I still don't think this sounds right. See the definition of bio_chain(): https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/block/bio.c?h=v5.10-rc6#n344 And in turn bio_inc_remaining(): https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/bio.h?h=v5.10-rc6#n667 With the current blk_next_bio(), the first bio will never even be handled by bio_chain()/bio_inc_remaining()/bio_set_flag(bio, BIO_CHAIN). When the first submit_bio() is called, it won't have any idea that the next/new bio exists. What you said actually sounds a bit like the current situation. On Sun, 6 Dec 2020 at 19:20, Hannes Reinecke <hare@xxxxxxx> wrote: > > On 12/6/20 6:18 AM, Tom Yan wrote: > > While it seems to have worked for so long, it doesn't seem right > > that we set the new bio as the parent. bio_chain() seems to be used > > in the other way everywhere else anyway. > > > > Signed-off-by: Tom Yan <tom.ty89@xxxxxxxxx> > > --- > > block/blk-lib.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/block/blk-lib.c b/block/blk-lib.c > > index e90614fd8d6a..918deaf5c8a4 100644 > > --- a/block/blk-lib.c > > +++ b/block/blk-lib.c > > @@ -15,7 +15,7 @@ struct bio *blk_next_bio(struct bio *bio, unsigned int nr_pages, gfp_t gfp) > > struct bio *new = bio_alloc(gfp, nr_pages); > > > > if (bio) { > > - bio_chain(bio, new); > > + bio_chain(new, bio); > > submit_bio(bio); > > } > > > > > I don't think this is correct. > This code is submitting the original bio, and we _want_ to keep the > newly allocated one even though the original might have been completed > already. If we were setting the 'parent' to the original bio upper > layers might infer that the entire request has been completed (as the > original bio is now the 'parent' bio), which is patently not true. > > So, rather not. > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke Kernel Storage Architect > hare@xxxxxxx +49 911 74053 688 > SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg > HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
