Re: [PATCH 2/3] block: Introduce blk_rq_completed()

["hch@infradead.org" <hch@infradead.org>]

On Tue, May 27, 2014 at 09:49:48AM +0200, Bart Van Assche wrote:
> > I don't see the value of patches 2,3 they're checking for an impossible
> > condition ... why might it be possible?
> 
> 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. Although it is possible to mention this as a comment above these
> functions, such comments are not checked at runtime. It would require
> additional work from the reader to verify whether or not such source
> code comments are up to date. However, the condition inside a
> WARN_ON_ONCE() statement is checked every time the code is executed.
> Hence my preference for a WARN_ON_ONCE() statement instead of writing
> down somewhere that these three functions operate on requests in which
> the REQ_ATOM_COMPLETE bit has been set.


I really do like the REQ_ATOM_COMPLETE asserts - as experience tells
these kinds of assumptions are best checked, otherwise they will
unintentionally be violated.

I'm less excited about the list walk I have to say, as the overhead is
getting fairly large for a simple assertation.