Re: [PATCH v3 06/11] md/md-bitmap: delay registration of bitmap_ops until creating bitmap

[Yu Kuai]

Hi,

在 2025/7/19 14:49, Li Nan 写道:

在 2025/7/18 17:23, Yu Kuai 写道:

From: Yu Kuai <yuku...@huawei.com>

Currently bitmap_ops is registered while allocating mddev, this is fine
when there is only one bitmap_ops.

Delay setting bitmap_ops until creating bitmap, so that user can choose
which bitmap to use before running the array.

Signed-off-by: Yu Kuai <yuku...@huawei.com>
---
  Documentation/admin-guide/md.rst |  3 ++
  drivers/md/md.c                  | 82 +++++++++++++++++++++-----------
  2 files changed, 56 insertions(+), 29 deletions(-)

diff --git a/Documentation/admin-guide/md.rst b/Documentation/admin-guide/md.rst

index 356d2a344f08..03a9f5025f99 100644
--- a/Documentation/admin-guide/md.rst
+++ b/Documentation/admin-guide/md.rst
@@ -388,6 +388,9 @@ All md devices contain:
       bitmap
           The default internal bitmap
  +If bitmap_type is not none, then additional bitmap attributes will be
+created after md device KOBJ_CHANGE event.
+
  If bitmap_type is bitmap, then the md device will also contain:
      bitmap/location
diff --git a/drivers/md/md.c b/drivers/md/md.c
index d8b0dfdb4bfc..639b0143cbb1 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -674,9 +674,11 @@ static void no_op(struct percpu_ref *r) {}
    static bool mddev_set_bitmap_ops(struct mddev *mddev)
  {
+    struct bitmap_operations *old = mddev->bitmap_ops;
      struct md_submodule_head *head;
  -    if (mddev->bitmap_id == ID_BITMAP_NONE)
+    if (mddev->bitmap_id == ID_BITMAP_NONE ||
+        (old && old->head.id == mddev->bitmap_id))
          return true;
        xa_lock(&md_submodule);

@@ -694,6 +696,18 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)

        mddev->bitmap_ops = (void *)head;
      xa_unlock(&md_submodule);
+
+    if (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));
+        else
+            /*
+             * Inform user with KOBJ_CHANGE about new bitmap
+             * attributes.
+             */
+            kobject_uevent(&mddev->kobj, KOBJ_CHANGE);
+    }
      return true;
    err:

@@ -703,28 +717,25 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)

    static void mddev_clear_bitmap_ops(struct mddev *mddev)
  {
+    if (mddev->bitmap_ops && mddev->bitmap_ops->group)
+        sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
+
      mddev->bitmap_ops = NULL;
  }
    int mddev_init(struct mddev *mddev)
  {
-    if (!IS_ENABLED(CONFIG_MD_BITMAP)) {
+    if (!IS_ENABLED(CONFIG_MD_BITMAP))
          mddev->bitmap_id = ID_BITMAP_NONE;
-    } else {
+    else
          mddev->bitmap_id = ID_BITMAP;

'bitmap_id' is set here.

-        if (!mddev_set_bitmap_ops(mddev))
-            return -EINVAL;
-    }
        if (percpu_ref_init(&mddev->active_io, active_io_release,
-                PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
-        mddev_clear_bitmap_ops(mddev);
+                PERCPU_REF_ALLOW_REINIT, GFP_KERNEL))
          return -ENOMEM;
-    }
        if (percpu_ref_init(&mddev->writes_pending, no_op,
                  PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
-        mddev_clear_bitmap_ops(mddev);
          percpu_ref_exit(&mddev->active_io);
          return -ENOMEM;
      }
@@ -752,6 +763,7 @@ int mddev_init(struct mddev *mddev)
      mddev->resync_min = 0;
      mddev->resync_max = MaxSector;
      mddev->level = LEVEL_NONE;
+    mddev->bitmap_id = ID_BITMAP;

This change is wrong.

Yes, this line from v1 should be removed. Will change this in v3.

Thanks,
Kuai