On Wed, 2014-06-25 at 06:13 -0700, Christoph Hellwig wrote: > On Thu, Jun 19, 2014 at 11:05:59AM -0700, James Bottomley wrote: > > 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. > > Looking at the places where the SCSI midlayer cares about the request > type: > > - scsi_finish_command to call ->done for non-PC requests. Given that > we called into the driver to setup flush/discard/etc we should also > call into the driver on request completion > - scsi_eh_action: ditto for error handling > - scsi_noretry_cmd: I don't see why we'd want to treat flush request > as having an implicit failfast flag > - scsi_io_completion: this mostly opts out of all kinds of error > handling and retries, not really what we'd want either > - scsi_unprep_fn: calls ->uninit_command only for !PC request, > so your patch introduces a leak for discard requests Yes, but think what you're proposing. Every block command with a special setup goes via scsi_setup_blk_pc_cmnd() because they usually have to translate to something special. If we did what you propose, every time we add one, we'd have to modify these five places in the code plus the setup ... it's a bit insane plus a maintenance nightmare. Since block gave us the command to set up as we please, we're entitled to change the fields including cmd_type. James -- 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
