[PATCH 0/3] md: bitmap grow fixes

[PATCH 0/3] md: bitmap grow fixes

[Su Yue]

Hi, This series is to fixes bugs exposed by mdadm/clustermd-autotest.
The bugs are not cluster md only but also bitmap related.

I'll find time to add test cases to mdadm. Thanks.

Su Yue (3):
  md: restore bitmap/location to fix wrong bitmap offset while growing
  md: call md_bitmap_create,destroy in location_store
  md: remove member group from bitmap_operations

 drivers/md/md-bitmap.c   | 45 +++++++++++++++++++++++++++++++++++-----
 drivers/md/md-bitmap.h   |  5 ++++-
 drivers/md/md-llbitmap.c | 13 +++++++++++-
 drivers/md/md.c          | 20 ++++++++++++------
 drivers/md/md.h          |  2 ++
 5 files changed, 72 insertions(+), 13 deletions(-)

-- 
2.53.0

[PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing

[Su Yue]

Before commit fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
if CONFIG_MD_BITMAP is enabled, both bitmap none, internal and clustered have
the sysfs file bitmap/location.

After the commit, if bitmap is none, bitmap/location doesn't exist anymore.
It breaks 'grow' behavior of a md array of madam with MD_FEATURE_BITMAP_OFFSET.
Take level=mirror and metadata=1.2 as an example:

$ mdadm  --create  /dev/md0 -f --bitmap=none --raid-devices=2 --level=mirror \
  --metadata=1.2 /dev/vdd /dev/vde
$ mdadm --grow /dev/md0 --bitmap=internal
$ cat /sys/block/md0/md/bitmap/location
Before:+8
After: +2

While growing bitmap from none to internal, clustered and llbitmap,
mdadm/Grow.c:Grow_addbitmap() tries to detect bitmap/location first.
1)If bitmap/location exists, it sets bitmap/location after getinfo_super().
2)If bitmap/location doesn't exist, mdadm just calls md_set_array_info() then
mddev->bitmap_info.default_offset will be used.
Situation can be worse if growing none to clustered, bitmap offset of the node
calling `madm --grow` will be changed but the other node are reading bitmap sb from
the old location.

Here restore sysfs file bitmap/location for ID_BITMAP_NONE and ID_BITMAP.
And it d adds the entry for llbitmap too.

New attribute_group md_bitmap_common_group is introduced and created in
md_alloc() as before commit fb8cc3b0d9db.
Add New operations register_group and unregister_group to struct bitmap_operations.

Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
Signed-off-by: Su Yue <glass.su@suse.com>
---
 drivers/md/md-bitmap.c   | 32 +++++++++++++++++++++++++++++++-
 drivers/md/md-bitmap.h   |  5 +++++
 drivers/md/md-llbitmap.c | 13 +++++++++++++
 drivers/md/md.c          | 16 ++++++++++++----
 4 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 83378c033c72..8ff1dc94ed78 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -2956,7 +2956,6 @@ __ATTR(max_backlog_used, S_IRUGO | S_IWUSR,
        behind_writes_used_show, behind_writes_used_reset);
 
 static struct attribute *md_bitmap_attrs[] = {
-	&bitmap_location.attr,
 	&bitmap_space.attr,
 	&bitmap_timeout.attr,
 	&bitmap_backlog.attr,
@@ -2967,11 +2966,40 @@ static struct attribute *md_bitmap_attrs[] = {
 	NULL
 };
 
+static struct attribute *md_bitmap_common_attrs[] = {
+	&bitmap_location.attr,
+	NULL
+};
+
 static struct attribute_group md_bitmap_group = {
 	.name = "bitmap",
 	.attrs = md_bitmap_attrs,
 };
 
+static struct attribute_group md_bitmap_common_group = {
+	.name = "bitmap",
+	.attrs = md_bitmap_common_attrs,
+};
+
+int md_sysfs_create_common_group(struct mddev *mddev)
+{
+	return sysfs_create_group(&mddev->kobj, &md_bitmap_common_group);
+}
+
+static int bitmap_register_group(struct mddev *mddev)
+{
+	/*
+	 * md_bitmap_group and md_bitmap_common_group are using same name
+	 * 'bitmap'.
+	 */
+	return sysfs_merge_group(&mddev->kobj, &md_bitmap_group);
+}
+
+static void bitmap_unregister_group(struct mddev *mddev)
+{
+	sysfs_unmerge_group(&mddev->kobj, &md_bitmap_group);
+}
+
 static struct bitmap_operations bitmap_ops = {
 	.head = {
 		.type	= MD_BITMAP,
@@ -3013,6 +3041,8 @@ static struct bitmap_operations bitmap_ops = {
 	.set_pages		= bitmap_set_pages,
 	.free			= md_bitmap_free,
 
+	.register_group		= bitmap_register_group,
+	.unregister_group	= bitmap_unregister_group,
 	.group			= &md_bitmap_group,
 };
 
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index b42a28fa83a0..371791e9011d 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -125,6 +125,9 @@ struct bitmap_operations {
 	void (*set_pages)(void *data, unsigned long pages);
 	void (*free)(void *data);
 
+	int (*register_group)(struct mddev *mddev);
+	void (*unregister_group)(struct mddev *mddev);
+
 	struct attribute_group *group;
 };
 
@@ -169,6 +172,8 @@ static inline void md_bitmap_end_sync(struct mddev *mddev, sector_t offset,
 	mddev->bitmap_ops->end_sync(mddev, offset, blocks);
 }
 
+int md_sysfs_create_common_group(struct mddev *mddev);
+
 #ifdef CONFIG_MD_BITMAP
 int md_bitmap_init(void);
 void md_bitmap_exit(void);
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index bf398d7476b3..24ff5f7f8751 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1561,6 +1561,16 @@ static struct attribute_group md_llbitmap_group = {
 	.attrs = md_llbitmap_attrs,
 };
 
+static int llbitmap_register_group(struct mddev *mddev)
+{
+	return sysfs_create_group(&mddev->kobj, &md_llbitmap_group);
+}
+
+static void llbitmap_unregister_group(struct mddev *mddev)
+{
+	sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
+}
+
 static struct bitmap_operations llbitmap_ops = {
 	.head = {
 		.type	= MD_BITMAP,
@@ -1597,6 +1607,9 @@ static struct bitmap_operations llbitmap_ops = {
 	.dirty_bits		= llbitmap_dirty_bits,
 	.write_all		= llbitmap_write_all,
 
+	.register_group		= llbitmap_register_group,
+	.unregister_group	= llbitmap_unregister_group,
+
 	.group			= &md_llbitmap_group,
 };
 
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 3ce6f9e9d38e..ab969e950ea8 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -703,8 +703,8 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
 	mddev->bitmap_ops = (void *)head;
 	xa_unlock(&md_submodule);
 
-	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
-		if (sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group))
+	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->register_group) {
+		if (mddev->bitmap_ops->register_group(mddev))
 			pr_warn("md: cannot register extra bitmap attributes for %s\n",
 				mdname(mddev));
 		else
@@ -724,8 +724,8 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
 static void mddev_clear_bitmap_ops(struct mddev *mddev)
 {
 	if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
-	    mddev->bitmap_ops->group)
-		sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
+	    mddev->bitmap_ops->unregister_group)
+		mddev->bitmap_ops->unregister_group(mddev);
 
 	mddev->bitmap_ops = NULL;
 }
@@ -6369,6 +6369,14 @@ struct mddev *md_alloc(dev_t dev, char *name)
 		return ERR_PTR(error);
 	}
 
+	/*
+	 * md_sysfs_remove_common_group is not needed because mddev_delayed_delete
+	 * calls kobject_put(&mddev->kobj) if mddev is to be deleted.
+	 */
+	if (md_sysfs_create_common_group(mddev))
+		pr_warn("md: cannot register common bitmap attributes for %s\n",
+			mdname(mddev));
+
 	kobject_uevent(&mddev->kobj, KOBJ_ADD);
 	mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd, "array_state");
 	mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd, "level");
-- 
2.53.0

[PATCH 2/3] md: call md_bitmap_create,destroy in location_store

[Su Yue]

If commit
'md: restore bitmap/location to fix wrong bitmap offset while growing'
is applied, mdadm will call update_array_info() while growing bitmap from
none to internal via location_store(). md_bitmap_create() is needed to set
mddev->bitmap_ops otherwise mddev->bitmap_ops->get_stats() in
update_array_info() will trigger kernel NULL pointer dereference.

Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
Signed-off-by: Su Yue <glass.su@suse.com>
---
 drivers/md/md-bitmap.c | 12 +++++++++---
 drivers/md/md.c        |  4 ++--
 drivers/md/md.h        |  2 ++
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 8ff1dc94ed78..8abec00496b4 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -2618,7 +2618,7 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
 			goto out;
 		}
 
-		bitmap_destroy(mddev);
+		md_bitmap_destroy(mddev);
 		mddev->bitmap_info.offset = 0;
 		if (mddev->bitmap_info.file) {
 			struct file *f = mddev->bitmap_info.file;
@@ -2653,15 +2653,21 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
 				goto out;
 			}
 
+			/*
+			 * lockless bitmap shoudle have set bitmap_id
+			 * using bitmap_type, so always ID_BITMAP.
+			 */
+			if (mddev->bitmap_id == ID_BITMAP_NONE)
+				mddev->bitmap_id = ID_BITMAP;
 			mddev->bitmap_info.offset = offset;
-			rv = bitmap_create(mddev);
+			rv = md_bitmap_create(mddev);
 			if (rv)
 				goto out;
 
 			rv = bitmap_load(mddev);
 			if (rv) {
 				mddev->bitmap_info.offset = 0;
-				bitmap_destroy(mddev);
+				md_bitmap_destroy(mddev);
 				goto out;
 			}
 		}
diff --git a/drivers/md/md.c b/drivers/md/md.c
index ab969e950ea8..80beaff5ad39 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6455,7 +6455,7 @@ static void md_safemode_timeout(struct timer_list *t)
 
 static int start_dirty_degraded;
 
-static int md_bitmap_create(struct mddev *mddev)
+int md_bitmap_create(struct mddev *mddev)
 {
 	if (mddev->bitmap_id == ID_BITMAP_NONE)
 		return -EINVAL;
@@ -6466,7 +6466,7 @@ static int md_bitmap_create(struct mddev *mddev)
 	return mddev->bitmap_ops->create(mddev);
 }
 
-static void md_bitmap_destroy(struct mddev *mddev)
+void md_bitmap_destroy(struct mddev *mddev)
 {
 	if (!md_bitmap_registered(mddev))
 		return;
diff --git a/drivers/md/md.h b/drivers/md/md.h
index ac84289664cd..ed69244af00d 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -895,6 +895,8 @@ static inline void safe_put_page(struct page *p)
 
 int register_md_submodule(struct md_submodule_head *msh);
 void unregister_md_submodule(struct md_submodule_head *msh);
+int md_bitmap_create(struct mddev *mddev);
+void md_bitmap_destroy(struct mddev *mddev);
 
 extern struct md_thread *md_register_thread(
 	void (*run)(struct md_thread *thread),
-- 
2.53.0

[PATCH 3/3] md: remove member group from bitmap_operations

[Su Yue]

The member is unused now so remove it.

Signed-off-by: Su Yue <glass.su@suse.com>
---
 drivers/md/md-bitmap.c   | 1 -
 drivers/md/md-bitmap.h   | 2 --
 drivers/md/md-llbitmap.c | 2 --
 3 files changed, 5 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 8abec00496b4..bb04e3c17043 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -3049,7 +3049,6 @@ static struct bitmap_operations bitmap_ops = {
 
 	.register_group		= bitmap_register_group,
 	.unregister_group	= bitmap_unregister_group,
-	.group			= &md_bitmap_group,
 };
 
 int md_bitmap_init(void)
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index 371791e9011d..c81489aaa767 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -127,8 +127,6 @@ struct bitmap_operations {
 
 	int (*register_group)(struct mddev *mddev);
 	void (*unregister_group)(struct mddev *mddev);
-
-	struct attribute_group *group;
 };
 
 /* the bitmap API */
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 24ff5f7f8751..78f5fafd59ba 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1609,8 +1609,6 @@ static struct bitmap_operations llbitmap_ops = {
 
 	.register_group		= llbitmap_register_group,
 	.unregister_group	= llbitmap_unregister_group,
-
-	.group			= &md_llbitmap_group,
 };
 
 int md_llbitmap_init(void)
-- 
2.53.0

Re: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing

[Yu Kuai]

Hi,

在 2026/3/3 11:37, Su Yue 写道:
> Before commit fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
> if CONFIG_MD_BITMAP is enabled, both bitmap none, internal and clustered have
> the sysfs file bitmap/location.
>
> After the commit, if bitmap is none, bitmap/location doesn't exist anymore.
> It breaks 'grow' behavior of a md array of madam with MD_FEATURE_BITMAP_OFFSET.
> Take level=mirror and metadata=1.2 as an example:
>
> $ mdadm  --create  /dev/md0 -f --bitmap=none --raid-devices=2 --level=mirror \
>    --metadata=1.2 /dev/vdd /dev/vde
> $ mdadm --grow /dev/md0 --bitmap=internal
> $ cat /sys/block/md0/md/bitmap/location
> Before:+8
> After: +2
>
> While growing bitmap from none to internal, clustered and llbitmap,
> mdadm/Grow.c:Grow_addbitmap() tries to detect bitmap/location first.
> 1)If bitmap/location exists, it sets bitmap/location after getinfo_super().
> 2)If bitmap/location doesn't exist, mdadm just calls md_set_array_info() then
> mddev->bitmap_info.default_offset will be used.
> Situation can be worse if growing none to clustered, bitmap offset of the node
> calling `madm --grow` will be changed but the other node are reading bitmap sb from
> the old location.

Now that we have a new sysfs attribute bitmap_type, can we fix this by:
- in the kernel, allow writing to this file in this case;
- in mdadm and the grow case above, write to this file first, and change
bitmap_type from none to bitmap(For llbitmap, there is still more work to do).

>
> Here restore sysfs file bitmap/location for ID_BITMAP_NONE and ID_BITMAP.
> And it d adds the entry for llbitmap too.
>
> New attribute_group md_bitmap_common_group is introduced and created in
> md_alloc() as before commit fb8cc3b0d9db.
> Add New operations register_group and unregister_group to struct bitmap_operations.
>
> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
> Signed-off-by: Su Yue <glass.su@suse.com>
> ---
>   drivers/md/md-bitmap.c   | 32 +++++++++++++++++++++++++++++++-
>   drivers/md/md-bitmap.h   |  5 +++++
>   drivers/md/md-llbitmap.c | 13 +++++++++++++
>   drivers/md/md.c          | 16 ++++++++++++----
>   4 files changed, 61 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 83378c033c72..8ff1dc94ed78 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -2956,7 +2956,6 @@ __ATTR(max_backlog_used, S_IRUGO | S_IWUSR,
>          behind_writes_used_show, behind_writes_used_reset);
>   
>   static struct attribute *md_bitmap_attrs[] = {
> -	&bitmap_location.attr,
>   	&bitmap_space.attr,
>   	&bitmap_timeout.attr,
>   	&bitmap_backlog.attr,
> @@ -2967,11 +2966,40 @@ static struct attribute *md_bitmap_attrs[] = {
>   	NULL
>   };
>   
> +static struct attribute *md_bitmap_common_attrs[] = {
> +	&bitmap_location.attr,
> +	NULL
> +};
> +
>   static struct attribute_group md_bitmap_group = {
>   	.name = "bitmap",
>   	.attrs = md_bitmap_attrs,
>   };
>   
> +static struct attribute_group md_bitmap_common_group = {
> +	.name = "bitmap",
> +	.attrs = md_bitmap_common_attrs,
> +};
> +
> +int md_sysfs_create_common_group(struct mddev *mddev)
> +{
> +	return sysfs_create_group(&mddev->kobj, &md_bitmap_common_group);
> +}
> +
> +static int bitmap_register_group(struct mddev *mddev)
> +{
> +	/*
> +	 * md_bitmap_group and md_bitmap_common_group are using same name
> +	 * 'bitmap'.
> +	 */
> +	return sysfs_merge_group(&mddev->kobj, &md_bitmap_group);
> +}
> +
> +static void bitmap_unregister_group(struct mddev *mddev)
> +{
> +	sysfs_unmerge_group(&mddev->kobj, &md_bitmap_group);
> +}
> +
>   static struct bitmap_operations bitmap_ops = {
>   	.head = {
>   		.type	= MD_BITMAP,
> @@ -3013,6 +3041,8 @@ static struct bitmap_operations bitmap_ops = {
>   	.set_pages		= bitmap_set_pages,
>   	.free			= md_bitmap_free,
>   
> +	.register_group		= bitmap_register_group,
> +	.unregister_group	= bitmap_unregister_group,
>   	.group			= &md_bitmap_group,
>   };
>   
> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
> index b42a28fa83a0..371791e9011d 100644
> --- a/drivers/md/md-bitmap.h
> +++ b/drivers/md/md-bitmap.h
> @@ -125,6 +125,9 @@ struct bitmap_operations {
>   	void (*set_pages)(void *data, unsigned long pages);
>   	void (*free)(void *data);
>   
> +	int (*register_group)(struct mddev *mddev);
> +	void (*unregister_group)(struct mddev *mddev);
> +
>   	struct attribute_group *group;
>   };
>   
> @@ -169,6 +172,8 @@ static inline void md_bitmap_end_sync(struct mddev *mddev, sector_t offset,
>   	mddev->bitmap_ops->end_sync(mddev, offset, blocks);
>   }
>   
> +int md_sysfs_create_common_group(struct mddev *mddev);
> +
>   #ifdef CONFIG_MD_BITMAP
>   int md_bitmap_init(void);
>   void md_bitmap_exit(void);
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index bf398d7476b3..24ff5f7f8751 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1561,6 +1561,16 @@ static struct attribute_group md_llbitmap_group = {
>   	.attrs = md_llbitmap_attrs,
>   };
>   
> +static int llbitmap_register_group(struct mddev *mddev)
> +{
> +	return sysfs_create_group(&mddev->kobj, &md_llbitmap_group);
> +}
> +
> +static void llbitmap_unregister_group(struct mddev *mddev)
> +{
> +	sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
> +}
> +
>   static struct bitmap_operations llbitmap_ops = {
>   	.head = {
>   		.type	= MD_BITMAP,
> @@ -1597,6 +1607,9 @@ static struct bitmap_operations llbitmap_ops = {
>   	.dirty_bits		= llbitmap_dirty_bits,
>   	.write_all		= llbitmap_write_all,
>   
> +	.register_group		= llbitmap_register_group,
> +	.unregister_group	= llbitmap_unregister_group,
> +
>   	.group			= &md_llbitmap_group,
>   };
>   
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 3ce6f9e9d38e..ab969e950ea8 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -703,8 +703,8 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
>   	mddev->bitmap_ops = (void *)head;
>   	xa_unlock(&md_submodule);
>   
> -	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
> -		if (sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group))
> +	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->register_group) {
> +		if (mddev->bitmap_ops->register_group(mddev))
>   			pr_warn("md: cannot register extra bitmap attributes for %s\n",
>   				mdname(mddev));
>   		else
> @@ -724,8 +724,8 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
>   static void mddev_clear_bitmap_ops(struct mddev *mddev)
>   {
>   	if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
> -	    mddev->bitmap_ops->group)
> -		sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
> +	    mddev->bitmap_ops->unregister_group)
> +		mddev->bitmap_ops->unregister_group(mddev);
>   
>   	mddev->bitmap_ops = NULL;
>   }
> @@ -6369,6 +6369,14 @@ struct mddev *md_alloc(dev_t dev, char *name)
>   		return ERR_PTR(error);
>   	}
>   
> +	/*
> +	 * md_sysfs_remove_common_group is not needed because mddev_delayed_delete
> +	 * calls kobject_put(&mddev->kobj) if mddev is to be deleted.
> +	 */
> +	if (md_sysfs_create_common_group(mddev))
> +		pr_warn("md: cannot register common bitmap attributes for %s\n",
> +			mdname(mddev));
> +
>   	kobject_uevent(&mddev->kobj, KOBJ_ADD);
>   	mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd, "array_state");
>   	mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd, "level");

-- 
Thansk,
Kuai

Re: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing

[Su Yue]

On Wed 04 Mar 2026 at 10:30, "Yu Kuai" <yukuai@fnnas.com> wrote:

> Hi,
>
> 在 2026/3/3 11:37, Su Yue 写道:
>> Before commit fb8cc3b0d9db ("md/md-bitmap: delay registration 
>> of bitmap_ops until creating bitmap")
>> if CONFIG_MD_BITMAP is enabled, both bitmap none, internal and 
>> clustered have
>> the sysfs file bitmap/location.
>>
>> After the commit, if bitmap is none, bitmap/location doesn't 
>> exist anymore.
>> It breaks 'grow' behavior of a md array of madam with 
>> MD_FEATURE_BITMAP_OFFSET.
>> Take level=mirror and metadata=1.2 as an example:
>>
>> $ mdadm  --create  /dev/md0 -f --bitmap=none --raid-devices=2 
>> --level=mirror \
>>    --metadata=1.2 /dev/vdd /dev/vde
>> $ mdadm --grow /dev/md0 --bitmap=internal
>> $ cat /sys/block/md0/md/bitmap/location
>> Before:+8
>> After: +2
>>
>> While growing bitmap from none to internal, clustered and 
>> llbitmap,
>> mdadm/Grow.c:Grow_addbitmap() tries to detect bitmap/location 
>> first.
>> 1)If bitmap/location exists, it sets bitmap/location after 
>> getinfo_super().
>> 2)If bitmap/location doesn't exist, mdadm just calls 
>> md_set_array_info() then
>> mddev->bitmap_info.default_offset will be used.
>> Situation can be worse if growing none to clustered, bitmap 
>> offset of the node
>> calling `madm --grow` will be changed but the other node are 
>> reading bitmap sb from
>> the old location.
>
> Now that we have a new sysfs attribute bitmap_type, can we fix 
> this by:
> - in the kernel, allow writing to this file in this case;
> - in mdadm and the grow case above, write to this file first, 
> and change
> bitmap_type from none to bitmap(For llbitmap, there is still 
> more work to do).
>
Yes. It's indeed feasible. But how about old versions mdadm? We 
can't require
users' madadm + kernel combinations for old feature. Kernel part 
should keep
compatibility with userspace. sysfs changes and broken haviros are 
not ideal
especially userspace depends on it unless there's a strong reason.
That's why linux/Documentation/ABI exists.

--
Su

>>
>> Here restore sysfs file bitmap/location for ID_BITMAP_NONE and 
>> ID_BITMAP.
>> And it d adds the entry for llbitmap too.
>>
>> New attribute_group md_bitmap_common_group is introduced and 
>> created in
>> md_alloc() as before commit fb8cc3b0d9db.
>> Add New operations register_group and unregister_group to 
>> struct bitmap_operations.
>>
>> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of 
>> bitmap_ops until creating bitmap")
>> Signed-off-by: Su Yue <glass.su@suse.com>
>> ---
>>   drivers/md/md-bitmap.c   | 32 
>>   +++++++++++++++++++++++++++++++-
>>   drivers/md/md-bitmap.h   |  5 +++++
>>   drivers/md/md-llbitmap.c | 13 +++++++++++++
>>   drivers/md/md.c          | 16 ++++++++++++----
>>   4 files changed, 61 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index 83378c033c72..8ff1dc94ed78 100644
>> --- a/drivers/md/md-bitmap.c
>> +++ b/drivers/md/md-bitmap.c
>> @@ -2956,7 +2956,6 @@ __ATTR(max_backlog_used, S_IRUGO | 
>> S_IWUSR,
>>          behind_writes_used_show, behind_writes_used_reset);
>>
>>   static struct attribute *md_bitmap_attrs[] = {
>> -	&bitmap_location.attr,
>>   	&bitmap_space.attr,
>>   	&bitmap_timeout.attr,
>>   	&bitmap_backlog.attr,
>> @@ -2967,11 +2966,40 @@ static struct attribute 
>> *md_bitmap_attrs[] = {
>>   	NULL
>>   };
>>
>> +static struct attribute *md_bitmap_common_attrs[] = {
>> +	&bitmap_location.attr,
>> +	NULL
>> +};
>> +
>>   static struct attribute_group md_bitmap_group = {
>>   	.name = "bitmap",
>>   	.attrs = md_bitmap_attrs,
>>   };
>>
>> +static struct attribute_group md_bitmap_common_group = {
>> +	.name = "bitmap",
>> +	.attrs = md_bitmap_common_attrs,
>> +};
>> +
>> +int md_sysfs_create_common_group(struct mddev *mddev)
>> +{
>> +	return sysfs_create_group(&mddev->kobj, 
>> &md_bitmap_common_group);
>> +}
>> +
>> +static int bitmap_register_group(struct mddev *mddev)
>> +{
>> +	/*
>> +	 * md_bitmap_group and md_bitmap_common_group are using 
>> same name
>> +	 * 'bitmap'.
>> +	 */
>> +	return sysfs_merge_group(&mddev->kobj, &md_bitmap_group);
>> +}
>> +
>> +static void bitmap_unregister_group(struct mddev *mddev)
>> +{
>> +	sysfs_unmerge_group(&mddev->kobj, &md_bitmap_group);
>> +}
>> +
>>   static struct bitmap_operations bitmap_ops = {
>>   	.head = {
>>   		.type	= MD_BITMAP,
>> @@ -3013,6 +3041,8 @@ static struct bitmap_operations 
>> bitmap_ops = {
>>   	.set_pages		= bitmap_set_pages,
>>   	.free			= md_bitmap_free,
>>
>> +	.register_group		= bitmap_register_group,
>> +	.unregister_group	= bitmap_unregister_group,
>>   	.group			= &md_bitmap_group,
>>   };
>>
>> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
>> index b42a28fa83a0..371791e9011d 100644
>> --- a/drivers/md/md-bitmap.h
>> +++ b/drivers/md/md-bitmap.h
>> @@ -125,6 +125,9 @@ struct bitmap_operations {
>>   	void (*set_pages)(void *data, unsigned long pages);
>>   	void (*free)(void *data);
>>
>> +	int (*register_group)(struct mddev *mddev);
>> +	void (*unregister_group)(struct mddev *mddev);
>> +
>>   	struct attribute_group *group;
>>   };
>>
>> @@ -169,6 +172,8 @@ static inline void 
>> md_bitmap_end_sync(struct mddev *mddev, sector_t offset,
>>   	mddev->bitmap_ops->end_sync(mddev, offset, blocks);
>>   }
>>
>> +int md_sysfs_create_common_group(struct mddev *mddev);
>> +
>>   #ifdef CONFIG_MD_BITMAP
>>   int md_bitmap_init(void);
>>   void md_bitmap_exit(void);
>> diff --git a/drivers/md/md-llbitmap.c 
>> b/drivers/md/md-llbitmap.c
>> index bf398d7476b3..24ff5f7f8751 100644
>> --- a/drivers/md/md-llbitmap.c
>> +++ b/drivers/md/md-llbitmap.c
>> @@ -1561,6 +1561,16 @@ static struct attribute_group 
>> md_llbitmap_group = {
>>   	.attrs = md_llbitmap_attrs,
>>   };
>>
>> +static int llbitmap_register_group(struct mddev *mddev)
>> +{
>> +	return sysfs_create_group(&mddev->kobj, 
>> &md_llbitmap_group);
>> +}
>> +
>> +static void llbitmap_unregister_group(struct mddev *mddev)
>> +{
>> +	sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
>> +}
>> +
>>   static struct bitmap_operations llbitmap_ops = {
>>   	.head = {
>>   		.type	= MD_BITMAP,
>> @@ -1597,6 +1607,9 @@ static struct bitmap_operations 
>> llbitmap_ops = {
>>   	.dirty_bits		= llbitmap_dirty_bits,
>>   	.write_all		= llbitmap_write_all,
>>
>> +	.register_group		= llbitmap_register_group,
>> +	.unregister_group	= llbitmap_unregister_group,
>> +
>>   	.group			= &md_llbitmap_group,
>>   };
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 3ce6f9e9d38e..ab969e950ea8 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -703,8 +703,8 @@ static bool mddev_set_bitmap_ops(struct 
>> mddev *mddev)
>>   	mddev->bitmap_ops = (void *)head;
>>   	xa_unlock(&md_submodule);
>>
>> -	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
>> -		if (sysfs_create_group(&mddev->kobj, 
>> mddev->bitmap_ops->group))
>> +	if (!mddev_is_dm(mddev) && 
>> mddev->bitmap_ops->register_group) {
>> +		if (mddev->bitmap_ops->register_group(mddev))
>>   			pr_warn("md: cannot register extra bitmap 
>>   attributes for %s\n",
>>   				mdname(mddev));
>>   		else
>> @@ -724,8 +724,8 @@ static bool mddev_set_bitmap_ops(struct 
>> mddev *mddev)
>>   static void mddev_clear_bitmap_ops(struct mddev *mddev)
>>   {
>>   	if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
>> -	    mddev->bitmap_ops->group)
>> -		sysfs_remove_group(&mddev->kobj, 
>> mddev->bitmap_ops->group);
>> +	    mddev->bitmap_ops->unregister_group)
>> +		mddev->bitmap_ops->unregister_group(mddev);
>>
>>   	mddev->bitmap_ops = NULL;
>>   }
>> @@ -6369,6 +6369,14 @@ struct mddev *md_alloc(dev_t dev, char 
>> *name)
>>   		return ERR_PTR(error);
>>   	}
>>
>> +	/*
>> +	 * md_sysfs_remove_common_group is not needed because 
>> mddev_delayed_delete
>> +	 * calls kobject_put(&mddev->kobj) if mddev is to be 
>> deleted.
>> +	 */
>> +	if (md_sysfs_create_common_group(mddev))
>> +		pr_warn("md: cannot register common bitmap attributes 
>> for %s\n",
>> +			mdname(mddev));
>> +
>>   	kobject_uevent(&mddev->kobj, KOBJ_ADD);
>>   	mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd, 
>>   "array_state");
>>   	mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd, 
>>   "level");

Re: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing

[Yu Kuai]

Hi,

在 2026/3/4 11:14, Su Yue 写道:
> On Wed 04 Mar 2026 at 10:30, "Yu Kuai" <yukuai@fnnas.com> wrote:
>
>> Hi,
>>
>> 在 2026/3/3 11:37, Su Yue 写道:
>>> Before commit fb8cc3b0d9db ("md/md-bitmap: delay registration of 
>>> bitmap_ops until creating bitmap")
>>> if CONFIG_MD_BITMAP is enabled, both bitmap none, internal and 
>>> clustered have
>>> the sysfs file bitmap/location.
>>>
>>> After the commit, if bitmap is none, bitmap/location doesn't exist 
>>> anymore.
>>> It breaks 'grow' behavior of a md array of madam with 
>>> MD_FEATURE_BITMAP_OFFSET.
>>> Take level=mirror and metadata=1.2 as an example:
>>>
>>> $ mdadm  --create  /dev/md0 -f --bitmap=none --raid-devices=2 
>>> --level=mirror \
>>>    --metadata=1.2 /dev/vdd /dev/vde
>>> $ mdadm --grow /dev/md0 --bitmap=internal
>>> $ cat /sys/block/md0/md/bitmap/location
>>> Before:+8
>>> After: +2
>>>
>>> While growing bitmap from none to internal, clustered and llbitmap,
>>> mdadm/Grow.c:Grow_addbitmap() tries to detect bitmap/location first.
>>> 1)If bitmap/location exists, it sets bitmap/location after 
>>> getinfo_super().
>>> 2)If bitmap/location doesn't exist, mdadm just calls 
>>> md_set_array_info() then
>>> mddev->bitmap_info.default_offset will be used.
>>> Situation can be worse if growing none to clustered, bitmap offset 
>>> of the node
>>> calling `madm --grow` will be changed but the other node are reading 
>>> bitmap sb from
>>> the old location.
>>
>> Now that we have a new sysfs attribute bitmap_type, can we fix this by:
>> - in the kernel, allow writing to this file in this case;
>> - in mdadm and the grow case above, write to this file first, and change
>> bitmap_type from none to bitmap(For llbitmap, there is still more 
>> work to do).
>>
> Yes. It's indeed feasible. But how about old versions mdadm? We can't 
> require
> users' madadm + kernel combinations for old feature. Kernel part 
> should keep
> compatibility with userspace. sysfs changes and broken haviros are not 
> ideal
> especially userspace depends on it unless there's a strong reason.
> That's why linux/Documentation/ABI exists.

Okay, I can accept keep this old behavior.

However, instead of introducing a new common_group with the same name "bitmap",
I'll prefer to introducing a separate bitmap_ops for none bitmap as well, and
you can define the attrs that are necessary.

For llbitmap, I think it's fine, we don't need this old sysfs attr anyway. I'll
support to convert from none/bitmap to llbitmap by writing the new bitmap_type
file.
  

>
> -- 
> Su
>
>>>
>>> Here restore sysfs file bitmap/location for ID_BITMAP_NONE and 
>>> ID_BITMAP.
>>> And it d adds the entry for llbitmap too.
>>>
>>> New attribute_group md_bitmap_common_group is introduced and created in
>>> md_alloc() as before commit fb8cc3b0d9db.
>>> Add New operations register_group and unregister_group to struct 
>>> bitmap_operations.
>>>
>>> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops 
>>> until creating bitmap")
>>> Signed-off-by: Su Yue <glass.su@suse.com>
>>> ---
>>>   drivers/md/md-bitmap.c   | 32 +++++++++++++++++++++++++++++++-
>>>   drivers/md/md-bitmap.h   |  5 +++++
>>>   drivers/md/md-llbitmap.c | 13 +++++++++++++
>>>   drivers/md/md.c          | 16 ++++++++++++----
>>>   4 files changed, 61 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>>> index 83378c033c72..8ff1dc94ed78 100644
>>> --- a/drivers/md/md-bitmap.c
>>> +++ b/drivers/md/md-bitmap.c
>>> @@ -2956,7 +2956,6 @@ __ATTR(max_backlog_used, S_IRUGO | S_IWUSR,
>>>          behind_writes_used_show, behind_writes_used_reset);
>>>
>>>   static struct attribute *md_bitmap_attrs[] = {
>>> -    &bitmap_location.attr,
>>>       &bitmap_space.attr,
>>>       &bitmap_timeout.attr,
>>>       &bitmap_backlog.attr,
>>> @@ -2967,11 +2966,40 @@ static struct attribute *md_bitmap_attrs[] = {
>>>       NULL
>>>   };
>>>
>>> +static struct attribute *md_bitmap_common_attrs[] = {
>>> +    &bitmap_location.attr,
>>> +    NULL
>>> +};
>>> +
>>>   static struct attribute_group md_bitmap_group = {
>>>       .name = "bitmap",
>>>       .attrs = md_bitmap_attrs,
>>>   };
>>>
>>> +static struct attribute_group md_bitmap_common_group = {
>>> +    .name = "bitmap",
>>> +    .attrs = md_bitmap_common_attrs,
>>> +};
>>> +
>>> +int md_sysfs_create_common_group(struct mddev *mddev)
>>> +{
>>> +    return sysfs_create_group(&mddev->kobj, &md_bitmap_common_group);
>>> +}
>>> +
>>> +static int bitmap_register_group(struct mddev *mddev)
>>> +{
>>> +    /*
>>> +     * md_bitmap_group and md_bitmap_common_group are using same name
>>> +     * 'bitmap'.
>>> +     */
>>> +    return sysfs_merge_group(&mddev->kobj, &md_bitmap_group);
>>> +}
>>> +
>>> +static void bitmap_unregister_group(struct mddev *mddev)
>>> +{
>>> +    sysfs_unmerge_group(&mddev->kobj, &md_bitmap_group);
>>> +}
>>> +
>>>   static struct bitmap_operations bitmap_ops = {
>>>       .head = {
>>>           .type    = MD_BITMAP,
>>> @@ -3013,6 +3041,8 @@ static struct bitmap_operations bitmap_ops = {
>>>       .set_pages        = bitmap_set_pages,
>>>       .free            = md_bitmap_free,
>>>
>>> +    .register_group        = bitmap_register_group,
>>> +    .unregister_group    = bitmap_unregister_group,
>>>       .group            = &md_bitmap_group,
>>>   };
>>>
>>> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
>>> index b42a28fa83a0..371791e9011d 100644
>>> --- a/drivers/md/md-bitmap.h
>>> +++ b/drivers/md/md-bitmap.h
>>> @@ -125,6 +125,9 @@ struct bitmap_operations {
>>>       void (*set_pages)(void *data, unsigned long pages);
>>>       void (*free)(void *data);
>>>
>>> +    int (*register_group)(struct mddev *mddev);
>>> +    void (*unregister_group)(struct mddev *mddev);
>>> +
>>>       struct attribute_group *group;
>>>   };
>>>
>>> @@ -169,6 +172,8 @@ static inline void md_bitmap_end_sync(struct 
>>> mddev *mddev, sector_t offset,
>>>       mddev->bitmap_ops->end_sync(mddev, offset, blocks);
>>>   }
>>>
>>> +int md_sysfs_create_common_group(struct mddev *mddev);
>>> +
>>>   #ifdef CONFIG_MD_BITMAP
>>>   int md_bitmap_init(void);
>>>   void md_bitmap_exit(void);
>>> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
>>> index bf398d7476b3..24ff5f7f8751 100644
>>> --- a/drivers/md/md-llbitmap.c
>>> +++ b/drivers/md/md-llbitmap.c
>>> @@ -1561,6 +1561,16 @@ static struct attribute_group 
>>> md_llbitmap_group = {
>>>       .attrs = md_llbitmap_attrs,
>>>   };
>>>
>>> +static int llbitmap_register_group(struct mddev *mddev)
>>> +{
>>> +    return sysfs_create_group(&mddev->kobj, &md_llbitmap_group);
>>> +}
>>> +
>>> +static void llbitmap_unregister_group(struct mddev *mddev)
>>> +{
>>> +    sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
>>> +}
>>> +
>>>   static struct bitmap_operations llbitmap_ops = {
>>>       .head = {
>>>           .type    = MD_BITMAP,
>>> @@ -1597,6 +1607,9 @@ static struct bitmap_operations llbitmap_ops = {
>>>       .dirty_bits        = llbitmap_dirty_bits,
>>>       .write_all        = llbitmap_write_all,
>>>
>>> +    .register_group        = llbitmap_register_group,
>>> +    .unregister_group    = llbitmap_unregister_group,
>>> +
>>>       .group            = &md_llbitmap_group,
>>>   };
>>>
>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>> index 3ce6f9e9d38e..ab969e950ea8 100644
>>> --- a/drivers/md/md.c
>>> +++ b/drivers/md/md.c
>>> @@ -703,8 +703,8 @@ static bool mddev_set_bitmap_ops(struct mddev 
>>> *mddev)
>>>       mddev->bitmap_ops = (void *)head;
>>>       xa_unlock(&md_submodule);
>>>
>>> -    if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
>>> -        if (sysfs_create_group(&mddev->kobj, 
>>> mddev->bitmap_ops->group))
>>> +    if (!mddev_is_dm(mddev) && mddev->bitmap_ops->register_group) {
>>> +        if (mddev->bitmap_ops->register_group(mddev))
>>>               pr_warn("md: cannot register extra bitmap attributes 
>>> for %s\n",
>>>                   mdname(mddev));
>>>           else
>>> @@ -724,8 +724,8 @@ static bool mddev_set_bitmap_ops(struct mddev 
>>> *mddev)
>>>   static void mddev_clear_bitmap_ops(struct mddev *mddev)
>>>   {
>>>       if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
>>> -        mddev->bitmap_ops->group)
>>> -        sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
>>> +        mddev->bitmap_ops->unregister_group)
>>> +        mddev->bitmap_ops->unregister_group(mddev);
>>>
>>>       mddev->bitmap_ops = NULL;
>>>   }
>>> @@ -6369,6 +6369,14 @@ struct mddev *md_alloc(dev_t dev, char *name)
>>>           return ERR_PTR(error);
>>>       }
>>>
>>> +    /*
>>> +     * md_sysfs_remove_common_group is not needed because 
>>> mddev_delayed_delete
>>> +     * calls kobject_put(&mddev->kobj) if mddev is to be deleted.
>>> +     */
>>> +    if (md_sysfs_create_common_group(mddev))
>>> +        pr_warn("md: cannot register common bitmap attributes for 
>>> %s\n",
>>> +            mdname(mddev));
>>> +
>>>       kobject_uevent(&mddev->kobj, KOBJ_ADD);
>>>       mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd,   
>>> "array_state");
>>>       mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd,   
>>> "level");

-- 
Thansk,
Kuai

Re: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing

[kernel test robot]

Hi Su,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v7.0-rc2 next-20260305]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Su-Yue/md-restore-bitmap-location-to-fix-wrong-bitmap-offset-while-growing/20260303-114108
base:   linus/master
patch link:    https://lore.kernel.org/r/20260303033731.83885-2-glass.su%40suse.com
patch subject: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing
config: i386-randconfig-r064-20260305 (https://download.01.org/0day-ci/archive/20260306/202603060651.tJ5tGHRo-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260306/202603060651.tJ5tGHRo-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603060651.tJ5tGHRo-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: md_sysfs_create_common_group
   >>> referenced by md.c:6376 (drivers/md/md.c:6376)
   >>>               drivers/md/md.o:(md_alloc) in archive vmlinux.a

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing

[kernel test robot]

Hi Su,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v7.0-rc2 next-20260305]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Su-Yue/md-restore-bitmap-location-to-fix-wrong-bitmap-offset-while-growing/20260303-114108
base:   linus/master
patch link:    https://lore.kernel.org/r/20260303033731.83885-2-glass.su%40suse.com
patch subject: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing
config: i386-randconfig-051-20260305 (https://download.01.org/0day-ci/archive/20260306/202603060614.AZY6J9w9-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260306/202603060614.AZY6J9w9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603060614.AZY6J9w9-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: drivers/md/md.o: in function `md_alloc':
>> drivers/md/md.c:6376:(.text+0xb36c): undefined reference to `md_sysfs_create_common_group'


vim +6376 drivers/md/md.c

  6276	
  6277	struct mddev *md_alloc(dev_t dev, char *name)
  6278	{
  6279		/*
  6280		 * If dev is zero, name is the name of a device to allocate with
  6281		 * an arbitrary minor number.  It will be "md_???"
  6282		 * If dev is non-zero it must be a device number with a MAJOR of
  6283		 * MD_MAJOR or mdp_major.  In this case, if "name" is NULL, then
  6284		 * the device is being created by opening a node in /dev.
  6285		 * If "name" is not NULL, the device is being created by
  6286		 * writing to /sys/module/md_mod/parameters/new_array.
  6287		 */
  6288		static DEFINE_MUTEX(disks_mutex);
  6289		struct mddev *mddev;
  6290		struct gendisk *disk;
  6291		int partitioned;
  6292		int shift;
  6293		int unit;
  6294		int error;
  6295	
  6296		/*
  6297		 * Wait for any previous instance of this device to be completely
  6298		 * removed (mddev_delayed_delete).
  6299		 */
  6300		flush_workqueue(md_misc_wq);
  6301	
  6302		mutex_lock(&disks_mutex);
  6303		mddev = mddev_alloc(dev);
  6304		if (IS_ERR(mddev)) {
  6305			error = PTR_ERR(mddev);
  6306			goto out_unlock;
  6307		}
  6308	
  6309		partitioned = (MAJOR(mddev->unit) != MD_MAJOR);
  6310		shift = partitioned ? MdpMinorShift : 0;
  6311		unit = MINOR(mddev->unit) >> shift;
  6312	
  6313		if (name && !dev) {
  6314			/* Need to ensure that 'name' is not a duplicate.
  6315			 */
  6316			struct mddev *mddev2;
  6317			spin_lock(&all_mddevs_lock);
  6318	
  6319			list_for_each_entry(mddev2, &all_mddevs, all_mddevs)
  6320				if (mddev2->gendisk &&
  6321				    strcmp(mddev2->gendisk->disk_name, name) == 0) {
  6322					spin_unlock(&all_mddevs_lock);
  6323					error = -EEXIST;
  6324					goto out_free_mddev;
  6325				}
  6326			spin_unlock(&all_mddevs_lock);
  6327		}
  6328		if (name && dev)
  6329			/*
  6330			 * Creating /dev/mdNNN via "newarray", so adjust hold_active.
  6331			 */
  6332			mddev->hold_active = UNTIL_STOP;
  6333	
  6334		disk = blk_alloc_disk(NULL, NUMA_NO_NODE);
  6335		if (IS_ERR(disk)) {
  6336			error = PTR_ERR(disk);
  6337			goto out_free_mddev;
  6338		}
  6339	
  6340		disk->major = MAJOR(mddev->unit);
  6341		disk->first_minor = unit << shift;
  6342		disk->minors = 1 << shift;
  6343		if (name)
  6344			strcpy(disk->disk_name, name);
  6345		else if (partitioned)
  6346			sprintf(disk->disk_name, "md_d%d", unit);
  6347		else
  6348			sprintf(disk->disk_name, "md%d", unit);
  6349		disk->fops = &md_fops;
  6350		disk->private_data = mddev;
  6351	
  6352		disk->events |= DISK_EVENT_MEDIA_CHANGE;
  6353		mddev->gendisk = disk;
  6354		error = add_disk(disk);
  6355		if (error)
  6356			goto out_put_disk;
  6357	
  6358		kobject_init(&mddev->kobj, &md_ktype);
  6359		error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");
  6360		if (error) {
  6361			/*
  6362			 * The disk is already live at this point.  Clear the hold flag
  6363			 * and let mddev_put take care of the deletion, as it isn't any
  6364			 * different from a normal close on last release now.
  6365			 */
  6366			mddev->hold_active = 0;
  6367			mutex_unlock(&disks_mutex);
  6368			mddev_put(mddev);
  6369			return ERR_PTR(error);
  6370		}
  6371	
  6372		/*
  6373		 * md_sysfs_remove_common_group is not needed because mddev_delayed_delete
  6374		 * calls kobject_put(&mddev->kobj) if mddev is to be deleted.
  6375		 */
> 6376		if (md_sysfs_create_common_group(mddev))
  6377			pr_warn("md: cannot register common bitmap attributes for %s\n",
  6378				mdname(mddev));
  6379	
  6380		kobject_uevent(&mddev->kobj, KOBJ_ADD);
  6381		mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd, "array_state");
  6382		mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd, "level");
  6383		mutex_unlock(&disks_mutex);
  6384		return mddev;
  6385	
  6386	out_put_disk:
  6387		put_disk(disk);
  6388	out_free_mddev:
  6389		mddev_free(mddev);
  6390	out_unlock:
  6391		mutex_unlock(&disks_mutex);
  6392		return ERR_PTR(error);
  6393	}
  6394	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing

[Glass Su]

> On Mar 6, 2026, at 06:50, kernel test robot <lkp@intel.com> wrote:
> 
> Hi Su,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v7.0-rc2 next-20260305]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Su-Yue/md-restore-bitmap-location-to-fix-wrong-bitmap-offset-while-growing/20260303-114108
> base:   linus/master
> patch link:    https://lore.kernel.org/r/20260303033731.83885-2-glass.su%40suse.com
> patch subject: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing
> config: i386-randconfig-051-20260305 (https://download.01.org/0day-ci/archive/20260306/202603060614.AZY6J9w9-lkp@intel.com/config)
> compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260306/202603060614.AZY6J9w9-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202603060614.AZY6J9w9-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>   ld: drivers/md/md.o: in function `md_alloc':
>>> drivers/md/md.c:6376:(.text+0xb36c): undefined reference to `md_sysfs_create_common_group'

Forget to test it with 

# CONFIG_MD_BITMAP is not set

Anyway, I will do  revision as Yu Kuai’s suggestion.

— 
Su

> 

> 
> vim +6376 drivers/md/md.c
> 
>  6276 
>  6277 struct mddev *md_alloc(dev_t dev, char *name)
>  6278 {
>  6279 /*
>  6280 * If dev is zero, name is the name of a device to allocate with
>  6281 * an arbitrary minor number.  It will be "md_???"
>  6282 * If dev is non-zero it must be a device number with a MAJOR of
>  6283 * MD_MAJOR or mdp_major.  In this case, if "name" is NULL, then
>  6284 * the device is being created by opening a node in /dev.
>  6285 * If "name" is not NULL, the device is being created by
>  6286 * writing to /sys/module/md_mod/parameters/new_array.
>  6287 */
>  6288 static DEFINE_MUTEX(disks_mutex);
>  6289 struct mddev *mddev;
>  6290 struct gendisk *disk;
>  6291 int partitioned;
>  6292 int shift;
>  6293 int unit;
>  6294 int error;
>  6295 
>  6296 /*
>  6297 * Wait for any previous instance of this device to be completely
>  6298 * removed (mddev_delayed_delete).
>  6299 */
>  6300 flush_workqueue(md_misc_wq);
>  6301 
>  6302 mutex_lock(&disks_mutex);
>  6303 mddev = mddev_alloc(dev);
>  6304 if (IS_ERR(mddev)) {
>  6305 error = PTR_ERR(mddev);
>  6306 goto out_unlock;
>  6307 }
>  6308 
>  6309 partitioned = (MAJOR(mddev->unit) != MD_MAJOR);
>  6310 shift = partitioned ? MdpMinorShift : 0;
>  6311 unit = MINOR(mddev->unit) >> shift;
>  6312 
>  6313 if (name && !dev) {
>  6314 /* Need to ensure that 'name' is not a duplicate.
>  6315 */
>  6316 struct mddev *mddev2;
>  6317 spin_lock(&all_mddevs_lock);
>  6318 
>  6319 list_for_each_entry(mddev2, &all_mddevs, all_mddevs)
>  6320 if (mddev2->gendisk &&
>  6321    strcmp(mddev2->gendisk->disk_name, name) == 0) {
>  6322 spin_unlock(&all_mddevs_lock);
>  6323 error = -EEXIST;
>  6324 goto out_free_mddev;
>  6325 }
>  6326 spin_unlock(&all_mddevs_lock);
>  6327 }
>  6328 if (name && dev)
>  6329 /*
>  6330 * Creating /dev/mdNNN via "newarray", so adjust hold_active.
>  6331 */
>  6332 mddev->hold_active = UNTIL_STOP;
>  6333 
>  6334 disk = blk_alloc_disk(NULL, NUMA_NO_NODE);
>  6335 if (IS_ERR(disk)) {
>  6336 error = PTR_ERR(disk);
>  6337 goto out_free_mddev;
>  6338 }
>  6339 
>  6340 disk->major = MAJOR(mddev->unit);
>  6341 disk->first_minor = unit << shift;
>  6342 disk->minors = 1 << shift;
>  6343 if (name)
>  6344 strcpy(disk->disk_name, name);
>  6345 else if (partitioned)
>  6346 sprintf(disk->disk_name, "md_d%d", unit);
>  6347 else
>  6348 sprintf(disk->disk_name, "md%d", unit);
>  6349 disk->fops = &md_fops;
>  6350 disk->private_data = mddev;
>  6351 
>  6352 disk->events |= DISK_EVENT_MEDIA_CHANGE;
>  6353 mddev->gendisk = disk;
>  6354 error = add_disk(disk);
>  6355 if (error)
>  6356 goto out_put_disk;
>  6357 
>  6358 kobject_init(&mddev->kobj, &md_ktype);
>  6359 error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");
>  6360 if (error) {
>  6361 /*
>  6362 * The disk is already live at this point.  Clear the hold flag
>  6363 * and let mddev_put take care of the deletion, as it isn't any
>  6364 * different from a normal close on last release now.
>  6365 */
>  6366 mddev->hold_active = 0;
>  6367 mutex_unlock(&disks_mutex);
>  6368 mddev_put(mddev);
>  6369 return ERR_PTR(error);
>  6370 }
>  6371 
>  6372 /*
>  6373 * md_sysfs_remove_common_group is not needed because mddev_delayed_delete
>  6374 * calls kobject_put(&mddev->kobj) if mddev is to be deleted.
>  6375 */
>> 6376 if (md_sysfs_create_common_group(mddev))
>  6377 pr_warn("md: cannot register common bitmap attributes for %s\n",
>  6378 mdname(mddev));
>  6379 
>  6380 kobject_uevent(&mddev->kobj, KOBJ_ADD);
>  6381 mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd, "array_state");
>  6382 mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd, "level");
>  6383 mutex_unlock(&disks_mutex);
>  6384 return mddev;
>  6385 
>  6386 out_put_disk:
>  6387 put_disk(disk);
>  6388 out_free_mddev:
>  6389 mddev_free(mddev);
>  6390 out_unlock:
>  6391 mutex_unlock(&disks_mutex);
>  6392 return ERR_PTR(error);
>  6393 }
>  6394 
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing

[Glass Su]

[-- Attachment #1: Type: text/plain, Size: 5392 bytes --]



> On Mar 6, 2026, at 01:57, Yu Kuai <yukuai@fnnas.com> wrote:
> 
> Hi,
> 
> 在 2026/3/4 11:14, Su Yue 写道:
>> On Wed 04 Mar 2026 at 10:30, "Yu Kuai" <yukuai@fnnas.com> wrote:
>> 
>>> Hi,
>>> 
>>> 在 2026/3/3 11:37, Su Yue 写道:
>>>> Before commit fb8cc3b0d9db ("md/md-bitmap: delay registration of 
>>>> bitmap_ops until creating bitmap")
>>>> if CONFIG_MD_BITMAP is enabled, both bitmap none, internal and 
>>>> clustered have
>>>> the sysfs file bitmap/location.
>>>> 
>>>> After the commit, if bitmap is none, bitmap/location doesn't exist 
>>>> anymore.
>>>> It breaks 'grow' behavior of a md array of madam with 
>>>> MD_FEATURE_BITMAP_OFFSET.
>>>> Take level=mirror and metadata=1.2 as an example:
>>>> 
>>>> $ mdadm  --create  /dev/md0 -f --bitmap=none --raid-devices=2 
>>>> --level=mirror \
>>>>    --metadata=1.2 /dev/vdd /dev/vde
>>>> $ mdadm --grow /dev/md0 --bitmap=internal
>>>> $ cat /sys/block/md0/md/bitmap/location
>>>> Before:+8
>>>> After: +2
>>>> 
>>>> While growing bitmap from none to internal, clustered and llbitmap,
>>>> mdadm/Grow.c:Grow_addbitmap() tries to detect bitmap/location first.
>>>> 1)If bitmap/location exists, it sets bitmap/location after 
>>>> getinfo_super().
>>>> 2)If bitmap/location doesn't exist, mdadm just calls 
>>>> md_set_array_info() then
>>>> mddev->bitmap_info.default_offset will be used.
>>>> Situation can be worse if growing none to clustered, bitmap offset 
>>>> of the node
>>>> calling `madm --grow` will be changed but the other node are reading 
>>>> bitmap sb from
>>>> the old location.
>>> 
>>> Now that we have a new sysfs attribute bitmap_type, can we fix this by:
>>> - in the kernel, allow writing to this file in this case;
>>> - in mdadm and the grow case above, write to this file first, and change
>>> bitmap_type from none to bitmap(For llbitmap, there is still more 
>>> work to do).
>>> 
>> Yes. It's indeed feasible. But how about old versions mdadm? We can't 
>> require
>> users' madadm + kernel combinations for old feature. Kernel part 
>> should keep
>> compatibility with userspace. sysfs changes and broken haviros are not 
>> ideal
>> especially userspace depends on it unless there's a strong reason.
>> That's why linux/Documentation/ABI exists.
> 
> Okay, I can accept keep this old behavior.
> 
> However, instead of introducing a new common_group with the same name "bitmap",
> I'll prefer to introducing a separate bitmap_ops for none bitmap as well, and
> you can define the attrs that are necessary.
> 

After a try, the thing I realized is that a common group is unavoidable for bitmap and none bitmap.

mdadm writes to /sys/block/md0/md/bitmap/location, if remove_files() called by sysfs_remove_group()/
sysfs_update_group() on same kernfs node, recursive locks will be triggered:

[  139.516750] ============================================
[  139.517363] WARNING: possible recursive locking detected
[  139.517953] 7.0.0-rc1-custom+ #282 Tainted: G           OE
[  139.518628] --------------------------------------------
[  139.519233] mdadm/2346 is trying to acquire lock:
[  139.519836] ffff8e6b24d85000 (kn->active#116){++++}-{0:0}, at: __kernfs_remove+0xd1/0x3e0
[  139.520848]
               but task is already holding lock:
[  139.521561] ffff8e6b24d85000 (kn->active#116){++++}-{0:0}, at: kernfs_fop_write_iter+0x12d/0x250
[  139.522603]
               other info that might help us debug this:
[  139.523383]  Possible unsafe locking scenario:

[  139.524169]        CPU0
[  139.524430]        ----
[  139.524682]   lock(kn->active#116);
[  139.525047]   lock(kn->active#116);
[  139.525398]
                *** DEADLOCK ***

[  139.525990]  May be due to missing lock nesting notation

[  139.526658] 4 locks held by mdadm/2346:
[  139.527049]  #0: ffff8e6acd5ec420 (sb_writers#5){.+.+}-{0:0}, at: ksys_write+0x6c/0xe0
[  139.527838]  #1: ffff8e6acd54fa88 (&of->mutex){+.+.}-{4:4}, at: kernfs_fop_write_iter+0x118/0x250
[  139.528713]  #2: ffff8e6b24d85000 (kn->active#116){++++}-{0:0}, at: kernfs_fop_write_iter+0x12d/0x250
[  139.529594]  #3: ffff8e6b26b51370 (&mddev->reconfig_mutex){+.+.}-{4:4}, at: location_store+0x6c/0x360 [md_mod]

[  139.530535]  dump_stack_lvl+0x68/0x90
[  139.530540]  print_deadlock_bug.cold+0xc0/0xcd
[  139.530549]  __lock_acquire+0x1324/0x2250
[  139.530556]  lock_acquire+0xc6/0x2f0
[  139.530564]  kernfs_drain+0x1eb/0x200
[  139.530568]  __kernfs_remove+0xd1/0x3e0
[  139.530570]  kernfs_remove_by_name_ns+0x5e/0xb0
[  139.530572]  internal_create_group+0x221/0x4d0
[  139.530578]  md_bitmap_create+0x122/0x130 [md_mod]
[  139.530586]  location_store+0x1e9/0x360 [md_mod]
[  139.530594]  md_attr_store+0xb8/0x1a0 [md_mod]
[  139.530602]  kernfs_fop_write_iter+0x176/0x250
[  139.530605]  vfs_write+0x21b/0x560
[  139.530609]  ksys_write+0x6c/0xe0
[  139.530611]  do_syscall_64+0x10f/0x5f0
[  139.530623]  entry_SYSCALL_64_after_hwframe+0x76/0x7e

The  patch implemented by separated bitmap_ops is attached. This version looks pretty similar to the first
version because of the reason listed above and IMO ungraceful. Dummy ops and functions are a little unnecessary.

I would like to ask your opinion since you are the maintainer even though I prefer v1 version + (remove the location entry for llbitmap).
Thanks.


[-- Attachment #2: 000-md-add-dummy-bitmap-ops-for-none-to-fix-wrong-bitmap.patch --]
[-- Type: application/octet-stream, Size: 7408 bytes --]

From 1dd81c439b0097d0a9a5975682801ce75448e108 Mon Sep 17 00:00:00 2001
From: Su Yue <glass.su@suse.com>
Date: Sat, 7 Mar 2026 12:30:36 +0800
Subject: [PATCH 1/2] md: add dummy bitmap ops for none to fix wrong bitmap
 offset

Before commit fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
if CONFIG_MD_BITMAP is enabled, both bitmap none, internal and clustered have
the sysfs file bitmap/location.

After the commit, if bitmap is none, bitmap/location doesn't exist anymore.
It breaks 'grow' behavior of a md array of madam with MD_FEATURE_BITMAP_OFFSET.
Take level=mirror and metadata=1.2 as an example:

$ mdadm  --create  /dev/md0 -f --bitmap=none --raid-devices=2 --level=mirror \
  --metadata=1.2 /dev/vdd /dev/vde
$ mdadm --grow /dev/md0 --bitmap=internal
$ cat /sys/block/md0/md/bitmap/location
Before:+8
After: +2

While growing bitmap from none to internal, clustered and llbitmap,
mdadm/Grow.c:Grow_addbitmap() tries to detect bitmap/location first.
1)If bitmap/location exists, it sets bitmap/location after getinfo_super().
2)If bitmap/location doesn't exist, mdadm just calls md_set_array_info() then
mddev->bitmap_info.default_offset will be used.
Situation can be worse if growing none to clustered, bitmap offset of the node
calling `madm --grow` will be changed but the other node are reading bitmap sb from
the old location.

Here introducing a dummy bitmap_operations for ID_BITMAP_NONE to restore sysfs
file bitmap/location for ID_BITMAP_NONE and ID_BITMAP.

Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
Signed-off-by: Su Yue <glass.su@suse.com>
---
 drivers/md/md-bitmap.c | 48 ++++++++++++++++++++++++++++++++++++++++--
 drivers/md/md.c        | 46 ++++++++++++++++++++++++++++------------
 2 files changed, 78 insertions(+), 16 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 83378c033c72..d5354cf35a65 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -240,6 +240,11 @@ static bool bitmap_enabled(void *data, bool flush)
 	       bitmap->storage.filemap != NULL;
 }
 
+static bool dummy_bitmap_enabled(void *data, bool flush)
+{
+	return false;
+}
+
 /*
  * check a page and, if necessary, allocate it (or hijack it if the alloc fails)
  *
@@ -2201,6 +2206,11 @@ static int bitmap_create(struct mddev *mddev)
 	return 0;
 }
 
+static int dummy_bitmap_create(struct mddev *mddev)
+{
+	return 0;
+}
+
 static int bitmap_load(struct mddev *mddev)
 {
 	int err = 0;
@@ -2956,7 +2966,7 @@ __ATTR(max_backlog_used, S_IRUGO | S_IWUSR,
        behind_writes_used_show, behind_writes_used_reset);
 
 static struct attribute *md_bitmap_attrs[] = {
-	&bitmap_location.attr,
+//	&bitmap_location.attr,
 	&bitmap_space.attr,
 	&bitmap_timeout.attr,
 	&bitmap_backlog.attr,
@@ -2972,6 +2982,17 @@ static struct attribute_group md_bitmap_group = {
 	.attrs = md_bitmap_attrs,
 };
 
+/* Only necessary attrs for compatibility */
+static struct attribute *md_dummy_bitmap_attrs[] = {
+	&bitmap_location.attr,
+	NULL
+};
+
+static struct attribute_group md_dummy_bitmap_group = {
+	.name = "bitmap",
+	.attrs = md_dummy_bitmap_attrs,
+};
+
 static struct bitmap_operations bitmap_ops = {
 	.head = {
 		.type	= MD_BITMAP,
@@ -3016,18 +3037,41 @@ static struct bitmap_operations bitmap_ops = {
 	.group			= &md_bitmap_group,
 };
 
+static struct bitmap_operations dummy_bitmap_ops = {
+	.head = {
+		.type	= MD_BITMAP,
+		.id	= ID_BITMAP_NONE,
+		.name	= "none",
+	},
+
+	.enabled		= dummy_bitmap_enabled,
+	.create			= dummy_bitmap_create,
+	.destroy		= bitmap_destroy,
+	.load			= bitmap_load,
+	.get_stats		= bitmap_get_stats,
+	.free			= md_bitmap_free,
+	.group			= &md_dummy_bitmap_group,
+};
+
 int md_bitmap_init(void)
 {
+	int ret;
+
 	md_bitmap_wq = alloc_workqueue("md_bitmap", WQ_MEM_RECLAIM | WQ_UNBOUND,
 				       0);
 	if (!md_bitmap_wq)
 		return -ENOMEM;
 
-	return register_md_submodule(&bitmap_ops.head);
+	ret = register_md_submodule(&bitmap_ops.head);
+	if (ret)
+		return ret;
+
+	return register_md_submodule(&dummy_bitmap_ops.head);
 }
 
 void md_bitmap_exit(void)
 {
 	destroy_workqueue(md_bitmap_wq);
 	unregister_md_submodule(&bitmap_ops.head);
+	unregister_md_submodule(&dummy_bitmap_ops.head);
 }
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 3ce6f9e9d38e..94ded8efb725 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -682,9 +682,9 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
 {
 	struct bitmap_operations *old = mddev->bitmap_ops;
 	struct md_submodule_head *head;
+	int err = 0;
 
-	if (mddev->bitmap_id == ID_BITMAP_NONE ||
-	    (old && old->head.id == mddev->bitmap_id))
+	if (old && old->head.id == mddev->bitmap_id)
 		return true;
 
 	xa_lock(&md_submodule);
@@ -704,13 +704,24 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
 	xa_unlock(&md_submodule);
 
 	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
-		if (sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group))
+
+		if (old && old->head.id == ID_BITMAP_NONE) {
+			if (head->id == ID_BITMAP) {
+				err = sysfs_merge_group(&mddev->kobj, mddev->bitmap_ops->group);
+			} else if (head->id == ID_LLBITMAP) {
+				sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
+				err = sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group);
+			}
+		} else {
+			err = sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group);
+		}
+
+		if (err)
 			pr_warn("md: cannot register extra bitmap attributes for %s\n",
 				mdname(mddev));
 		else
 			/*
-			 * Inform user with KOBJ_CHANGE about new bitmap
-			 * attributes.
+			 * Inform user with KOBJ_CHANGE about bitmap attributes changes.
 			 */
 			kobject_uevent(&mddev->kobj, KOBJ_CHANGE);
 	}
@@ -723,11 +734,20 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
 
 static void mddev_clear_bitmap_ops(struct mddev *mddev)
 {
-	if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
-	    mddev->bitmap_ops->group)
-		sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
+	if (mddev_is_dm(mddev) || !mddev->bitmap_ops)
+		return;
 
-	mddev->bitmap_ops = NULL;
+	if (mddev->bitmap_ops->head.id == ID_BITMAP_NONE)
+		return;
+
+	if (mddev->bitmap_ops->group)
+		sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
+	/*
+	 * group of ID_BITMAP_NONE is removed when new bitmap is
+	 * creating in mddev_set_bitmap_ops().
+	 */
+	mddev->bitmap_id = ID_BITMAP_NONE;
+	md_bitmap_create(mddev);
 }
 
 int mddev_init(struct mddev *mddev)
@@ -6449,9 +6469,6 @@ static int start_dirty_degraded;
 
 static int md_bitmap_create(struct mddev *mddev)
 {
-	if (mddev->bitmap_id == ID_BITMAP_NONE)
-		return -EINVAL;
-
 	if (!mddev_set_bitmap_ops(mddev))
 		return -ENOENT;
 
@@ -6610,8 +6627,9 @@ int md_run(struct mddev *mddev)
 			(unsigned long long)pers->size(mddev, 0, 0) / 2);
 		err = -EINVAL;
 	}
-	if (err == 0 && pers->sync_request &&
-	    (mddev->bitmap_info.file || mddev->bitmap_info.offset)) {
+	if (err == 0 && pers->sync_request) {
+		if (mddev->bitmap_info.offset == 0)
+			mddev->bitmap_id = ID_BITMAP_NONE;
 		err = md_bitmap_create(mddev);
 		if (err)
 			pr_warn("%s: failed to create bitmap (%d)\n",
-- 
2.53.0


[-- Attachment #3: Type: text/plain, Size: 7309 bytes --]



— 
Su



> For llbitmap, I think it's fine, we don't need this old sysfs attr anyway. I'll
> support to convert from none/bitmap to llbitmap by writing the new bitmap_type
> file.
> 
> 
>> 
>> -- 
>> Su
>> 
>>>> 
>>>> Here restore sysfs file bitmap/location for ID_BITMAP_NONE and 
>>>> ID_BITMAP.
>>>> And it d adds the entry for llbitmap too.
>>>> 
>>>> New attribute_group md_bitmap_common_group is introduced and created in
>>>> md_alloc() as before commit fb8cc3b0d9db.
>>>> Add New operations register_group and unregister_group to struct 
>>>> bitmap_operations.
>>>> 
>>>> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops 
>>>> until creating bitmap")
>>>> Signed-off-by: Su Yue <glass.su@suse.com>
>>>> ---
>>>>   drivers/md/md-bitmap.c   | 32 +++++++++++++++++++++++++++++++-
>>>>   drivers/md/md-bitmap.h   |  5 +++++
>>>>   drivers/md/md-llbitmap.c | 13 +++++++++++++
>>>>   drivers/md/md.c          | 16 ++++++++++++----
>>>>   4 files changed, 61 insertions(+), 5 deletions(-)
>>>> 
>>>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>>>> index 83378c033c72..8ff1dc94ed78 100644
>>>> --- a/drivers/md/md-bitmap.c
>>>> +++ b/drivers/md/md-bitmap.c
>>>> @@ -2956,7 +2956,6 @@ __ATTR(max_backlog_used, S_IRUGO | S_IWUSR,
>>>>          behind_writes_used_show, behind_writes_used_reset);
>>>> 
>>>>   static struct attribute *md_bitmap_attrs[] = {
>>>> -    &bitmap_location.attr,
>>>>       &bitmap_space.attr,
>>>>       &bitmap_timeout.attr,
>>>>       &bitmap_backlog.attr,
>>>> @@ -2967,11 +2966,40 @@ static struct attribute *md_bitmap_attrs[] = {
>>>>       NULL
>>>>   };
>>>> 
>>>> +static struct attribute *md_bitmap_common_attrs[] = {
>>>> +    &bitmap_location.attr,
>>>> +    NULL
>>>> +};
>>>> +
>>>>   static struct attribute_group md_bitmap_group = {
>>>>       .name = "bitmap",
>>>>       .attrs = md_bitmap_attrs,
>>>>   };
>>>> 
>>>> +static struct attribute_group md_bitmap_common_group = {
>>>> +    .name = "bitmap",
>>>> +    .attrs = md_bitmap_common_attrs,
>>>> +};
>>>> +
>>>> +int md_sysfs_create_common_group(struct mddev *mddev)
>>>> +{
>>>> +    return sysfs_create_group(&mddev->kobj, &md_bitmap_common_group);
>>>> +}
>>>> +
>>>> +static int bitmap_register_group(struct mddev *mddev)
>>>> +{
>>>> +    /*
>>>> +     * md_bitmap_group and md_bitmap_common_group are using same name
>>>> +     * 'bitmap'.
>>>> +     */
>>>> +    return sysfs_merge_group(&mddev->kobj, &md_bitmap_group);
>>>> +}
>>>> +
>>>> +static void bitmap_unregister_group(struct mddev *mddev)
>>>> +{
>>>> +    sysfs_unmerge_group(&mddev->kobj, &md_bitmap_group);
>>>> +}
>>>> +
>>>>   static struct bitmap_operations bitmap_ops = {
>>>>       .head = {
>>>>           .type    = MD_BITMAP,
>>>> @@ -3013,6 +3041,8 @@ static struct bitmap_operations bitmap_ops = {
>>>>       .set_pages        = bitmap_set_pages,
>>>>       .free            = md_bitmap_free,
>>>> 
>>>> +    .register_group        = bitmap_register_group,
>>>> +    .unregister_group    = bitmap_unregister_group,
>>>>       .group            = &md_bitmap_group,
>>>>   };
>>>> 
>>>> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
>>>> index b42a28fa83a0..371791e9011d 100644
>>>> --- a/drivers/md/md-bitmap.h
>>>> +++ b/drivers/md/md-bitmap.h
>>>> @@ -125,6 +125,9 @@ struct bitmap_operations {
>>>>       void (*set_pages)(void *data, unsigned long pages);
>>>>       void (*free)(void *data);
>>>> 
>>>> +    int (*register_group)(struct mddev *mddev);
>>>> +    void (*unregister_group)(struct mddev *mddev);
>>>> +
>>>>       struct attribute_group *group;
>>>>   };
>>>> 
>>>> @@ -169,6 +172,8 @@ static inline void md_bitmap_end_sync(struct 
>>>> mddev *mddev, sector_t offset,
>>>>       mddev->bitmap_ops->end_sync(mddev, offset, blocks);
>>>>   }
>>>> 
>>>> +int md_sysfs_create_common_group(struct mddev *mddev);
>>>> +
>>>>   #ifdef CONFIG_MD_BITMAP
>>>>   int md_bitmap_init(void);
>>>>   void md_bitmap_exit(void);
>>>> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
>>>> index bf398d7476b3..24ff5f7f8751 100644
>>>> --- a/drivers/md/md-llbitmap.c
>>>> +++ b/drivers/md/md-llbitmap.c
>>>> @@ -1561,6 +1561,16 @@ static struct attribute_group 
>>>> md_llbitmap_group = {
>>>>       .attrs = md_llbitmap_attrs,
>>>>   };
>>>> 
>>>> +static int llbitmap_register_group(struct mddev *mddev)
>>>> +{
>>>> +    return sysfs_create_group(&mddev->kobj, &md_llbitmap_group);
>>>> +}
>>>> +
>>>> +static void llbitmap_unregister_group(struct mddev *mddev)
>>>> +{
>>>> +    sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
>>>> +}
>>>> +
>>>>   static struct bitmap_operations llbitmap_ops = {
>>>>       .head = {
>>>>           .type    = MD_BITMAP,
>>>> @@ -1597,6 +1607,9 @@ static struct bitmap_operations llbitmap_ops = {
>>>>       .dirty_bits        = llbitmap_dirty_bits,
>>>>       .write_all        = llbitmap_write_all,
>>>> 
>>>> +    .register_group        = llbitmap_register_group,
>>>> +    .unregister_group    = llbitmap_unregister_group,
>>>> +
>>>>       .group            = &md_llbitmap_group,
>>>>   };
>>>> 
>>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>>> index 3ce6f9e9d38e..ab969e950ea8 100644
>>>> --- a/drivers/md/md.c
>>>> +++ b/drivers/md/md.c
>>>> @@ -703,8 +703,8 @@ static bool mddev_set_bitmap_ops(struct mddev 
>>>> *mddev)
>>>>       mddev->bitmap_ops = (void *)head;
>>>>       xa_unlock(&md_submodule);
>>>> 
>>>> -    if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
>>>> -        if (sysfs_create_group(&mddev->kobj, 
>>>> mddev->bitmap_ops->group))
>>>> +    if (!mddev_is_dm(mddev) && mddev->bitmap_ops->register_group) {
>>>> +        if (mddev->bitmap_ops->register_group(mddev))
>>>>               pr_warn("md: cannot register extra bitmap attributes 
>>>> for %s\n",
>>>>                   mdname(mddev));
>>>>           else
>>>> @@ -724,8 +724,8 @@ static bool mddev_set_bitmap_ops(struct mddev 
>>>> *mddev)
>>>>   static void mddev_clear_bitmap_ops(struct mddev *mddev)
>>>>   {
>>>>       if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
>>>> -        mddev->bitmap_ops->group)
>>>> -        sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
>>>> +        mddev->bitmap_ops->unregister_group)
>>>> +        mddev->bitmap_ops->unregister_group(mddev);
>>>> 
>>>>       mddev->bitmap_ops = NULL;
>>>>   }
>>>> @@ -6369,6 +6369,14 @@ struct mddev *md_alloc(dev_t dev, char *name)
>>>>           return ERR_PTR(error);
>>>>       }
>>>> 
>>>> +    /*
>>>> +     * md_sysfs_remove_common_group is not needed because 
>>>> mddev_delayed_delete
>>>> +     * calls kobject_put(&mddev->kobj) if mddev is to be deleted.
>>>> +     */
>>>> +    if (md_sysfs_create_common_group(mddev))
>>>> +        pr_warn("md: cannot register common bitmap attributes for 
>>>> %s\n",
>>>> +            mdname(mddev));
>>>> +
>>>>       kobject_uevent(&mddev->kobj, KOBJ_ADD);
>>>>       mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd,   
>>>> "array_state");
>>>>       mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd,   
>>>> "level");
> 
> -- 
> Thansk,
> Kuai

Re: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing

[Yu Kuai]

Hi,

在 2026/3/15 16:56, Glass Su 写道:
>
>> On Mar 6, 2026, at 01:57, Yu Kuai <yukuai@fnnas.com> wrote:
>>
>> Hi,
>>
>> 在 2026/3/4 11:14, Su Yue 写道:
>>> On Wed 04 Mar 2026 at 10:30, "Yu Kuai" <yukuai@fnnas.com> wrote:
>>>
>>>> Hi,
>>>>
>>>> 在 2026/3/3 11:37, Su Yue 写道:
>>>>> Before commit fb8cc3b0d9db ("md/md-bitmap: delay registration of
>>>>> bitmap_ops until creating bitmap")
>>>>> if CONFIG_MD_BITMAP is enabled, both bitmap none, internal and
>>>>> clustered have
>>>>> the sysfs file bitmap/location.
>>>>>
>>>>> After the commit, if bitmap is none, bitmap/location doesn't exist
>>>>> anymore.
>>>>> It breaks 'grow' behavior of a md array of madam with
>>>>> MD_FEATURE_BITMAP_OFFSET.
>>>>> Take level=mirror and metadata=1.2 as an example:
>>>>>
>>>>> $ mdadm  --create  /dev/md0 -f --bitmap=none --raid-devices=2
>>>>> --level=mirror \
>>>>>     --metadata=1.2 /dev/vdd /dev/vde
>>>>> $ mdadm --grow /dev/md0 --bitmap=internal
>>>>> $ cat /sys/block/md0/md/bitmap/location
>>>>> Before:+8
>>>>> After: +2
>>>>>
>>>>> While growing bitmap from none to internal, clustered and llbitmap,
>>>>> mdadm/Grow.c:Grow_addbitmap() tries to detect bitmap/location first.
>>>>> 1)If bitmap/location exists, it sets bitmap/location after
>>>>> getinfo_super().
>>>>> 2)If bitmap/location doesn't exist, mdadm just calls
>>>>> md_set_array_info() then
>>>>> mddev->bitmap_info.default_offset will be used.
>>>>> Situation can be worse if growing none to clustered, bitmap offset
>>>>> of the node
>>>>> calling `madm --grow` will be changed but the other node are reading
>>>>> bitmap sb from
>>>>> the old location.
>>>> Now that we have a new sysfs attribute bitmap_type, can we fix this by:
>>>> - in the kernel, allow writing to this file in this case;
>>>> - in mdadm and the grow case above, write to this file first, and change
>>>> bitmap_type from none to bitmap(For llbitmap, there is still more
>>>> work to do).
>>>>
>>> Yes. It's indeed feasible. But how about old versions mdadm? We can't
>>> require
>>> users' madadm + kernel combinations for old feature. Kernel part
>>> should keep
>>> compatibility with userspace. sysfs changes and broken haviros are not
>>> ideal
>>> especially userspace depends on it unless there's a strong reason.
>>> That's why linux/Documentation/ABI exists.
>> Okay, I can accept keep this old behavior.
>>
>> However, instead of introducing a new common_group with the same name "bitmap",
>> I'll prefer to introducing a separate bitmap_ops for none bitmap as well, and
>> you can define the attrs that are necessary.
>>
> After a try, the thing I realized is that a common group is unavoidable for bitmap and none bitmap.
>
> mdadm writes to /sys/block/md0/md/bitmap/location, if remove_files() called by sysfs_remove_group()/
> sysfs_update_group() on same kernfs node, recursive locks will be triggered:
>
> [  139.516750] ============================================
> [  139.517363] WARNING: possible recursive locking detected
> [  139.517953] 7.0.0-rc1-custom+ #282 Tainted: G           OE
> [  139.518628] --------------------------------------------
> [  139.519233] mdadm/2346 is trying to acquire lock:
> [  139.519836] ffff8e6b24d85000 (kn->active#116){++++}-{0:0}, at: __kernfs_remove+0xd1/0x3e0
> [  139.520848]
>                 but task is already holding lock:
> [  139.521561] ffff8e6b24d85000 (kn->active#116){++++}-{0:0}, at: kernfs_fop_write_iter+0x12d/0x250
> [  139.522603]
>                 other info that might help us debug this:
> [  139.523383]  Possible unsafe locking scenario:
>
> [  139.524169]        CPU0
> [  139.524430]        ----
> [  139.524682]   lock(kn->active#116);
> [  139.525047]   lock(kn->active#116);
> [  139.525398]
>                  *** DEADLOCK ***
>
> [  139.525990]  May be due to missing lock nesting notation
>
> [  139.526658] 4 locks held by mdadm/2346:
> [  139.527049]  #0: ffff8e6acd5ec420 (sb_writers#5){.+.+}-{0:0}, at: ksys_write+0x6c/0xe0
> [  139.527838]  #1: ffff8e6acd54fa88 (&of->mutex){+.+.}-{4:4}, at: kernfs_fop_write_iter+0x118/0x250
> [  139.528713]  #2: ffff8e6b24d85000 (kn->active#116){++++}-{0:0}, at: kernfs_fop_write_iter+0x12d/0x250
> [  139.529594]  #3: ffff8e6b26b51370 (&mddev->reconfig_mutex){+.+.}-{4:4}, at: location_store+0x6c/0x360 [md_mod]
>
> [  139.530535]  dump_stack_lvl+0x68/0x90
> [  139.530540]  print_deadlock_bug.cold+0xc0/0xcd
> [  139.530549]  __lock_acquire+0x1324/0x2250
> [  139.530556]  lock_acquire+0xc6/0x2f0
> [  139.530564]  kernfs_drain+0x1eb/0x200
> [  139.530568]  __kernfs_remove+0xd1/0x3e0
> [  139.530570]  kernfs_remove_by_name_ns+0x5e/0xb0
> [  139.530572]  internal_create_group+0x221/0x4d0
> [  139.530578]  md_bitmap_create+0x122/0x130 [md_mod]
> [  139.530586]  location_store+0x1e9/0x360 [md_mod]
> [  139.530594]  md_attr_store+0xb8/0x1a0 [md_mod]
> [  139.530602]  kernfs_fop_write_iter+0x176/0x250
> [  139.530605]  vfs_write+0x21b/0x560
> [  139.530609]  ksys_write+0x6c/0xe0
> [  139.530611]  do_syscall_64+0x10f/0x5f0
> [  139.530623]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> The  patch implemented by separated bitmap_ops is attached. This version looks pretty similar to the first
> version because of the reason listed above and IMO ungraceful. Dummy ops and functions are a little unnecessary.
>
> I would like to ask your opinion since you are the maintainer even though I prefer v1 version + (remove the location entry for llbitmap).

Hi, I don't think you'll need to remove and recreate sysfs entry here, looks like
this problem is introduced by patch 2?

1) split bitmap group into a common group that contain location attr; and a internal
bitmap group(the name is NULL), for example:

struct attribute_group *none_bitmap_group[] = {
	&common_bitmap_group,
	NULL
};

struct attribute_group *internal_bitmap_group[] = {
	&common_bitmap_group,
	&internal_bitmap_group,
	NULL,
};

2) create none bitmap with common group only, and create internal bitmap with common group and
internal bitmap group; Notice we should convert to user sysfs_create_groups().

3) while growing from none to bitmap, create internal bitmap group attrs

4) while growing from bitmap to none, remove internal bitmap group attrs

> Thanks.
>
>
>
> —
> Su
>
>
>
>> For llbitmap, I think it's fine, we don't need this old sysfs attr anyway. I'll
>> support to convert from none/bitmap to llbitmap by writing the new bitmap_type
>> file.
>>
>>
>>> -- 
>>> Su
>>>
>>>>> Here restore sysfs file bitmap/location for ID_BITMAP_NONE and
>>>>> ID_BITMAP.
>>>>> And it d adds the entry for llbitmap too.
>>>>>
>>>>> New attribute_group md_bitmap_common_group is introduced and created in
>>>>> md_alloc() as before commit fb8cc3b0d9db.
>>>>> Add New operations register_group and unregister_group to struct
>>>>> bitmap_operations.
>>>>>
>>>>> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops
>>>>> until creating bitmap")
>>>>> Signed-off-by: Su Yue <glass.su@suse.com>
>>>>> ---
>>>>>    drivers/md/md-bitmap.c   | 32 +++++++++++++++++++++++++++++++-
>>>>>    drivers/md/md-bitmap.h   |  5 +++++
>>>>>    drivers/md/md-llbitmap.c | 13 +++++++++++++
>>>>>    drivers/md/md.c          | 16 ++++++++++++----
>>>>>    4 files changed, 61 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>>>>> index 83378c033c72..8ff1dc94ed78 100644
>>>>> --- a/drivers/md/md-bitmap.c
>>>>> +++ b/drivers/md/md-bitmap.c
>>>>> @@ -2956,7 +2956,6 @@ __ATTR(max_backlog_used, S_IRUGO | S_IWUSR,
>>>>>           behind_writes_used_show, behind_writes_used_reset);
>>>>>
>>>>>    static struct attribute *md_bitmap_attrs[] = {
>>>>> -    &bitmap_location.attr,
>>>>>        &bitmap_space.attr,
>>>>>        &bitmap_timeout.attr,
>>>>>        &bitmap_backlog.attr,
>>>>> @@ -2967,11 +2966,40 @@ static struct attribute *md_bitmap_attrs[] = {
>>>>>        NULL
>>>>>    };
>>>>>
>>>>> +static struct attribute *md_bitmap_common_attrs[] = {
>>>>> +    &bitmap_location.attr,
>>>>> +    NULL
>>>>> +};
>>>>> +
>>>>>    static struct attribute_group md_bitmap_group = {
>>>>>        .name = "bitmap",
>>>>>        .attrs = md_bitmap_attrs,
>>>>>    };
>>>>>
>>>>> +static struct attribute_group md_bitmap_common_group = {
>>>>> +    .name = "bitmap",
>>>>> +    .attrs = md_bitmap_common_attrs,
>>>>> +};
>>>>> +
>>>>> +int md_sysfs_create_common_group(struct mddev *mddev)
>>>>> +{
>>>>> +    return sysfs_create_group(&mddev->kobj, &md_bitmap_common_group);
>>>>> +}
>>>>> +
>>>>> +static int bitmap_register_group(struct mddev *mddev)
>>>>> +{
>>>>> +    /*
>>>>> +     * md_bitmap_group and md_bitmap_common_group are using same name
>>>>> +     * 'bitmap'.
>>>>> +     */
>>>>> +    return sysfs_merge_group(&mddev->kobj, &md_bitmap_group);
>>>>> +}
>>>>> +
>>>>> +static void bitmap_unregister_group(struct mddev *mddev)
>>>>> +{
>>>>> +    sysfs_unmerge_group(&mddev->kobj, &md_bitmap_group);
>>>>> +}
>>>>> +
>>>>>    static struct bitmap_operations bitmap_ops = {
>>>>>        .head = {
>>>>>            .type    = MD_BITMAP,
>>>>> @@ -3013,6 +3041,8 @@ static struct bitmap_operations bitmap_ops = {
>>>>>        .set_pages        = bitmap_set_pages,
>>>>>        .free            = md_bitmap_free,
>>>>>
>>>>> +    .register_group        = bitmap_register_group,
>>>>> +    .unregister_group    = bitmap_unregister_group,
>>>>>        .group            = &md_bitmap_group,
>>>>>    };
>>>>>
>>>>> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
>>>>> index b42a28fa83a0..371791e9011d 100644
>>>>> --- a/drivers/md/md-bitmap.h
>>>>> +++ b/drivers/md/md-bitmap.h
>>>>> @@ -125,6 +125,9 @@ struct bitmap_operations {
>>>>>        void (*set_pages)(void *data, unsigned long pages);
>>>>>        void (*free)(void *data);
>>>>>
>>>>> +    int (*register_group)(struct mddev *mddev);
>>>>> +    void (*unregister_group)(struct mddev *mddev);
>>>>> +
>>>>>        struct attribute_group *group;
>>>>>    };
>>>>>
>>>>> @@ -169,6 +172,8 @@ static inline void md_bitmap_end_sync(struct
>>>>> mddev *mddev, sector_t offset,
>>>>>        mddev->bitmap_ops->end_sync(mddev, offset, blocks);
>>>>>    }
>>>>>
>>>>> +int md_sysfs_create_common_group(struct mddev *mddev);
>>>>> +
>>>>>    #ifdef CONFIG_MD_BITMAP
>>>>>    int md_bitmap_init(void);
>>>>>    void md_bitmap_exit(void);
>>>>> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
>>>>> index bf398d7476b3..24ff5f7f8751 100644
>>>>> --- a/drivers/md/md-llbitmap.c
>>>>> +++ b/drivers/md/md-llbitmap.c
>>>>> @@ -1561,6 +1561,16 @@ static struct attribute_group
>>>>> md_llbitmap_group = {
>>>>>        .attrs = md_llbitmap_attrs,
>>>>>    };
>>>>>
>>>>> +static int llbitmap_register_group(struct mddev *mddev)
>>>>> +{
>>>>> +    return sysfs_create_group(&mddev->kobj, &md_llbitmap_group);
>>>>> +}
>>>>> +
>>>>> +static void llbitmap_unregister_group(struct mddev *mddev)
>>>>> +{
>>>>> +    sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
>>>>> +}
>>>>> +
>>>>>    static struct bitmap_operations llbitmap_ops = {
>>>>>        .head = {
>>>>>            .type    = MD_BITMAP,
>>>>> @@ -1597,6 +1607,9 @@ static struct bitmap_operations llbitmap_ops = {
>>>>>        .dirty_bits        = llbitmap_dirty_bits,
>>>>>        .write_all        = llbitmap_write_all,
>>>>>
>>>>> +    .register_group        = llbitmap_register_group,
>>>>> +    .unregister_group    = llbitmap_unregister_group,
>>>>> +
>>>>>        .group            = &md_llbitmap_group,
>>>>>    };
>>>>>
>>>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>>>> index 3ce6f9e9d38e..ab969e950ea8 100644
>>>>> --- a/drivers/md/md.c
>>>>> +++ b/drivers/md/md.c
>>>>> @@ -703,8 +703,8 @@ static bool mddev_set_bitmap_ops(struct mddev
>>>>> *mddev)
>>>>>        mddev->bitmap_ops = (void *)head;
>>>>>        xa_unlock(&md_submodule);
>>>>>
>>>>> -    if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
>>>>> -        if (sysfs_create_group(&mddev->kobj,
>>>>> mddev->bitmap_ops->group))
>>>>> +    if (!mddev_is_dm(mddev) && mddev->bitmap_ops->register_group) {
>>>>> +        if (mddev->bitmap_ops->register_group(mddev))
>>>>>                pr_warn("md: cannot register extra bitmap attributes
>>>>> for %s\n",
>>>>>                    mdname(mddev));
>>>>>            else
>>>>> @@ -724,8 +724,8 @@ static bool mddev_set_bitmap_ops(struct mddev
>>>>> *mddev)
>>>>>    static void mddev_clear_bitmap_ops(struct mddev *mddev)
>>>>>    {
>>>>>        if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
>>>>> -        mddev->bitmap_ops->group)
>>>>> -        sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
>>>>> +        mddev->bitmap_ops->unregister_group)
>>>>> +        mddev->bitmap_ops->unregister_group(mddev);
>>>>>
>>>>>        mddev->bitmap_ops = NULL;
>>>>>    }
>>>>> @@ -6369,6 +6369,14 @@ struct mddev *md_alloc(dev_t dev, char *name)
>>>>>            return ERR_PTR(error);
>>>>>        }
>>>>>
>>>>> +    /*
>>>>> +     * md_sysfs_remove_common_group is not needed because
>>>>> mddev_delayed_delete
>>>>> +     * calls kobject_put(&mddev->kobj) if mddev is to be deleted.
>>>>> +     */
>>>>> +    if (md_sysfs_create_common_group(mddev))
>>>>> +        pr_warn("md: cannot register common bitmap attributes for
>>>>> %s\n",
>>>>> +            mdname(mddev));
>>>>> +
>>>>>        kobject_uevent(&mddev->kobj, KOBJ_ADD);
>>>>>        mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd,
>>>>> "array_state");
>>>>>        mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd,
>>>>> "level");
>> -- 
>> Thansk,
>> Kuai
>
-- 
Thansk,
Kuai