On Tue, 2014-05-27 at 11:00 +0200, Bart Van Assche wrote: > On 05/27/14 10:23, James Bottomley wrote: > > On Tue, 2014-05-27 at 09:49 +0200, Bart Van Assche wrote: > >> When reading the source code in scsi_error.c it's easy to overlook that > >> scmd_eh_abort_handler(), scsi_abort_command() and scsi_eh_scmd_add() are > >> all invoked for requests in which the REQ_ATOM_COMPLETE bit has been > >> set. > > > > I really don't like this entanglement of state of block and SCSI. > > "complete" in block terms isn't the same as in SCSI terms. The > > REQ_ATOM_COMPLETE flag is fully internal to block and indicates that > > we've taken over processing the command and any completions into block > > get ignored. This is for the possible race between timeout and inbound > > command completion. If you start coding SCSI assertions in terms of it, > > you're entangling layers that should be separate. > > > > The assertion in SCSI terms is that abort and ->done cannot race. > > Recent e-mail threads on the linux-scsi mailing list have shown that it > is easily overlooked which error handler functions are called only for > requests marked by the block layer as "complete" and hence cannot race > with scsi_softirq_done(). So far my proposals for how to improve the > documentation of how this race is avoided were rejected. Do you perhaps > have a proposal of how this should be documented properly ? Um, if that's your goal, then I don't see how adding a WARN_ON_ONCE(!blk_rq_completed(scmd->request)); makes it clear that timeout and the softirq cannot race. I suppose for me, it's just an obvious systems assertion since every command must be in a defined state for the state model, either a command has timed out or it's in normal completion. But since we inherit this race mediation from block, isn't that the place to document it if people are confused? I could also see us one day extending the TMF capability to abort any running command, which would make even an assertion of block timed out or completed invalid. 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
