linux-kernel - Re: [PATCH v4 5/5] md/raid10: Atomic write support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPhsuW6nJGQMQsEzJFZasg4LuHb3Qf-+JRTgqjaBtbYj_uBNGQ@mail.gmail.com>
Date: Fri, 15 Nov 2024 10:19:44 -0800
From: Song Liu <song@...nel.org>
To: John Garry <john.g.garry@...cle.com>
Cc: axboe@...nel.dk, yukuai3@...wei.com, hch@....de, 
	linux-block@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-raid@...r.kernel.org, martin.petersen@...cle.com
Subject: Re: [PATCH v4 5/5] md/raid10: Atomic write support

On Tue, Nov 12, 2024 at 4:43 AM John Garry <john.g.garry@...cle.com> wrote:
>
> Set BLK_FEAT_ATOMIC_WRITES_STACKED to enable atomic writes.
>
> For an attempt to atomic write to a region which has bad blocks, error
> the write as we just cannot do this. It is unlikely to find devices which
> support atomic writes and bad blocks.
>
> Signed-off-by: John Garry <john.g.garry@...cle.com>
> ---
>  drivers/md/raid10.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 8c7f5daa073a..a3936a67e1e8 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1255,6 +1255,7 @@ static void raid10_write_one_disk(struct mddev *mddev, struct r10bio *r10_bio,
>         const enum req_op op = bio_op(bio);
>         const blk_opf_t do_sync = bio->bi_opf & REQ_SYNC;
>         const blk_opf_t do_fua = bio->bi_opf & REQ_FUA;
> +       const blk_opf_t do_atomic = bio->bi_opf & REQ_ATOMIC;
>         unsigned long flags;
>         struct r10conf *conf = mddev->private;
>         struct md_rdev *rdev;
> @@ -1273,7 +1274,7 @@ static void raid10_write_one_disk(struct mddev *mddev, struct r10bio *r10_bio,
>         mbio->bi_iter.bi_sector = (r10_bio->devs[n_copy].addr +
>                                    choose_data_offset(r10_bio, rdev));
>         mbio->bi_end_io = raid10_end_write_request;
> -       mbio->bi_opf = op | do_sync | do_fua;
> +       mbio->bi_opf = op | do_sync | do_fua | do_atomic;
>         if (!replacement && test_bit(FailFast,
>                                      &conf->mirrors[devnum].rdev->flags)
>                          && enough(conf, devnum))
> @@ -1468,7 +1469,15 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
>                                 continue;
>                         }
>                         if (is_bad) {
> -                               int good_sectors = first_bad - dev_sector;
> +                               int good_sectors;
> +
> +                               if (bio->bi_opf & REQ_ATOMIC) {
> +                                       /* We just cannot atomically write this ... */
> +                                       error = -EFAULT;

Is EFAULT the right error code here? I think we should return something
covered by blk_errors?

Other than this, 4/5 and 5/5 look good to me.

Thanks,
Song