On Thu, 2012-11-08 at 10:10 -0800, Nicholas A. Bellinger wrote: > On Thu, 2012-11-08 at 14:25 +0000, Prantis, Kelsey wrote: > > On 11/7/12 4:26 PM, "Prantis, Kelsey" <kelsey.prantis@xxxxxxxxx> wrote: > > >>So I'll try to reproduce using some different combinations of > > >>fabric_max_sectors, but for the moment please apply the following debug > > >>patch to dump struct scatterlist + struct bio members and try to > > >>reproduce. > > > > > >Am working on applying the patch now, and will get back to you when I have > > >a result. > > > > There was quite a bit of output, which can be fetched from: > > ftp://ftp.whamcloud.com/uploads/lio-debug.txt > > > > Hi Kelsey, > > Thanks for the log. After an quick look, here's what I see so far: > <SNIP> > So AFAICT this looks like a iblock bio completion callback is being > invoked more than once, that is being triggered when using multiple BIOs > to complete a single se_cmd descriptor. > > Looking at the code in question from target_core_iblock.c, I think I see > what's going on now. There is a double completion happening between the > iblock_complete_cmd call in iblock_execute_rw() and iblock_bio_done() > that appears to have been introduced with: > > commit 5787cacd0bd5ee016ad807b244550d34fe2beebe > Author: Christoph Hellwig <hch@xxxxxxxxxxxxx> > Date: Tue Apr 24 00:25:06 2012 -0400 > > target: remove struct se_task > > I'm currently testing the following patch to address this bug. > > Please give it a shot with your setup. > > Thanks! > > --nab > > diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c > index 57d7674..305aa41 100644 > --- a/drivers/target/target_core_iblock.c > +++ b/drivers/target/target_core_iblock.c > @@ -658,7 +658,7 @@ static int iblock_execute_rw(struct se_cmd *cmd) > bio_list_init(&list); > bio_list_add(&list, bio); > > - atomic_set(&ibr->pending, 2); > + atomic_set(&ibr->pending, 1); > bio_cnt = 1; > > for_each_sg(sgl, sg, sgl_nents, i) { > @@ -689,7 +689,6 @@ static int iblock_execute_rw(struct se_cmd *cmd) > } > > iblock_submit_bios(&list, rw); > - iblock_complete_cmd(cmd); > return 0; > > fail_put_bios: > > -- Ok, a slightly updated version of this patch to make sure that ibr->pending in incremented ahead of the possible call to iblock_submit_bios() when bio_cnt >= IBLOCK_MAX_BIO_PER_TASK. This is not happening in your particular case, but is still required to ensure that iblock_bio_done() waits for all outstanding bios to return before completing to target core. --nab diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c index 57d7674..70fd5b5 100644 --- a/drivers/target/target_core_iblock.c +++ b/drivers/target/target_core_iblock.c @@ -658,7 +658,7 @@ static int iblock_execute_rw(struct se_cmd *cmd) bio_list_init(&list); bio_list_add(&list, bio); - atomic_set(&ibr->pending, 2); + atomic_set(&ibr->pending, 1); bio_cnt = 1; for_each_sg(sgl, sg, sgl_nents, i) { @@ -669,16 +669,23 @@ static int iblock_execute_rw(struct se_cmd *cmd) */ while (bio_add_page(bio, sg_page(sg), sg->length, sg->offset) != sg->length) { + /* + * Increment pending now before possible call to + * iblock_submit_bios() -> async iblock_bio_done() + */ + atomic_inc(&ibr->pending); + if (bio_cnt >= IBLOCK_MAX_BIO_PER_TASK) { iblock_submit_bios(&list, rw); bio_cnt = 0; } bio = iblock_get_bio(cmd, block_lba, sg_num); - if (!bio) + if (!bio) { + atomic_dec(&ibr->pending); goto fail_put_bios; + } - atomic_inc(&ibr->pending); bio_list_add(&list, bio); bio_cnt++; } @@ -689,7 +696,6 @@ static int iblock_execute_rw(struct se_cmd *cmd) } iblock_submit_bios(&list, rw); - iblock_complete_cmd(cmd); return 0; fail_put_bios: -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
