Re: [PATCH] md/raid1: honor REQ_NOWAIT when waiting for behind writes

On Thu, Jun 11, 2026 at 08:49 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Leaking pre-allocated `r1_bio` when `REQ_NOWAIT` reads fail `wait_read_barrier()` during retry.
> - [High] Permanent barrier leak if a REQ_NOWAIT write encounters a blocked device.
> - [Medium] REQ_NOWAIT semantics violation: memory allocations use GFP_NOIO unconditionally.

These appear to be pre-existing issues rather than being introduced by
this patch. I plan to address them in separate patches.

> --
>
> commit 7fed7389f4fe20e79eece974eaf3b8d317519e27
> Author: Abd-Alrhman Masalkhi <[email protected]>
>
> md/raid1: honor REQ_NOWAIT when waiting for behind writes
>
> This commit ensures that raid1 consistently honors REQ_NOWAIT reads
> across paths that may otherwise wait for behind writes. It modifies
> bitmap_wait_behind_writes() to return early instead of sleeping if
> the nowait flag is set.
>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index 5b9368bd9e700..d03da046a5920 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
> [ ... ]
>> @@ -1360,8 +1361,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
>>  	 * Still need barrier for READ in case that whole
>>  	 * array is frozen.
>>  	 */
>> -	if (!wait_read_barrier(conf, bio->bi_iter.bi_sector,
>> -				bio->bi_opf & REQ_NOWAIT)) {
>> +	if (!wait_read_barrier(conf, bio->bi_iter.bi_sector, nowait)) {
>>  		bio_wouldblock_error(bio);
>>  		return;
>>  	}
>
> [Severity: High]
> This is a pre-existing issue, but does this early return leak the 
> pre-allocated r1_bio when wait_read_barrier() fails during a retry?
>
> When a REQ_NOWAIT read fails its initial attempt, it is queued to the 
> retry_list and handled by the raid1d thread via handle_read_error().
>
> That function passes the existing, pre-allocated r1_bio back to 
> raid1_read_request():
>
> drivers/md/raid1.c:handle_read_error() {
>     ...
>     raid1_read_request(mddev, bio, r1_bio->sectors, r1_bio);
>     allow_barrier(conf, sector);
>     ...
> }
>
> If the array is concurrently frozen, wait_read_barrier() returns false 
> since REQ_NOWAIT is still set on the master bio. The code handles this 
> by returning immediately, bypassing the err_handle cleanup path and 
> failing to call raid_end_bio_io().
>
> Since handle_read_error() assumes ownership was taken and only releases 
> its old barrier, does this result in a permanent memory leak of the 
> r1_bio from the r1bio_pool mempool?
>
> [Severity: Medium]
> This is another pre-existing issue, but does the allocation path for 
> REQ_NOWAIT bios violate block layer non-blocking semantics by 
> unconditionally using GFP_NOIO?
>
> Even when REQ_NOWAIT is requested, alloc_r1bio() uses GFP_NOIO 
> unconditionally:
>
> drivers/md/raid1.c:alloc_r1bio() {
>     ...
>     r1_bio = mempool_alloc(conf->r1bio_pool, GFP_NOIO);
>     ...
> }
>
> Similarly, bio_alloc_clone() in raid1_read_request() unconditionally 
> uses gfp flags that include __GFP_DIRECT_RECLAIM.
>
> Since mempool-backed allocations will sleep under memory pressure 
> instead of failing immediately when these flags are used, will this 
> cause unexpected high latencies and potential priority inversion in 
> asynchronous contexts?
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does 
> raid1_write_request() permanently leak the barrier if a REQ_NOWAIT 
> write encounters a blocked device?
>
> In raid1_write_request(), the code successfully acquires a barrier by 
> calling wait_barrier(), which internally increments conf->nr_pending.
>
> Immediately after, it calls wait_blocked_rdev():
>
> drivers/md/raid1.c:raid1_write_request() {
>     ...
>     if (!wait_blocked_rdev(mddev, bio)) {
>         bio_wouldblock_error(bio);
>         return;
>     }
>     ...
> }
>
> If the device is blocked and the request is REQ_NOWAIT, 
> wait_blocked_rdev() returns false and the function returns early. 
> However, it fails to call allow_barrier() before returning to drop 
> the previously acquired barrier increment.
>
> Because freeze_array() waits for nr_pending to reach 0, could this 
> permanently prevent the array from ever freezing again, deadlocking 
> resyncs, reconfiguration, and array teardown?
>
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

-- 
Best Regards,
Abd-Alrhman