Hi, Jeff. Jeff Garzik wrote:
Tejun Heo wrote:06_blk_update_scsi_to_use_new_ordered.patch All ordered request related stuff delegated to HLD. Midlayer now doens't deal with ordered setting or prepare_flush callback. sd.c updated to deal with blk_queue_ordered setting. Currently, ordered tag isn't used as SCSI midlayer cannot guarantee request ordering. Signed-off-by: Tejun Heo <[email protected]> drivers/scsi/hosts.c | 9 ----- drivers/scsi/scsi_lib.c | 46 --------------------------drivers/scsi/sd.c | 79 +++++++++++++++++++-------------------------- include/scsi/scsi_driver.h | 1 include/scsi/scsi_host.h | 1 5 files changed, 34 insertions(+), 102 deletions(-)Index: blk-fixes/drivers/scsi/sd.c ===================================================================--- blk-fixes.orig/drivers/scsi/sd.c 2005-06-05 14:53:32.000000000 +0900+++ blk-fixes/drivers/scsi/sd.c 2005-06-05 14:53:35.000000000 +0900 @@ -103,6 +103,7 @@ struct scsi_disk { u8 write_prot; unsigned WCE : 1; /* state of disk WCE bit */ unsigned RCD : 1; /* state of disk RCD bit, unused */ + unsigned DPOFUA : 1; /* state of disk DPOFUA bit */ };static DEFINE_IDR(sd_index_idr);@@ -122,8 +123,7 @@ static void sd_shutdown(struct device *d static void sd_rescan(struct device *); static int sd_init_command(struct scsi_cmnd *); static int sd_issue_flush(struct device *, sector_t *); -static void sd_end_flush(request_queue_t *, struct request *); -static int sd_prepare_flush(request_queue_t *, struct request *); +static void sd_prepare_flush(request_queue_t *, struct request *); static void sd_read_capacity(struct scsi_disk *sdkp, char *diskname, struct scsi_request *SRpnt, unsigned char *buffer);@@ -138,8 +138,6 @@ static struct scsi_driver sd_template = .rescan = sd_rescan,.init_command = sd_init_command, .issue_flush = sd_issue_flush, - .prepare_flush = sd_prepare_flush, - .end_flush = sd_end_flush, };/*@@ -346,6 +344,7 @@ static int sd_init_command(struct scsi_cif (block > 0xffffffff) {SCpnt->cmnd[0] += READ_16 - READ_6; + SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0;SCpnt->cmnd[2] = sizeof(block) > 4 ? (unsigned char) (block >> 56) & 0xff : 0; SCpnt->cmnd[3] = sizeof(block) > 4 ? (unsigned char) (block >> 48) & 0xff : 0; SCpnt->cmnd[4] = sizeof(block) > 4 ? (unsigned char) (block >> 40) & 0xff : 0;@@ -360,11 +359,12 @@ static int sd_init_command(struct scsi_c SCpnt->cmnd[13] = (unsigned char) this_count & 0xff; SCpnt->cmnd[14] = SCpnt->cmnd[15] = 0; } else if ((this_count > 0xff) || (block > 0x1fffff) || - SCpnt->device->use_10_for_rw) { + SCpnt->device->use_10_for_rw || blk_fua_rq(rq)) {This seems suspicious, like it would cause unwanted use of READ(10) for some devices that prefer READ(6) ?
As READ/WRITE(6) doesn't have FUA field, we have to use WRITE(10) or WRITE(16) on FUA writes. Above addition affects only FUA writes (no effect on READs or normal WRITEs).
FUA write requests are generated only when a device reports DPOFUA capability during sd attach, and only READ/WRITE(>=10) have FUA field, so I think it's logical to assume that devices which report DPOFUA support READ/WRITE(10).
Also, currently all SCSI disks default to READ/WRITE(10) and use_10_for_rw is turned off only when READ/WRITE(10) fails w/ ILLEGAL_REQUEST. So, the assumption is that devices are at least capable of properly terminating READ/WRITE(10)s with ILLEGAL REQUEST when it doesn't like'em (thus turning off FUA).
Hmmm... Do you think I should add more tests before enabling FUA such that we know WRITE(10) is supported (capacity test comes to mind).
if (this_count > 0xffff) this_count = 0xffff;SCpnt->cmnd[0] += READ_10 - READ_6;+ SCpnt->cmnd[1] |= blk_fua_rq(rq) ? 0x8 : 0; SCpnt->cmnd[2] = (unsigned char) (block >> 24) & 0xff; SCpnt->cmnd[3] = (unsigned char) (block >> 16) & 0xff; SCpnt->cmnd[4] = (unsigned char) (block >> 8) & 0xff;@@ -739,43 +739,12 @@ static int sd_issue_flush(struct device return sd_sync_cache(sdp);}-static void sd_end_flush(request_queue_t *q, struct request *flush_rq)+static void sd_prepare_flush(request_queue_t *q, struct request *rq) { - struct request *rq = flush_rq->end_io_data; - struct scsi_cmnd *cmd = rq->special; - unsigned int bytes = rq->hard_nr_sectors << 9; - - if (!flush_rq->errors) { - spin_unlock(q->queue_lock); - scsi_io_completion(cmd, bytes, 0); - spin_lock(q->queue_lock); - } else if (blk_barrier_postflush(rq)) { - spin_unlock(q->queue_lock); - scsi_io_completion(cmd, 0, bytes); - spin_lock(q->queue_lock); - } else { - /* - * force journal abort of barriers - */ - end_that_request_first(rq, -EOPNOTSUPP, rq->hard_nr_sectors); - end_that_request_last(rq, -EOPNOTSUPP); - } -} - -static int sd_prepare_flush(request_queue_t *q, struct request *rq) -{ - struct scsi_device *sdev = q->queuedata; - struct scsi_disk *sdkp = dev_get_drvdata(&sdev->sdev_gendev); - - if (sdkp->WCE) { - memset(rq->cmd, 0, sizeof(rq->cmd)); - rq->flags |= REQ_BLOCK_PC | REQ_SOFTBARRIER; - rq->timeout = SD_TIMEOUT; - rq->cmd[0] = SYNCHRONIZE_CACHE; - return 1; - } - - return 0; + memset(rq->cmd, 0, sizeof(rq->cmd)); + rq->flags |= REQ_BLOCK_PC | REQ_SOFTBARRIER; + rq->timeout = SD_TIMEOUT; + rq->cmd[0] = SYNCHRONIZE_CACHE; }static void sd_rescan(struct device *dev)@@ -1433,10 +1402,13 @@ sd_read_cache_type(struct scsi_disk *sdk sdkp->RCD = 0; }+ sdkp->DPOFUA = (data.device_specific & 0x10) != 0;+ ct = sdkp->RCD + 2*sdkp->WCE;- printk(KERN_NOTICE "SCSI device %s: drive cache: %s\n",- diskname, types[ct]); + printk(KERN_NOTICE "SCSI device %s: drive cache: %s%s\n", + diskname, types[ct], + sdkp->DPOFUA ? " with forced unit access" : "");This is IMO a bit verbose. Just " w/ FUA" might be better.
Sure.
return; } @@ -1469,6 +1441,7 @@ static int sd_revalidate_disk(struct gen struct scsi_device *sdp = sdkp->device; struct scsi_request *sreq; unsigned char *buffer; + unsigned ordered;SCSI_LOG_HLQUEUE(3, printk("sd_revalidate_disk: disk=%s\n", disk->disk_name)); @@ -1514,7 +1487,22 @@ static int sd_revalidate_disk(struct gensreq, buffer); sd_read_cache_type(sdkp, disk->disk_name, sreq, buffer); }- ++ /* + * We now have all cache related info, determine how we deal + * with ordered requests. Note that as the current SCSI + * dispatch function can alter request order, we cannot use + * QUEUE_ORDERED_TAG_* even when ordered tag is supported. + */ + if (sdkp->WCE) + ordered = sdkp->DPOFUA + ? QUEUE_ORDERED_DRAIN_FUA : QUEUE_ORDERED_DRAIN_FLUSH;Certainly 'DPO and FUA' implies we have FUA, but I wonder if this test is unnecessarily narrow.
Meaning...? -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
- References:
- [PATCH Linux 2.6.12-rc5-mm2 00/09] blk: ordered request reimplementation (take 2, for review)
- From: Tejun Heo <[email protected]>
- Re: [PATCH Linux 2.6.12-rc5-mm2 06/09] blk: update SCSI to use the new blk_ordered
- From: Tejun Heo <[email protected]>
- Re: [PATCH Linux 2.6.12-rc5-mm2 06/09] blk: update SCSI to use the new blk_ordered
- From: Jeff Garzik <[email protected]>
- [PATCH Linux 2.6.12-rc5-mm2 00/09] blk: ordered request reimplementation (take 2, for review)
- Prev by Date: Re: [SATA] libata-dev queue updated
- Next by Date: Re: [PATCH Linux 2.6.12-rc5-mm2 07/09] blk: update libata to use the new blk_ordered.
- Previous by thread: Re: [PATCH Linux 2.6.12-rc5-mm2 06/09] blk: update SCSI to use the new blk_ordered
- Next by thread: Re: [PATCH Linux 2.6.12-rc5-mm2 07/09] blk: update libata to use the new blk_ordered.
- Index(es):