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: <CALTww28iG8rUWNkYLcHkAM0o0k-9F_hjvZhBx=4VW=fhugkOsA@mail.gmail.com>
Date: Fri, 22 Nov 2024 10:16:29 +0800
From: Xiao Ni <xni@...hat.com>
To: Yu Kuai <yukuai1@...weicloud.com>
Cc: song@...nel.org, linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org, 
	yukuai3@...wei.com, yi.zhang@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH md-6.13 1/5] md/md-bitmap: factor behind write counters
 out from bitmap_{start/end}write()

On Mon, Nov 18, 2024 at 7:44 PM Yu Kuai <yukuai1@...weicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@...wei.com>
>
> behind_write is only used in raid1, prepare to refactor
> bitmap_{start/end}write(), there are no functional changes.
>
> Signed-off-by: Yu Kuai <yukuai3@...wei.com>
> ---
>  drivers/md/md-bitmap.c   | 57 +++++++++++++++++++++++++---------------
>  drivers/md/md-bitmap.h   |  7 +++--
>  drivers/md/raid1.c       | 12 +++++----
>  drivers/md/raid10.c      |  6 ++---
>  drivers/md/raid5-cache.c |  3 +--
>  drivers/md/raid5.c       | 11 ++++----
>  6 files changed, 56 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index c3a42dd66ce5..84fb4cc67d5e 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -1671,24 +1671,13 @@ __acquires(bitmap->lock)
>  }
>
>  static int bitmap_startwrite(struct mddev *mddev, sector_t offset,
> -                            unsigned long sectors, bool behind)
> +                            unsigned long sectors)
>  {
>         struct bitmap *bitmap = mddev->bitmap;
>
>         if (!bitmap)
>                 return 0;
>
> -       if (behind) {
> -               int bw;
> -               atomic_inc(&bitmap->behind_writes);
> -               bw = atomic_read(&bitmap->behind_writes);
> -               if (bw > bitmap->behind_writes_used)
> -                       bitmap->behind_writes_used = bw;
> -
> -               pr_debug("inc write-behind count %d/%lu\n",
> -                        bw, bitmap->mddev->bitmap_info.max_write_behind);
> -       }
> -
>         while (sectors) {
>                 sector_t blocks;
>                 bitmap_counter_t *bmc;
> @@ -1737,21 +1726,13 @@ static int bitmap_startwrite(struct mddev *mddev, sector_t offset,
>  }
>
>  static void bitmap_endwrite(struct mddev *mddev, sector_t offset,
> -                           unsigned long sectors, bool success, bool behind)
> +                           unsigned long sectors, bool success)
>  {
>         struct bitmap *bitmap = mddev->bitmap;
>
>         if (!bitmap)
>                 return;
>
> -       if (behind) {
> -               if (atomic_dec_and_test(&bitmap->behind_writes))
> -                       wake_up(&bitmap->behind_wait);
> -               pr_debug("dec write-behind count %d/%lu\n",
> -                        atomic_read(&bitmap->behind_writes),
> -                        bitmap->mddev->bitmap_info.max_write_behind);
> -       }
> -
>         while (sectors) {
>                 sector_t blocks;
>                 unsigned long flags;
> @@ -2062,6 +2043,37 @@ static void md_bitmap_free(void *data)
>         kfree(bitmap);
>  }
>
> +static void bitmap_start_behind_write(struct mddev *mddev)
> +{
> +       struct bitmap *bitmap = mddev->bitmap;
> +       int bw;
> +
> +       if (!bitmap)
> +               return;
> +
> +       atomic_inc(&bitmap->behind_writes);
> +       bw = atomic_read(&bitmap->behind_writes);
> +       if (bw > bitmap->behind_writes_used)
> +               bitmap->behind_writes_used = bw;
> +
> +       pr_debug("inc write-behind count %d/%lu\n",
> +                bw, bitmap->mddev->bitmap_info.max_write_behind);
> +}
> +
> +static void bitmap_end_behind_write(struct mddev *mddev)
> +{
> +       struct bitmap *bitmap = mddev->bitmap;
> +
> +       if (!bitmap)
> +               return;
> +
> +       if (atomic_dec_and_test(&bitmap->behind_writes))
> +               wake_up(&bitmap->behind_wait);
> +       pr_debug("dec write-behind count %d/%lu\n",
> +                atomic_read(&bitmap->behind_writes),
> +                bitmap->mddev->bitmap_info.max_write_behind);
> +}
> +
>  static void bitmap_wait_behind_writes(struct mddev *mddev)
>  {
>         struct bitmap *bitmap = mddev->bitmap;
> @@ -2981,6 +2993,9 @@ static struct bitmap_operations bitmap_ops = {
>         .dirty_bits             = bitmap_dirty_bits,
>         .unplug                 = bitmap_unplug,
>         .daemon_work            = bitmap_daemon_work,
> +
> +       .start_behind_write     = bitmap_start_behind_write,
> +       .end_behind_write       = bitmap_end_behind_write,
>         .wait_behind_writes     = bitmap_wait_behind_writes,
>
>         .startwrite             = bitmap_startwrite,
> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
> index 662e6fc141a7..e87a1f493d3c 100644
> --- a/drivers/md/md-bitmap.h
> +++ b/drivers/md/md-bitmap.h
> @@ -84,12 +84,15 @@ struct bitmap_operations {
>                            unsigned long e);
>         void (*unplug)(struct mddev *mddev, bool sync);
>         void (*daemon_work)(struct mddev *mddev);
> +
> +       void (*start_behind_write)(struct mddev *mddev);
> +       void (*end_behind_write)(struct mddev *mddev);
>         void (*wait_behind_writes)(struct mddev *mddev);
>
>         int (*startwrite)(struct mddev *mddev, sector_t offset,
> -                         unsigned long sectors, bool behind);
> +                         unsigned long sectors);
>         void (*endwrite)(struct mddev *mddev, sector_t offset,
> -                        unsigned long sectors, bool success, bool behind);
> +                        unsigned long sectors, bool success);
>         bool (*start_sync)(struct mddev *mddev, sector_t offset,
>                            sector_t *blocks, bool degraded);
>         void (*end_sync)(struct mddev *mddev, sector_t offset, sector_t *blocks);
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index cd3e94dceabc..e6e1228d9b3a 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -420,10 +420,11 @@ static void close_write(struct r1bio *r1_bio)
>                 r1_bio->behind_master_bio = NULL;
>         }
>
> +       if (test_bit(R1BIO_BehindIO, &r1_bio->state))
> +               mddev->bitmap_ops->end_behind_write(mddev);
>         /* clear the bitmap if all writes complete successfully */
>         mddev->bitmap_ops->endwrite(mddev, r1_bio->sector, r1_bio->sectors,
> -                                   !test_bit(R1BIO_Degraded, &r1_bio->state),
> -                                   test_bit(R1BIO_BehindIO, &r1_bio->state));
> +                                   !test_bit(R1BIO_Degraded, &r1_bio->state));
>         md_write_end(mddev);
>  }
>
> @@ -1614,9 +1615,10 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>                             stats.behind_writes < max_write_behind)
>                                 alloc_behind_master_bio(r1_bio, bio);
>
> -                       mddev->bitmap_ops->startwrite(
> -                               mddev, r1_bio->sector, r1_bio->sectors,
> -                               test_bit(R1BIO_BehindIO, &r1_bio->state));
> +                       if (test_bit(R1BIO_BehindIO, &r1_bio->state))
> +                               mddev->bitmap_ops->start_behind_write(mddev);
> +                       mddev->bitmap_ops->startwrite(mddev, r1_bio->sector,
> +                                                     r1_bio->sectors);
>                         first_clone = 0;
>                 }
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index ff73db2f6c41..61ac074e821f 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -430,8 +430,7 @@ static void close_write(struct r10bio *r10_bio)
>
>         /* clear the bitmap if all writes complete successfully */
>         mddev->bitmap_ops->endwrite(mddev, r10_bio->sector, r10_bio->sectors,
> -                                   !test_bit(R10BIO_Degraded, &r10_bio->state),
> -                                   false);
> +                                   !test_bit(R10BIO_Degraded, &r10_bio->state));
>         md_write_end(mddev);
>  }
>
> @@ -1489,8 +1488,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
>         md_account_bio(mddev, &bio);
>         r10_bio->master_bio = bio;
>         atomic_set(&r10_bio->remaining, 1);
> -       mddev->bitmap_ops->startwrite(mddev, r10_bio->sector, r10_bio->sectors,
> -                                     false);
> +       mddev->bitmap_ops->startwrite(mddev, r10_bio->sector, r10_bio->sectors);
>
>         for (i = 0; i < conf->copies; i++) {
>                 if (r10_bio->devs[i].bio)
> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
> index b4f7b79fd187..4c7ecdd5c1f3 100644
> --- a/drivers/md/raid5-cache.c
> +++ b/drivers/md/raid5-cache.c
> @@ -315,8 +315,7 @@ void r5c_handle_cached_data_endio(struct r5conf *conf,
>                         r5c_return_dev_pending_writes(conf, &sh->dev[i]);
>                         conf->mddev->bitmap_ops->endwrite(conf->mddev,
>                                         sh->sector, RAID5_STRIPE_SECTORS(conf),
> -                                       !test_bit(STRIPE_DEGRADED, &sh->state),
> -                                       false);
> +                                       !test_bit(STRIPE_DEGRADED, &sh->state));
>                 }
>         }
>  }
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index f5ac81dd21b2..84f3d27e0fa4 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3564,7 +3564,7 @@ static void __add_stripe_bio(struct stripe_head *sh, struct bio *bi,
>                 set_bit(STRIPE_BITMAP_PENDING, &sh->state);
>                 spin_unlock_irq(&sh->stripe_lock);
>                 conf->mddev->bitmap_ops->startwrite(conf->mddev, sh->sector,
> -                                       RAID5_STRIPE_SECTORS(conf), false);
> +                                       RAID5_STRIPE_SECTORS(conf));
>                 spin_lock_irq(&sh->stripe_lock);
>                 clear_bit(STRIPE_BITMAP_PENDING, &sh->state);
>                 if (!sh->batch_head) {
> @@ -3665,7 +3665,7 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
>                 if (bitmap_end)
>                         conf->mddev->bitmap_ops->endwrite(conf->mddev,
>                                         sh->sector, RAID5_STRIPE_SECTORS(conf),
> -                                       false, false);
> +                                       false);
>                 bitmap_end = 0;
>                 /* and fail all 'written' */
>                 bi = sh->dev[i].written;
> @@ -3712,7 +3712,7 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
>                 if (bitmap_end)
>                         conf->mddev->bitmap_ops->endwrite(conf->mddev,
>                                         sh->sector, RAID5_STRIPE_SECTORS(conf),
> -                                       false, false);
> +                                       false);
>                 /* If we were in the middle of a write the parity block might
>                  * still be locked - so just clear all R5_LOCKED flags
>                  */
> @@ -4063,8 +4063,7 @@ static void handle_stripe_clean_event(struct r5conf *conf,
>                                 }
>                                 conf->mddev->bitmap_ops->endwrite(conf->mddev,
>                                         sh->sector, RAID5_STRIPE_SECTORS(conf),
> -                                       !test_bit(STRIPE_DEGRADED, &sh->state),
> -                                       false);
> +                                       !test_bit(STRIPE_DEGRADED, &sh->state));
>                                 if (head_sh->batch_head) {
>                                         sh = list_first_entry(&sh->batch_list,
>                                                               struct stripe_head,
> @@ -5787,7 +5786,7 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
>                         for (d = 0; d < conf->raid_disks - conf->max_degraded;
>                              d++)
>                                 mddev->bitmap_ops->startwrite(mddev, sh->sector,
> -                                       RAID5_STRIPE_SECTORS(conf), false);
> +                                       RAID5_STRIPE_SECTORS(conf));
>                         sh->bm_seq = conf->seq_flush + 1;
>                         set_bit(STRIPE_BIT_DELAY, &sh->state);
>                 }
> --
> 2.39.2
>

Reviewed-by: Xiao Ni <xni@...hat.com>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ