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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

--
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


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux