* [PATCH v2 0/5] md: bitmap grow fixes
@ 2026-04-07 10:26 Su Yue
2026-04-07 10:26 ` [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store Su Yue
` (6 more replies)
0 siblings, 7 replies; 22+ messages in thread
From: Su Yue @ 2026-04-07 10:26 UTC (permalink / raw)
To: linux-raid; +Cc: song, xni, linan122, yukuai, heming.zhao, l, Su Yue
Hi, This v2 series is to fixes bugs exposed by mdadm/clustermd-autotest.
The bugs are not cluster md only but also bitmap related.
The series based on v7.0-rc7 passes tests with mdadm v4.6 without regression.
v2:
Add a dummy bitmap operations per Kuai's suggestion.
To Yu Kuai:
A NULL group can't used for internal_bitmap_group since
the entries from internal_bitmap_attrs should be under bitmap.
So instead of sysfs_create_groups(), sysfs_create_group() and sysfs_merge_group()
are still needed /sigh.
Su Yue (5):
md/md-bitmap: call md_bitmap_create,destroy in location_store
md/md-bitmap: add an extra sysfs argument to md_bitmap_create and
destroy
md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset
md: skip ID_BITMAP_NONE when show available bitmap types
md/md-bitmap: remove member group from bitmap_operations
drivers/md/md-bitmap.c | 121 ++++++++++++++++++++++++++++++++++++---
drivers/md/md-bitmap.h | 3 +-
drivers/md/md-llbitmap.c | 13 ++++-
drivers/md/md.c | 55 +++++++++---------
drivers/md/md.h | 2 +
5 files changed, 155 insertions(+), 39 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store
2026-04-07 10:26 [PATCH v2 0/5] md: bitmap grow fixes Su Yue
@ 2026-04-07 10:26 ` Su Yue
2026-04-13 7:47 ` Li Nan
2026-04-15 10:34 ` Xiao Ni
2026-04-07 10:26 ` [PATCH v2 2/5] md/md-bitmap: add an extra sysfs argument to md_bitmap_create and destroy Su Yue
` (5 subsequent siblings)
6 siblings, 2 replies; 22+ messages in thread
From: Su Yue @ 2026-04-07 10:26 UTC (permalink / raw)
To: linux-raid; +Cc: song, xni, linan122, yukuai, heming.zhao, l, Su Yue
If bitmap/location is present, 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 | 11 ++++++++---
drivers/md/md.c | 4 ++--
drivers/md/md.h | 2 ++
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 83378c033c72..2f24aae05552 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,20 @@ 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.
+ */
+ 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 3ce6f9e9d38e..8b1ecc370ad6 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6447,7 +6447,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;
@@ -6458,7 +6458,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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 2/5] md/md-bitmap: add an extra sysfs argument to md_bitmap_create and destroy
2026-04-07 10:26 [PATCH v2 0/5] md: bitmap grow fixes Su Yue
2026-04-07 10:26 ` [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store Su Yue
@ 2026-04-07 10:26 ` Su Yue
2026-04-20 5:24 ` Xiao Ni
2026-04-07 10:26 ` [PATCH v2 3/5] md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset Su Yue
` (4 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Su Yue @ 2026-04-07 10:26 UTC (permalink / raw)
To: linux-raid; +Cc: song, xni, linan122, yukuai, heming.zhao, l, Su Yue
For further use, no functional change.
Signed-off-by: Su Yue <glass.su@suse.com>
---
drivers/md/md-bitmap.c | 6 +++---
drivers/md/md.c | 36 ++++++++++++++++++------------------
drivers/md/md.h | 4 ++--
3 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 2f24aae05552..ac06c9647bf0 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;
}
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
mddev->bitmap_info.offset = 0;
if (mddev->bitmap_info.file) {
struct file *f = mddev->bitmap_info.file;
@@ -2659,14 +2659,14 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
*/
mddev->bitmap_id = ID_BITMAP;
mddev->bitmap_info.offset = offset;
- rv = md_bitmap_create(mddev);
+ rv = md_bitmap_create(mddev, true);
if (rv)
goto out;
rv = bitmap_load(mddev);
if (rv) {
mddev->bitmap_info.offset = 0;
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
goto out;
}
}
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 8b1ecc370ad6..d3c8f77b4fe3 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -678,7 +678,7 @@ static void active_io_release(struct percpu_ref *ref)
static void no_op(struct percpu_ref *r) {}
-static bool mddev_set_bitmap_ops(struct mddev *mddev)
+static bool mddev_set_bitmap_ops(struct mddev *mddev, bool create_sysfs)
{
struct bitmap_operations *old = mddev->bitmap_ops;
struct md_submodule_head *head;
@@ -703,7 +703,7 @@ 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 (create_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
if (sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group))
pr_warn("md: cannot register extra bitmap attributes for %s\n",
mdname(mddev));
@@ -721,9 +721,9 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
return false;
}
-static void mddev_clear_bitmap_ops(struct mddev *mddev)
+static void mddev_clear_bitmap_ops(struct mddev *mddev, bool remove_sysfs)
{
- if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
+ if (remove_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops &&
mddev->bitmap_ops->group)
sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
@@ -6447,24 +6447,24 @@ static void md_safemode_timeout(struct timer_list *t)
static int start_dirty_degraded;
-int md_bitmap_create(struct mddev *mddev)
+int md_bitmap_create(struct mddev *mddev, bool create_sysfs)
{
if (mddev->bitmap_id == ID_BITMAP_NONE)
return -EINVAL;
- if (!mddev_set_bitmap_ops(mddev))
+ if (!mddev_set_bitmap_ops(mddev, create_sysfs))
return -ENOENT;
return mddev->bitmap_ops->create(mddev);
}
-void md_bitmap_destroy(struct mddev *mddev)
+void md_bitmap_destroy(struct mddev *mddev, bool remove_sysfs)
{
if (!md_bitmap_registered(mddev))
return;
mddev->bitmap_ops->destroy(mddev);
- mddev_clear_bitmap_ops(mddev);
+ mddev_clear_bitmap_ops(mddev, remove_sysfs);
}
int md_run(struct mddev *mddev)
@@ -6612,7 +6612,7 @@ int md_run(struct mddev *mddev)
}
if (err == 0 && pers->sync_request &&
(mddev->bitmap_info.file || mddev->bitmap_info.offset)) {
- err = md_bitmap_create(mddev);
+ err = md_bitmap_create(mddev, true);
if (err)
pr_warn("%s: failed to create bitmap (%d)\n",
mdname(mddev), err);
@@ -6685,7 +6685,7 @@ int md_run(struct mddev *mddev)
pers->free(mddev, mddev->private);
mddev->private = NULL;
put_pers(pers);
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
return err;
}
EXPORT_SYMBOL_GPL(md_run);
@@ -6702,7 +6702,7 @@ int do_md_run(struct mddev *mddev)
if (md_bitmap_registered(mddev)) {
err = mddev->bitmap_ops->load(mddev);
if (err) {
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
goto out;
}
}
@@ -6903,7 +6903,7 @@ static void __md_stop(struct mddev *mddev)
{
struct md_personality *pers = mddev->pers;
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
mddev_detach(mddev);
spin_lock(&mddev->lock);
mddev->pers = NULL;
@@ -7680,16 +7680,16 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
err = 0;
if (mddev->pers) {
if (fd >= 0) {
- err = md_bitmap_create(mddev);
+ err = md_bitmap_create(mddev, true);
if (!err)
err = mddev->bitmap_ops->load(mddev);
if (err) {
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
fd = -1;
}
} else if (fd < 0) {
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
}
}
@@ -7996,12 +7996,12 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
mddev->bitmap_info.default_offset;
mddev->bitmap_info.space =
mddev->bitmap_info.default_space;
- rv = md_bitmap_create(mddev);
+ rv = md_bitmap_create(mddev, true);
if (!rv)
rv = mddev->bitmap_ops->load(mddev);
if (rv)
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
} else {
struct md_bitmap_stats stats;
@@ -8027,7 +8027,7 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
put_cluster_ops(mddev);
mddev->safemode_delay = DEFAULT_SAFEMODE_DELAY;
}
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
mddev->bitmap_info.offset = 0;
}
}
diff --git a/drivers/md/md.h b/drivers/md/md.h
index ed69244af00d..4aaba3d7015c 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -895,8 +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);
+int md_bitmap_create(struct mddev *mddev, bool create_sysfs);
+void md_bitmap_destroy(struct mddev *mddev, bool remove_sysfs);
extern struct md_thread *md_register_thread(
void (*run)(struct md_thread *thread),
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 3/5] md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset
2026-04-07 10:26 [PATCH v2 0/5] md: bitmap grow fixes Su Yue
2026-04-07 10:26 ` [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store Su Yue
2026-04-07 10:26 ` [PATCH v2 2/5] md/md-bitmap: add an extra sysfs argument to md_bitmap_create and destroy Su Yue
@ 2026-04-07 10:26 ` Su Yue
2026-04-20 7:05 ` Xiao Ni
2026-04-07 10:26 ` [PATCH v2 4/5] md: skip ID_BITMAP_NONE when show available bitmap types Su Yue
` (3 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Su Yue @ 2026-04-07 10:26 UTC (permalink / raw)
To: linux-raid; +Cc: song, xni, linan122, yukuai, heming.zhao, l, 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 introducing a dummy bitmap_operations for ID_BITMAP_NONE to restore sysfs
file bitmap/location for ID_BITMAP_NONE and ID_BITMAP.
location_store() now calls md_bitmap_(create|destroy) with false sysfs parameter then
(add,remove)_internal_bitmap() will be called to manage sysfs entries.
Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
Suggested-by: Yu Kuai <yukuai@fnnas.com>
Signed-off-by: Su Yue <glass.su@suse.com>
---
drivers/md/md-bitmap.c | 116 ++++++++++++++++++++++++++++++++++++---
drivers/md/md-bitmap.h | 3 +
drivers/md/md-llbitmap.c | 12 ++++
drivers/md/md.c | 16 +++---
4 files changed, 130 insertions(+), 17 deletions(-)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index ac06c9647bf0..a8a176428c61 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;
@@ -2594,6 +2604,9 @@ location_show(struct mddev *mddev, char *page)
return len;
}
+static int create_internal_group(struct mddev *mddev);
+static void remove_internal_group(struct mddev *mddev);
+
static ssize_t
location_store(struct mddev *mddev, const char *buf, size_t len)
{
@@ -2618,7 +2631,8 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
goto out;
}
- md_bitmap_destroy(mddev, true);
+ md_bitmap_destroy(mddev, false);
+ remove_internal_group(mddev);
mddev->bitmap_info.offset = 0;
if (mddev->bitmap_info.file) {
struct file *f = mddev->bitmap_info.file;
@@ -2659,14 +2673,22 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
*/
mddev->bitmap_id = ID_BITMAP;
mddev->bitmap_info.offset = offset;
- rv = md_bitmap_create(mddev, true);
+ rv = md_bitmap_create(mddev, false);
if (rv)
goto out;
+ rv = create_internal_group(mddev);
+ if (rv) {
+ mddev->bitmap_info.offset = 0;
+ bitmap_destroy(mddev);
+ goto out;
+ }
+
rv = bitmap_load(mddev);
if (rv) {
+ remove_internal_group(mddev);
mddev->bitmap_info.offset = 0;
- md_bitmap_destroy(mddev, true);
+ md_bitmap_destroy(mddev, false);
goto out;
}
}
@@ -2960,8 +2982,7 @@ static struct md_sysfs_entry max_backlog_used =
__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,
+static struct attribute *internal_bitmap_attrs[] = {
&bitmap_space.attr,
&bitmap_timeout.attr,
&bitmap_backlog.attr,
@@ -2972,11 +2993,57 @@ static struct attribute *md_bitmap_attrs[] = {
NULL
};
-static struct attribute_group md_bitmap_group = {
+static struct attribute_group internal_bitmap_group = {
.name = "bitmap",
- .attrs = md_bitmap_attrs,
+ .attrs = internal_bitmap_attrs,
};
+/* Only necessary attrs for compatibility */
+static struct attribute *common_bitmap_attrs[] = {
+ &bitmap_location.attr,
+ NULL
+};
+
+static const struct attribute_group common_bitmap_group = {
+ .name = "bitmap",
+ .attrs = common_bitmap_attrs,
+};
+
+static int create_internal_group(struct mddev *mddev)
+{
+ /*
+ * md_bitmap_group and md_bitmap_common_group are using same name
+ * 'bitmap'.
+ */
+ return sysfs_merge_group(&mddev->kobj, &internal_bitmap_group);
+}
+
+static void remove_internal_group(struct mddev *mddev)
+{
+ sysfs_unmerge_group(&mddev->kobj, &internal_bitmap_group);
+}
+
+static int bitmap_register_groups(struct mddev *mddev)
+{
+ int ret;
+
+ ret = sysfs_create_group(&mddev->kobj, &common_bitmap_group);
+
+ if (ret)
+ return ret;
+
+ ret = sysfs_merge_group(&mddev->kobj, &internal_bitmap_group);
+ if (ret)
+ sysfs_remove_group(&mddev->kobj, &common_bitmap_group);
+
+ return ret;
+}
+
+static void bitmap_unregister_groups(struct mddev *mddev)
+{
+ sysfs_unmerge_group(&mddev->kobj, &internal_bitmap_group);
+}
+
static struct bitmap_operations bitmap_ops = {
.head = {
.type = MD_BITMAP,
@@ -3018,21 +3085,52 @@ static struct bitmap_operations bitmap_ops = {
.set_pages = bitmap_set_pages,
.free = md_bitmap_free,
- .group = &md_bitmap_group,
+ .register_groups = bitmap_register_groups,
+ .unregister_groups = bitmap_unregister_groups,
+};
+
+static int none_bitmap_register_groups(struct mddev *mddev)
+{
+ return sysfs_create_group(&mddev->kobj, &common_bitmap_group);
+}
+
+static struct bitmap_operations none_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,
+
+ .register_groups = none_bitmap_register_groups,
+ .unregister_groups = NULL,
};
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(&none_bitmap_ops.head);
}
void md_bitmap_exit(void)
{
destroy_workqueue(md_bitmap_wq);
unregister_md_submodule(&bitmap_ops.head);
+ unregister_md_submodule(&none_bitmap_ops.head);
}
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index b42a28fa83a0..10bc6854798c 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_groups)(struct mddev *mddev);
+ void (*unregister_groups)(struct mddev *mddev);
+
struct attribute_group *group;
};
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index bf398d7476b3..9b3ea4f1d268 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_groups(struct mddev *mddev)
+{
+ return sysfs_create_group(&mddev->kobj, &md_llbitmap_group);
+}
+
+static void llbitmap_unregister_groups(struct mddev *mddev)
+{
+ sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
+}
+
static struct bitmap_operations llbitmap_ops = {
.head = {
.type = MD_BITMAP,
@@ -1597,6 +1607,8 @@ static struct bitmap_operations llbitmap_ops = {
.dirty_bits = llbitmap_dirty_bits,
.write_all = llbitmap_write_all,
+ .register_groups = llbitmap_register_groups,
+ .unregister_groups = llbitmap_unregister_groups,
.group = &md_llbitmap_group,
};
diff --git a/drivers/md/md.c b/drivers/md/md.c
index d3c8f77b4fe3..55a95b227b83 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -683,8 +683,7 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev, bool create_sysfs)
struct bitmap_operations *old = mddev->bitmap_ops;
struct md_submodule_head *head;
- 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);
@@ -703,8 +702,8 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev, bool create_sysfs)
mddev->bitmap_ops = (void *)head;
xa_unlock(&md_submodule);
- if (create_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
- if (sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group))
+ if (create_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops->register_groups) {
+ if (mddev->bitmap_ops->register_groups(mddev))
pr_warn("md: cannot register extra bitmap attributes for %s\n",
mdname(mddev));
else
@@ -724,8 +723,8 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev, bool create_sysfs)
static void mddev_clear_bitmap_ops(struct mddev *mddev, bool remove_sysfs)
{
if (remove_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops &&
- mddev->bitmap_ops->group)
- sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
+ mddev->bitmap_ops->unregister_groups)
+ mddev->bitmap_ops->unregister_groups(mddev);
mddev->bitmap_ops = NULL;
}
@@ -6610,8 +6609,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, true);
if (err)
pr_warn("%s: failed to create bitmap (%d)\n",
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 4/5] md: skip ID_BITMAP_NONE when show available bitmap types
2026-04-07 10:26 [PATCH v2 0/5] md: bitmap grow fixes Su Yue
` (2 preceding siblings ...)
2026-04-07 10:26 ` [PATCH v2 3/5] md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset Su Yue
@ 2026-04-07 10:26 ` Su Yue
2026-04-13 8:15 ` Li Nan
2026-04-07 10:26 ` [PATCH v2 5/5] md/md-bitmap: remove member group from bitmap_operations Su Yue
` (2 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Su Yue @ 2026-04-07 10:26 UTC (permalink / raw)
To: linux-raid; +Cc: song, xni, linan122, yukuai, heming.zhao, l, Su Yue
As none_bitmap_ops is introduced, ID_BITMAP_NONE should be skipped while
iterating md submodules, otherwise:
$ cat /sys/block/md0/md/bitmap_type
[none] bitmap llbitmap [none]
Signed-off-by: Su Yue <glass.su@suse.com>
---
drivers/md/md.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 55a95b227b83..20a953676319 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4269,6 +4269,8 @@ bitmap_type_show(struct mddev *mddev, char *page)
xa_for_each(&md_submodule, i, head) {
if (head->type != MD_BITMAP)
continue;
+ if (head->id == ID_BITMAP_NONE)
+ continue;
if (mddev->bitmap_id == head->id)
len += sprintf(page + len, "[%s] ", head->name);
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 5/5] md/md-bitmap: remove member group from bitmap_operations
2026-04-07 10:26 [PATCH v2 0/5] md: bitmap grow fixes Su Yue
` (3 preceding siblings ...)
2026-04-07 10:26 ` [PATCH v2 4/5] md: skip ID_BITMAP_NONE when show available bitmap types Su Yue
@ 2026-04-07 10:26 ` Su Yue
2026-04-16 14:10 ` [PATCH v2 0/5] md: bitmap grow fixes Su Yue
2026-04-21 5:15 ` Yu Kuai
6 siblings, 0 replies; 22+ messages in thread
From: Su Yue @ 2026-04-07 10:26 UTC (permalink / raw)
To: linux-raid; +Cc: song, xni, linan122, yukuai, heming.zhao, l, Su Yue
The member is unused now so remove it.
Signed-off-by: Su Yue <glass.su@suse.com>
---
drivers/md/md-bitmap.h | 2 --
drivers/md/md-llbitmap.c | 1 -
2 files changed, 3 deletions(-)
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index 10bc6854798c..7fba97a09b66 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -127,8 +127,6 @@ struct bitmap_operations {
int (*register_groups)(struct mddev *mddev);
void (*unregister_groups)(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 9b3ea4f1d268..4fb9003cdc49 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1609,7 +1609,6 @@ static struct bitmap_operations llbitmap_ops = {
.register_groups = llbitmap_register_groups,
.unregister_groups = llbitmap_unregister_groups,
- .group = &md_llbitmap_group,
};
int md_llbitmap_init(void)
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store
2026-04-07 10:26 ` [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store Su Yue
@ 2026-04-13 7:47 ` Li Nan
2026-04-13 10:18 ` Su Yue
2026-04-15 10:34 ` Xiao Ni
1 sibling, 1 reply; 22+ messages in thread
From: Li Nan @ 2026-04-13 7:47 UTC (permalink / raw)
To: Su Yue, linux-raid; +Cc: song, xni, yukuai, heming.zhao, l
在 2026/4/7 18:26, Su Yue 写道:
> If bitmap/location is present, 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 | 11 ++++++++---
> drivers/md/md.c | 4 ++--
> drivers/md/md.h | 2 ++
> 3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 83378c033c72..2f24aae05552 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,20 @@ 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.
> + */
> + 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);
mddev->bitmap_ops->load() should also be used here.
--
Thanks,
Nan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 4/5] md: skip ID_BITMAP_NONE when show available bitmap types
2026-04-07 10:26 ` [PATCH v2 4/5] md: skip ID_BITMAP_NONE when show available bitmap types Su Yue
@ 2026-04-13 8:15 ` Li Nan
2026-04-13 10:23 ` Su Yue
0 siblings, 1 reply; 22+ messages in thread
From: Li Nan @ 2026-04-13 8:15 UTC (permalink / raw)
To: Su Yue, linux-raid; +Cc: song, xni, yukuai, heming.zhao, l
在 2026/4/7 18:26, Su Yue 写道:
> As none_bitmap_ops is introduced, ID_BITMAP_NONE should be skipped while
> iterating md submodules, otherwise:
>
> $ cat /sys/block/md0/md/bitmap_type
> [none] bitmap llbitmap [none]
>
> Signed-off-by: Su Yue <glass.su@suse.com>
> ---
> drivers/md/md.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 55a95b227b83..20a953676319 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -4269,6 +4269,8 @@ bitmap_type_show(struct mddev *mddev, char *page)
> xa_for_each(&md_submodule, i, head) {
> if (head->type != MD_BITMAP)
> continue;
> + if (head->id == ID_BITMAP_NONE)
> + continue;
>
> if (mddev->bitmap_id == head->id)
> len += sprintf(page + len, "[%s] ", head->name);
If we indeed add ID_BITMAP_NONE as patch 3, would it be better to delete
this piece of code instead?
if (mddev->bitmap_id == ID_BITMAP_NONE)
len += sprintf(page + len, "[none] ");
else
len += sprintf(page + len, "none ");
--
Thanks,
Nan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store
2026-04-13 7:47 ` Li Nan
@ 2026-04-13 10:18 ` Su Yue
0 siblings, 0 replies; 22+ messages in thread
From: Su Yue @ 2026-04-13 10:18 UTC (permalink / raw)
To: Li Nan; +Cc: Su Yue, linux-raid, song, xni, yukuai, heming.zhao
aOn Mon 13 Apr 2026 at 15:47, Li Nan <linan666@huaweicloud.com>
wrote:
> 在 2026/4/7 18:26, Su Yue 写道:
>> If bitmap/location is present, 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 | 11 ++++++++---
>> drivers/md/md.c | 4 ++--
>> drivers/md/md.h | 2 ++
>> 3 files changed, 12 insertions(+), 5 deletions(-)
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index 83378c033c72..2f24aae05552 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,20 @@ 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.
>> + */
>> + 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);
>
> mddev->bitmap_ops->load() should also be used here.
/NOD.
location_store() is only used for ID_BITMAP_NONE and ID_BITMAP, so
mddev->bitmap_ops->load() is always bitmap_load().
But for code consistency, mddev->bitmap_ops->load() is better,
will fix it.
--
Su
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 4/5] md: skip ID_BITMAP_NONE when show available bitmap types
2026-04-13 8:15 ` Li Nan
@ 2026-04-13 10:23 ` Su Yue
0 siblings, 0 replies; 22+ messages in thread
From: Su Yue @ 2026-04-13 10:23 UTC (permalink / raw)
To: Li Nan; +Cc: Su Yue, linux-raid, song, xni, yukuai, heming.zhao
On Mon 13 Apr 2026 at 16:15, Li Nan <linan666@huaweicloud.com>
wrote:
> 在 2026/4/7 18:26, Su Yue 写道:
>> As none_bitmap_ops is introduced, ID_BITMAP_NONE should be
>> skipped while
>> iterating md submodules, otherwise:
>> $ cat /sys/block/md0/md/bitmap_type
>> [none] bitmap llbitmap [none]
>> Signed-off-by: Su Yue <glass.su@suse.com>
>> ---
>> drivers/md/md.c | 2 ++
>> 1 file changed, 2 insertions(+)
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 55a95b227b83..20a953676319 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -4269,6 +4269,8 @@ bitmap_type_show(struct mddev *mddev,
>> char *page)
>> xa_for_each(&md_submodule, i, head) {
>> if (head->type != MD_BITMAP)
>> continue;
>> + if (head->id == ID_BITMAP_NONE)
>> + continue;
>> if (mddev->bitmap_id == head->id)
>> len += sprintf(page + len, "[%s] ", head->name);
>
> If we indeed add ID_BITMAP_NONE as patch 3, would it be better
> to delete
> this piece of code instead?
>
I dont' think so. There is one case CONFIG_MD_BITMAP is not
enabled which
means register_md_submodule(&none_bitmap_ops.head) is not called
but in
mddev_init():
int mddev_init(struct mddev *mddev)
|
{
|
int err = 0;
|
|
if (!IS_ENABLED(CONFIG_MD_BITMAP))
|
mddev->bitmap_id = ID_BITMAP_NONE;
...
}
--
Su
>
> if (mddev->bitmap_id == ID_BITMAP_NONE)
> len += sprintf(page + len, "[none] ");
> else
> len += sprintf(page + len, "none ");
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store
2026-04-07 10:26 ` [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store Su Yue
2026-04-13 7:47 ` Li Nan
@ 2026-04-15 10:34 ` Xiao Ni
2026-04-16 14:08 ` Su Yue
1 sibling, 1 reply; 22+ messages in thread
From: Xiao Ni @ 2026-04-15 10:34 UTC (permalink / raw)
To: Su Yue; +Cc: linux-raid, song, linan122, yukuai, heming.zhao, l
On Tue, Apr 7, 2026 at 6:26 PM Su Yue <glass.su@suse.com> wrote:
>
> If bitmap/location is present, 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.
Hi Su Yue
How can bitmap/location be present when bitmap is none? Could you
provide the test commands that reproduce this problem?
mdadm -CR /dev/md0 -l1 -n2 /dev/loop0 /dev/loop1 --bitmap=none (There
is not bitmap/location, because bitmap directory is not created)
mdadm /dev/md0 --grow --bitmap=internal
Grow.c md_set_array_info runs
451 array.state |= (1 << MD_SB_BITMAP_PRESENT);
452 rv = md_set_array_info(fd, &array);
In kernel space, it runs
8125 rv = md_bitmap_create(mddev);
8126 if (!rv)
8127 rv = mddev->bitmap_ops->load(mddev);
Best Regards
Xiao
>
> 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 | 11 ++++++++---
> drivers/md/md.c | 4 ++--
> drivers/md/md.h | 2 ++
> 3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 83378c033c72..2f24aae05552 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,20 @@ 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.
> + */
> + 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 3ce6f9e9d38e..8b1ecc370ad6 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6447,7 +6447,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;
> @@ -6458,7 +6458,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
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store
2026-04-15 10:34 ` Xiao Ni
@ 2026-04-16 14:08 ` Su Yue
2026-04-20 5:21 ` Xiao Ni
0 siblings, 1 reply; 22+ messages in thread
From: Su Yue @ 2026-04-16 14:08 UTC (permalink / raw)
To: Xiao Ni; +Cc: Su Yue, linux-raid, song, linan122, yukuai, heming.zhao
On Wed 15 Apr 2026 at 18:34, Xiao Ni <xni@redhat.com> wrote:
> On Tue, Apr 7, 2026 at 6:26 PM Su Yue <glass.su@suse.com> wrote:
>>
>> If bitmap/location is present, 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.
>
>
> Hi Su Yue
>
> How can bitmap/location be present when bitmap is none? Could
> you
> provide the test commands that reproduce this problem?
>
Sorry for the misleading commit message. It can only be reproduced
patch 3 is appiled.
I adjusted the sequence of this patch for easy review because
md_bitmap_create,destroy
are touched in patch1,2 and 3. Also if put the patch after 3rd
patch,
it will break ability to bisect.
# mdadm --create --assume-clean /dev/md0 -f --bitmap=internal
--raid-devices=2 --level=mirror --metadata=1.2 /dev/vdc /dev/vdd
# mdadm --grow /dev/md0 --bitmap=none
# mdadm --grow /dev/md0 --bitmap=internal # step 3
# mdadm --grow /dev/md0 --bitmap=none # step 4
[1] 2325 killed mdadm --grow /dev/md0 --bitmap=none
When step 3 is called,
md_bitmap_destroy() is called in update_array_info() to set NULL
mddev->bitmap_ops
then in step 4 kernel Oops is triggered.
I am willing to amend commit message or move it after patch 3 if
you would like.
--
Su
>
> mdadm -CR /dev/md0 -l1 -n2 /dev/loop0 /dev/loop1 --bitmap=none
> (There
> is not bitmap/location, because bitmap directory is not created)
> mdadm /dev/md0 --grow --bitmap=internal
> Grow.c md_set_array_info runs
> 451 array.state |= (1 << MD_SB_BITMAP_PRESENT);
> 452 rv = md_set_array_info(fd, &array);
> In kernel space, it runs
> 8125 rv = md_bitmap_create(mddev);
> 8126 if (!rv)
> 8127 rv = mddev->bitmap_ops->load(mddev);
>
> Best Regards
> Xiao
>
>>
>> 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 | 11 ++++++++---
>> drivers/md/md.c | 4 ++--
>> drivers/md/md.h | 2 ++
>> 3 files changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index 83378c033c72..2f24aae05552 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,20 @@ 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.
>> + */
>> + 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 3ce6f9e9d38e..8b1ecc370ad6 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -6447,7 +6447,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;
>> @@ -6458,7 +6458,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
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 0/5] md: bitmap grow fixes
2026-04-07 10:26 [PATCH v2 0/5] md: bitmap grow fixes Su Yue
` (4 preceding siblings ...)
2026-04-07 10:26 ` [PATCH v2 5/5] md/md-bitmap: remove member group from bitmap_operations Su Yue
@ 2026-04-16 14:10 ` Su Yue
2026-04-21 5:15 ` Yu Kuai
6 siblings, 0 replies; 22+ messages in thread
From: Su Yue @ 2026-04-16 14:10 UTC (permalink / raw)
To: Su Yue; +Cc: linux-raid, song, xni, linan122, yukuai, heming.zhao
On Tue 07 Apr 2026 at 18:26, Su Yue <glass.su@suse.com> wrote:
> Hi, This v2 series is to fixes bugs exposed by
> mdadm/clustermd-autotest.
> The bugs are not cluster md only but also bitmap related.
>
> The series based on v7.0-rc7 passes tests with mdadm v4.6
> without regression.
>
> v2:
> Add a dummy bitmap operations per Kuai's suggestion.
>
> To Yu Kuai:
> A NULL group can't used for internal_bitmap_group since
> the entries from internal_bitmap_attrs should be under bitmap.
> So instead of sysfs_create_groups(), sysfs_create_group() and
> sysfs_merge_group()
> are still needed /sigh.
>
> Su Yue (5):
> md/md-bitmap: call md_bitmap_create,destroy in location_store
> md/md-bitmap: add an extra sysfs argument to md_bitmap_create
> and
> destroy
> md/md-bitmap: add dummy bitmap ops for none to fix wrong
> bitmap offset
> md: skip ID_BITMAP_NONE when show available bitmap types
> md/md-bitmap: remove member group from bitmap_operations
>
Missing part should be added in v3:
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 20a953676319..453b89941db2 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6450,9 +6450,6 @@ static int start_dirty_degraded;
int md_bitmap_create(struct mddev *mddev, bool create_sysfs)
{
- if (mddev->bitmap_id == ID_BITMAP_NONE)
- return -EINVAL;
-
}
> drivers/md/md-bitmap.c | 121
> ++++++++++++++++++++++++++++++++++++---
> drivers/md/md-bitmap.h | 3 +-
> drivers/md/md-llbitmap.c | 13 ++++-
> drivers/md/md.c | 55 +++++++++---------
> drivers/md/md.h | 2 +
> 5 files changed, 155 insertions(+), 39 deletions(-)
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store
2026-04-16 14:08 ` Su Yue
@ 2026-04-20 5:21 ` Xiao Ni
2026-04-21 1:26 ` Su Yue
0 siblings, 1 reply; 22+ messages in thread
From: Xiao Ni @ 2026-04-20 5:21 UTC (permalink / raw)
To: Su Yue; +Cc: Su Yue, linux-raid, song, linan122, yukuai, heming.zhao
On Thu, Apr 16, 2026 at 10:09 PM Su Yue <l@damenly.org> wrote:
>
> On Wed 15 Apr 2026 at 18:34, Xiao Ni <xni@redhat.com> wrote:
>
> > On Tue, Apr 7, 2026 at 6:26 PM Su Yue <glass.su@suse.com> wrote:
> >>
> >> If bitmap/location is present, 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.
> >
> >
> > Hi Su Yue
> >
> > How can bitmap/location be present when bitmap is none? Could
> > you
> > provide the test commands that reproduce this problem?
> >
> Sorry for the misleading commit message. It can only be reproduced
> patch 3 is appiled.
> I adjusted the sequence of this patch for easy review because
> md_bitmap_create,destroy
> are touched in patch1,2 and 3. Also if put the patch after 3rd
> patch,
> it will break ability to bisect.
>
> # mdadm --create --assume-clean /dev/md0 -f --bitmap=internal
> --raid-devices=2 --level=mirror --metadata=1.2 /dev/vdc /dev/vdd
> # mdadm --grow /dev/md0 --bitmap=none
> # mdadm --grow /dev/md0 --bitmap=internal # step 3
> # mdadm --grow /dev/md0 --bitmap=none # step 4
> [1] 2325 killed mdadm --grow /dev/md0 --bitmap=none
>
> When step 3 is called,
> md_bitmap_destroy() is called in update_array_info() to set NULL
> mddev->bitmap_ops
> then in step 4 kernel Oops is triggered.
>
>
> I am willing to amend commit message or move it after patch 3 if
> you would like.
Hi Su
Thanks for the detail explanation. After reading patch3, I totoally
understand. The sequence is good to me. And yes, it's better to
explain that this is needed after patch3.
Best Regards
Xiao
>
> --
> Su
>
> >
> > mdadm -CR /dev/md0 -l1 -n2 /dev/loop0 /dev/loop1 --bitmap=none
> > (There
> > is not bitmap/location, because bitmap directory is not created)
> > mdadm /dev/md0 --grow --bitmap=internal
> > Grow.c md_set_array_info runs
> > 451 array.state |= (1 << MD_SB_BITMAP_PRESENT);
> > 452 rv = md_set_array_info(fd, &array);
> > In kernel space, it runs
> > 8125 rv = md_bitmap_create(mddev);
> > 8126 if (!rv)
> > 8127 rv = mddev->bitmap_ops->load(mddev);
> >
> > Best Regards
> > Xiao
> >
> >>
> >> 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 | 11 ++++++++---
> >> drivers/md/md.c | 4 ++--
> >> drivers/md/md.h | 2 ++
> >> 3 files changed, 12 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> >> index 83378c033c72..2f24aae05552 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,20 @@ 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.
> >> + */
> >> + 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 3ce6f9e9d38e..8b1ecc370ad6 100644
> >> --- a/drivers/md/md.c
> >> +++ b/drivers/md/md.c
> >> @@ -6447,7 +6447,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;
> >> @@ -6458,7 +6458,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
> >>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/5] md/md-bitmap: add an extra sysfs argument to md_bitmap_create and destroy
2026-04-07 10:26 ` [PATCH v2 2/5] md/md-bitmap: add an extra sysfs argument to md_bitmap_create and destroy Su Yue
@ 2026-04-20 5:24 ` Xiao Ni
0 siblings, 0 replies; 22+ messages in thread
From: Xiao Ni @ 2026-04-20 5:24 UTC (permalink / raw)
To: Su Yue; +Cc: linux-raid, song, linan122, yukuai, heming.zhao, l
On Tue, Apr 7, 2026 at 6:26 PM Su Yue <glass.su@suse.com> wrote:
>
> For further use, no functional change.
>
> Signed-off-by: Su Yue <glass.su@suse.com>
> ---
> drivers/md/md-bitmap.c | 6 +++---
> drivers/md/md.c | 36 ++++++++++++++++++------------------
> drivers/md/md.h | 4 ++--
> 3 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 2f24aae05552..ac06c9647bf0 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;
> }
>
> - md_bitmap_destroy(mddev);
> + md_bitmap_destroy(mddev, true);
> mddev->bitmap_info.offset = 0;
> if (mddev->bitmap_info.file) {
> struct file *f = mddev->bitmap_info.file;
> @@ -2659,14 +2659,14 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
> */
> mddev->bitmap_id = ID_BITMAP;
> mddev->bitmap_info.offset = offset;
> - rv = md_bitmap_create(mddev);
> + rv = md_bitmap_create(mddev, true);
> if (rv)
> goto out;
>
> rv = bitmap_load(mddev);
> if (rv) {
> mddev->bitmap_info.offset = 0;
> - md_bitmap_destroy(mddev);
> + md_bitmap_destroy(mddev, true);
> goto out;
> }
> }
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 8b1ecc370ad6..d3c8f77b4fe3 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -678,7 +678,7 @@ static void active_io_release(struct percpu_ref *ref)
>
> static void no_op(struct percpu_ref *r) {}
>
> -static bool mddev_set_bitmap_ops(struct mddev *mddev)
> +static bool mddev_set_bitmap_ops(struct mddev *mddev, bool create_sysfs)
> {
> struct bitmap_operations *old = mddev->bitmap_ops;
> struct md_submodule_head *head;
> @@ -703,7 +703,7 @@ 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 (create_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
> if (sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group))
> pr_warn("md: cannot register extra bitmap attributes for %s\n",
> mdname(mddev));
> @@ -721,9 +721,9 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
> return false;
> }
>
> -static void mddev_clear_bitmap_ops(struct mddev *mddev)
> +static void mddev_clear_bitmap_ops(struct mddev *mddev, bool remove_sysfs)
> {
> - if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
> + if (remove_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops &&
> mddev->bitmap_ops->group)
> sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
>
> @@ -6447,24 +6447,24 @@ static void md_safemode_timeout(struct timer_list *t)
>
> static int start_dirty_degraded;
>
> -int md_bitmap_create(struct mddev *mddev)
> +int md_bitmap_create(struct mddev *mddev, bool create_sysfs)
> {
> if (mddev->bitmap_id == ID_BITMAP_NONE)
> return -EINVAL;
>
> - if (!mddev_set_bitmap_ops(mddev))
> + if (!mddev_set_bitmap_ops(mddev, create_sysfs))
> return -ENOENT;
>
> return mddev->bitmap_ops->create(mddev);
> }
>
> -void md_bitmap_destroy(struct mddev *mddev)
> +void md_bitmap_destroy(struct mddev *mddev, bool remove_sysfs)
> {
> if (!md_bitmap_registered(mddev))
> return;
>
> mddev->bitmap_ops->destroy(mddev);
> - mddev_clear_bitmap_ops(mddev);
> + mddev_clear_bitmap_ops(mddev, remove_sysfs);
> }
>
> int md_run(struct mddev *mddev)
> @@ -6612,7 +6612,7 @@ int md_run(struct mddev *mddev)
> }
> if (err == 0 && pers->sync_request &&
> (mddev->bitmap_info.file || mddev->bitmap_info.offset)) {
> - err = md_bitmap_create(mddev);
> + err = md_bitmap_create(mddev, true);
> if (err)
> pr_warn("%s: failed to create bitmap (%d)\n",
> mdname(mddev), err);
> @@ -6685,7 +6685,7 @@ int md_run(struct mddev *mddev)
> pers->free(mddev, mddev->private);
> mddev->private = NULL;
> put_pers(pers);
> - md_bitmap_destroy(mddev);
> + md_bitmap_destroy(mddev, true);
> return err;
> }
> EXPORT_SYMBOL_GPL(md_run);
> @@ -6702,7 +6702,7 @@ int do_md_run(struct mddev *mddev)
> if (md_bitmap_registered(mddev)) {
> err = mddev->bitmap_ops->load(mddev);
> if (err) {
> - md_bitmap_destroy(mddev);
> + md_bitmap_destroy(mddev, true);
> goto out;
> }
> }
> @@ -6903,7 +6903,7 @@ static void __md_stop(struct mddev *mddev)
> {
> struct md_personality *pers = mddev->pers;
>
> - md_bitmap_destroy(mddev);
> + md_bitmap_destroy(mddev, true);
> mddev_detach(mddev);
> spin_lock(&mddev->lock);
> mddev->pers = NULL;
> @@ -7680,16 +7680,16 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
> err = 0;
> if (mddev->pers) {
> if (fd >= 0) {
> - err = md_bitmap_create(mddev);
> + err = md_bitmap_create(mddev, true);
> if (!err)
> err = mddev->bitmap_ops->load(mddev);
>
> if (err) {
> - md_bitmap_destroy(mddev);
> + md_bitmap_destroy(mddev, true);
> fd = -1;
> }
> } else if (fd < 0) {
> - md_bitmap_destroy(mddev);
> + md_bitmap_destroy(mddev, true);
> }
> }
>
> @@ -7996,12 +7996,12 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
> mddev->bitmap_info.default_offset;
> mddev->bitmap_info.space =
> mddev->bitmap_info.default_space;
> - rv = md_bitmap_create(mddev);
> + rv = md_bitmap_create(mddev, true);
> if (!rv)
> rv = mddev->bitmap_ops->load(mddev);
>
> if (rv)
> - md_bitmap_destroy(mddev);
> + md_bitmap_destroy(mddev, true);
> } else {
> struct md_bitmap_stats stats;
>
> @@ -8027,7 +8027,7 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
> put_cluster_ops(mddev);
> mddev->safemode_delay = DEFAULT_SAFEMODE_DELAY;
> }
> - md_bitmap_destroy(mddev);
> + md_bitmap_destroy(mddev, true);
> mddev->bitmap_info.offset = 0;
> }
> }
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index ed69244af00d..4aaba3d7015c 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -895,8 +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);
> +int md_bitmap_create(struct mddev *mddev, bool create_sysfs);
> +void md_bitmap_destroy(struct mddev *mddev, bool remove_sysfs);
>
> extern struct md_thread *md_register_thread(
> void (*run)(struct md_thread *thread),
> --
> 2.53.0
>
Looks good to me.
Reviewed-by: Xiao Ni <xni@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 3/5] md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset
2026-04-07 10:26 ` [PATCH v2 3/5] md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset Su Yue
@ 2026-04-20 7:05 ` Xiao Ni
2026-04-21 2:29 ` Su Yue
0 siblings, 1 reply; 22+ messages in thread
From: Xiao Ni @ 2026-04-20 7:05 UTC (permalink / raw)
To: Su Yue; +Cc: linux-raid, song, linan122, yukuai, heming.zhao, l
On Tue, Apr 7, 2026 at 6:27 PM Su Yue <glass.su@suse.com> wrote:
>
> 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.
> location_store() now calls md_bitmap_(create|destroy) with false sysfs parameter then
> (add,remove)_internal_bitmap() will be called to manage sysfs entries.
>
> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
> Suggested-by: Yu Kuai <yukuai@fnnas.com>
> Signed-off-by: Su Yue <glass.su@suse.com>
> ---
> drivers/md/md-bitmap.c | 116 ++++++++++++++++++++++++++++++++++++---
> drivers/md/md-bitmap.h | 3 +
> drivers/md/md-llbitmap.c | 12 ++++
> drivers/md/md.c | 16 +++---
> 4 files changed, 130 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index ac06c9647bf0..a8a176428c61 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;
> @@ -2594,6 +2604,9 @@ location_show(struct mddev *mddev, char *page)
> return len;
> }
>
> +static int create_internal_group(struct mddev *mddev);
> +static void remove_internal_group(struct mddev *mddev);
> +
> static ssize_t
> location_store(struct mddev *mddev, const char *buf, size_t len)
> {
> @@ -2618,7 +2631,8 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
> goto out;
> }
>
> - md_bitmap_destroy(mddev, true);
> + md_bitmap_destroy(mddev, false);
> + remove_internal_group(mddev);
> mddev->bitmap_info.offset = 0;
> if (mddev->bitmap_info.file) {
> struct file *f = mddev->bitmap_info.file;
> @@ -2659,14 +2673,22 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
> */
> mddev->bitmap_id = ID_BITMAP;
> mddev->bitmap_info.offset = offset;
> - rv = md_bitmap_create(mddev, true);
> + rv = md_bitmap_create(mddev, false);
> if (rv)
> goto out;
>
> + rv = create_internal_group(mddev);
> + if (rv) {
> + mddev->bitmap_info.offset = 0;
> + bitmap_destroy(mddev);
> + goto out;
> + }
> +
> rv = bitmap_load(mddev);
> if (rv) {
> + remove_internal_group(mddev);
> mddev->bitmap_info.offset = 0;
> - md_bitmap_destroy(mddev, true);
> + md_bitmap_destroy(mddev, false);
> goto out;
> }
> }
> @@ -2960,8 +2982,7 @@ static struct md_sysfs_entry max_backlog_used =
> __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,
> +static struct attribute *internal_bitmap_attrs[] = {
> &bitmap_space.attr,
> &bitmap_timeout.attr,
> &bitmap_backlog.attr,
> @@ -2972,11 +2993,57 @@ static struct attribute *md_bitmap_attrs[] = {
> NULL
> };
>
> -static struct attribute_group md_bitmap_group = {
> +static struct attribute_group internal_bitmap_group = {
> .name = "bitmap",
> - .attrs = md_bitmap_attrs,
> + .attrs = internal_bitmap_attrs,
> };
>
> +/* Only necessary attrs for compatibility */
> +static struct attribute *common_bitmap_attrs[] = {
> + &bitmap_location.attr,
> + NULL
> +};
> +
> +static const struct attribute_group common_bitmap_group = {
> + .name = "bitmap",
> + .attrs = common_bitmap_attrs,
> +};
> +
> +static int create_internal_group(struct mddev *mddev)
> +{
> + /*
> + * md_bitmap_group and md_bitmap_common_group are using same name
> + * 'bitmap'.
> + */
> + return sysfs_merge_group(&mddev->kobj, &internal_bitmap_group);
> +}
> +
> +static void remove_internal_group(struct mddev *mddev)
> +{
> + sysfs_unmerge_group(&mddev->kobj, &internal_bitmap_group);
> +}
> +
> +static int bitmap_register_groups(struct mddev *mddev)
> +{
> + int ret;
> +
> + ret = sysfs_create_group(&mddev->kobj, &common_bitmap_group);
> +
> + if (ret)
> + return ret;
> +
> + ret = sysfs_merge_group(&mddev->kobj, &internal_bitmap_group);
> + if (ret)
> + sysfs_remove_group(&mddev->kobj, &common_bitmap_group);
> +
> + return ret;
> +}
> +
> +static void bitmap_unregister_groups(struct mddev *mddev)
> +{
> + sysfs_unmerge_group(&mddev->kobj, &internal_bitmap_group);
> +}
Hi Su
bitmap_unregister_groups should also remove common_bitmap_group, right?
> +
> static struct bitmap_operations bitmap_ops = {
You already changed md_bitmap_group to internal_bitmap_group. It's
better to change bitmap_ops to internal_bitmap_ops?
> .head = {
> .type = MD_BITMAP,
> @@ -3018,21 +3085,52 @@ static struct bitmap_operations bitmap_ops = {
> .set_pages = bitmap_set_pages,
> .free = md_bitmap_free,
>
> - .group = &md_bitmap_group,
> + .register_groups = bitmap_register_groups,
> + .unregister_groups = bitmap_unregister_groups,
> +};
> +
> +static int none_bitmap_register_groups(struct mddev *mddev)
> +{
> + return sysfs_create_group(&mddev->kobj, &common_bitmap_group);
> +}
> +
> +static struct bitmap_operations none_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,
> +
> + .register_groups = none_bitmap_register_groups,
> + .unregister_groups = NULL,
How does bitmap/location can be deleted if array is created with no
bitmap? Can the `mdadm --stop` command get stuck?
Best Regards
Xiao
> };
>
> 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(&none_bitmap_ops.head);
> }
>
> void md_bitmap_exit(void)
> {
> destroy_workqueue(md_bitmap_wq);
> unregister_md_submodule(&bitmap_ops.head);
> + unregister_md_submodule(&none_bitmap_ops.head);
> }
> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
> index b42a28fa83a0..10bc6854798c 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_groups)(struct mddev *mddev);
> + void (*unregister_groups)(struct mddev *mddev);
> +
> struct attribute_group *group;
> };
>
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index bf398d7476b3..9b3ea4f1d268 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_groups(struct mddev *mddev)
> +{
> + return sysfs_create_group(&mddev->kobj, &md_llbitmap_group);
> +}
> +
> +static void llbitmap_unregister_groups(struct mddev *mddev)
> +{
> + sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
> +}
> +
> static struct bitmap_operations llbitmap_ops = {
> .head = {
> .type = MD_BITMAP,
> @@ -1597,6 +1607,8 @@ static struct bitmap_operations llbitmap_ops = {
> .dirty_bits = llbitmap_dirty_bits,
> .write_all = llbitmap_write_all,
>
> + .register_groups = llbitmap_register_groups,
> + .unregister_groups = llbitmap_unregister_groups,
> .group = &md_llbitmap_group,
> };
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d3c8f77b4fe3..55a95b227b83 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -683,8 +683,7 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev, bool create_sysfs)
> struct bitmap_operations *old = mddev->bitmap_ops;
> struct md_submodule_head *head;
>
> - 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);
> @@ -703,8 +702,8 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev, bool create_sysfs)
> mddev->bitmap_ops = (void *)head;
> xa_unlock(&md_submodule);
>
> - if (create_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
> - if (sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group))
> + if (create_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops->register_groups) {
> + if (mddev->bitmap_ops->register_groups(mddev))
> pr_warn("md: cannot register extra bitmap attributes for %s\n",
> mdname(mddev));
> else
> @@ -724,8 +723,8 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev, bool create_sysfs)
> static void mddev_clear_bitmap_ops(struct mddev *mddev, bool remove_sysfs)
> {
> if (remove_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops &&
> - mddev->bitmap_ops->group)
> - sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
> + mddev->bitmap_ops->unregister_groups)
> + mddev->bitmap_ops->unregister_groups(mddev);
>
> mddev->bitmap_ops = NULL;
> }
> @@ -6610,8 +6609,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, true);
> if (err)
> pr_warn("%s: failed to create bitmap (%d)\n",
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store
2026-04-20 5:21 ` Xiao Ni
@ 2026-04-21 1:26 ` Su Yue
0 siblings, 0 replies; 22+ messages in thread
From: Su Yue @ 2026-04-21 1:26 UTC (permalink / raw)
To: Xiao Ni; +Cc: Su Yue, linux-raid, song, linan122, yukuai, heming.zhao
On Mon 20 Apr 2026 at 13:21, Xiao Ni <xni@redhat.com> wrote:
> On Thu, Apr 16, 2026 at 10:09 PM Su Yue <l@damenly.org> wrote:
>>
>> On Wed 15 Apr 2026 at 18:34, Xiao Ni <xni@redhat.com> wrote:
>>
>> > On Tue, Apr 7, 2026 at 6:26 PM Su Yue <glass.su@suse.com>
>> > wrote:
>> >>
>> >> If bitmap/location is present, 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.
>> >
>> >
>> > Hi Su Yue
>> >
>> > How can bitmap/location be present when bitmap is none? Could
>> > you
>> > provide the test commands that reproduce this problem?
>> >
>> Sorry for the misleading commit message. It can only be
>> reproduced
>> patch 3 is appiled.
>> I adjusted the sequence of this patch for easy review because
>> md_bitmap_create,destroy
>> are touched in patch1,2 and 3. Also if put the patch after 3rd
>> patch,
>> it will break ability to bisect.
>>
>> # mdadm --create --assume-clean /dev/md0 -f --bitmap=internal
>> --raid-devices=2 --level=mirror --metadata=1.2 /dev/vdc
>> /dev/vdd
>> # mdadm --grow /dev/md0 --bitmap=none
>> # mdadm --grow /dev/md0 --bitmap=internal # step 3
>> # mdadm --grow /dev/md0 --bitmap=none # step 4
>> [1] 2325 killed mdadm --grow /dev/md0 --bitmap=none
>>
>> When step 3 is called,
>> md_bitmap_destroy() is called in update_array_info() to set
>> NULL
>> mddev->bitmap_ops
>> then in step 4 kernel Oops is triggered.
>>
>>
>> I am willing to amend commit message or move it after patch 3
>> if
>> you would like.
>
> Hi Su
>
> Thanks for the detail explanation. After reading patch3, I
> totoally
> understand. The sequence is good to me. And yes, it's better to
> explain that this is needed after patch3.
>
Sure. I will do it in next version.
--
Su
>
> Best Regards
> Xiao
>>
>> --
>> Su
>>
>> >
>> > mdadm -CR /dev/md0 -l1 -n2 /dev/loop0 /dev/loop1
>> > --bitmap=none
>> > (There
>> > is not bitmap/location, because bitmap directory is not
>> > created)
>> > mdadm /dev/md0 --grow --bitmap=internal
>> > Grow.c md_set_array_info runs
>> > 451 array.state |= (1 << MD_SB_BITMAP_PRESENT);
>> > 452 rv = md_set_array_info(fd, &array);
>> > In kernel space, it runs
>> > 8125 rv = md_bitmap_create(mddev);
>> > 8126 if (!rv)
>> > 8127 rv = mddev->bitmap_ops->load(mddev);
>> >
>> > Best Regards
>> > Xiao
>> >
>> >>
>> >> 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 | 11 ++++++++---
>> >> drivers/md/md.c | 4 ++--
>> >> drivers/md/md.h | 2 ++
>> >> 3 files changed, 12 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> >> index 83378c033c72..2f24aae05552 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,20 @@ 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.
>> >> + */
>> >> + 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 3ce6f9e9d38e..8b1ecc370ad6 100644
>> >> --- a/drivers/md/md.c
>> >> +++ b/drivers/md/md.c
>> >> @@ -6447,7 +6447,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;
>> >> @@ -6458,7 +6458,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
>> >>
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 3/5] md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset
2026-04-20 7:05 ` Xiao Ni
@ 2026-04-21 2:29 ` Su Yue
2026-04-21 7:36 ` Xiao Ni
0 siblings, 1 reply; 22+ messages in thread
From: Su Yue @ 2026-04-21 2:29 UTC (permalink / raw)
To: Xiao Ni; +Cc: Su Yue, linux-raid, song, linan122, yukuai, heming.zhao
On Mon 20 Apr 2026 at 15:05, Xiao Ni <xni@redhat.com> wrote:
> On Tue, Apr 7, 2026 at 6:27 PM Su Yue <glass.su@suse.com> wrote:
>>
>> 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.
>> location_store() now calls md_bitmap_(create|destroy) with
>> false sysfs parameter then
>> (add,remove)_internal_bitmap() will be called to manage sysfs
>> entries.
>>
>> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of
>> bitmap_ops until creating bitmap")
>> Suggested-by: Yu Kuai <yukuai@fnnas.com>
>> Signed-off-by: Su Yue <glass.su@suse.com>
>> ---
>> drivers/md/md-bitmap.c | 116
>> ++++++++++++++++++++++++++++++++++++---
>> drivers/md/md-bitmap.h | 3 +
>> drivers/md/md-llbitmap.c | 12 ++++
>> drivers/md/md.c | 16 +++---
>> 4 files changed, 130 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index ac06c9647bf0..a8a176428c61 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;
>> @@ -2594,6 +2604,9 @@ location_show(struct mddev *mddev, char
>> *page)
>> return len;
>> }
>>
>> +static int create_internal_group(struct mddev *mddev);
>> +static void remove_internal_group(struct mddev *mddev);
>> +
>> static ssize_t
>> location_store(struct mddev *mddev, const char *buf, size_t
>> len)
>> {
>> @@ -2618,7 +2631,8 @@ location_store(struct mddev *mddev, const
>> char *buf, size_t len)
>> goto out;
>> }
>>
>> - md_bitmap_destroy(mddev, true);
>> + md_bitmap_destroy(mddev, false);
>> + remove_internal_group(mddev);
>> mddev->bitmap_info.offset = 0;
>> if (mddev->bitmap_info.file) {
>> struct file *f =
>> mddev->bitmap_info.file;
>> @@ -2659,14 +2673,22 @@ location_store(struct mddev *mddev,
>> const char *buf, size_t len)
>> */
>> mddev->bitmap_id = ID_BITMAP;
>> mddev->bitmap_info.offset = offset;
>> - rv = md_bitmap_create(mddev, true);
>> + rv = md_bitmap_create(mddev, false);
>> if (rv)
>> goto out;
>>
>> + rv = create_internal_group(mddev);
>> + if (rv) {
>> + mddev->bitmap_info.offset = 0;
>> + bitmap_destroy(mddev);
>> + goto out;
>> + }
>> +
>> rv = bitmap_load(mddev);
>> if (rv) {
>> + remove_internal_group(mddev);
>> mddev->bitmap_info.offset = 0;
>> - md_bitmap_destroy(mddev, true);
>> + md_bitmap_destroy(mddev,
>> false);
>> goto out;
>> }
>> }
>> @@ -2960,8 +2982,7 @@ static struct md_sysfs_entry
>> max_backlog_used =
>> __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,
>> +static struct attribute *internal_bitmap_attrs[] = {
>> &bitmap_space.attr,
>> &bitmap_timeout.attr,
>> &bitmap_backlog.attr,
>> @@ -2972,11 +2993,57 @@ static struct attribute
>> *md_bitmap_attrs[] = {
>> NULL
>> };
>>
>> -static struct attribute_group md_bitmap_group = {
>> +static struct attribute_group internal_bitmap_group = {
>> .name = "bitmap",
>> - .attrs = md_bitmap_attrs,
>> + .attrs = internal_bitmap_attrs,
>> };
>>
>> +/* Only necessary attrs for compatibility */
>> +static struct attribute *common_bitmap_attrs[] = {
>> + &bitmap_location.attr,
>> + NULL
>> +};
>> +
>> +static const struct attribute_group common_bitmap_group = {
>> + .name = "bitmap",
>> + .attrs = common_bitmap_attrs,
>> +};
>> +
>> +static int create_internal_group(struct mddev *mddev)
>> +{
>> + /*
>> + * md_bitmap_group and md_bitmap_common_group are using
>> same name
>> + * 'bitmap'.
>> + */
>> + return sysfs_merge_group(&mddev->kobj,
>> &internal_bitmap_group);
>> +}
>> +
>> +static void remove_internal_group(struct mddev *mddev)
>> +{
>> + sysfs_unmerge_group(&mddev->kobj,
>> &internal_bitmap_group);
>> +}
>> +
>> +static int bitmap_register_groups(struct mddev *mddev)
>> +{
>> + int ret;
>> +
>> + ret = sysfs_create_group(&mddev->kobj,
>> &common_bitmap_group);
>> +
>> + if (ret)
>> + return ret;
>> +
>> + ret = sysfs_merge_group(&mddev->kobj,
>> &internal_bitmap_group);
>> + if (ret)
>> + sysfs_remove_group(&mddev->kobj,
>> &common_bitmap_group);
>> +
>> + return ret;
>> +}
>> +
>> +static void bitmap_unregister_groups(struct mddev *mddev)
>> +{
>> + sysfs_unmerge_group(&mddev->kobj,
>> &internal_bitmap_group);
>> +}
>
> Hi Su
>
> bitmap_unregister_groups should also remove common_bitmap_group,
> right?
>
As you pasted before, mdadm:Grow.c:
int Grow_addbitmap(char *devname, int fd, struct context *c,
struct shape *s)
{
...
if (array.state & (1 << MD_SB_BITMAP_PRESENT)) {
if (s->btype == BitmapNone) {
array.state &= ~(1 << MD_SB_BITMAP_PRESENT);
if (md_set_array_info(fd, &array) != 0) {
if (array.state & (1 << MD_SB_CLUSTERED))
pr_err("failed to remove clustered
bitmap.\n");
else
pr_err("failed to remove internal bitmap.\n");
return 1;
}
return 0;
}
pr_err("bitmap already present on %s\n", devname);
return 1;
}
...
}
In case of growing from internal to none,
bitmap_unregister_groups() will
be called, if common_bitmap_group is removed, bitmap/location
won't exist.
update_array_info() is a common function used by many call paths.
Also
llbitmap is involved here. I don't want to make situation and code
more
complicated like adding more codes in update_array_info().
>> +
>> static struct bitmap_operations bitmap_ops = {
>
> You already changed md_bitmap_group to internal_bitmap_group.
> It's
> better to change bitmap_ops to internal_bitmap_ops?
>
>> .head = {
>> .type = MD_BITMAP,
>> @@ -3018,21 +3085,52 @@ static struct bitmap_operations
>> bitmap_ops = {
>> .set_pages = bitmap_set_pages,
>> .free = md_bitmap_free,
>>
>> - .group = &md_bitmap_group,
>> + .register_groups = bitmap_register_groups,
>> + .unregister_groups = bitmap_unregister_groups,
>> +};
>> +
>> +static int none_bitmap_register_groups(struct mddev *mddev)
>> +{
>> + return sysfs_create_group(&mddev->kobj,
>> &common_bitmap_group);
>> +}
>> +
>> +static struct bitmap_operations none_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,
>> +
>> + .register_groups = none_bitmap_register_groups,
>> + .unregister_groups = NULL,
>
> How does bitmap/location can be deleted if array is created with
> no
> bitmap? Can the `mdadm --stop` command get stuck?
>
No. It wont get stuck.
I guess here your concern is the timing of removing
common_bitmap_group.
The life cycle is same as before fb8cc3b0d9db. At the time
llbitmap is
not introduced, because there is no need to switch bitmap ops, so
all
attrs of bitmap are removed in kobject_put() in
mddev_delayed_delete()
queued by mddev_put(). This is now where common_bitmap_group is
being removed.
--
Su
> Best Regards
> Xiao
>
>> };
>>
>> 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(&none_bitmap_ops.head);
>> }
>>
>> void md_bitmap_exit(void)
>> {
>> destroy_workqueue(md_bitmap_wq);
>> unregister_md_submodule(&bitmap_ops.head);
>> + unregister_md_submodule(&none_bitmap_ops.head);
>> }
>> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
>> index b42a28fa83a0..10bc6854798c 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_groups)(struct mddev *mddev);
>> + void (*unregister_groups)(struct mddev *mddev);
>> +
>> struct attribute_group *group;
>> };
>>
>> diff --git a/drivers/md/md-llbitmap.c
>> b/drivers/md/md-llbitmap.c
>> index bf398d7476b3..9b3ea4f1d268 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_groups(struct mddev *mddev)
>> +{
>> + return sysfs_create_group(&mddev->kobj,
>> &md_llbitmap_group);
>> +}
>> +
>> +static void llbitmap_unregister_groups(struct mddev *mddev)
>> +{
>> + sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
>> +}
>> +
>> static struct bitmap_operations llbitmap_ops = {
>> .head = {
>> .type = MD_BITMAP,
>> @@ -1597,6 +1607,8 @@ static struct bitmap_operations
>> llbitmap_ops = {
>> .dirty_bits = llbitmap_dirty_bits,
>> .write_all = llbitmap_write_all,
>>
>> + .register_groups = llbitmap_register_groups,
>> + .unregister_groups = llbitmap_unregister_groups,
>> .group = &md_llbitmap_group,
>> };
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index d3c8f77b4fe3..55a95b227b83 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -683,8 +683,7 @@ static bool mddev_set_bitmap_ops(struct
>> mddev *mddev, bool create_sysfs)
>> struct bitmap_operations *old = mddev->bitmap_ops;
>> struct md_submodule_head *head;
>>
>> - 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);
>> @@ -703,8 +702,8 @@ static bool mddev_set_bitmap_ops(struct
>> mddev *mddev, bool create_sysfs)
>> mddev->bitmap_ops = (void *)head;
>> xa_unlock(&md_submodule);
>>
>> - if (create_sysfs && !mddev_is_dm(mddev) &&
>> mddev->bitmap_ops->group) {
>> - if (sysfs_create_group(&mddev->kobj,
>> mddev->bitmap_ops->group))
>> + if (create_sysfs && !mddev_is_dm(mddev) &&
>> mddev->bitmap_ops->register_groups) {
>> + if (mddev->bitmap_ops->register_groups(mddev))
>> pr_warn("md: cannot register extra
>> bitmap attributes for %s\n",
>> mdname(mddev));
>> else
>> @@ -724,8 +723,8 @@ static bool mddev_set_bitmap_ops(struct
>> mddev *mddev, bool create_sysfs)
>> static void mddev_clear_bitmap_ops(struct mddev *mddev, bool
>> remove_sysfs)
>> {
>> if (remove_sysfs && !mddev_is_dm(mddev) &&
>> mddev->bitmap_ops &&
>> - mddev->bitmap_ops->group)
>> - sysfs_remove_group(&mddev->kobj,
>> mddev->bitmap_ops->group);
>> + mddev->bitmap_ops->unregister_groups)
>> + mddev->bitmap_ops->unregister_groups(mddev);
>>
>> mddev->bitmap_ops = NULL;
>> }
>> @@ -6610,8 +6609,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, true);
>> if (err)
>> pr_warn("%s: failed to create bitmap
>> (%d)\n",
>> --
>> 2.53.0
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 0/5] md: bitmap grow fixes
2026-04-07 10:26 [PATCH v2 0/5] md: bitmap grow fixes Su Yue
` (5 preceding siblings ...)
2026-04-16 14:10 ` [PATCH v2 0/5] md: bitmap grow fixes Su Yue
@ 2026-04-21 5:15 ` Yu Kuai
2026-04-21 5:39 ` Su Yue
6 siblings, 1 reply; 22+ messages in thread
From: Yu Kuai @ 2026-04-21 5:15 UTC (permalink / raw)
To: Su Yue, linux-raid
Cc: song, xni, linan122@huawei.com >> Li Nan, yukuai,
Heming Zhao
Hi,
在 2026/4/7 18:26, Su Yue 写道:
> Hi, This v2 series is to fixes bugs exposed by mdadm/clustermd-autotest.
> The bugs are not cluster md only but also bitmap related.
>
> The series based on v7.0-rc7 passes tests with mdadm v4.6 without regression.
>
> v2:
> Add a dummy bitmap operations per Kuai's suggestion.
>
> To Yu Kuai:
> A NULL group can't used for internal_bitmap_group since
> the entries from internal_bitmap_attrs should be under bitmap.
> So instead of sysfs_create_groups(), sysfs_create_group() and sysfs_merge_group()
> are still needed /sigh.
Thanks for the set and sorry for the delay.
TBO, this set is still a bit complex than expected. It's true sysfs_create_groups()
can't be used for the groups with same name. However, there is still another api
sysfs_update_groups() that can be used in this case.
I just finish a local version and confirm following 3 commits should be enough:
1) factor out bitmap sysfs creation from mddev_set_bitmap_ops(), and create sysfs entries
after bitmap is created.
2) split bitmap/location out as the common bitmap groups, and convert bitmap filed groups
to struct attribute_group **groups;
3) add a separate bitmap_ops for none bitmap, location_store() can be similar to
patch 3 in your set, just create/remove additional internal_bitmap_group.
> Su Yue (5):
> md/md-bitmap: call md_bitmap_create,destroy in location_store
> md/md-bitmap: add an extra sysfs argument to md_bitmap_create and
> destroy
> md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset
> md: skip ID_BITMAP_NONE when show available bitmap types
> md/md-bitmap: remove member group from bitmap_operations
>
> drivers/md/md-bitmap.c | 121 ++++++++++++++++++++++++++++++++++++---
> drivers/md/md-bitmap.h | 3 +-
> drivers/md/md-llbitmap.c | 13 ++++-
> drivers/md/md.c | 55 +++++++++---------
> drivers/md/md.h | 2 +
> 5 files changed, 155 insertions(+), 39 deletions(-)
>
--
Thansk,
Kuai
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 0/5] md: bitmap grow fixes
2026-04-21 5:15 ` Yu Kuai
@ 2026-04-21 5:39 ` Su Yue
0 siblings, 0 replies; 22+ messages in thread
From: Su Yue @ 2026-04-21 5:39 UTC (permalink / raw)
To: Yu Kuai
Cc: Su Yue, linux-raid, song, xni,
linan122@huawei.com >> Li Nan, Heming Zhao
On Tue 21 Apr 2026 at 13:15, "Yu Kuai" <yukuai@fnnas.com> wrote:
> Hi,
>
> 在 2026/4/7 18:26, Su Yue 写道:
>> Hi, This v2 series is to fixes bugs exposed by
>> mdadm/clustermd-autotest.
>> The bugs are not cluster md only but also bitmap related.
>>
>> The series based on v7.0-rc7 passes tests with mdadm v4.6
>> without regression.
>>
>> v2:
>> Add a dummy bitmap operations per Kuai's suggestion.
>>
>> To Yu Kuai:
>> A NULL group can't used for internal_bitmap_group since
>> the entries from internal_bitmap_attrs should be under bitmap.
>> So instead of sysfs_create_groups(), sysfs_create_group() and
>> sysfs_merge_group()
>> are still needed /sigh.
>
> Thanks for the set and sorry for the delay.
>
> TBO, this set is still a bit complex than expected. It's true
> sysfs_create_groups()
> can't be used for the groups with same name. However, there is
> still another api
> sysfs_update_groups() that can be used in this case.
>
IIUC, sysfs_update_groups() can't be used because it first removes
then re-add files
see linux/fs/sysfs/group.c:create_files() for details.
>
> I just finish a local version and confirm following 3 commits
> should be enough:
>
If you would like to send your own patchset. I am happy to give
up my patchset then
review yours :) The only thing I want to fix the issue. Thanks.
--
Su
> 1) factor out bitmap sysfs creation from mddev_set_bitmap_ops(),
> and create sysfs entries
> after bitmap is created.
> 2) split bitmap/location out as the common bitmap groups, and
> convert bitmap filed groups
> to struct attribute_group **groups;
> 3) add a separate bitmap_ops for none bitmap, location_store()
> can be similar to
> patch 3 in your set, just create/remove additional
> internal_bitmap_group.
>
>> Su Yue (5):
>> md/md-bitmap: call md_bitmap_create,destroy in
>> location_store
>> md/md-bitmap: add an extra sysfs argument to
>> md_bitmap_create and
>> destroy
>> md/md-bitmap: add dummy bitmap ops for none to fix wrong
>> bitmap offset
>> md: skip ID_BITMAP_NONE when show available bitmap types
>> md/md-bitmap: remove member group from bitmap_operations
>>
>> drivers/md/md-bitmap.c | 121
>> ++++++++++++++++++++++++++++++++++++---
>> drivers/md/md-bitmap.h | 3 +-
>> drivers/md/md-llbitmap.c | 13 ++++-
>> drivers/md/md.c | 55 +++++++++---------
>> drivers/md/md.h | 2 +
>> 5 files changed, 155 insertions(+), 39 deletions(-)
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 3/5] md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset
2026-04-21 2:29 ` Su Yue
@ 2026-04-21 7:36 ` Xiao Ni
2026-04-21 9:21 ` Su Yue
0 siblings, 1 reply; 22+ messages in thread
From: Xiao Ni @ 2026-04-21 7:36 UTC (permalink / raw)
To: Su Yue; +Cc: Su Yue, linux-raid, song, linan122, yukuai, heming.zhao
On Tue, Apr 21, 2026 at 10:29 AM Su Yue <l@damenly.org> wrote:
>
> On Mon 20 Apr 2026 at 15:05, Xiao Ni <xni@redhat.com> wrote:
>
> > On Tue, Apr 7, 2026 at 6:27 PM Su Yue <glass.su@suse.com> wrote:
> >>
> >> 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.
> >> location_store() now calls md_bitmap_(create|destroy) with
> >> false sysfs parameter then
> >> (add,remove)_internal_bitmap() will be called to manage sysfs
> >> entries.
> >>
> >> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of
> >> bitmap_ops until creating bitmap")
> >> Suggested-by: Yu Kuai <yukuai@fnnas.com>
> >> Signed-off-by: Su Yue <glass.su@suse.com>
> >> ---
> >> drivers/md/md-bitmap.c | 116
> >> ++++++++++++++++++++++++++++++++++++---
> >> drivers/md/md-bitmap.h | 3 +
> >> drivers/md/md-llbitmap.c | 12 ++++
> >> drivers/md/md.c | 16 +++---
> >> 4 files changed, 130 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> >> index ac06c9647bf0..a8a176428c61 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;
> >> @@ -2594,6 +2604,9 @@ location_show(struct mddev *mddev, char
> >> *page)
> >> return len;
> >> }
> >>
> >> +static int create_internal_group(struct mddev *mddev);
> >> +static void remove_internal_group(struct mddev *mddev);
> >> +
> >> static ssize_t
> >> location_store(struct mddev *mddev, const char *buf, size_t
> >> len)
> >> {
> >> @@ -2618,7 +2631,8 @@ location_store(struct mddev *mddev, const
> >> char *buf, size_t len)
> >> goto out;
> >> }
> >>
> >> - md_bitmap_destroy(mddev, true);
> >> + md_bitmap_destroy(mddev, false);
> >> + remove_internal_group(mddev);
> >> mddev->bitmap_info.offset = 0;
> >> if (mddev->bitmap_info.file) {
> >> struct file *f =
> >> mddev->bitmap_info.file;
> >> @@ -2659,14 +2673,22 @@ location_store(struct mddev *mddev,
> >> const char *buf, size_t len)
> >> */
> >> mddev->bitmap_id = ID_BITMAP;
> >> mddev->bitmap_info.offset = offset;
> >> - rv = md_bitmap_create(mddev, true);
> >> + rv = md_bitmap_create(mddev, false);
> >> if (rv)
> >> goto out;
> >>
> >> + rv = create_internal_group(mddev);
> >> + if (rv) {
> >> + mddev->bitmap_info.offset = 0;
> >> + bitmap_destroy(mddev);
> >> + goto out;
> >> + }
> >> +
> >> rv = bitmap_load(mddev);
> >> if (rv) {
> >> + remove_internal_group(mddev);
> >> mddev->bitmap_info.offset = 0;
> >> - md_bitmap_destroy(mddev, true);
> >> + md_bitmap_destroy(mddev,
> >> false);
> >> goto out;
> >> }
> >> }
> >> @@ -2960,8 +2982,7 @@ static struct md_sysfs_entry
> >> max_backlog_used =
> >> __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,
> >> +static struct attribute *internal_bitmap_attrs[] = {
> >> &bitmap_space.attr,
> >> &bitmap_timeout.attr,
> >> &bitmap_backlog.attr,
> >> @@ -2972,11 +2993,57 @@ static struct attribute
> >> *md_bitmap_attrs[] = {
> >> NULL
> >> };
> >>
> >> -static struct attribute_group md_bitmap_group = {
> >> +static struct attribute_group internal_bitmap_group = {
> >> .name = "bitmap",
> >> - .attrs = md_bitmap_attrs,
> >> + .attrs = internal_bitmap_attrs,
> >> };
> >>
> >> +/* Only necessary attrs for compatibility */
> >> +static struct attribute *common_bitmap_attrs[] = {
> >> + &bitmap_location.attr,
> >> + NULL
> >> +};
> >> +
> >> +static const struct attribute_group common_bitmap_group = {
> >> + .name = "bitmap",
> >> + .attrs = common_bitmap_attrs,
> >> +};
> >> +
> >> +static int create_internal_group(struct mddev *mddev)
> >> +{
> >> + /*
> >> + * md_bitmap_group and md_bitmap_common_group are using
> >> same name
> >> + * 'bitmap'.
> >> + */
> >> + return sysfs_merge_group(&mddev->kobj,
> >> &internal_bitmap_group);
> >> +}
> >> +
> >> +static void remove_internal_group(struct mddev *mddev)
> >> +{
> >> + sysfs_unmerge_group(&mddev->kobj,
> >> &internal_bitmap_group);
> >> +}
> >> +
> >> +static int bitmap_register_groups(struct mddev *mddev)
> >> +{
> >> + int ret;
> >> +
> >> + ret = sysfs_create_group(&mddev->kobj,
> >> &common_bitmap_group);
> >> +
> >> + if (ret)
> >> + return ret;
> >> +
> >> + ret = sysfs_merge_group(&mddev->kobj,
> >> &internal_bitmap_group);
> >> + if (ret)
> >> + sysfs_remove_group(&mddev->kobj,
> >> &common_bitmap_group);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static void bitmap_unregister_groups(struct mddev *mddev)
> >> +{
> >> + sysfs_unmerge_group(&mddev->kobj,
> >> &internal_bitmap_group);
> >> +}
> >
> > Hi Su
> >
> > bitmap_unregister_groups should also remove common_bitmap_group,
> > right?
> >
>
> As you pasted before, mdadm:Grow.c:
>
> int Grow_addbitmap(char *devname, int fd, struct context *c,
> struct shape *s)
> {
> ...
> if (array.state & (1 << MD_SB_BITMAP_PRESENT)) {
> if (s->btype == BitmapNone) {
> array.state &= ~(1 << MD_SB_BITMAP_PRESENT);
> if (md_set_array_info(fd, &array) != 0) {
> if (array.state & (1 << MD_SB_CLUSTERED))
> pr_err("failed to remove clustered
> bitmap.\n");
> else
> pr_err("failed to remove internal bitmap.\n");
> return 1;
> }
> return 0;
> }
> pr_err("bitmap already present on %s\n", devname);
> return 1;
> }
> ...
> }
>
> In case of growing from internal to none,
> bitmap_unregister_groups() will
> be called, if common_bitmap_group is removed, bitmap/location
> won't exist.
> update_array_info() is a common function used by many call paths.
> Also
> llbitmap is involved here. I don't want to make situation and code
> more
> complicated like adding more codes in update_array_info().
The above mdadm codes use bitmap/location. In your patch, you already
pass false to md_bitmap_destroy in location_store. So removing
common_bitmap_group in bitmap_unregister_groups can't affect the above
case.
But after reading your below comments. It's good to me that
bitmap_unregister_groups doesn't remove common_bitmap_group.
>
>
> >> +
> >> static struct bitmap_operations bitmap_ops = {
> >
> > You already changed md_bitmap_group to internal_bitmap_group.
> > It's
> > better to change bitmap_ops to internal_bitmap_ops?
> >
> >> .head = {
> >> .type = MD_BITMAP,
> >> @@ -3018,21 +3085,52 @@ static struct bitmap_operations
> >> bitmap_ops = {
> >> .set_pages = bitmap_set_pages,
> >> .free = md_bitmap_free,
> >>
> >> - .group = &md_bitmap_group,
> >> + .register_groups = bitmap_register_groups,
> >> + .unregister_groups = bitmap_unregister_groups,
> >> +};
> >> +
> >> +static int none_bitmap_register_groups(struct mddev *mddev)
> >> +{
> >> + return sysfs_create_group(&mddev->kobj,
> >> &common_bitmap_group);
> >> +}
> >> +
> >> +static struct bitmap_operations none_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,
> >> +
> >> + .register_groups = none_bitmap_register_groups,
> >> + .unregister_groups = NULL,
> >
> > How does bitmap/location can be deleted if array is created with
> > no
> > bitmap? Can the `mdadm --stop` command get stuck?
> >
> No. It wont get stuck.
>
> I guess here your concern is the timing of removing
> common_bitmap_group.
> The life cycle is same as before fb8cc3b0d9db. At the time
> llbitmap is
> not introduced, because there is no need to switch bitmap ops, so
> all
> attrs of bitmap are removed in kobject_put() in
> mddev_delayed_delete()
> queued by mddev_put(). This is now where common_bitmap_group is
> being removed.
Thanks for the explanation.
Best Regards
Xiao
>
> --
> Su
> > Best Regards
> > Xiao
> >
> >> };
> >>
> >> 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(&none_bitmap_ops.head);
> >> }
> >>
> >> void md_bitmap_exit(void)
> >> {
> >> destroy_workqueue(md_bitmap_wq);
> >> unregister_md_submodule(&bitmap_ops.head);
> >> + unregister_md_submodule(&none_bitmap_ops.head);
> >> }
> >> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
> >> index b42a28fa83a0..10bc6854798c 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_groups)(struct mddev *mddev);
> >> + void (*unregister_groups)(struct mddev *mddev);
> >> +
> >> struct attribute_group *group;
> >> };
> >>
> >> diff --git a/drivers/md/md-llbitmap.c
> >> b/drivers/md/md-llbitmap.c
> >> index bf398d7476b3..9b3ea4f1d268 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_groups(struct mddev *mddev)
> >> +{
> >> + return sysfs_create_group(&mddev->kobj,
> >> &md_llbitmap_group);
> >> +}
> >> +
> >> +static void llbitmap_unregister_groups(struct mddev *mddev)
> >> +{
> >> + sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
> >> +}
> >> +
> >> static struct bitmap_operations llbitmap_ops = {
> >> .head = {
> >> .type = MD_BITMAP,
> >> @@ -1597,6 +1607,8 @@ static struct bitmap_operations
> >> llbitmap_ops = {
> >> .dirty_bits = llbitmap_dirty_bits,
> >> .write_all = llbitmap_write_all,
> >>
> >> + .register_groups = llbitmap_register_groups,
> >> + .unregister_groups = llbitmap_unregister_groups,
> >> .group = &md_llbitmap_group,
> >> };
> >>
> >> diff --git a/drivers/md/md.c b/drivers/md/md.c
> >> index d3c8f77b4fe3..55a95b227b83 100644
> >> --- a/drivers/md/md.c
> >> +++ b/drivers/md/md.c
> >> @@ -683,8 +683,7 @@ static bool mddev_set_bitmap_ops(struct
> >> mddev *mddev, bool create_sysfs)
> >> struct bitmap_operations *old = mddev->bitmap_ops;
> >> struct md_submodule_head *head;
> >>
> >> - 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);
> >> @@ -703,8 +702,8 @@ static bool mddev_set_bitmap_ops(struct
> >> mddev *mddev, bool create_sysfs)
> >> mddev->bitmap_ops = (void *)head;
> >> xa_unlock(&md_submodule);
> >>
> >> - if (create_sysfs && !mddev_is_dm(mddev) &&
> >> mddev->bitmap_ops->group) {
> >> - if (sysfs_create_group(&mddev->kobj,
> >> mddev->bitmap_ops->group))
> >> + if (create_sysfs && !mddev_is_dm(mddev) &&
> >> mddev->bitmap_ops->register_groups) {
> >> + if (mddev->bitmap_ops->register_groups(mddev))
> >> pr_warn("md: cannot register extra
> >> bitmap attributes for %s\n",
> >> mdname(mddev));
> >> else
> >> @@ -724,8 +723,8 @@ static bool mddev_set_bitmap_ops(struct
> >> mddev *mddev, bool create_sysfs)
> >> static void mddev_clear_bitmap_ops(struct mddev *mddev, bool
> >> remove_sysfs)
> >> {
> >> if (remove_sysfs && !mddev_is_dm(mddev) &&
> >> mddev->bitmap_ops &&
> >> - mddev->bitmap_ops->group)
> >> - sysfs_remove_group(&mddev->kobj,
> >> mddev->bitmap_ops->group);
> >> + mddev->bitmap_ops->unregister_groups)
> >> + mddev->bitmap_ops->unregister_groups(mddev);
> >>
> >> mddev->bitmap_ops = NULL;
> >> }
> >> @@ -6610,8 +6609,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, true);
> >> if (err)
> >> pr_warn("%s: failed to create bitmap
> >> (%d)\n",
> >> --
> >> 2.53.0
> >>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 3/5] md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset
2026-04-21 7:36 ` Xiao Ni
@ 2026-04-21 9:21 ` Su Yue
0 siblings, 0 replies; 22+ messages in thread
From: Su Yue @ 2026-04-21 9:21 UTC (permalink / raw)
To: Xiao Ni; +Cc: Su Yue, linux-raid, song, linan122, yukuai, heming.zhao
On Tue 21 Apr 2026 at 15:36, Xiao Ni <xni@redhat.com> wrote:
> On Tue, Apr 21, 2026 at 10:29 AM Su Yue <l@damenly.org> wrote:
>>
>> On Mon 20 Apr 2026 at 15:05, Xiao Ni <xni@redhat.com> wrote:
>>
>> > On Tue, Apr 7, 2026 at 6:27 PM Su Yue <glass.su@suse.com>
>> > wrote:
>> >>
>> >> 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.
>> >> location_store() now calls md_bitmap_(create|destroy) with
>> >> false sysfs parameter then
>> >> (add,remove)_internal_bitmap() will be called to manage
>> >> sysfs
>> >> entries.
>> >>
>> >> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of
>> >> bitmap_ops until creating bitmap")
>> >> Suggested-by: Yu Kuai <yukuai@fnnas.com>
>> >> Signed-off-by: Su Yue <glass.su@suse.com>
>> >> ---
>> >> drivers/md/md-bitmap.c | 116
>> >> ++++++++++++++++++++++++++++++++++++---
>> >> drivers/md/md-bitmap.h | 3 +
>> >> drivers/md/md-llbitmap.c | 12 ++++
>> >> drivers/md/md.c | 16 +++---
>> >> 4 files changed, 130 insertions(+), 17 deletions(-)
>> >>
>> >> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> >> index ac06c9647bf0..a8a176428c61 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;
>> >> @@ -2594,6 +2604,9 @@ location_show(struct mddev *mddev,
>> >> char
>> >> *page)
>> >> return len;
>> >> }
>> >>
>> >> +static int create_internal_group(struct mddev *mddev);
>> >> +static void remove_internal_group(struct mddev *mddev);
>> >> +
>> >> static ssize_t
>> >> location_store(struct mddev *mddev, const char *buf, size_t
>> >> len)
>> >> {
>> >> @@ -2618,7 +2631,8 @@ location_store(struct mddev *mddev,
>> >> const
>> >> char *buf, size_t len)
>> >> goto out;
>> >> }
>> >>
>> >> - md_bitmap_destroy(mddev, true);
>> >> + md_bitmap_destroy(mddev, false);
>> >> + remove_internal_group(mddev);
>> >> mddev->bitmap_info.offset = 0;
>> >> if (mddev->bitmap_info.file) {
>> >> struct file *f =
>> >> mddev->bitmap_info.file;
>> >> @@ -2659,14 +2673,22 @@ location_store(struct mddev *mddev,
>> >> const char *buf, size_t len)
>> >> */
>> >> mddev->bitmap_id = ID_BITMAP;
>> >> mddev->bitmap_info.offset = offset;
>> >> - rv = md_bitmap_create(mddev, true);
>> >> + rv = md_bitmap_create(mddev, false);
>> >> if (rv)
>> >> goto out;
>> >>
>> >> + rv = create_internal_group(mddev);
>> >> + if (rv) {
>> >> + mddev->bitmap_info.offset =
>> >> 0;
>> >> + bitmap_destroy(mddev);
>> >> + goto out;
>> >> + }
>> >> +
>> >> rv = bitmap_load(mddev);
>> >> if (rv) {
>> >> +
>> >> remove_internal_group(mddev);
>> >> mddev->bitmap_info.offset =
>> >> 0;
>> >> - md_bitmap_destroy(mddev,
>> >> true);
>> >> + md_bitmap_destroy(mddev,
>> >> false);
>> >> goto out;
>> >> }
>> >> }
>> >> @@ -2960,8 +2982,7 @@ static struct md_sysfs_entry
>> >> max_backlog_used =
>> >> __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,
>> >> +static struct attribute *internal_bitmap_attrs[] = {
>> >> &bitmap_space.attr,
>> >> &bitmap_timeout.attr,
>> >> &bitmap_backlog.attr,
>> >> @@ -2972,11 +2993,57 @@ static struct attribute
>> >> *md_bitmap_attrs[] = {
>> >> NULL
>> >> };
>> >>
>> >> -static struct attribute_group md_bitmap_group = {
>> >> +static struct attribute_group internal_bitmap_group = {
>> >> .name = "bitmap",
>> >> - .attrs = md_bitmap_attrs,
>> >> + .attrs = internal_bitmap_attrs,
>> >> };
>> >>
>> >> +/* Only necessary attrs for compatibility */
>> >> +static struct attribute *common_bitmap_attrs[] = {
>> >> + &bitmap_location.attr,
>> >> + NULL
>> >> +};
>> >> +
>> >> +static const struct attribute_group common_bitmap_group = {
>> >> + .name = "bitmap",
>> >> + .attrs = common_bitmap_attrs,
>> >> +};
>> >> +
>> >> +static int create_internal_group(struct mddev *mddev)
>> >> +{
>> >> + /*
>> >> + * md_bitmap_group and md_bitmap_common_group are
>> >> using
>> >> same name
>> >> + * 'bitmap'.
>> >> + */
>> >> + return sysfs_merge_group(&mddev->kobj,
>> >> &internal_bitmap_group);
>> >> +}
>> >> +
>> >> +static void remove_internal_group(struct mddev *mddev)
>> >> +{
>> >> + sysfs_unmerge_group(&mddev->kobj,
>> >> &internal_bitmap_group);
>> >> +}
>> >> +
>> >> +static int bitmap_register_groups(struct mddev *mddev)
>> >> +{
>> >> + int ret;
>> >> +
>> >> + ret = sysfs_create_group(&mddev->kobj,
>> >> &common_bitmap_group);
>> >> +
>> >> + if (ret)
>> >> + return ret;
>> >> +
>> >> + ret = sysfs_merge_group(&mddev->kobj,
>> >> &internal_bitmap_group);
>> >> + if (ret)
>> >> + sysfs_remove_group(&mddev->kobj,
>> >> &common_bitmap_group);
>> >> +
>> >> + return ret;
>> >> +}
>> >> +
>> >> +static void bitmap_unregister_groups(struct mddev *mddev)
>> >> +{
>> >> + sysfs_unmerge_group(&mddev->kobj,
>> >> &internal_bitmap_group);
>> >> +}
>> >
>> > Hi Su
>> >
>> > bitmap_unregister_groups should also remove
>> > common_bitmap_group,
>> > right?
>> >
>>
>> As you pasted before, mdadm:Grow.c:
>>
>> int Grow_addbitmap(char *devname, int fd, struct context *c,
>> struct shape *s)
>> {
>> ...
>> if (array.state & (1 << MD_SB_BITMAP_PRESENT)) {
>> if (s->btype == BitmapNone) {
>> array.state &= ~(1 <<
>> MD_SB_BITMAP_PRESENT);
>> if (md_set_array_info(fd, &array) != 0)
>> {
>> if (array.state & (1 <<
>> MD_SB_CLUSTERED))
>> pr_err("failed to
>> remove clustered
>> bitmap.\n");
>> else
>> pr_err("failed to
>> remove internal
>> bitmap.\n");
>> return 1;
>> }
>> return 0;
>> }
>> pr_err("bitmap already present on %s\n",
>> devname);
>> return 1;
>> }
>> ...
>> }
>>
>> In case of growing from internal to none,
>> bitmap_unregister_groups() will
>> be called, if common_bitmap_group is removed, bitmap/location
>> won't exist.
>> update_array_info() is a common function used by many call
>> paths.
>> Also
>> llbitmap is involved here. I don't want to make situation and
>> code
>> more
>> complicated like adding more codes in update_array_info().
>
> The above mdadm codes use bitmap/location. In your patch, you
> already
> pass false to md_bitmap_destroy in location_store. So removing
> common_bitmap_group in bitmap_unregister_groups can't affect the
> above
> case.
>
Right. Got confusion between md_set_array_info() and
update_array_info().
>
> But after reading your below comments. It's good to me that
> bitmap_unregister_groups doesn't remove common_bitmap_group.
>
Thanks.
--
Su
>>
>>
>> >> +
>> >> static struct bitmap_operations bitmap_ops = {
>> >
>> > You already changed md_bitmap_group to internal_bitmap_group.
>> > It's
>> > better to change bitmap_ops to internal_bitmap_ops?
>> >
>> >> .head = {
>> >> .type = MD_BITMAP,
>> >> @@ -3018,21 +3085,52 @@ static struct bitmap_operations
>> >> bitmap_ops = {
>> >> .set_pages = bitmap_set_pages,
>> >> .free = md_bitmap_free,
>> >>
>> >> - .group = &md_bitmap_group,
>> >> + .register_groups = bitmap_register_groups,
>> >> + .unregister_groups = bitmap_unregister_groups,
>> >> +};
>> >> +
>> >> +static int none_bitmap_register_groups(struct mddev *mddev)
>> >> +{
>> >> + return sysfs_create_group(&mddev->kobj,
>> >> &common_bitmap_group);
>> >> +}
>> >> +
>> >> +static struct bitmap_operations none_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,
>> >> +
>> >> + .register_groups =
>> >> none_bitmap_register_groups,
>> >> + .unregister_groups = NULL,
>> >
>> > How does bitmap/location can be deleted if array is created
>> > with
>> > no
>> > bitmap? Can the `mdadm --stop` command get stuck?
>> >
>> No. It wont get stuck.
>>
>> I guess here your concern is the timing of removing
>> common_bitmap_group.
>> The life cycle is same as before fb8cc3b0d9db. At the time
>> llbitmap is
>> not introduced, because there is no need to switch bitmap ops,
>> so
>> all
>> attrs of bitmap are removed in kobject_put() in
>> mddev_delayed_delete()
>> queued by mddev_put(). This is now where common_bitmap_group is
>> being removed.
>
> Thanks for the explanation.
>
> Best Regards
> Xiao
>>
>> --
>> Su
>> > Best Regards
>> > Xiao
>> >
>> >> };
>> >>
>> >> 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(&none_bitmap_ops.head);
>> >> }
>> >>
>> >> void md_bitmap_exit(void)
>> >> {
>> >> destroy_workqueue(md_bitmap_wq);
>> >> unregister_md_submodule(&bitmap_ops.head);
>> >> + unregister_md_submodule(&none_bitmap_ops.head);
>> >> }
>> >> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
>> >> index b42a28fa83a0..10bc6854798c 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_groups)(struct mddev *mddev);
>> >> + void (*unregister_groups)(struct mddev *mddev);
>> >> +
>> >> struct attribute_group *group;
>> >> };
>> >>
>> >> diff --git a/drivers/md/md-llbitmap.c
>> >> b/drivers/md/md-llbitmap.c
>> >> index bf398d7476b3..9b3ea4f1d268 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_groups(struct mddev *mddev)
>> >> +{
>> >> + return sysfs_create_group(&mddev->kobj,
>> >> &md_llbitmap_group);
>> >> +}
>> >> +
>> >> +static void llbitmap_unregister_groups(struct mddev *mddev)
>> >> +{
>> >> + sysfs_remove_group(&mddev->kobj,
>> >> &md_llbitmap_group);
>> >> +}
>> >> +
>> >> static struct bitmap_operations llbitmap_ops = {
>> >> .head = {
>> >> .type = MD_BITMAP,
>> >> @@ -1597,6 +1607,8 @@ static struct bitmap_operations
>> >> llbitmap_ops = {
>> >> .dirty_bits = llbitmap_dirty_bits,
>> >> .write_all = llbitmap_write_all,
>> >>
>> >> + .register_groups = llbitmap_register_groups,
>> >> + .unregister_groups =
>> >> llbitmap_unregister_groups,
>> >> .group = &md_llbitmap_group,
>> >> };
>> >>
>> >> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> >> index d3c8f77b4fe3..55a95b227b83 100644
>> >> --- a/drivers/md/md.c
>> >> +++ b/drivers/md/md.c
>> >> @@ -683,8 +683,7 @@ static bool mddev_set_bitmap_ops(struct
>> >> mddev *mddev, bool create_sysfs)
>> >> struct bitmap_operations *old = mddev->bitmap_ops;
>> >> struct md_submodule_head *head;
>> >>
>> >> - 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);
>> >> @@ -703,8 +702,8 @@ static bool mddev_set_bitmap_ops(struct
>> >> mddev *mddev, bool create_sysfs)
>> >> mddev->bitmap_ops = (void *)head;
>> >> xa_unlock(&md_submodule);
>> >>
>> >> - if (create_sysfs && !mddev_is_dm(mddev) &&
>> >> mddev->bitmap_ops->group) {
>> >> - if (sysfs_create_group(&mddev->kobj,
>> >> mddev->bitmap_ops->group))
>> >> + if (create_sysfs && !mddev_is_dm(mddev) &&
>> >> mddev->bitmap_ops->register_groups) {
>> >> + if
>> >> (mddev->bitmap_ops->register_groups(mddev))
>> >> pr_warn("md: cannot register extra
>> >> bitmap attributes for %s\n",
>> >> mdname(mddev));
>> >> else
>> >> @@ -724,8 +723,8 @@ static bool mddev_set_bitmap_ops(struct
>> >> mddev *mddev, bool create_sysfs)
>> >> static void mddev_clear_bitmap_ops(struct mddev *mddev,
>> >> bool
>> >> remove_sysfs)
>> >> {
>> >> if (remove_sysfs && !mddev_is_dm(mddev) &&
>> >> mddev->bitmap_ops &&
>> >> - mddev->bitmap_ops->group)
>> >> - sysfs_remove_group(&mddev->kobj,
>> >> mddev->bitmap_ops->group);
>> >> + mddev->bitmap_ops->unregister_groups)
>> >> + mddev->bitmap_ops->unregister_groups(mddev);
>> >>
>> >> mddev->bitmap_ops = NULL;
>> >> }
>> >> @@ -6610,8 +6609,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, true);
>> >> if (err)
>> >> pr_warn("%s: failed to create bitmap
>> >> (%d)\n",
>> >> --
>> >> 2.53.0
>> >>
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2026-04-21 9:26 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-07 10:26 [PATCH v2 0/5] md: bitmap grow fixes Su Yue
2026-04-07 10:26 ` [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store Su Yue
2026-04-13 7:47 ` Li Nan
2026-04-13 10:18 ` Su Yue
2026-04-15 10:34 ` Xiao Ni
2026-04-16 14:08 ` Su Yue
2026-04-20 5:21 ` Xiao Ni
2026-04-21 1:26 ` Su Yue
2026-04-07 10:26 ` [PATCH v2 2/5] md/md-bitmap: add an extra sysfs argument to md_bitmap_create and destroy Su Yue
2026-04-20 5:24 ` Xiao Ni
2026-04-07 10:26 ` [PATCH v2 3/5] md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset Su Yue
2026-04-20 7:05 ` Xiao Ni
2026-04-21 2:29 ` Su Yue
2026-04-21 7:36 ` Xiao Ni
2026-04-21 9:21 ` Su Yue
2026-04-07 10:26 ` [PATCH v2 4/5] md: skip ID_BITMAP_NONE when show available bitmap types Su Yue
2026-04-13 8:15 ` Li Nan
2026-04-13 10:23 ` Su Yue
2026-04-07 10:26 ` [PATCH v2 5/5] md/md-bitmap: remove member group from bitmap_operations Su Yue
2026-04-16 14:10 ` [PATCH v2 0/5] md: bitmap grow fixes Su Yue
2026-04-21 5:15 ` Yu Kuai
2026-04-21 5:39 ` Su Yue
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox