Re: [PATCH 4/7] md/raid10: raid10_write_request() drops the barrier before calling

[Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>]

Hi Kuai,

On Wed, Jun 24, 2026 at 10:01 +0200, Abd-Alrhman Masalkhi wrote:
> On Wed, Jun 24, 2026 at 15:23 +0800, yu kuai wrote:
>> Hi,
>>
>> 在 2026/6/23 15:24, Abd-Alrhman Masalkhi 写道:
>>> bio_submit_split_bioset() and reacquires it afterwards. This is
>>> unnecessary because bio_submit_split_bioset() does not require
>>> releasing the barrier protection.
>>
>> Why is this safe?
>>
>> It's possible plug is not enabled in this context, and the allocated new
>> aplit bio will be submitted and enter wait_barrier() again. wait_barrier()
>> is not supposed to be called recursively. Otherwise deadlock can happend:
>>
>> t1: sync_thread grab barrier and waiting for nr_pending to be 0 from raise_barrier()
>> t2: wait_barrier() first inc nr_pending, then recursive wait_barrier() waiting
>> for barrier to be released.
>>
>
> raid10_write_request() is only called from the bio submission path and
> not from raid10d thread, so we do not re-enter it while already holding
> barrier. Therefore, the split bio submitted by bio_submit_split_bioset()
> cannot recurse back into wait_barrier() from the same execution path.

I looked at commit e820d55cb99d. At the time, it addressed this issue
because submit_flushes() called md_handle_request() directly. Since
v5.2, submit_flushes() has gone through submit_bio() instead before
submit_flushes() was eventually removed.

Today, md_handle_request() is called from md_submit_bio() and dm-raid.
I don't have much experience with dm, but at first glance it seems safe
to remove these calls. However, if md_handle_request() were ever invoked
from a worker thread or kthread, we could run into the same kind of
issues, not only for RAID10, but also for RAID1.

What do you think? Should I drop this patch and guard RAID1 as well, or
is it safe to assume that dm-raid cannot hit this path from a kthread
or a workqueue worker?

>
> The situation is different for reads, where raid10_read_request() will
> be called both from the submission path and from raid10d. There,
> dropping and reacquiring the barrier protection remains necessary.
>
>>>
>>> Remove the redundant allow_barrier()/wait_barrier() pair around
>>> bio_submit_split_bioset().
>>>
>>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>>> ---
>>>   drivers/md/raid10.c | 2 --
>>>   1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>>> index 840f0446c231..4bc1d5553ec7 100644
>>> --- a/drivers/md/raid10.c
>>> +++ b/drivers/md/raid10.c
>>> @@ -1493,10 +1493,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
>>>   		if (atomic)
>>>   			goto err_handle;
>>>   
>>> -		allow_barrier(conf);
>>>   		bio = bio_submit_split_bioset(bio, r10_bio->sectors,
>>>   					      &conf->bio_split);
>>> -		wait_barrier(conf, false);
>>>   		if (!bio) {
>>>   			set_bit(R10BIO_Returned, &r10_bio->state);
>>>   			goto err_handle;
>>
>> -- 
>> Thanks,
>> Kuai
>
> -- 
> Best Regards,
> Abd-Alrhman

-- 
Best Regards,
Abd-Alrhman