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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html