Re: [PATCH v7 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio()

[Chen Cheng]

在 2026/7/12 01:43, yu kuai 写道:
>> -	if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
>> -		if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
>> -			bio->bi_status = BLK_STS_IOERR;
>> +	returned = test_and_set_bit(R10BIO_Returned, &state);
>> +	if (!returned && !test_bit(R10BIO_Uptodate, &state))
>> +		bio->bi_status = BLK_STS_IOERR;
>> +
>> +	free_r10bio(r10_bio);
>> +
>> +	if (!returned)
>>    		bio_endio(bio);
>> -	}
> Apparently this change is not enough, the R10BIO_Returned case still return
> the master_bio before freeing r10_bio.
> 



hi Kuai,

thanks for review.

After read the relate part, two scenarios goto raid_end_bio_io:
	a. bio_submit_split_bioset() failed cause early return, and
	b. normal raid-10 completion -> finially call raid_end_bio_io()

for (a), protected by md_handle_request()'s active_io ref life-scope.
	so, even close master io earily , active_io still alive.

for (b), after fix,  free r10bio first. then, close master bio, which
	md_end_clone_io() put active_io refcnt.

both is ok , I think.

thanks.
cheng


> I think you should get an active_io reference while allocating r10_bio to
> guarantee that no r10_bio is active while array is suspended.
> 
>>    
>>    	/*
>>    	 * Wake up any possible resync thread that waits for the device
>>    	 * to go idle.
>>    	 */
>>    	allow_barrier(conf);
>> -
>> -	free_r10bio(r10_bio);
>>    }
>>    
>>    /*
>>     * Update disk head position estimator based on IRQ completion info.
>>     */
>> @@ -1579,13 +1582,15 @@ static void raid_end_discard_bio(struct r10bio *r10bio)
>>    		if (!test_bit(R10BIO_Discard, &r10bio->state)) {
>>    			first_r10bio = (struct r10bio *)r10bio->master_bio;