[PATCH 04/23] md/md-bitmap: support discard for bitmap ops

Yu Kuai posted 23 patches 6 months, 4 weeks ago
There is a newer version of this series
[PATCH 04/23] md/md-bitmap: support discard for bitmap ops
Posted by Yu Kuai 6 months, 4 weeks ago
From: Yu Kuai <yukuai3@huawei.com>

Use two new methods {start, end}_discard to handle discard IO, prepare
to support new md bitmap.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/md/md-bitmap.c |  3 +++
 drivers/md/md-bitmap.h | 12 ++++++++----
 drivers/md/md.c        | 15 +++++++++++----
 drivers/md/md.h        |  1 +
 4 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 2997e09d463d..848626049dea 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -2991,6 +2991,9 @@ static struct bitmap_operations bitmap_ops = {
 
 	.start_write		= bitmap_start_write,
 	.end_write		= bitmap_end_write,
+	.start_discard		= bitmap_start_write,
+	.end_discard		= bitmap_end_write,
+
 	.start_sync		= bitmap_start_sync,
 	.end_sync		= bitmap_end_sync,
 	.cond_end_sync		= bitmap_cond_end_sync,
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index 9474e0d86fc6..4d804c07dbdd 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -70,6 +70,9 @@ struct md_bitmap_stats {
 	struct file	*file;
 };
 
+typedef void (md_bitmap_fn)(struct mddev *mddev, sector_t offset,
+			    unsigned long sectors);
+
 struct bitmap_operations {
 	struct md_submodule_head head;
 
@@ -90,10 +93,11 @@ struct bitmap_operations {
 	void (*end_behind_write)(struct mddev *mddev);
 	void (*wait_behind_writes)(struct mddev *mddev);
 
-	void (*start_write)(struct mddev *mddev, sector_t offset,
-			    unsigned long sectors);
-	void (*end_write)(struct mddev *mddev, sector_t offset,
-			  unsigned long sectors);
+	md_bitmap_fn *start_write;
+	md_bitmap_fn *end_write;
+	md_bitmap_fn *start_discard;
+	md_bitmap_fn *end_discard;
+
 	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/md.c b/drivers/md/md.c
index 04a659f40cd6..466087cef4f9 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8845,18 +8845,24 @@ EXPORT_SYMBOL_GPL(md_submit_discard_bio);
 static void md_bitmap_start(struct mddev *mddev,
 			    struct md_io_clone *md_io_clone)
 {
+	md_bitmap_fn *fn = unlikely(md_io_clone->rw == STAT_DISCARD) ?
+			   mddev->bitmap_ops->start_discard :
+			   mddev->bitmap_ops->start_write;
+
 	if (mddev->pers->bitmap_sector)
 		mddev->pers->bitmap_sector(mddev, &md_io_clone->offset,
 					   &md_io_clone->sectors);
 
-	mddev->bitmap_ops->start_write(mddev, md_io_clone->offset,
-				       md_io_clone->sectors);
+	fn(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->end_write(mddev, md_io_clone->offset,
-				     md_io_clone->sectors);
+	md_bitmap_fn *fn = unlikely(md_io_clone->rw == STAT_DISCARD) ?
+			   mddev->bitmap_ops->end_discard :
+			   mddev->bitmap_ops->end_write;
+
+	fn(mddev, md_io_clone->offset, md_io_clone->sectors);
 }
 
 static void md_end_clone_io(struct bio *bio)
@@ -8895,6 +8901,7 @@ static void md_clone_bio(struct mddev *mddev, struct bio **bio)
 	if (bio_data_dir(*bio) == WRITE && md_bitmap_enabled(mddev)) {
 		md_io_clone->offset = (*bio)->bi_iter.bi_sector;
 		md_io_clone->sectors = bio_sectors(*bio);
+		md_io_clone->rw = op_stat_group(bio_op(*bio));
 		md_bitmap_start(mddev, md_io_clone);
 	}
 
diff --git a/drivers/md/md.h b/drivers/md/md.h
index c241119e6ef3..13e3f9ce1b79 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -850,6 +850,7 @@ struct md_io_clone {
 	unsigned long	start_time;
 	sector_t	offset;
 	unsigned long	sectors;
+	enum stat_group	rw;
 	struct bio	bio_clone;
 };
 
-- 
2.39.2
Re: [PATCH 04/23] md/md-bitmap: support discard for bitmap ops
Posted by Glass Su 6 months, 3 weeks ago

> On May 24, 2025, at 14:13, Yu Kuai <yukuai1@huaweicloud.com> wrote:
> 
> From: Yu Kuai <yukuai3@huawei.com>
> 
> Use two new methods {start, end}_discard to handle discard IO, prepare
> to support new md bitmap.
> 
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
> drivers/md/md-bitmap.c |  3 +++
> drivers/md/md-bitmap.h | 12 ++++++++----
> drivers/md/md.c        | 15 +++++++++++----
> drivers/md/md.h        |  1 +
> 4 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 2997e09d463d..848626049dea 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -2991,6 +2991,9 @@ static struct bitmap_operations bitmap_ops = {
> 
> .start_write = bitmap_start_write,
> .end_write = bitmap_end_write,
> + .start_discard = bitmap_start_write,
> + .end_discard = bitmap_end_write,
> +
> .start_sync = bitmap_start_sync,
> .end_sync = bitmap_end_sync,
> .cond_end_sync = bitmap_cond_end_sync,
> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
> index 9474e0d86fc6..4d804c07dbdd 100644
> --- a/drivers/md/md-bitmap.h
> +++ b/drivers/md/md-bitmap.h
> @@ -70,6 +70,9 @@ struct md_bitmap_stats {
> struct file *file;
> };
> 
> +typedef void (md_bitmap_fn)(struct mddev *mddev, sector_t offset,
> +    unsigned long sectors);
> +
> struct bitmap_operations {
> struct md_submodule_head head;
> 
> @@ -90,10 +93,11 @@ struct bitmap_operations {
> void (*end_behind_write)(struct mddev *mddev);
> void (*wait_behind_writes)(struct mddev *mddev);
> 
> - void (*start_write)(struct mddev *mddev, sector_t offset,
> -    unsigned long sectors);
> - void (*end_write)(struct mddev *mddev, sector_t offset,
> -  unsigned long sectors);
> + md_bitmap_fn *start_write;
> + md_bitmap_fn *end_write;
> + md_bitmap_fn *start_discard;
> + md_bitmap_fn *end_discard;
> +
> 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/md.c b/drivers/md/md.c
> index 04a659f40cd6..466087cef4f9 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8845,18 +8845,24 @@ EXPORT_SYMBOL_GPL(md_submit_discard_bio);
> static void md_bitmap_start(struct mddev *mddev,
>    struct md_io_clone *md_io_clone)
> {
> + md_bitmap_fn *fn = unlikely(md_io_clone->rw == STAT_DISCARD) ?
> +   mddev->bitmap_ops->start_discard :
> +   mddev->bitmap_ops->start_write;
> +
> if (mddev->pers->bitmap_sector)
> mddev->pers->bitmap_sector(mddev, &md_io_clone->offset,
>   &md_io_clone->sectors);
> 
> - mddev->bitmap_ops->start_write(mddev, md_io_clone->offset,
> -       md_io_clone->sectors);
> + fn(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->end_write(mddev, md_io_clone->offset,
> -     md_io_clone->sectors);
> + md_bitmap_fn *fn = unlikely(md_io_clone->rw == STAT_DISCARD) ?
> +   mddev->bitmap_ops->end_discard :
> +   mddev->bitmap_ops->end_write;
> +
> + fn(mddev, md_io_clone->offset, md_io_clone->sectors);
> }
> 
> static void md_end_clone_io(struct bio *bio)
> @@ -8895,6 +8901,7 @@ static void md_clone_bio(struct mddev *mddev, struct bio **bio)
> if (bio_data_dir(*bio) == WRITE && md_bitmap_enabled(mddev)) {
> md_io_clone->offset = (*bio)->bi_iter.bi_sector;
> md_io_clone->sectors = bio_sectors(*bio);
> + md_io_clone->rw = op_stat_group(bio_op(*bio));
> md_bitmap_start(mddev, md_io_clone);
> }
> 
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index c241119e6ef3..13e3f9ce1b79 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -850,6 +850,7 @@ struct md_io_clone {
> unsigned long start_time;
> sector_t offset;
> unsigned long sectors;
> + enum stat_group rw;

Please also mention the change in commit message.

— 
Su
> struct bio bio_clone;
> };
> 
> -- 
> 2.39.2
> 
> 
Re: [PATCH 04/23] md/md-bitmap: support discard for bitmap ops
Posted by Hannes Reinecke 6 months, 3 weeks ago
On 5/24/25 08:13, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> Use two new methods {start, end}_discard to handle discard IO, prepare
> to support new md bitmap.
> 
This actually does more than just add new methods; but not sure if one
should list all of that. Maybe just 'Add typedef for bitmap functions
and new methods to support md bitmap'.

 > Signed-off-by: Yu Kuai <yukuai3@huawei.com>> ---
>   drivers/md/md-bitmap.c |  3 +++
>   drivers/md/md-bitmap.h | 12 ++++++++----
>   drivers/md/md.c        | 15 +++++++++++----
>   drivers/md/md.h        |  1 +
>   4 files changed, 23 insertions(+), 8 deletions(-)
> 
Other than that:

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH 04/23] md/md-bitmap: support discard for bitmap ops
Posted by Christoph Hellwig 6 months, 3 weeks ago
> +typedef void (md_bitmap_fn)(struct mddev *mddev, sector_t offset,
> +			    unsigned long sectors);

Does this typedef really add any value?

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Re: [PATCH 04/23] md/md-bitmap: support discard for bitmap ops
Posted by Xiao Ni 6 months, 4 weeks ago
On Sat, May 24, 2025 at 2:19 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@huawei.com>
>
> Use two new methods {start, end}_discard to handle discard IO, prepare
> to support new md bitmap.
>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  drivers/md/md-bitmap.c |  3 +++
>  drivers/md/md-bitmap.h | 12 ++++++++----
>  drivers/md/md.c        | 15 +++++++++++----
>  drivers/md/md.h        |  1 +
>  4 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 2997e09d463d..848626049dea 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -2991,6 +2991,9 @@ static struct bitmap_operations bitmap_ops = {
>
>         .start_write            = bitmap_start_write,
>         .end_write              = bitmap_end_write,
> +       .start_discard          = bitmap_start_write,
> +       .end_discard            = bitmap_end_write,
> +
>         .start_sync             = bitmap_start_sync,
>         .end_sync               = bitmap_end_sync,
>         .cond_end_sync          = bitmap_cond_end_sync,
> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
> index 9474e0d86fc6..4d804c07dbdd 100644
> --- a/drivers/md/md-bitmap.h
> +++ b/drivers/md/md-bitmap.h
> @@ -70,6 +70,9 @@ struct md_bitmap_stats {
>         struct file     *file;
>  };
>
> +typedef void (md_bitmap_fn)(struct mddev *mddev, sector_t offset,
> +                           unsigned long sectors);
> +
>  struct bitmap_operations {
>         struct md_submodule_head head;
>
> @@ -90,10 +93,11 @@ struct bitmap_operations {
>         void (*end_behind_write)(struct mddev *mddev);
>         void (*wait_behind_writes)(struct mddev *mddev);
>
> -       void (*start_write)(struct mddev *mddev, sector_t offset,
> -                           unsigned long sectors);
> -       void (*end_write)(struct mddev *mddev, sector_t offset,
> -                         unsigned long sectors);
> +       md_bitmap_fn *start_write;
> +       md_bitmap_fn *end_write;
> +       md_bitmap_fn *start_discard;
> +       md_bitmap_fn *end_discard;
> +
>         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/md.c b/drivers/md/md.c
> index 04a659f40cd6..466087cef4f9 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8845,18 +8845,24 @@ EXPORT_SYMBOL_GPL(md_submit_discard_bio);
>  static void md_bitmap_start(struct mddev *mddev,
>                             struct md_io_clone *md_io_clone)
>  {
> +       md_bitmap_fn *fn = unlikely(md_io_clone->rw == STAT_DISCARD) ?
> +                          mddev->bitmap_ops->start_discard :
> +                          mddev->bitmap_ops->start_write;
> +
>         if (mddev->pers->bitmap_sector)
>                 mddev->pers->bitmap_sector(mddev, &md_io_clone->offset,
>                                            &md_io_clone->sectors);
>
> -       mddev->bitmap_ops->start_write(mddev, md_io_clone->offset,
> -                                      md_io_clone->sectors);
> +       fn(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->end_write(mddev, md_io_clone->offset,
> -                                    md_io_clone->sectors);
> +       md_bitmap_fn *fn = unlikely(md_io_clone->rw == STAT_DISCARD) ?
> +                          mddev->bitmap_ops->end_discard :
> +                          mddev->bitmap_ops->end_write;
> +
> +       fn(mddev, md_io_clone->offset, md_io_clone->sectors);
>  }
>
>  static void md_end_clone_io(struct bio *bio)
> @@ -8895,6 +8901,7 @@ static void md_clone_bio(struct mddev *mddev, struct bio **bio)
>         if (bio_data_dir(*bio) == WRITE && md_bitmap_enabled(mddev)) {
>                 md_io_clone->offset = (*bio)->bi_iter.bi_sector;
>                 md_io_clone->sectors = bio_sectors(*bio);
> +               md_io_clone->rw = op_stat_group(bio_op(*bio));
>                 md_bitmap_start(mddev, md_io_clone);
>         }
>
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index c241119e6ef3..13e3f9ce1b79 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -850,6 +850,7 @@ struct md_io_clone {
>         unsigned long   start_time;
>         sector_t        offset;
>         unsigned long   sectors;
> +       enum stat_group rw;
>         struct bio      bio_clone;
>  };
>
> --
> 2.39.2
>
>

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