Linux RAID subsystem development
 help / color / mirror / Atom feed
* [PATCH] md/raid5: complete discard bios while reshape is active
@ 2026-07-11 16:13 Genjian
  2026-07-11 16:27 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Genjian @ 2026-07-11 16:13 UTC (permalink / raw)
  To: song, yukuai, magiclinan, xiao
  Cc: linux-raid, linux-kernel, zhanggenjian, Genjian Zhang

From: Genjian Zhang <zhanggenjian@kylinos.cn>

make_discard_request() returns without completing the bio when reshape
is in progress. Discard callers block in submit_bio_wait()
waiting for a completion that never arrives.  The caller hangs in
uninterruptible sleep, and this does not resolve when reshape finishes.

Complete the bio with BLK_STS_AGAIN so userspace can retry after reshape,
consistent with the existing policy of not processing discard during
reshape.

Tested on a loop-backed RAID5 array during mdadm --grow: without this
patch, blkdiscard hangs in bio_await() and remains in uninterruptible
sleep after md reports "reshape done"; with this patch it returns
-EAGAIN instead.

Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn>
---
 drivers/md/raid5.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ffb5fcde54a9..4c3b42b2af85 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5722,9 +5722,11 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
 	if (WARN_ON_ONCE(bi->bi_opf & REQ_NOWAIT))
 		return;
 
-	if (mddev->reshape_position != MaxSector)
+	if (mddev->reshape_position != MaxSector) {
 		/* Skip discard while reshape is happening */
+		bio_endio_status(bi, BLK_STS_AGAIN);
 		return;
+	}
 
 	if (!raid5_discard_limits(mddev, bi))
 		return;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] md/raid5: complete discard bios while reshape is active
  2026-07-11 16:13 [PATCH] md/raid5: complete discard bios while reshape is active Genjian
@ 2026-07-11 16:27 ` sashiko-bot
  2026-07-12 14:35   ` genjian zhang
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-07-11 16:27 UTC (permalink / raw)
  To: Genjian; +Cc: linux-raid, yukuai

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

New issues:
- [High] Completing blocking bios with BLK_STS_AGAIN violates block layer semantics and causes unexpected errors in synchronous paths.

Pre-existing issues:
- [High] make_discard_request() leaks the bio and causes an uninterruptible hang if REQ_NOWAIT is set.
--

--- Patch [1]: [PATCH] md/raid5: complete discard bios while reshape is active ---
commit f117d558ba13364f82645f866dea16210cadce51
Author: Genjian Zhang <zhanggenjian@kylinos.cn>

md/raid5: complete discard bios while reshape is active

This patch modifies the discard request handling to complete bios with
BLK_STS_AGAIN when an mdraid reshape is in progress. This prevents callers
from hanging in uninterruptible sleep waiting for a completion that never
arrives.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -5722,9 +5722,11 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
>  	if (WARN_ON_ONCE(bi->bi_opf & REQ_NOWAIT))
>  		return;

[Severity: High]
This isn't a bug introduced by this patch, but does this early return leak
the bio and cause an uninterruptible hang if REQ_NOWAIT is set?

If a caller submits a discard bio with REQ_NOWAIT, the driver encounters this
warning and returns immediately without calling bio_endio(). Because the
caller assumes the bio was successfully handled, could this lead to a
resource leak and task hang?

>  
> -	if (mddev->reshape_position != MaxSector)
> +	if (mddev->reshape_position != MaxSector) {
>  		/* Skip discard while reshape is happening */
> +		bio_endio_status(bi, BLK_STS_AGAIN);

[Severity: High]
Does completing a blocking bio with BLK_STS_AGAIN violate block layer
semantics?

Since the REQ_NOWAIT bios are filtered out just above this check, this path
is only reached by synchronous, blocking discard bios. Completing them with
BLK_STS_AGAIN will propagate an unexpected -EAGAIN error to synchronous IO
paths and userspace applications.

Block APIs typically handle BLK_STS_NOTSUPP gracefully, but passing -EAGAIN
back through synchronous paths might cause filesystem transaction aborts or
userspace failures. Should this return BLK_STS_NOTSUPP instead?

>  		return;
> +	}
>  
>  	if (!raid5_discard_limits(mddev, bi))
>  		return;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711161326.962336-1-zhanggenjian@126.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] md/raid5: complete discard bios while reshape is active
  2026-07-11 16:27 ` sashiko-bot
@ 2026-07-12 14:35   ` genjian zhang
  0 siblings, 0 replies; 3+ messages in thread
From: genjian zhang @ 2026-07-12 14:35 UTC (permalink / raw)
  To: sashiko-reviews, sashiko-bot; +Cc: linux-raid, yukuai, zhanggenjian

At 2026-07-12 00:27:27, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
>New issues:
>- [High] Completing blocking bios with BLK_STS_AGAIN violates block layer semantics and causes unexpected errors in synchronous paths.
>
>Pre-existing issues:
>- [High] make_discard_request() leaks the bio and causes an uninterruptible hang if REQ_NOWAIT is set.
>--
>
>--- Patch [1]: [PATCH] md/raid5: complete discard bios while reshape is active ---
>commit f117d558ba13364f82645f866dea16210cadce51
>Author: Genjian Zhang <zhanggenjian@kylinos.cn>
>
>md/raid5: complete discard bios while reshape is active
>
>This patch modifies the discard request handling to complete bios with
>BLK_STS_AGAIN when an mdraid reshape is in progress. This prevents callers
>from hanging in uninterruptible sleep waiting for a completion that never
>arrives.
>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -5722,9 +5722,11 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
>>  	if (WARN_ON_ONCE(bi->bi_opf & REQ_NOWAIT))
>>  		return;
>
>[Severity: High]
>This isn't a bug introduced by this patch, but does this early return leak
>the bio and cause an uninterruptible hang if REQ_NOWAIT is set?
>
>If a caller submits a discard bio with REQ_NOWAIT, the driver encounters this
>warning and returns immediately without calling bio_endio(). Because the
>caller assumes the bio was successfully handled, could this lead to a
>resource leak and task hang?

Agreed — the early return without bio_endio() is a pre-existing issue in the same function. This patch intentionally focuses on the reshape case that we reproduced, where blkdiscard hangs in bio_await() even after reshape has finished. I will fix the REQ_NOWAIT path in v2 using bio_wouldblock_error(), consistent with raid5_make_request().

>
>>  
>> -	if (mddev->reshape_position != MaxSector)
>> +	if (mddev->reshape_position != MaxSector) {
>>  		/* Skip discard while reshape is happening */
>> +		bio_endio_status(bi, BLK_STS_AGAIN);
>
>[Severity: High]
>Does completing a blocking bio with BLK_STS_AGAIN violate block layer
>semantics?
>
>Since the REQ_NOWAIT bios are filtered out just above this check, this path
>is only reached by synchronous, blocking discard bios. Completing them with
>BLK_STS_AGAIN will propagate an unexpected -EAGAIN error to synchronous IO
>paths and userspace applications.
>
>Block APIs typically handle BLK_STS_NOTSUPP gracefully, but passing -EAGAIN
>back through synchronous paths might cause filesystem transaction aborts or
>userspace failures. Should this return BLK_STS_NOTSUPP instead?
>

I did not find a status that fits perfectly.

In my view, BLK_STS_NOTSUPP does not fit well here either: discard is supported on the array, just not while reshape is in progress. In addition, blk_ioctl_discard() treats -EOPNOTSUPP as success (err = 0), so returning BLK_STS_NOTSUPP would report success to userspace even though no discard was performed.

BLK_STS_DEV_RESOURCE (-EBUSY) may also be worth considering.

Although the details differ from raid10, I used BLK_STS_AGAIN for the same case ( discard during reshape), consistent with raid10_handle_discard() returning -EAGAIN. The main goal remains completing the bio with a retryable error instead of hanging callers.

>>  		return;
>> +	}
>>  
>>  	if (!raid5_discard_limits(mddev, bi))
>>  		return;
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20260711161326.962336-1-zhanggenjian@126.com?part=1

Suggestions are welcome.

Thanks,
Genjian

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-12 14:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-11 16:13 [PATCH] md/raid5: complete discard bios while reshape is active Genjian
2026-07-11 16:27 ` sashiko-bot
2026-07-12 14:35   ` genjian zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox