linux-kernel - Re: [PATCH v3 06/11] md/md-bitmap: delay registration of bitmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d1ee0f33-40e9-5586-36ec-88192747998f@huaweicloud.com>
Date: Sat, 19 Jul 2025 14:49:06 +0800
From: Li Nan <linan666@...weicloud.com>
To: Yu Kuai <yukuai1@...weicloud.com>, corbet@....net, agk@...hat.com,
 snitzer@...nel.org, mpatocka@...hat.com, song@...nel.org,
 yukuai3@...wei.com, hare@...e.de
Cc: linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 dm-devel@...ts.linux.dev, linux-raid@...r.kernel.org, yi.zhang@...wei.com,
 yangerkun@...wei.com, johnny.chenyi@...wei.com
Subject: Re: [PATCH v3 06/11] md/md-bitmap: delay registration of bitmap_ops
 until creating bitmap



在 2025/7/18 17:23, Yu Kuai 写道:
> From: Yu Kuai <yukuai3@...wei.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 <yukuai3@...wei.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.

-- 
Thanks,
Nan