On Thu, 2012-11-15 at 14:36 -0800, Nicholas A. Bellinger wrote: > On Thu, 2012-11-15 at 05:49 -0500, Christoph Hellwig wrote: > > On Thu, Nov 08, 2012 at 07:58:59PM +0000, Nicholas A. Bellinger wrote: > > > From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx> > > > > > > This patch fixes a double completion bug where ibr->pending = 2 usage > > > plus the extra callback to iblock_complete_cmd() invoked after bio > > > submission in iblock_execute_rw() can interfere with the normal > > > bio->bi_done() -> iblock_bio_done() -> iblock_complete_cmd() completion > > > path, causing a double target_complete_cmd() call to occur. > > > > How so exactly? Please provide an explanation, and preferably a test > > case. > > > > So, I was getting a bit confused here while debugging Kelsey's issue > with dispatched multiple bios throwing an with a virtio-blk backend. > > 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..? > > > > Also drop the bio_cnt >= IBLOCK_MAX_BIO_PER_TASK rolling call to > > > iblock_submit_bios() to avoid the exception case where outstanding > > > bios completing via iblock_bio_done() are not accounted for when > > > returning returning non zero from iblock_execute_rw(). > > > > This will cause deadlocks when we can't allocate enough bios for > > a too large request. > > > > Mmmm.. > > 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. > Err, nevermind.. Dropping this patch as v3.7 material for the moment while debugging the Kelsey's multiple-bio IBLOCK+virtio-blk bug.. > How about returning an exception here instead when IBLOCK_MAX_BIOS in > reached..? Btw, where did the default of 32 for this come from..? > Still curious where this value comes from..? :) --nab -- 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
