Linux RAID subsystem development
 help / color / mirror / Atom feed
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 v3 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths
Date: Wed, 08 Jul 2026 10:33:02 +0000	[thread overview]
Message-ID: <20260708103303.4D7C61F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260708101341.473750-2-abd.masalkhi@gmail.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] REQ_NOWAIT bios can block waiting for blocked devices in wait_blocked_dev(), violating non-blocking semantics.
- [High] REQ_NOWAIT bios can stall during memory allocation due to hardcoded GFP_NOIO usage.
- [High] REQ_NOWAIT bios split by bio_submit_split_bioset block waiting for the barrier.
--

--- Patch [1]: [PATCH v3 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths ---
commit 3eda7e2f9e31ead6359429f927a806f3d762dc4b
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>

md/raid10: fix r10bio leak in raid10_write_request() error paths

This commit fixes a memory leak in raid10_write_request() by freeing the
allocated r10_bio when a write request fails due to REQ_NOWAIT being set.

Closes: https://lore.kernel.org/linux-raid/20260613184042.BCEC01F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-raid/20260628142420.1051027-2-abd.masalkhi@gmail.com/
Link: https://lore.kernel.org/linux-raid/20260623072456.333437-2-abd.masalkhi@gmail.com/

> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0a3cfdd3f5df8..bd322eccdc3ff 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1365,6 +1365,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
>  		/* Bail out if REQ_NOWAIT is set for the bio */
>  		if (bio->bi_opf & REQ_NOWAIT) {
>  			bio_wouldblock_error(bio);
> +			free_r10bio(r10_bio);
>  			return false;
>  		}
>  		for (;;) {

[ ... ]

> @@ -1398,6 +1399,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
>  		if (bio->bi_opf & REQ_NOWAIT) {
>  			allow_barrier(conf);
>  			bio_wouldblock_error(bio);
> +			free_r10bio(r10_bio);
>  			return false;
>  		}

[Severity: High]
This is a pre-existing issue, but since this patch improves REQ_NOWAIT
handling, can REQ_NOWAIT bios still block waiting for blocked devices?

In raid10_write_request(), wait_blocked_dev() is called unconditionally:

    wait_blocked_dev(mddev, r10_bio);

If an rdev is blocked, wait_blocked_dev() calls md_wait_for_blocked_rdev(),
which waits up to 5 seconds via wait_event_timeout(). Should this path check
if the bio is REQ_NOWAIT to avoid stalling the submission thread?

[Severity: High]
This is another pre-existing issue, but can REQ_NOWAIT bios stall during memory
allocation due to hardcoded GFP_NOIO usage?

In __make_request(), the initial allocation uses GFP_NOIO:

    r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);

And in raid10_write_one_disk(), the child bio allocation also uses GFP_NOIO:

    mbio = bio_alloc_clone(rdev->bdev, bio, GFP_NOIO, &mddev->bio_set);

Under memory pressure, GFP_NOIO will trigger direct reclaim and wait for
mempool replenishment. Should these allocations be attempted with GFP_NOWAIT
and aborted with BLK_STS_AGAIN for REQ_NOWAIT bios?

[Severity: High]
This is also a pre-existing issue, but do REQ_NOWAIT bios that are split block
waiting for the barrier?

If a REQ_NOWAIT bio spans multiple chunks, it can be split. The wait_barrier()
call uses a hardcoded false for the nowait parameter:

    wait_barrier(conf, false);

This will block if the array barrier is raised. Since returning BLK_STS_AGAIN
is no longer viable once the first half is submitted, is there a way to prevent
stalling REQ_NOWAIT callers on split bios?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708101341.473750-1-abd.masalkhi@gmail.com?part=1

  reply	other threads:[~2026-07-08 10:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-08 10:13 [PATCH v3 0/7] md/raid10: fixes, atomic write handling, and error-path cleanup Abd-Alrhman Masalkhi
2026-07-08 10:13 ` [PATCH v3 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths Abd-Alrhman Masalkhi
2026-07-08 10:33   ` sashiko-bot [this message]
2026-07-08 10:13 ` [PATCH v3 2/7] md/raid1: advertise atomic write limits and handle runtime constraints Abd-Alrhman Masalkhi
2026-07-09 12:11   ` John Garry
2026-07-10  9:42     ` Abd-Alrhman Masalkhi
2026-07-08 10:13 ` [PATCH v3 3/7] md/raid10: consistently fail atomic writes that require splitting Abd-Alrhman Masalkhi
2026-07-08 10:31   ` sashiko-bot
2026-07-08 10:13 ` [PATCH v3 4/7] md/raid10: remove unnecessary barrier around bio_submit_split_bioset() Abd-Alrhman Masalkhi
2026-07-08 10:13 ` [PATCH v3 5/7] md/raid10: replace wait loop with wait_event_idle() Abd-Alrhman Masalkhi
2026-07-08 10:13 ` [PATCH v3 6/7] md/raid10: simplify write request error handling Abd-Alrhman Masalkhi
2026-07-08 10:13 ` [PATCH v3 7/7] md/raid10: simplify read " 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=20260708103303.4D7C61F000E9@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