Re: [PATCH 06/23] md/md-bitmap: add a new sysfs api bitmap_type

[Xiao Ni <xni@redhat.com>]

On Sat, May 24, 2025 at 2:18 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@huawei.com>
>
> The api will be used by mdadm to set bitmap_ops while creating new array

Hi Kuai

Maybe you want to say "set bitmap type" here? And can you explain more
here, why does it need this sys file while creating a new array? The
reason I ask is that it doesn't use a sys file when creating an array
with bitmap.

And if it really needs this, can this be gotten by superblock?

Best Regards
Xiao

> or assemble array, prepare to add a new bitmap.
>
> Currently available options are:
>
> cat /sys/block/md0/md/bitmap_type
> none [bitmap]
>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  Documentation/admin-guide/md.rst | 73 ++++++++++++++----------
>  drivers/md/md.c                  | 96 ++++++++++++++++++++++++++++++--
>  drivers/md/md.h                  |  2 +
>  3 files changed, 135 insertions(+), 36 deletions(-)
>
> diff --git a/Documentation/admin-guide/md.rst b/Documentation/admin-guide/md.rst
> index 4ff2cc291d18..356d2a344f08 100644
> --- a/Documentation/admin-guide/md.rst
> +++ b/Documentation/admin-guide/md.rst
> @@ -347,6 +347,49 @@ All md devices contain:
>       active-idle
>           like active, but no writes have been seen for a while (safe_mode_delay).
>
> +  consistency_policy
> +     This indicates how the array maintains consistency in case of unexpected
> +     shutdown. It can be:
> +
> +     none
> +       Array has no redundancy information, e.g. raid0, linear.
> +
> +     resync
> +       Full resync is performed and all redundancy is regenerated when the
> +       array is started after unclean shutdown.
> +
> +     bitmap
> +       Resync assisted by a write-intent bitmap.
> +
> +     journal
> +       For raid4/5/6, journal device is used to log transactions and replay
> +       after unclean shutdown.
> +
> +     ppl
> +       For raid5 only, Partial Parity Log is used to close the write hole and
> +       eliminate resync.
> +
> +     The accepted values when writing to this file are ``ppl`` and ``resync``,
> +     used to enable and disable PPL.
> +
> +  uuid
> +     This indicates the UUID of the array in the following format:
> +     xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> +
> +  bitmap_type
> +     [RW] When read, this file will display the current and available
> +     bitmap for this array. The currently active bitmap will be enclosed
> +     in [] brackets. Writing an bitmap name or ID to this file will switch
> +     control of this array to that new bitmap. Note that writing a new
> +     bitmap for created array is forbidden.
> +
> +     none
> +         No bitmap
> +     bitmap
> +         The default internal bitmap
> +
> +If bitmap_type is bitmap, then the md device will also contain:
> +
>    bitmap/location
>       This indicates where the write-intent bitmap for the array is
>       stored.
> @@ -401,36 +444,6 @@ All md devices contain:
>       once the array becomes non-degraded, and this fact has been
>       recorded in the metadata.
>
> -  consistency_policy
> -     This indicates how the array maintains consistency in case of unexpected
> -     shutdown. It can be:
> -
> -     none
> -       Array has no redundancy information, e.g. raid0, linear.
> -
> -     resync
> -       Full resync is performed and all redundancy is regenerated when the
> -       array is started after unclean shutdown.
> -
> -     bitmap
> -       Resync assisted by a write-intent bitmap.
> -
> -     journal
> -       For raid4/5/6, journal device is used to log transactions and replay
> -       after unclean shutdown.
> -
> -     ppl
> -       For raid5 only, Partial Parity Log is used to close the write hole and
> -       eliminate resync.
> -
> -     The accepted values when writing to this file are ``ppl`` and ``resync``,
> -     used to enable and disable PPL.
> -
> -  uuid
> -     This indicates the UUID of the array in the following format:
> -     xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> -
> -
>  As component devices are added to an md array, they appear in the ``md``
>  directory as new directories named::
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 311e52d5173d..4eb0c6effd5b 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -672,13 +672,18 @@ static void active_io_release(struct percpu_ref *ref)
>
>  static void no_op(struct percpu_ref *r) {}
>
> -static void mddev_set_bitmap_ops(struct mddev *mddev, enum md_submodule_id id)
> +static bool mddev_set_bitmap_ops(struct mddev *mddev)
>  {
>         xa_lock(&md_submodule);
> -       mddev->bitmap_ops = xa_load(&md_submodule, id);
> +       mddev->bitmap_ops = xa_load(&md_submodule, mddev->bitmap_id);
>         xa_unlock(&md_submodule);
> -       if (!mddev->bitmap_ops)
> -               pr_warn_once("md: can't find bitmap id %d\n", id);
> +
> +       if (!mddev->bitmap_ops) {
> +               pr_warn_once("md: can't find bitmap id %d\n", mddev->bitmap_id);
> +               return false;
> +       }
> +
> +       return true;
>  }
>
>  static void mddev_clear_bitmap_ops(struct mddev *mddev)
> @@ -688,8 +693,10 @@ static void mddev_clear_bitmap_ops(struct mddev *mddev)
>
>  int mddev_init(struct mddev *mddev)
>  {
> -       /* TODO: support more versions */
> -       mddev_set_bitmap_ops(mddev, ID_BITMAP);
> +       mddev->bitmap_id = ID_BITMAP;
> +
> +       if (!mddev_set_bitmap_ops(mddev))
> +               return -EINVAL;
>
>         if (percpu_ref_init(&mddev->active_io, active_io_release,
>                             PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
> @@ -4155,6 +4162,82 @@ new_level_store(struct mddev *mddev, const char *buf, size_t len)
>  static struct md_sysfs_entry md_new_level =
>  __ATTR(new_level, 0664, new_level_show, new_level_store);
>
> +static ssize_t
> +bitmap_type_show(struct mddev *mddev, char *page)
> +{
> +       struct md_submodule_head *head;
> +       unsigned long i;
> +       ssize_t len = 0;
> +
> +       if (mddev->bitmap_id == ID_BITMAP_NONE)
> +               len += sprintf(page + len, "[none] ");
> +       else
> +               len += sprintf(page + len, "none ");
> +
> +       xa_lock(&md_submodule);
> +       xa_for_each(&md_submodule, i, head) {
> +               if (head->type != MD_BITMAP)
> +                       continue;
> +
> +               if (mddev->bitmap_id == head->id)
> +                       len += sprintf(page + len, "[%s] ", head->name);
> +               else
> +                       len += sprintf(page + len, "%s ", head->name);
> +       }
> +       xa_unlock(&md_submodule);
> +
> +       len += sprintf(page + len, "\n");
> +       return len;
> +}
> +
> +static ssize_t
> +bitmap_type_store(struct mddev *mddev, const char *buf, size_t len)
> +{
> +       struct md_submodule_head *head;
> +       enum md_submodule_id id;
> +       unsigned long i;
> +       int err;
> +
> +       if (mddev->bitmap_ops)
> +               return -EBUSY;
> +
> +       err = kstrtoint(buf, 10, &id);
> +       if (!err) {
> +               if (id == ID_BITMAP_NONE) {
> +                       mddev->bitmap_id = id;
> +                       return len;
> +               }
> +
> +               xa_lock(&md_submodule);
> +               head = xa_load(&md_submodule, id);
> +               xa_unlock(&md_submodule);
> +
> +               if (head && head->type == MD_BITMAP) {
> +                       mddev->bitmap_id = id;
> +                       return len;
> +               }
> +       }
> +
> +       if (cmd_match(buf, "none")) {
> +               mddev->bitmap_id = ID_BITMAP_NONE;
> +               return len;
> +       }
> +
> +       xa_lock(&md_submodule);
> +       xa_for_each(&md_submodule, i, head) {
> +               if (head->type == MD_BITMAP && cmd_match(buf, head->name)) {
> +                       mddev->bitmap_id = head->id;
> +                       xa_unlock(&md_submodule);
> +                       return len;
> +               }
> +       }
> +       xa_unlock(&md_submodule);
> +       return -ENOENT;
> +}
> +
> +static struct md_sysfs_entry md_bitmap_type =
> +__ATTR(bitmap_type, 0664, bitmap_type_show, bitmap_type_store);
> +
>  static ssize_t
>  layout_show(struct mddev *mddev, char *page)
>  {
> @@ -5719,6 +5802,7 @@ __ATTR(serialize_policy, S_IRUGO | S_IWUSR, serialize_policy_show,
>  static struct attribute *md_default_attrs[] = {
>         &md_level.attr,
>         &md_new_level.attr,
> +       &md_bitmap_type.attr,
>         &md_layout.attr,
>         &md_raid_disks.attr,
>         &md_uuid.attr,
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 13e3f9ce1b79..bf34c0a36551 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -40,6 +40,7 @@ enum md_submodule_id {
>         ID_CLUSTER,
>         ID_BITMAP,
>         ID_LLBITMAP,    /* TODO */
> +       ID_BITMAP_NONE,
>  };
>
>  struct md_submodule_head {
> @@ -565,6 +566,7 @@ struct mddev {
>         struct percpu_ref               writes_pending;
>         int                             sync_checkers;  /* # of threads checking writes_pending */
>
> +       enum md_submodule_id            bitmap_id;
>         void                            *bitmap; /* the bitmap for the device */
>         struct bitmap_operations        *bitmap_ops;
>         struct {
> --
> 2.39.2
>