* [PATCH md-6.13 1/5] md/md-bitmap: factor behind write counters out from bitmap_{start/end}write()
2024-11-18 11:41 [PATCH md-6.13 0/5] md/md-bitmap: move bitmap_{start,end}write to md upper layer Yu Kuai
@ 2024-11-18 11:41 ` Yu Kuai
2024-11-22 2:16 ` Xiao Ni
2024-11-18 11:41 ` [PATCH md-6.13 2/5] md/md-bitmap: remove the last parameter for bimtap_ops->endwrite() Yu Kuai
` (3 subsequent siblings)
4 siblings, 1 reply; 28+ messages in thread
From: Yu Kuai @ 2024-11-18 11:41 UTC (permalink / raw)
To: song, xni; +Cc: linux-raid, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun
From: Yu Kuai <yukuai3@huawei.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@huawei.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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH md-6.13 1/5] md/md-bitmap: factor behind write counters out from bitmap_{start/end}write()
2024-11-18 11:41 ` [PATCH md-6.13 1/5] md/md-bitmap: factor behind write counters out from bitmap_{start/end}write() Yu Kuai
@ 2024-11-22 2:16 ` Xiao Ni
0 siblings, 0 replies; 28+ messages in thread
From: Xiao Ni @ 2024-11-22 2:16 UTC (permalink / raw)
To: Yu Kuai; +Cc: song, linux-raid, linux-kernel, yukuai3, yi.zhang, yangerkun
On Mon, Nov 18, 2024 at 7:44 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@huawei.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@huawei.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@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH md-6.13 2/5] md/md-bitmap: remove the last parameter for bimtap_ops->endwrite()
2024-11-18 11:41 [PATCH md-6.13 0/5] md/md-bitmap: move bitmap_{start,end}write to md upper layer Yu Kuai
2024-11-18 11:41 ` [PATCH md-6.13 1/5] md/md-bitmap: factor behind write counters out from bitmap_{start/end}write() Yu Kuai
@ 2024-11-18 11:41 ` Yu Kuai
2024-11-18 11:41 ` [PATCH md-6.13 3/5] md: add a new callback pers->bitmap_sector() Yu Kuai
` (2 subsequent siblings)
4 siblings, 0 replies; 28+ messages in thread
From: Yu Kuai @ 2024-11-18 11:41 UTC (permalink / raw)
To: song, xni; +Cc: linux-raid, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun
From: Yu Kuai <yukuai3@huawei.com>
It is useless, because for the case IO failed for one rdev:
- If badblocks is set and rdev is not faulty, there is no need to
mark the bit as NEEDED;
- If rdev is faulty, then mddev->degraded will be set, and we can use
it to mard the bit as NEEDED;
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/md/md-bitmap.c | 19 ++++++++++---------
drivers/md/md-bitmap.h | 2 +-
drivers/md/raid1.c | 3 +--
drivers/md/raid10.c | 3 +--
drivers/md/raid5-cache.c | 3 +--
drivers/md/raid5.c | 9 +++------
6 files changed, 17 insertions(+), 22 deletions(-)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 84fb4cc67d5e..b40a84b01085 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -1726,7 +1726,7 @@ 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)
+ unsigned long sectors)
{
struct bitmap *bitmap = mddev->bitmap;
@@ -1745,15 +1745,16 @@ static void bitmap_endwrite(struct mddev *mddev, sector_t offset,
return;
}
- if (success && !bitmap->mddev->degraded &&
- bitmap->events_cleared < bitmap->mddev->events) {
- bitmap->events_cleared = bitmap->mddev->events;
- bitmap->need_sync = 1;
- sysfs_notify_dirent_safe(bitmap->sysfs_can_clear);
- }
-
- if (!success && !NEEDED(*bmc))
+ if (!bitmap->mddev->degraded) {
+ if (bitmap->events_cleared < bitmap->mddev->events) {
+ bitmap->events_cleared = bitmap->mddev->events;
+ bitmap->need_sync = 1;
+ sysfs_notify_dirent_safe(
+ bitmap->sysfs_can_clear);
+ }
+ } else if (!NEEDED(*bmc)) {
*bmc |= NEEDED_MASK;
+ }
if (COUNTER(*bmc) == COUNTER_MAX)
wake_up(&bitmap->overflow_wait);
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index e87a1f493d3c..31c93019c76b 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -92,7 +92,7 @@ struct bitmap_operations {
int (*startwrite)(struct mddev *mddev, sector_t offset,
unsigned long sectors);
void (*endwrite)(struct mddev *mddev, sector_t offset,
- unsigned long sectors, bool success);
+ unsigned long sectors);
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 e6e1228d9b3a..b9819f9c8ed2 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -423,8 +423,7 @@ static void close_write(struct r1bio *r1_bio)
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));
+ mddev->bitmap_ops->endwrite(mddev, r1_bio->sector, r1_bio->sectors);
md_write_end(mddev);
}
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 61ac074e821f..efbc0bd92479 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -429,8 +429,7 @@ static void close_write(struct r10bio *r10_bio)
struct mddev *mddev = r10_bio->mddev;
/* 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));
+ mddev->bitmap_ops->endwrite(mddev, r10_bio->sector, r10_bio->sectors);
md_write_end(mddev);
}
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 4c7ecdd5c1f3..ba4f9577c737 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -314,8 +314,7 @@ void r5c_handle_cached_data_endio(struct r5conf *conf,
set_bit(R5_UPTODATE, &sh->dev[i].flags);
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));
+ sh->sector, RAID5_STRIPE_SECTORS(conf));
}
}
}
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 84f3d27e0fa4..2f10cca4b7b1 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3664,8 +3664,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);
+ sh->sector, RAID5_STRIPE_SECTORS(conf));
bitmap_end = 0;
/* and fail all 'written' */
bi = sh->dev[i].written;
@@ -3711,8 +3710,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);
+ sh->sector, RAID5_STRIPE_SECTORS(conf));
/* If we were in the middle of a write the parity block might
* still be locked - so just clear all R5_LOCKED flags
*/
@@ -4062,8 +4060,7 @@ static void handle_stripe_clean_event(struct r5conf *conf,
wbi = wbi2;
}
conf->mddev->bitmap_ops->endwrite(conf->mddev,
- sh->sector, RAID5_STRIPE_SECTORS(conf),
- !test_bit(STRIPE_DEGRADED, &sh->state));
+ sh->sector, RAID5_STRIPE_SECTORS(conf));
if (head_sh->batch_head) {
sh = list_first_entry(&sh->batch_list,
struct stripe_head,
--
2.39.2
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH md-6.13 3/5] md: add a new callback pers->bitmap_sector()
2024-11-18 11:41 [PATCH md-6.13 0/5] md/md-bitmap: move bitmap_{start,end}write to md upper layer Yu Kuai
2024-11-18 11:41 ` [PATCH md-6.13 1/5] md/md-bitmap: factor behind write counters out from bitmap_{start/end}write() Yu Kuai
2024-11-18 11:41 ` [PATCH md-6.13 2/5] md/md-bitmap: remove the last parameter for bimtap_ops->endwrite() Yu Kuai
@ 2024-11-18 11:41 ` Yu Kuai
2024-11-22 2:17 ` Xiao Ni
2024-11-18 11:41 ` [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector() Yu Kuai
2024-11-18 11:41 ` [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer Yu Kuai
4 siblings, 1 reply; 28+ messages in thread
From: Yu Kuai @ 2024-11-18 11:41 UTC (permalink / raw)
To: song, xni; +Cc: linux-raid, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun
From: Yu Kuai <yukuai3@huawei.com>
This callback will be used in raid5 to convert io ranges from array to
bitmap.
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/md/md.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 4ba93af36126..de6dadb9a40b 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -746,6 +746,9 @@ struct md_personality
void *(*takeover) (struct mddev *mddev);
/* Changes the consistency policy of an active array. */
int (*change_consistency_policy)(struct mddev *mddev, const char *buf);
+ /* convert io ranges from array to bitmap */
+ void (*bitmap_sector)(struct mddev *mddev, sector_t *offset,
+ unsigned long *sectors);
};
struct md_sysfs_entry {
--
2.39.2
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH md-6.13 3/5] md: add a new callback pers->bitmap_sector()
2024-11-18 11:41 ` [PATCH md-6.13 3/5] md: add a new callback pers->bitmap_sector() Yu Kuai
@ 2024-11-22 2:17 ` Xiao Ni
0 siblings, 0 replies; 28+ messages in thread
From: Xiao Ni @ 2024-11-22 2:17 UTC (permalink / raw)
To: Yu Kuai; +Cc: song, linux-raid, linux-kernel, yukuai3, yi.zhang, yangerkun
On Mon, Nov 18, 2024 at 7:44 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@huawei.com>
>
> This callback will be used in raid5 to convert io ranges from array to
> bitmap.
>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
> drivers/md/md.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 4ba93af36126..de6dadb9a40b 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -746,6 +746,9 @@ struct md_personality
> void *(*takeover) (struct mddev *mddev);
> /* Changes the consistency policy of an active array. */
> int (*change_consistency_policy)(struct mddev *mddev, const char *buf);
> + /* convert io ranges from array to bitmap */
> + void (*bitmap_sector)(struct mddev *mddev, sector_t *offset,
> + unsigned long *sectors);
> };
>
> struct md_sysfs_entry {
> --
> 2.39.2
>
Reviewed-by: Xiao Ni <xni@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-18 11:41 [PATCH md-6.13 0/5] md/md-bitmap: move bitmap_{start,end}write to md upper layer Yu Kuai
` (2 preceding siblings ...)
2024-11-18 11:41 ` [PATCH md-6.13 3/5] md: add a new callback pers->bitmap_sector() Yu Kuai
@ 2024-11-18 11:41 ` Yu Kuai
2024-11-18 11:41 ` [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer Yu Kuai
4 siblings, 0 replies; 28+ messages in thread
From: Yu Kuai @ 2024-11-18 11:41 UTC (permalink / raw)
To: song, xni; +Cc: linux-raid, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun
From: Yu Kuai <yukuai3@huawei.com>
Bitmap is used for the whole array for raid1/raid10, hence IO for the
array can be used directly for bitmap. However, bitmap is used for
underlying disks for raid5, hence IO for the array can't be used
directly for bitmap.
Implement pers->bitmap_sector() for raid5 to convert IO ranges from the
array to the underlying disks.
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/md/raid5.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 2f10cca4b7b1..95caed41654c 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2950,6 +2950,23 @@ static void raid5_error(struct mddev *mddev, struct md_rdev *rdev)
r5c_update_on_rdev_error(mddev, rdev);
}
+static void raid5_bitmap_sector(struct mddev *mddev, sector_t *offset,
+ unsigned long *sectors)
+{
+ struct r5conf *conf = mddev->private;
+ int sectors_per_chunk = conf->chunk_sectors * (conf->raid_disks -
+ conf->max_degraded);
+ sector_t start = *offset;
+ sector_t end = start + *sectors;
+ int dd_idx;
+
+ start = round_down(start, sectors_per_chunk);
+ end = round_up(end, sectors_per_chunk);
+
+ *offset = raid5_compute_sector(conf, start, 0, &dd_idx, NULL);
+ *sectors = raid5_compute_sector(conf, end, 0, &dd_idx, NULL) - *offset;
+}
+
/*
* Input: a 'big' sector number,
* Output: index of the data and parity disk, and the sector # in them.
@@ -8968,6 +8985,7 @@ static struct md_personality raid6_personality =
.takeover = raid6_takeover,
.change_consistency_policy = raid5_change_consistency_policy,
.prepare_suspend = raid5_prepare_suspend,
+ .bitmap_sector = raid5_bitmap_sector,
};
static struct md_personality raid5_personality =
{
@@ -8993,6 +9011,7 @@ static struct md_personality raid5_personality =
.takeover = raid5_takeover,
.change_consistency_policy = raid5_change_consistency_policy,
.prepare_suspend = raid5_prepare_suspend,
+ .bitmap_sector = raid5_bitmap_sector,
};
static struct md_personality raid4_personality =
@@ -9019,6 +9038,7 @@ static struct md_personality raid4_personality =
.takeover = raid4_takeover,
.change_consistency_policy = raid5_change_consistency_policy,
.prepare_suspend = raid5_prepare_suspend,
+ .bitmap_sector = raid5_bitmap_sector,
};
static int __init raid5_init(void)
--
2.39.2
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer
2024-11-18 11:41 [PATCH md-6.13 0/5] md/md-bitmap: move bitmap_{start,end}write to md upper layer Yu Kuai
` (3 preceding siblings ...)
2024-11-18 11:41 ` [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector() Yu Kuai
@ 2024-11-18 11:41 ` Yu Kuai
2024-11-19 9:49 ` [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector() Jack Wang
2024-11-22 2:06 ` [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer Xiao Ni
4 siblings, 2 replies; 28+ messages in thread
From: Yu Kuai @ 2024-11-18 11:41 UTC (permalink / raw)
To: song, xni; +Cc: linux-raid, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun
From: Yu Kuai <yukuai3@huawei.com>
There are two BUG reports that raid5 will hang at
bitmap_startwrite([1],[2]), root cause is that bitmap start write and end
write is unbalanced, and while reviewing raid5 code, it's found that
bitmap operations can be optimized. For example, for a 4 disks raid5, with
chunksize=8k, if user issue a IO (0 + 48k) to the array:
┌────────────────────────────────────────────────────────────┐
│chunk 0 │
│ ┌────────────┬─────────────┬─────────────┬────────────┼
│ sh0 │A0: 0 + 4k │A1: 8k + 4k │A2: 16k + 4k │A3: P │
│ ┼────────────┼─────────────┼─────────────┼────────────┼
│ sh1 │B0: 4k + 4k │B1: 12k + 4k │B2: 20k + 4k │B3: P │
┼──────┴────────────┴─────────────┴─────────────┴────────────┼
│chunk 1 │
│ ┌────────────┬─────────────┬─────────────┬────────────┤
│ sh2 │C0: 24k + 4k│C1: 32k + 4k │C2: P │C3: 40k + 4k│
│ ┼────────────┼─────────────┼─────────────┼────────────┼
│ sh3 │D0: 28k + 4k│D1: 36k + 4k │D2: P │D3: 44k + 4k│
└──────┴────────────┴─────────────┴─────────────┴────────────┘
Before this patch, 4 stripe head will be used, and each sh will attach
bio for 3 disks, and each attached bio will trigger
bitmap_startwrite() once, which means total 12 times.
- 3 times (0 + 4k), for (A0, A1 and A2)
- 3 times (4 + 4k), for (B0, B1 and B2)
- 3 times (8 + 4k), for (C0, C1 and C3)
- 3 times (12 + 4k), for (D0, D1 and D3)
After this patch, md upper layer will calculate that IO range (0 + 48k)
is corresponding to the bitmap (0 + 16k), and call bitmap_startwrite()
just once.
Noted that this patch will align bitmap ranges to the chunks, for example,
if user issue a IO (0 + 4k) to array:
- Before this patch, 1 time (0 + 4k), for A0;
- After this patch, 1 time (0 + 8k) for chunk 0;
Usually, one bitmap bit will represent more than one disk chunk, and this
doesn't have any difference. And even if user really created a array
that one chunk contain multiple bits, the overhead is that more data
will be recovered after power failure.
[1] https://lore.kernel.org/all/CAJpMwyjmHQLvm6zg1cmQErttNNQPDAAXPKM3xgTjMhbfts986Q@mail.gmail.com/
[2] https://lore.kernel.org/all/ADF7D720-5764-4AF3-B68E-1845988737AA@flyingcircus.io/
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/md/md.c | 29 +++++++++++++++++++++++++++++
drivers/md/md.h | 2 ++
drivers/md/raid1.c | 4 ----
drivers/md/raid10.c | 3 ---
drivers/md/raid5-cache.c | 2 --
drivers/md/raid5.c | 24 +-----------------------
6 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index bbe002ebd584..ab37c9939ac6 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8723,12 +8723,32 @@ void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
}
EXPORT_SYMBOL_GPL(md_submit_discard_bio);
+static void md_bitmap_start(struct mddev *mddev,
+ struct md_io_clone *md_io_clone)
+{
+ if (mddev->pers->bitmap_sector)
+ mddev->pers->bitmap_sector(mddev, &md_io_clone->offset,
+ &md_io_clone->sectors);
+
+ mddev->bitmap_ops->startwrite(mddev, md_io_clone->offset,
+ md_io_clone->sectors);
+}
+
+static void md_bitmap_end(struct mddev *mddev, struct md_io_clone *md_io_clone)
+{
+ mddev->bitmap_ops->endwrite(mddev, md_io_clone->offset,
+ md_io_clone->sectors);
+}
+
static void md_end_clone_io(struct bio *bio)
{
struct md_io_clone *md_io_clone = bio->bi_private;
struct bio *orig_bio = md_io_clone->orig_bio;
struct mddev *mddev = md_io_clone->mddev;
+ if (bio_data_dir(orig_bio) == WRITE && mddev->bitmap)
+ md_bitmap_end(mddev, md_io_clone);
+
if (bio->bi_status && !orig_bio->bi_status)
orig_bio->bi_status = bio->bi_status;
@@ -8753,6 +8773,12 @@ static void md_clone_bio(struct mddev *mddev, struct bio **bio)
if (blk_queue_io_stat(bdev->bd_disk->queue))
md_io_clone->start_time = bio_start_io_acct(*bio);
+ if (bio_data_dir(*bio) == WRITE && mddev->bitmap) {
+ md_io_clone->offset = (*bio)->bi_iter.bi_sector;
+ md_io_clone->sectors = bio_sectors(*bio);
+ md_bitmap_start(mddev, md_io_clone);
+ }
+
clone->bi_end_io = md_end_clone_io;
clone->bi_private = md_io_clone;
*bio = clone;
@@ -8771,6 +8797,9 @@ void md_free_cloned_bio(struct bio *bio)
struct bio *orig_bio = md_io_clone->orig_bio;
struct mddev *mddev = md_io_clone->mddev;
+ if (bio_data_dir(orig_bio) == WRITE && mddev->bitmap)
+ md_bitmap_end(mddev, md_io_clone);
+
if (bio->bi_status && !orig_bio->bi_status)
orig_bio->bi_status = bio->bi_status;
diff --git a/drivers/md/md.h b/drivers/md/md.h
index de6dadb9a40b..def808064ad8 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -831,6 +831,8 @@ struct md_io_clone {
struct mddev *mddev;
struct bio *orig_bio;
unsigned long start_time;
+ sector_t offset;
+ unsigned long sectors;
struct bio bio_clone;
};
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index b9819f9c8ed2..e2adfeff5ae6 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -422,8 +422,6 @@ static void close_write(struct r1bio *r1_bio)
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);
md_write_end(mddev);
}
@@ -1616,8 +1614,6 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
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 efbc0bd92479..79a209236c9f 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -428,8 +428,6 @@ static void close_write(struct r10bio *r10_bio)
{
struct mddev *mddev = r10_bio->mddev;
- /* clear the bitmap if all writes complete successfully */
- mddev->bitmap_ops->endwrite(mddev, r10_bio->sector, r10_bio->sectors);
md_write_end(mddev);
}
@@ -1487,7 +1485,6 @@ 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);
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 ba4f9577c737..011246e16a99 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -313,8 +313,6 @@ void r5c_handle_cached_data_endio(struct r5conf *conf,
if (sh->dev[i].written) {
set_bit(R5_UPTODATE, &sh->dev[i].flags);
r5c_return_dev_pending_writes(conf, &sh->dev[i]);
- conf->mddev->bitmap_ops->endwrite(conf->mddev,
- sh->sector, RAID5_STRIPE_SECTORS(conf));
}
}
}
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 95caed41654c..976788138a6e 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3578,12 +3578,6 @@ static void __add_stripe_bio(struct stripe_head *sh, struct bio *bi,
* is added to a batch, STRIPE_BIT_DELAY cannot be changed
* any more.
*/
- 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));
- spin_lock_irq(&sh->stripe_lock);
- clear_bit(STRIPE_BITMAP_PENDING, &sh->state);
if (!sh->batch_head) {
sh->bm_seq = conf->seq_flush+1;
set_bit(STRIPE_BIT_DELAY, &sh->state);
@@ -3638,7 +3632,6 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
BUG_ON(sh->batch_head);
for (i = disks; i--; ) {
struct bio *bi;
- int bitmap_end = 0;
if (test_bit(R5_ReadError, &sh->dev[i].flags)) {
struct md_rdev *rdev = conf->disks[i].rdev;
@@ -3663,8 +3656,6 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
sh->dev[i].towrite = NULL;
sh->overwrite_disks = 0;
spin_unlock_irq(&sh->stripe_lock);
- if (bi)
- bitmap_end = 1;
log_stripe_write_finished(sh);
@@ -3679,10 +3670,6 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
bio_io_error(bi);
bi = nextbi;
}
- if (bitmap_end)
- conf->mddev->bitmap_ops->endwrite(conf->mddev,
- sh->sector, RAID5_STRIPE_SECTORS(conf));
- bitmap_end = 0;
/* and fail all 'written' */
bi = sh->dev[i].written;
sh->dev[i].written = NULL;
@@ -3691,7 +3678,6 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
sh->dev[i].page = sh->dev[i].orig_page;
}
- if (bi) bitmap_end = 1;
while (bi && bi->bi_iter.bi_sector <
sh->dev[i].sector + RAID5_STRIPE_SECTORS(conf)) {
struct bio *bi2 = r5_next_bio(conf, bi, sh->dev[i].sector);
@@ -3725,9 +3711,6 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
bi = nextbi;
}
}
- if (bitmap_end)
- conf->mddev->bitmap_ops->endwrite(conf->mddev,
- sh->sector, RAID5_STRIPE_SECTORS(conf));
/* If we were in the middle of a write the parity block might
* still be locked - so just clear all R5_LOCKED flags
*/
@@ -4076,8 +4059,7 @@ static void handle_stripe_clean_event(struct r5conf *conf,
bio_endio(wbi);
wbi = wbi2;
}
- conf->mddev->bitmap_ops->endwrite(conf->mddev,
- sh->sector, RAID5_STRIPE_SECTORS(conf));
+
if (head_sh->batch_head) {
sh = list_first_entry(&sh->batch_list,
struct stripe_head,
@@ -5797,10 +5779,6 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
}
spin_unlock_irq(&sh->stripe_lock);
if (conf->mddev->bitmap) {
- for (d = 0; d < conf->raid_disks - conf->max_degraded;
- d++)
- mddev->bitmap_ops->startwrite(mddev, sh->sector,
- RAID5_STRIPE_SECTORS(conf));
sh->bm_seq = conf->seq_flush + 1;
set_bit(STRIPE_BIT_DELAY, &sh->state);
}
--
2.39.2
^ permalink raw reply [flat|nested] 28+ messages in thread* RE: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-18 11:41 ` [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer Yu Kuai
@ 2024-11-19 9:49 ` Jack Wang
2024-11-19 10:58 ` Yu Kuai
2024-11-22 2:06 ` [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer Xiao Ni
1 sibling, 1 reply; 28+ messages in thread
From: Jack Wang @ 2024-11-19 9:49 UTC (permalink / raw)
To: yukuai1; +Cc: linux-kernel, linux-raid, song, xni, yangerkun, yi.zhang, yukuai3
Hi Kuai,
Thanks for the patchset, as you mentioned both both hung problem regarding raid5
bitmap, just want to get confirmation, will this patchset fix the hung or just a
performance improvement/code cleanup?
In patch4, as you removed the set/clear bit STRIPE_BITMAP_PENDING, I think you
should also remove the test_bit line in stripe_can_batch, also the definition
enum in raif5.h and the few lines in comments in __add_stripe_bio, same for the
line in break_stripe_batch_list.
Regards!
Jinpu Wang @ IONOS
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-19 9:49 ` [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector() Jack Wang
@ 2024-11-19 10:58 ` Yu Kuai
2024-11-19 15:29 ` Jack Wang
0 siblings, 1 reply; 28+ messages in thread
From: Yu Kuai @ 2024-11-19 10:58 UTC (permalink / raw)
To: Jack Wang, yukuai1
Cc: linux-kernel, linux-raid, song, xni, yangerkun, yi.zhang, yukuai (C)
Hi,
在 2024/11/19 17:49, Jack Wang 写道:
> Hi Kuai,
>
> Thanks for the patchset, as you mentioned both both hung problem regarding raid5
> bitmap, just want to get confirmation, will this patchset fix the hung or just a
> performance improvement/code cleanup?
I'm hoping both. :)
After review, I'll CC related folks and see if they can comfirm this
will fix the hang problem.
>
>
> In patch4, as you removed the set/clear bit STRIPE_BITMAP_PENDING, I think you
> should also remove the test_bit line in stripe_can_batch, also the definition
> enum in raif5.h and the few lines in comments in __add_stripe_bio, same for the
> line in break_stripe_batch_list.
Yes, and I assume you mean patch 5.
Thanks,
Kuai
>
>
> Regards!
> Jinpu Wang @ IONOS
>
>
> .
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-19 10:58 ` Yu Kuai
@ 2024-11-19 15:29 ` Jack Wang
2024-11-21 8:10 ` Jinpu Wang
0 siblings, 1 reply; 28+ messages in thread
From: Jack Wang @ 2024-11-19 15:29 UTC (permalink / raw)
To: yukuai1
Cc: jinpu.wang, linux-kernel, linux-raid, song, xni, yangerkun,
yi.zhang, yukuai3
Hi Kuai,
We will test on our side and report back.
Yes, I meant patch5.
Regards!
Jinpu Wang @ IONOS
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-19 15:29 ` Jack Wang
@ 2024-11-21 8:10 ` Jinpu Wang
2024-11-21 8:33 ` Yu Kuai
0 siblings, 1 reply; 28+ messages in thread
From: Jinpu Wang @ 2024-11-21 8:10 UTC (permalink / raw)
To: yukuai1, Haris Iqbal
Cc: linux-kernel, linux-raid, song, xni, yangerkun, yi.zhang,
yukuai3, Florian-Ewald Müller
On Tue, Nov 19, 2024 at 4:29 PM Jack Wang <jinpu.wang@ionos.com> wrote:
>
> Hi Kuai,
>
> We will test on our side and report back.
Hi Kuai,
Haris tested the new patchset, and it works fine.
Thanks for the work.
>
> Yes, I meant patch5.
>
> Regards!
> Jinpu Wang @ IONOS
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-21 8:10 ` Jinpu Wang
@ 2024-11-21 8:33 ` Yu Kuai
2024-11-21 8:40 ` Jinpu Wang
` (2 more replies)
0 siblings, 3 replies; 28+ messages in thread
From: Yu Kuai @ 2024-11-21 8:33 UTC (permalink / raw)
To: Jinpu Wang, yukuai1, Haris Iqbal
Cc: linux-kernel, linux-raid, song, xni, yangerkun, yi.zhang,
Florian-Ewald Müller, Christian Theune, yukuai (C)
Hi,
在 2024/11/21 16:10, Jinpu Wang 写道:
> On Tue, Nov 19, 2024 at 4:29 PM Jack Wang <jinpu.wang@ionos.com> wrote:
>>
>> Hi Kuai,
>>
>> We will test on our side and report back.
> Hi Kuai,
>
> Haris tested the new patchset, and it works fine.
> Thanks for the work.
Thanks for the test! And just to be sure, the BUG_ON() problem in the
other thread is not triggered as well, right?
+CC Christian
Are you able to test this set for lastest kernel?
Thanks,
Kuai
>>
>> Yes, I meant patch5.
>>
>> Regards!
>> Jinpu Wang @ IONOS
>>
>
> .
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-21 8:33 ` Yu Kuai
@ 2024-11-21 8:40 ` Jinpu Wang
2024-11-21 8:44 ` Yu Kuai
2024-11-21 9:30 ` Christian Theune
2024-11-21 12:21 ` Christian Theune
2 siblings, 1 reply; 28+ messages in thread
From: Jinpu Wang @ 2024-11-21 8:40 UTC (permalink / raw)
To: Yu Kuai
Cc: Haris Iqbal, linux-kernel, linux-raid, song, xni, yangerkun,
yi.zhang, Florian-Ewald Müller, Christian Theune, yukuai (C)
Hi
On Thu, Nov 21, 2024 at 9:33 AM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> Hi,
>
> 在 2024/11/21 16:10, Jinpu Wang 写道:
> > On Tue, Nov 19, 2024 at 4:29 PM Jack Wang <jinpu.wang@ionos.com> wrote:
> >>
> >> Hi Kuai,
> >>
> >> We will test on our side and report back.
> > Hi Kuai,
> >
> > Haris tested the new patchset, and it works fine.
> > Thanks for the work.
>
> Thanks for the test! And just to be sure, the BUG_ON() problem in the
> other thread is not triggered as well, right?
Yes, we tested the patchset on top of md/for-6.13 branch, no hang, no
BUG_ON, it was running fine
>
> +CC Christian
>
> Are you able to test this set for lastest kernel?
see above.
>
> Thanks,
> Kuai
Thx!
>
> >>
> >> Yes, I meant patch5.
> >>
> >> Regards!
> >> Jinpu Wang @ IONOS
> >>
> >
> > .
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-21 8:40 ` Jinpu Wang
@ 2024-11-21 8:44 ` Yu Kuai
2024-11-21 8:46 ` Jinpu Wang
0 siblings, 1 reply; 28+ messages in thread
From: Yu Kuai @ 2024-11-21 8:44 UTC (permalink / raw)
To: Jinpu Wang, Yu Kuai
Cc: Haris Iqbal, linux-kernel, linux-raid, song, xni, yangerkun,
yi.zhang, Florian-Ewald Müller, Christian Theune, yukuai (C)
Hi,
在 2024/11/21 16:40, Jinpu Wang 写道:
> Hi
> On Thu, Nov 21, 2024 at 9:33 AM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>>
>> Hi,
>>
>> 在 2024/11/21 16:10, Jinpu Wang 写道:
>>> On Tue, Nov 19, 2024 at 4:29 PM Jack Wang <jinpu.wang@ionos.com> wrote:
>>>>
>>>> Hi Kuai,
>>>>
>>>> We will test on our side and report back.
>>> Hi Kuai,
>>>
>>> Haris tested the new patchset, and it works fine.
>>> Thanks for the work.
>>
>> Thanks for the test! And just to be sure, the BUG_ON() problem in the
>> other thread is not triggered as well, right?
> Yes, we tested the patchset on top of md/for-6.13 branch, no hang, no
> BUG_ON, it was running fine
>>
>> +CC Christian
>>
>> Are you able to test this set for lastest kernel?
> see above.
Are you guys the same team with Christian? Because there is another
thread that he reported the same problem.
Thanks,
Kuai
>>
>> Thanks,
>> Kuai
> Thx!
>>
>>>>
>>>> Yes, I meant patch5.
>>>>
>>>> Regards!
>>>> Jinpu Wang @ IONOS
>>>>
>>>
>>> .
>>>
>>
>
> .
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-21 8:44 ` Yu Kuai
@ 2024-11-21 8:46 ` Jinpu Wang
0 siblings, 0 replies; 28+ messages in thread
From: Jinpu Wang @ 2024-11-21 8:46 UTC (permalink / raw)
To: Yu Kuai
Cc: Haris Iqbal, linux-kernel, linux-raid, song, xni, yangerkun,
yi.zhang, Florian-Ewald Müller, Christian Theune, yukuai (C)
On Thu, Nov 21, 2024 at 9:44 AM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> Hi,
>
> 在 2024/11/21 16:40, Jinpu Wang 写道:
> > Hi
> > On Thu, Nov 21, 2024 at 9:33 AM Yu Kuai <yukuai1@huaweicloud.com> wrote:
> >>
> >> Hi,
> >>
> >> 在 2024/11/21 16:10, Jinpu Wang 写道:
> >>> On Tue, Nov 19, 2024 at 4:29 PM Jack Wang <jinpu.wang@ionos.com> wrote:
> >>>>
> >>>> Hi Kuai,
> >>>>
> >>>> We will test on our side and report back.
> >>> Hi Kuai,
> >>>
> >>> Haris tested the new patchset, and it works fine.
> >>> Thanks for the work.
> >>
> >> Thanks for the test! And just to be sure, the BUG_ON() problem in the
> >> other thread is not triggered as well, right?
> > Yes, we tested the patchset on top of md/for-6.13 branch, no hang, no
> > BUG_ON, it was running fine
> >>
> >> +CC Christian
> >>
> >> Are you able to test this set for lastest kernel?
> > see above.
>
> Are you guys the same team with Christian? Because there is another
> thread that he reported the same problem.
No, we are not the same team with Christian, I guess he will test on his side.
>
> Thanks,
> Kuai
>
> >>
> >> Thanks,
> >> Kuai
> > Thx!
> >>
> >>>>
> >>>> Yes, I meant patch5.
> >>>>
> >>>> Regards!
> >>>> Jinpu Wang @ IONOS
> >>>>
> >>>
> >>> .
> >>>
> >>
> >
> > .
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-21 8:33 ` Yu Kuai
2024-11-21 8:40 ` Jinpu Wang
@ 2024-11-21 9:30 ` Christian Theune
2024-11-21 11:01 ` Yu Kuai
2024-11-21 12:21 ` Christian Theune
2 siblings, 1 reply; 28+ messages in thread
From: Christian Theune @ 2024-11-21 9:30 UTC (permalink / raw)
To: Yu Kuai
Cc: Jinpu Wang, Haris Iqbal, linux-kernel, linux-raid, song, xni,
yangerkun, yi.zhang, Florian-Ewald Müller, yukuai (C)
Hi,
> On 21. Nov 2024, at 09:33, Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> Hi,
>
> 在 2024/11/21 16:10, Jinpu Wang 写道:
>> On Tue, Nov 19, 2024 at 4:29 PM Jack Wang <jinpu.wang@ionos.com> wrote:
>>>
>>> Hi Kuai,
>>>
>>> We will test on our side and report back.
>> Hi Kuai,
>> Haris tested the new patchset, and it works fine.
>> Thanks for the work.
>
> Thanks for the test! And just to be sure, the BUG_ON() problem in the
> other thread is not triggered as well, right?
>
> +CC Christian
>
> Are you able to test this set for lastest kernel?
I have scheduled testing for later today. My current plan was to try Xiao Ni’s fix on 6.6 as that did fix it on 6.11 for me.
Which way forward makes more sense now? Are those two patches independent or amalgamated or might they be stepping on each others’ toes?
Christian
--
Christian Theune · ct@flyingcircus.io · +49 345 219401 0
Flying Circus Internet Operations GmbH · https://flyingcircus.io
Leipziger Str. 70/71 · 06108 Halle (Saale) · Deutschland
HR Stendal HRB 21169 · Geschäftsführer: Christian Theune, Christian Zagrodnick
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-21 9:30 ` Christian Theune
@ 2024-11-21 11:01 ` Yu Kuai
2024-11-21 12:05 ` Christian Theune
0 siblings, 1 reply; 28+ messages in thread
From: Yu Kuai @ 2024-11-21 11:01 UTC (permalink / raw)
To: Christian Theune, Yu Kuai
Cc: Jinpu Wang, Haris Iqbal, linux-kernel, linux-raid, song, xni,
yangerkun, yi.zhang, Florian-Ewald Müller, yukuai (C)
Hi,
在 2024/11/21 17:30, Christian Theune 写道:
> Hi,
>
>> On 21. Nov 2024, at 09:33, Yu Kuai <yukuai1@huaweicloud.com> wrote:
>>
>> Hi,
>>
>> 在 2024/11/21 16:10, Jinpu Wang 写道:
>>> On Tue, Nov 19, 2024 at 4:29 PM Jack Wang <jinpu.wang@ionos.com> wrote:
>>>>
>>>> Hi Kuai,
>>>>
>>>> We will test on our side and report back.
>>> Hi Kuai,
>>> Haris tested the new patchset, and it works fine.
>>> Thanks for the work.
>>
>> Thanks for the test! And just to be sure, the BUG_ON() problem in the
>> other thread is not triggered as well, right?
>>
>> +CC Christian
>>
>> Are you able to test this set for lastest kernel?
>
> I have scheduled testing for later today. My current plan was to try Xiao Ni’s fix on 6.6 as that did fix it on 6.11 for me.
>
> Which way forward makes more sense now? Are those two patches independent or amalgamated or might they be stepping on each others’ toes?
Our plan is to apply this set to latest kernel first, and then backport
this to older kernel. Xiao's fix will not be considered. :(
Thanks,
Kuai
>
> Christian
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-21 11:01 ` Yu Kuai
@ 2024-11-21 12:05 ` Christian Theune
0 siblings, 0 replies; 28+ messages in thread
From: Christian Theune @ 2024-11-21 12:05 UTC (permalink / raw)
To: Yu Kuai
Cc: Jinpu Wang, Haris Iqbal, linux-kernel, linux-raid, song, xni,
yangerkun, yi.zhang, Florian-Ewald Müller, yukuai (C)
Hi,
ok, good thing I asked, then … :)
I’ll try this out later today.
Christian
> On 21. Nov 2024, at 12:01, Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> Hi,
>
> 在 2024/11/21 17:30, Christian Theune 写道:
>> Hi,
>>> On 21. Nov 2024, at 09:33, Yu Kuai <yukuai1@huaweicloud.com> wrote:
>>>
>>> Hi,
>>>
>>> 在 2024/11/21 16:10, Jinpu Wang 写道:
>>>> On Tue, Nov 19, 2024 at 4:29 PM Jack Wang <jinpu.wang@ionos.com> wrote:
>>>>>
>>>>> Hi Kuai,
>>>>>
>>>>> We will test on our side and report back.
>>>> Hi Kuai,
>>>> Haris tested the new patchset, and it works fine.
>>>> Thanks for the work.
>>>
>>> Thanks for the test! And just to be sure, the BUG_ON() problem in the
>>> other thread is not triggered as well, right?
>>>
>>> +CC Christian
>>>
>>> Are you able to test this set for lastest kernel?
>> I have scheduled testing for later today. My current plan was to try Xiao Ni’s fix on 6.6 as that did fix it on 6.11 for me.
>> Which way forward makes more sense now? Are those two patches independent or amalgamated or might they be stepping on each others’ toes?
>
> Our plan is to apply this set to latest kernel first, and then backport
> this to older kernel. Xiao's fix will not be considered. :(
>
> Thanks,
> Kuai
>
>> Christian
Liebe Grüße,
Christian Theune
--
Christian Theune · ct@flyingcircus.io · +49 345 219401 0
Flying Circus Internet Operations GmbH · https://flyingcircus.io
Leipziger Str. 70/71 · 06108 Halle (Saale) · Deutschland
HR Stendal HRB 21169 · Geschäftsführer: Christian Theune, Christian Zagrodnick
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-21 8:33 ` Yu Kuai
2024-11-21 8:40 ` Jinpu Wang
2024-11-21 9:30 ` Christian Theune
@ 2024-11-21 12:21 ` Christian Theune
2024-11-21 12:37 ` Yu Kuai
2 siblings, 1 reply; 28+ messages in thread
From: Christian Theune @ 2024-11-21 12:21 UTC (permalink / raw)
To: Yu Kuai
Cc: Jinpu Wang, Haris Iqbal, linux-kernel, linux-raid, song, xni,
yangerkun, yi.zhang, Florian-Ewald Müller, yukuai (C)
Hi,
> On 21. Nov 2024, at 09:33, Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> Thanks for the test! And just to be sure, the BUG_ON() problem in the
> other thread is not triggered as well, right?
>
> +CC Christian
>
> Are you able to test this set for lastest kernel?
Reading through the list - I’m confused. You posted a patchset on 2024-11-18@12:48. There was discussion later around an adjustment but I’m not seeing the adjusted patchset.
Also, which kernel do you want me to test on? 6.12?
Christian
--
Christian Theune · ct@flyingcircus.io · +49 345 219401 0
Flying Circus Internet Operations GmbH · https://flyingcircus.io
Leipziger Str. 70/71 · 06108 Halle (Saale) · Deutschland
HR Stendal HRB 21169 · Geschäftsführer: Christian Theune, Christian Zagrodnick
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-21 12:21 ` Christian Theune
@ 2024-11-21 12:37 ` Yu Kuai
2024-11-21 16:14 ` Christian Theune
0 siblings, 1 reply; 28+ messages in thread
From: Yu Kuai @ 2024-11-21 12:37 UTC (permalink / raw)
To: Christian Theune, Yu Kuai
Cc: Jinpu Wang, Haris Iqbal, linux-kernel, linux-raid, song, xni,
yangerkun, yi.zhang, Florian-Ewald Müller, yukuai (C)
Hi,
在 2024/11/21 20:21, Christian Theune 写道:
> Hi,
>
>> On 21. Nov 2024, at 09:33, Yu Kuai <yukuai1@huaweicloud.com> wrote:
>>
>> Thanks for the test! And just to be sure, the BUG_ON() problem in the
>> other thread is not triggered as well, right?
>>
>> +CC Christian
>>
>> Are you able to test this set for lastest kernel?
>
> Reading through the list - I’m confused. You posted a patchset on 2024-11-18@12:48. There was discussion later around an adjustment but I’m not seeing the adjusted patchset.
I don't know you mean here, this is the only set. If you didn't
subscribe mail-list and don't know how to find the set:
https://lore.kernel.org/all/20241118114157.355749-1-yukuai1@huaweicloud.com/
>
> Also, which kernel do you want me to test on? 6.12?
The branch is from the title, md-6.13. If you don't know:
https://git.kernel.org/pub/scm/linux/kernel/git/song/md.git/log/?h=md-6.13
Thanks,
Kuai
>
> Christian
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector()
2024-11-21 12:37 ` Yu Kuai
@ 2024-11-21 16:14 ` Christian Theune
0 siblings, 0 replies; 28+ messages in thread
From: Christian Theune @ 2024-11-21 16:14 UTC (permalink / raw)
To: Yu Kuai
Cc: Jinpu Wang, Haris Iqbal, linux-kernel, linux-raid, song, xni,
yangerkun, yi.zhang, Florian-Ewald Müller, yukuai (C)
Hi,
> On 21. Nov 2024, at 13:37, Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
>
> I don't know you mean here, this is the only set. If you didn't
> subscribe mail-list and don't know how to find the set:
>
> https://lore.kernel.org/all/20241118114157.355749-1-yukuai1@huaweicloud.com/
Yes, I saw those and was talking about https://lore.kernel.org/all/adf796b9-2443-d29a-f4ac-fb9b8a657f93@huaweicloud.com/
>> Also, which kernel do you want me to test on? 6.12?
>
> The branch is from the title, md-6.13. If you don't know:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/song/md.git/log/?h=md-6.13
Ok, I can use that. Had to bail out for today and will try again tomorrow.
Christian
--
Christian Theune · ct@flyingcircus.io · +49 345 219401 0
Flying Circus Internet Operations GmbH · https://flyingcircus.io
Leipziger Str. 70/71 · 06108 Halle (Saale) · Deutschland
HR Stendal HRB 21169 · Geschäftsführer: Christian Theune, Christian Zagrodnick
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer
2024-11-18 11:41 ` [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer Yu Kuai
2024-11-19 9:49 ` [PATCH md-6.13 4/5] md/raid5: implement pers->bitmap_sector() Jack Wang
@ 2024-11-22 2:06 ` Xiao Ni
2024-11-22 2:45 ` Yu Kuai
1 sibling, 1 reply; 28+ messages in thread
From: Xiao Ni @ 2024-11-22 2:06 UTC (permalink / raw)
To: Yu Kuai; +Cc: song, linux-raid, linux-kernel, yukuai3, yi.zhang, yangerkun
On Mon, Nov 18, 2024 at 7:44 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@huawei.com>
>
> There are two BUG reports that raid5 will hang at
> bitmap_startwrite([1],[2]), root cause is that bitmap start write and end
> write is unbalanced, and while reviewing raid5 code, it's found that
Hi Kuai
It's better to describe more about "unbalanced" in the patch. For
raid5, bitmap is set and cleared based on stripe->dev[] now. It looks
like the set operation matches the clear operation already.
> bitmap operations can be optimized. For example, for a 4 disks raid5, with
> chunksize=8k, if user issue a IO (0 + 48k) to the array:
>
> ┌────────────────────────────────────────────────────────────┐
> │chunk 0 │
> │ ┌────────────┬─────────────┬─────────────┬────────────┼
> │ sh0 │A0: 0 + 4k │A1: 8k + 4k │A2: 16k + 4k │A3: P │
> │ ┼────────────┼─────────────┼─────────────┼────────────┼
> │ sh1 │B0: 4k + 4k │B1: 12k + 4k │B2: 20k + 4k │B3: P │
> ┼──────┴────────────┴─────────────┴─────────────┴────────────┼
> │chunk 1 │
> │ ┌────────────┬─────────────┬─────────────┬────────────┤
> │ sh2 │C0: 24k + 4k│C1: 32k + 4k │C2: P │C3: 40k + 4k│
> │ ┼────────────┼─────────────┼─────────────┼────────────┼
> │ sh3 │D0: 28k + 4k│D1: 36k + 4k │D2: P │D3: 44k + 4k│
> └──────┴────────────┴─────────────┴─────────────┴────────────┘
>
> Before this patch, 4 stripe head will be used, and each sh will attach
> bio for 3 disks, and each attached bio will trigger
> bitmap_startwrite() once, which means total 12 times.
> - 3 times (0 + 4k), for (A0, A1 and A2)
> - 3 times (4 + 4k), for (B0, B1 and B2)
> - 3 times (8 + 4k), for (C0, C1 and C3)
> - 3 times (12 + 4k), for (D0, D1 and D3)
>
> After this patch, md upper layer will calculate that IO range (0 + 48k)
> is corresponding to the bitmap (0 + 16k), and call bitmap_startwrite()
> just once.
>
> Noted that this patch will align bitmap ranges to the chunks, for example,
> if user issue a IO (0 + 4k) to array:
>
> - Before this patch, 1 time (0 + 4k), for A0;
> - After this patch, 1 time (0 + 8k) for chunk 0;
>
> Usually, one bitmap bit will represent more than one disk chunk, and this
> doesn't have any difference. And even if user really created a array
> that one chunk contain multiple bits, the overhead is that more data
> will be recovered after power failure.
>
> [1] https://lore.kernel.org/all/CAJpMwyjmHQLvm6zg1cmQErttNNQPDAAXPKM3xgTjMhbfts986Q@mail.gmail.com/
> [2] https://lore.kernel.org/all/ADF7D720-5764-4AF3-B68E-1845988737AA@flyingcircus.io/
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
> drivers/md/md.c | 29 +++++++++++++++++++++++++++++
> drivers/md/md.h | 2 ++
> drivers/md/raid1.c | 4 ----
> drivers/md/raid10.c | 3 ---
> drivers/md/raid5-cache.c | 2 --
> drivers/md/raid5.c | 24 +-----------------------
> 6 files changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index bbe002ebd584..ab37c9939ac6 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8723,12 +8723,32 @@ void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
> }
> EXPORT_SYMBOL_GPL(md_submit_discard_bio);
>
> +static void md_bitmap_start(struct mddev *mddev,
> + struct md_io_clone *md_io_clone)
> +{
> + if (mddev->pers->bitmap_sector)
> + mddev->pers->bitmap_sector(mddev, &md_io_clone->offset,
> + &md_io_clone->sectors);
> +
> + mddev->bitmap_ops->startwrite(mddev, md_io_clone->offset,
> + md_io_clone->sectors);
> +}
> +
> +static void md_bitmap_end(struct mddev *mddev, struct md_io_clone *md_io_clone)
> +{
> + mddev->bitmap_ops->endwrite(mddev, md_io_clone->offset,
> + md_io_clone->sectors);
> +}
> +
> static void md_end_clone_io(struct bio *bio)
> {
> struct md_io_clone *md_io_clone = bio->bi_private;
> struct bio *orig_bio = md_io_clone->orig_bio;
> struct mddev *mddev = md_io_clone->mddev;
>
> + if (bio_data_dir(orig_bio) == WRITE && mddev->bitmap)
> + md_bitmap_end(mddev, md_io_clone);
> +
> if (bio->bi_status && !orig_bio->bi_status)
> orig_bio->bi_status = bio->bi_status;
>
> @@ -8753,6 +8773,12 @@ static void md_clone_bio(struct mddev *mddev, struct bio **bio)
> if (blk_queue_io_stat(bdev->bd_disk->queue))
> md_io_clone->start_time = bio_start_io_acct(*bio);
>
> + if (bio_data_dir(*bio) == WRITE && mddev->bitmap) {
> + md_io_clone->offset = (*bio)->bi_iter.bi_sector;
> + md_io_clone->sectors = bio_sectors(*bio);
> + md_bitmap_start(mddev, md_io_clone);
> + }
> +
> clone->bi_end_io = md_end_clone_io;
> clone->bi_private = md_io_clone;
> *bio = clone;
> @@ -8771,6 +8797,9 @@ void md_free_cloned_bio(struct bio *bio)
> struct bio *orig_bio = md_io_clone->orig_bio;
> struct mddev *mddev = md_io_clone->mddev;
>
> + if (bio_data_dir(orig_bio) == WRITE && mddev->bitmap)
> + md_bitmap_end(mddev, md_io_clone);
> +
> if (bio->bi_status && !orig_bio->bi_status)
> orig_bio->bi_status = bio->bi_status;
>
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index de6dadb9a40b..def808064ad8 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -831,6 +831,8 @@ struct md_io_clone {
> struct mddev *mddev;
> struct bio *orig_bio;
> unsigned long start_time;
> + sector_t offset;
> + unsigned long sectors;
> struct bio bio_clone;
> };
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index b9819f9c8ed2..e2adfeff5ae6 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -422,8 +422,6 @@ static void close_write(struct r1bio *r1_bio)
>
> 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);
> md_write_end(mddev);
> }
>
> @@ -1616,8 +1614,6 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>
> 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 efbc0bd92479..79a209236c9f 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -428,8 +428,6 @@ static void close_write(struct r10bio *r10_bio)
> {
> struct mddev *mddev = r10_bio->mddev;
>
> - /* clear the bitmap if all writes complete successfully */
> - mddev->bitmap_ops->endwrite(mddev, r10_bio->sector, r10_bio->sectors);
> md_write_end(mddev);
> }
>
> @@ -1487,7 +1485,6 @@ 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);
>
> 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 ba4f9577c737..011246e16a99 100644
> --- a/drivers/md/raid5-cache.c
> +++ b/drivers/md/raid5-cache.c
> @@ -313,8 +313,6 @@ void r5c_handle_cached_data_endio(struct r5conf *conf,
> if (sh->dev[i].written) {
> set_bit(R5_UPTODATE, &sh->dev[i].flags);
> r5c_return_dev_pending_writes(conf, &sh->dev[i]);
> - conf->mddev->bitmap_ops->endwrite(conf->mddev,
> - sh->sector, RAID5_STRIPE_SECTORS(conf));
> }
> }
> }
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 95caed41654c..976788138a6e 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3578,12 +3578,6 @@ static void __add_stripe_bio(struct stripe_head *sh, struct bio *bi,
> * is added to a batch, STRIPE_BIT_DELAY cannot be changed
> * any more.
> */
> - 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));
> - spin_lock_irq(&sh->stripe_lock);
> - clear_bit(STRIPE_BITMAP_PENDING, &sh->state);
> if (!sh->batch_head) {
> sh->bm_seq = conf->seq_flush+1;
> set_bit(STRIPE_BIT_DELAY, &sh->state);
> @@ -3638,7 +3632,6 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
> BUG_ON(sh->batch_head);
> for (i = disks; i--; ) {
> struct bio *bi;
> - int bitmap_end = 0;
>
> if (test_bit(R5_ReadError, &sh->dev[i].flags)) {
> struct md_rdev *rdev = conf->disks[i].rdev;
> @@ -3663,8 +3656,6 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
> sh->dev[i].towrite = NULL;
> sh->overwrite_disks = 0;
> spin_unlock_irq(&sh->stripe_lock);
> - if (bi)
> - bitmap_end = 1;
>
> log_stripe_write_finished(sh);
>
> @@ -3679,10 +3670,6 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
> bio_io_error(bi);
> bi = nextbi;
> }
> - if (bitmap_end)
> - conf->mddev->bitmap_ops->endwrite(conf->mddev,
> - sh->sector, RAID5_STRIPE_SECTORS(conf));
> - bitmap_end = 0;
> /* and fail all 'written' */
> bi = sh->dev[i].written;
> sh->dev[i].written = NULL;
> @@ -3691,7 +3678,6 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
> sh->dev[i].page = sh->dev[i].orig_page;
> }
>
> - if (bi) bitmap_end = 1;
> while (bi && bi->bi_iter.bi_sector <
> sh->dev[i].sector + RAID5_STRIPE_SECTORS(conf)) {
> struct bio *bi2 = r5_next_bio(conf, bi, sh->dev[i].sector);
> @@ -3725,9 +3711,6 @@ handle_failed_stripe(struct r5conf *conf, struct stripe_head *sh,
> bi = nextbi;
> }
> }
> - if (bitmap_end)
> - conf->mddev->bitmap_ops->endwrite(conf->mddev,
> - sh->sector, RAID5_STRIPE_SECTORS(conf));
> /* If we were in the middle of a write the parity block might
> * still be locked - so just clear all R5_LOCKED flags
> */
> @@ -4076,8 +4059,7 @@ static void handle_stripe_clean_event(struct r5conf *conf,
> bio_endio(wbi);
> wbi = wbi2;
> }
> - conf->mddev->bitmap_ops->endwrite(conf->mddev,
> - sh->sector, RAID5_STRIPE_SECTORS(conf));
> +
> if (head_sh->batch_head) {
> sh = list_first_entry(&sh->batch_list,
> struct stripe_head,
> @@ -5797,10 +5779,6 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
> }
> spin_unlock_irq(&sh->stripe_lock);
> if (conf->mddev->bitmap) {
> - for (d = 0; d < conf->raid_disks - conf->max_degraded;
> - d++)
> - mddev->bitmap_ops->startwrite(mddev, sh->sector,
> - RAID5_STRIPE_SECTORS(conf));
> sh->bm_seq = conf->seq_flush + 1;
> set_bit(STRIPE_BIT_DELAY, &sh->state);
> }
> --
> 2.39.2
>
This patch looks good to me.
Reviewd-by: Xiao Ni <xni@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer
2024-11-22 2:06 ` [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer Xiao Ni
@ 2024-11-22 2:45 ` Yu Kuai
2024-11-22 3:14 ` Xiao Ni
0 siblings, 1 reply; 28+ messages in thread
From: Yu Kuai @ 2024-11-22 2:45 UTC (permalink / raw)
To: Xiao Ni, Yu Kuai
Cc: song, linux-raid, linux-kernel, yi.zhang, yangerkun, yukuai (C)
Hi,
在 2024/11/22 10:06, Xiao Ni 写道:
> On Mon, Nov 18, 2024 at 7:44 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>>
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> There are two BUG reports that raid5 will hang at
>> bitmap_startwrite([1],[2]), root cause is that bitmap start write and end
>> write is unbalanced, and while reviewing raid5 code, it's found that
>
> Hi Kuai
>
> It's better to describe more about "unbalanced" in the patch. For
> raid5, bitmap is set and cleared based on stripe->dev[] now. It looks
> like the set operation matches the clear operation already.
Ok, one place that I found is that raid5 can do extra end write while
stripe->dev[].towrite is NULL, the null checking is missing. I'll
mention that in the next version.
...
>
> This patch looks good to me.
>
> Reviewd-by: Xiao Ni <xni@redhat.com>
>
Thanks for the review. I'll also remove the unused STRIPE_BITMAP_PENDING
in v2.
Kuai
>
> .
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer
2024-11-22 2:45 ` Yu Kuai
@ 2024-11-22 3:14 ` Xiao Ni
2024-11-22 8:32 ` Yu Kuai
0 siblings, 1 reply; 28+ messages in thread
From: Xiao Ni @ 2024-11-22 3:14 UTC (permalink / raw)
To: Yu Kuai; +Cc: song, linux-raid, linux-kernel, yi.zhang, yangerkun, yukuai (C)
On Fri, Nov 22, 2024 at 10:46 AM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> Hi,
>
> 在 2024/11/22 10:06, Xiao Ni 写道:
> > On Mon, Nov 18, 2024 at 7:44 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
> >>
> >> From: Yu Kuai <yukuai3@huawei.com>
> >>
> >> There are two BUG reports that raid5 will hang at
> >> bitmap_startwrite([1],[2]), root cause is that bitmap start write and end
> >> write is unbalanced, and while reviewing raid5 code, it's found that
> >
> > Hi Kuai
> >
> > It's better to describe more about "unbalanced" in the patch. For
> > raid5, bitmap is set and cleared based on stripe->dev[] now. It looks
> > like the set operation matches the clear operation already.
>
> Ok, one place that I found is that raid5 can do extra end write while
> stripe->dev[].towrite is NULL, the null checking is missing. I'll
> mention that in the next version.
Does this can cause the deadlock?
Regards
Xiao
>
> ...
>
> >
> > This patch looks good to me.
> >
> > Reviewd-by: Xiao Ni <xni@redhat.com>
> >
>
> Thanks for the review. I'll also remove the unused STRIPE_BITMAP_PENDING
> in v2.
>
> Kuai
>
> >
> > .
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer
2024-11-22 3:14 ` Xiao Ni
@ 2024-11-22 8:32 ` Yu Kuai
2024-11-23 3:55 ` Xiao Ni
0 siblings, 1 reply; 28+ messages in thread
From: Yu Kuai @ 2024-11-22 8:32 UTC (permalink / raw)
To: Xiao Ni, Yu Kuai
Cc: song, linux-raid, linux-kernel, yi.zhang, yangerkun, yukuai (C)
Hi,
在 2024/11/22 11:14, Xiao Ni 写道:
>> Ok, one place that I found is that raid5 can do extra end write while
>> stripe->dev[].towrite is NULL, the null checking is missing. I'll
>> mention that in the next version.
> Does this can cause the deadlock?
Not deadlock, the bit counter will underflow and reach COUNTER_MAX, and
bitmap_startwrite() can wait forever for the counter.
Thanks,
Kuai
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer
2024-11-22 8:32 ` Yu Kuai
@ 2024-11-23 3:55 ` Xiao Ni
2024-11-25 1:15 ` Yu Kuai
0 siblings, 1 reply; 28+ messages in thread
From: Xiao Ni @ 2024-11-23 3:55 UTC (permalink / raw)
To: Yu Kuai; +Cc: song, linux-raid, linux-kernel, yi.zhang, yangerkun, yukuai (C)
On Fri, Nov 22, 2024 at 4:32 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> Hi,
>
> 在 2024/11/22 11:14, Xiao Ni 写道:
> >> Ok, one place that I found is that raid5 can do extra end write while
> >> stripe->dev[].towrite is NULL, the null checking is missing. I'll
> >> mention that in the next version.
> > Does this can cause the deadlock?
>
> Not deadlock, the bit counter will underflow and reach COUNTER_MAX, and
> bitmap_startwrite() can wait forever for the counter.
Hi Kuai
For normal io, endwrite is called in function
handle_stripe_clean_event when sh->dev[i].written has value.
For failed io, endwrite is called when bitmap_end has value.
bitmap_end is set when sh->dev[i].to_write is not NULL.
Which place does extra endwrite?
Regards
Xiao
>
> Thanks,
> Kuai
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH md-6.13 5/5] md/md-bitmap: move bitmap_{start, end}write to md upper layer
2024-11-23 3:55 ` Xiao Ni
@ 2024-11-25 1:15 ` Yu Kuai
0 siblings, 0 replies; 28+ messages in thread
From: Yu Kuai @ 2024-11-25 1:15 UTC (permalink / raw)
To: Xiao Ni, Yu Kuai
Cc: song, linux-raid, linux-kernel, yi.zhang, yangerkun, yukuai (C)
Hi,
在 2024/11/23 11:55, Xiao Ni 写道:
> For normal io, endwrite is called in function
> handle_stripe_clean_event when sh->dev[i].written has value.
> For failed io, endwrite is called when bitmap_end has value.
> bitmap_end is set when sh->dev[i].to_write is not NULL.
> Which place does extra endwrite?
I think it's related to the sh batch list. For example, the
returnbi tag from handle_stripe_clean_event() doesn't check
the written value.
Thanks,
Kuai
^ permalink raw reply [flat|nested] 28+ messages in thread