Also see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/block/bio.c?h=v5.10-rc6#n1384 btw. I really think the "new as parent" is a careless typo. (Christoph?) On Sun, 6 Dec 2020 at 21:17, Tom Yan <tom.ty89@xxxxxxxxx> wrote: > > 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
