From: sashiko-bot@kernel.org
To: "Abd-Alrhman Masalkhi" <abd.masalkhi@gmail.com>
Cc: yukuai@fygo.io, linux-raid@vger.kernel.org
Subject: Re: [PATCH] md/raid1: honor REQ_NOWAIT when waiting for behind writes
Date: Thu, 11 Jun 2026 08:49:53 +0000 [thread overview]
Message-ID: <20260611084953.E078E1F00898@smtp.kernel.org> (raw)
In-Reply-To: <20260611083514.754922-1-abd.masalkhi@gmail.com>
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.
--
commit 7fed7389f4fe20e79eece974eaf3b8d317519e27
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
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/20260611083514.754922-1-abd.masalkhi@gmail.com?part=1
next prev parent reply other threads:[~2026-06-11 8:49 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-11 8:35 [PATCH] md/raid1: honor REQ_NOWAIT when waiting for behind writes Abd-Alrhman Masalkhi
2026-06-11 8:49 ` sashiko-bot [this message]
2026-06-11 10:49 ` Abd-Alrhman Masalkhi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260611084953.E078E1F00898@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=abd.masalkhi@gmail.com \
--cc=linux-raid@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=yukuai@fygo.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox