On Wed, 2014-06-11 at 06:37 -0700, Christoph Hellwig wrote:
> On Mon, Jun 09, 2014 at 10:29:06AM -0700, James Bottomley wrote:
> > I'll do it as a bug fix, but I do need Jens to make sure nothing else
> > breaks first.  Best I can tell, the state model for compound commands
> > like flush doesn't expect us to change the request type ... nothing puts
> > it back to REQ_TYPE_FS.  In your case, the flush is the last command
> > sent, so there's no problem ... I just worry we will get an obscure
> > problem later on from something that does a BLOCK_PC prepared first
> > command.
> 
> Yes, I don't think resetting cmd_type is a good idea.  I'd much rather
> see a special case for rq->cmd_flags & REQ_FLUSH in the completion
> handler - we already treat it special during setup anyway.

That's not really a good idea either ... I did think of it.  We'll end
up with a cmd_type of REQ_TYPE_FS which because of REQ_FLUSH (or REQ_FUA
or REQ_DISCARD or any number of other things) we have to treat as though
it were REQ_TYPE_BLOCK_PC.  It's much better to tune handling
expectations according to req->cmd_type because that's what we already
do.  These commands are actually set up by our handlers, so it's up to
us to mark the request type correctly.

This, I think, does everything we need

James

---

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f7e3163..d1458ec 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1103,6 +1103,11 @@ int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, 
struct request *req)
        
        cmd->transfersize = blk_rq_bytes(req);
        cmd->allowed = req->retries;
+       /*
+        * Thanks to flush and other PC prepared commands, we may
+        * not be a REQ_TYPE_BLOCK_PC; make sure we are
+        */
+       req->cmd_type = REQ_TYPE_BLOCK_PC;
        return BLKPREP_OK;
 }
 EXPORT_SYMBOL(scsi_setup_blk_pc_cmnd);
@@ -1129,6 +1134,11 @@ int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct 
request *req)
        BUG_ON(!req->nr_phys_segments);
 
        memset(cmd->cmnd, 0, BLK_MAX_CDB);
+       /*
+        * Thanks to flush and other PC prepared commands, we may
+        * not be a REQ_TYPE_FS; make sure we are
+        */
+       req->cmd_type = REQ_TYPE_FS;
        return scsi_init_io(cmd, GFP_ATOMIC);
 }
 EXPORT_SYMBOL(scsi_setup_fs_cmnd);


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to