[<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