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

在 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 <[email protected]>
>>> ---
>>> Changes in v2:
>>>    - Reuse md_free_cloned_bio() instead of introducing a new helper
>>>    - Link to v1: https://lore.kernel.org/linux-raid/[email protected]
>>> ---
>>>    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 <[email protected]>

-- 
Thanks,
Nan