Re: [PATCH] target/iblock: Fix double iblock_complete_cmd callback bug

[Christoph Hellwig <hch@infradead.org>]

On Thu, Nov 15, 2012 at 02:36:30PM -0800, Nicholas A. Bellinger wrote:
> However, setting the default ibr->pending=2 value before dispatch, and
> making a extra iblock_complete_cmd() call from iblock_execute_rw(),
> while doing the normal iblock_complete_cmd() calls from
> iblock_bio_done() is AFAICT a pointless extra atomic_dec_and_test() call
> per I/O.
> 
> Was there a reason why you changed ->pending from 1 -> 2 during the
> se_task removal in commit 5787cacd0bd5ee01..?

It is to avoid having the request completed before even submitting all
bios.  As soon as one is submitted it could on a very fast device
complete before we've incremented the count for the next bio.

It's a very common scheme used all over the kernel when submitting
I/O in smaller subdivisions.

> So the case this patch tries to avoid is where iblock_submit_bio() is
> called multiple times before the final ibr->pending value is set, this
> could potentially cause bios completion calls to decrement the value +
> complete to core before iblock_execute_rw() is done incrementing
> ibr->pending.

That's exacltly what we try to avoid here.

> How about returning an exception here instead when IBLOCK_MAX_BIOS in
> reached..?

Why?  As soon as we kicked the first batch off we're guaranteed to make
progress allocating more as sson as the first of the submitted ones
completes.

> Btw, where did the default of 32 for this come from..?

It's a random number, with the important property that it's considerably
smaller than IBLOCK_BIO_POOL_SIZE.