Re: [PATCH v2] md: use md_free_cloned_bio() in md_end_clone_io()

[Li Nan]

在 2026/4/16 15:39, Abd-Alrhman Masalkhi 写道:
> Hi Nan,
>
> thank you for the feedback.
>
> On Thu, Apr 16, 2026 at 14:47 +0800, Li Nan wrote:
> > 在 2026/4/15 16:19, Abd-Alrhman Masalkhi 写道:
> > > md_end_clone_io() and md_free_cloned_bio() share identical teardown
> > > logic. Use md_free_cloned_bio() in md_end_clone_io() for cleanup and
> > > call bio_endio() afterwards.
> > >
> > > Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@xxxxxxxxx>
> > > ---
> > > Changes in v2:
> > >    - Reuse md_free_cloned_bio() instead of introducing a new helper
> > >    - Link to v1: https://lore.kernel.org/linux-raid/20260414103813.307601-1-abd.masalkhi@xxxxxxxxx
> > > ---
> > >    drivers/md/md.c | 13 +------------
> > >    1 file changed, 1 insertion(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > > index ac71640ff3a8..8565566a447b 100644
> > > --- a/drivers/md/md.c
> > > +++ b/drivers/md/md.c
> > > @@ -9212,20 +9212,9 @@ static void md_end_clone_io(struct bio *bio)
> > >    {
> > >    	struct md_io_clone *md_io_clone = bio->bi_private;
> > >    	struct bio *orig_bio = md_io_clone->orig_bio;
> > > -	struct mddev *mddev = md_io_clone->mddev;
> > > -
> > > -	if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
> > > -		md_bitmap_end(mddev, md_io_clone);
> > > -
> > > -	if (bio->bi_status && !orig_bio->bi_status)
> > > -		orig_bio->bi_status = bio->bi_status;
> > > -
> > > -	if (md_io_clone->start_time)
> > > -		bio_end_io_acct(orig_bio, md_io_clone->start_time);
> > >    
> > > -	bio_put(bio);
> > > +	md_free_cloned_bio(bio);
> > >    	bio_endio(orig_bio);
> >
> > Is it safe to end orig_bio after putting active_io?
>
> As I understand it, active_io is primarily used to account for
> in-flight bios, allowing the array to be safely suspended once no
> bios are outstanding.
>
> I have made a diagram, while i am reviewing the md driver:
>
> mddev->active_io
>
> md driver:
> md_handle_request()
>      mddev->active_io +1
> 	    pers->make_request()
>              md_account_bio() +1
> 			    if (err)
> 				    md_free_cloned_bio -1
>      mddev->active_io -1
>
> lower driver:
> bio_endio()
>      md_end_clone_io -1
>
> As shown, it incremented in two places, the first is before passing
> orig_bio to pers->make_request(), and decremented after it returns
> (regardless of what make_request() does with it). and the second
> is when cloning the original bio, and it should be decremented after
> the cloned bio is released.
>
> In this context, it should not matter whether bio_endio(orig_bio) is
> called before or after decrementing active_io, as long as the cloned
> bio teardown is completed correctly.
>
> Am I missing anything that would make this unsafe?

Thanks for your reply, LGTM.

Reviewed-by: Li Nan <linan122@xxxxxxxxxx>

--
Thanks,
Nan