Re: [PATCH v3 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths

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 <[email protected]>

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/[email protected]/
Link: https://lore.kernel.org/linux-raid/[email protected]/
Link: https://lore.kernel.org/linux-raid/[email protected]/

> 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/[email protected]?part=1