| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <837a4dd4-045f-a956-a241-cab1e8bc20fe@huaweicloud.com>
Date: Wed, 17 Sep 2025 17:24:57 +0800
From: Li Nan <linan666@...weicloud.com>
To: Kenta Akagi <k@...l.me>, Song Liu <song@...nel.org>,
Yu Kuai <yukuai3@...wei.com>, Mariusz Tkaczyk <mtkaczyk@...nel.org>,
Shaohua Li <shli@...com>, Guoqing Jiang <jgq516@...il.com>
Cc: linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 5/9] md/raid1,raid10: Set R{1,10}BIO_Uptodate when
successful retry of a failed bio
在 2025/9/15 11:42, Kenta Akagi 写道:
> In the current implementation, when a write bio fails, the retry flow
> is as follows:
> * In bi_end_io, e.g. raid1_end_write_request, R1BIO_WriteError is
> set on the r1bio.
> * The md thread calls handle_write_finished corresponding to this r1bio.
> * Inside handle_write_finished, narrow_write_error is invoked.
> * narrow_write_error rewrites the r1bio on a per-sector basis, marking
> any failed sectors as badblocks. It returns true if all sectors succeed,
> or if failed sectors are successfully recorded via rdev_set_badblock.
> It returns false if rdev_set_badblock fails or if badblocks are disabled.
> * handle_write_finished faults the rdev if it receives false from
> narrow_write_error. Otherwise, it does nothing.
>
> This can cause a problem where an r1bio that succeeded on retry is
> incorrectly reported as failed to the higher layer, for example in the
> following case:
> * Only one In_sync rdev exists, and
> * The write bio initially failed but all retries in
> narrow_write_error succeed.
>
> This commit ensures that if a write initially fails but all retries in
> narrow_write_error succeed, R1BIO_Uptodate or R10BIO_Uptodate is set
> and the higher layer receives a successful write status.
>
> Signed-off-by: Kenta Akagi <k@...l.me>
> ---
> drivers/md/raid1.c | 32 ++++++++++++++++++++++++++------
> drivers/md/raid10.c | 21 +++++++++++++++++++++
> 2 files changed, 47 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 8fff9dacc6e0..806f5cb33a8e 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -2517,6 +2517,21 @@ static void fix_read_error(struct r1conf *conf, struct r1bio *r1_bio)
> }
> }
>
> +/**
> + * narrow_write_error() - Retry write and set badblock
> + * @r1_bio: the r1bio containing the write error
> + * @i: which device to retry
> + *
> + * Rewrites the bio, splitting it at the least common multiple of the logical
> + * block size and the badblock size. Blocks that fail to be written are marked
> + * as bad. If badblocks are disabled, no write is attempted and false is
> + * returned immediately.
> + *
> + * Return:
> + * * %true - all blocks were written or marked bad successfully
> + * * %false - bbl disabled or
> + * one or more blocks write failed and could not be marked bad
> + */
> static bool narrow_write_error(struct r1bio *r1_bio, int i)
> {
> struct mddev *mddev = r1_bio->mddev;
> @@ -2614,9 +2629,9 @@ static void handle_write_finished(struct r1conf *conf, struct r1bio *r1_bio)
> int m, idx;
> bool fail = false;
>
> - for (m = 0; m < conf->raid_disks * 2 ; m++)
> + for (m = 0; m < conf->raid_disks * 2 ; m++) {
> + struct md_rdev *rdev = conf->mirrors[m].rdev;
> if (r1_bio->bios[m] == IO_MADE_GOOD) {
> - struct md_rdev *rdev = conf->mirrors[m].rdev;
> rdev_clear_badblocks(rdev,
> r1_bio->sector,
> r1_bio->sectors, 0);
> @@ -2628,12 +2643,17 @@ static void handle_write_finished(struct r1conf *conf, struct r1bio *r1_bio)
> */
> fail = true;
'fail' should be false when re-write is successful.
> if (!narrow_write_error(r1_bio, m))
> - md_error(conf->mddev,
> - conf->mirrors[m].rdev);
> + md_error(conf->mddev, rdev);
> /* an I/O failed, we can't clear the bitmap */
> - rdev_dec_pending(conf->mirrors[m].rdev,
> - conf->mddev);
> + else if (test_bit(In_sync, &rdev->flags) &&
> + !test_bit(Faulty, &rdev->flags) &&
> + rdev_has_badblock(rdev,
> + r1_bio->sector,
> + r1_bio->sectors) == 0)
Clear badblock and set R10BIO_Uptodate if rdev has badblock.
> + set_bit(R1BIO_Uptodate, &r1_bio->state);
> + rdev_dec_pending(rdev, conf->mddev);
> }
> + }
> if (fail) {
> spin_lock_irq(&conf->device_lock);
> list_add(&r1_bio->retry_list, &conf->bio_end_io_list);
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index b73af94a88b0..21c2821453e1 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -2809,6 +2809,21 @@ static void fix_read_error(struct r10conf *conf, struct mddev *mddev, struct r10
> }
> }
>
> +/**
> + * narrow_write_error() - Retry write and set badblock
> + * @r10_bio: the r10bio containing the write error
> + * @i: which device to retry
> + *
> + * Rewrites the bio, splitting it at the least common multiple of the logical
> + * block size and the badblock size. Blocks that fail to be written are marked
> + * as bad. If badblocks are disabled, no write is attempted and false is
> + * returned immediately.
> + *
> + * Return:
> + * * %true - all blocks were written or marked bad successfully
> + * * %false - bbl disabled or
> + * one or more blocks write failed and could not be marked bad
> + */
> static bool narrow_write_error(struct r10bio *r10_bio, int i)
> {
> struct bio *bio = r10_bio->master_bio;
> @@ -2975,6 +2990,12 @@ static void handle_write_completed(struct r10conf *conf, struct r10bio *r10_bio)
> fail = true;
> if (!narrow_write_error(r10_bio, m))
> md_error(conf->mddev, rdev);
> + else if (test_bit(In_sync, &rdev->flags) &&
> + !test_bit(Faulty, &rdev->flags) &&
> + rdev_has_badblock(rdev,
> + r10_bio->devs[m].addr,
> + r10_bio->sectors) == 0)
Same as raid1.
> + set_bit(R10BIO_Uptodate, &r10_bio->state);
> rdev_dec_pending(rdev, conf->mddev);
> }
> bio = r10_bio->devs[m].repl_bio;
--
Thanks,
Nan
Powered by blists - more mailing lists