[PATCH RFC 0/4] md/md-bitmap: support to build md-bitmap as kernel module

[PATCH RFC 0/4] md/md-bitmap: support to build md-bitmap as kernel module

[Yu Kuai]

From: Yu Kuai <yukuai3@huawei.com>

Now that all implementations are internal, it's sensible to build it as
kernel module now.

Currently, if md-bitmap is build as module and the module is not loaded,
creating new array will try to load the module, regardless that bitmap
is used or not. There are still lots of cleanups to prevent
deferencing "mddev->bitmap_ops" for the array without bitmap.

Yu Kuai (4):
  md/md-bitmap: remove the parameter 'init' for bitmap_ops->resize()
  md/md-bitmap: merge md_bitmap_group into bitmap_operations
  md: export some helpers
  md/md-bitmap: support to build md-bitmap as kernel module

 drivers/md/Kconfig      |  15 +++++
 drivers/md/Makefile     |   4 +-
 drivers/md/dm-raid.c    |   2 +-
 drivers/md/md-bitmap.c  |  38 +++++++++++--
 drivers/md/md-bitmap.h  |  13 +++--
 drivers/md/md-cluster.c |   2 +-
 drivers/md/md.c         | 118 +++++++++++++++++++++++++++++++++-------
 drivers/md/md.h         |  12 +++-
 drivers/md/raid1.c      |   2 +-
 drivers/md/raid10.c     |   8 +--
 drivers/md/raid5.c      |   2 +-
 11 files changed, 172 insertions(+), 44 deletions(-)

-- 
2.39.2

[PATCH RFC 1/4] md/md-bitmap: remove the parameter 'init' for bitmap_ops->resize()

[Yu Kuai]

From: Yu Kuai <yukuai3@huawei.com>

It's set to 'false' for all callers, hence it's useless and can be
removed.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/md/dm-raid.c    | 2 +-
 drivers/md/md-bitmap.c  | 5 ++---
 drivers/md/md-bitmap.h  | 3 +--
 drivers/md/md-cluster.c | 2 +-
 drivers/md/raid1.c      | 2 +-
 drivers/md/raid10.c     | 8 ++++----
 drivers/md/raid5.c      | 2 +-
 7 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 1e0d3b9b75d6..0ca73b571c7d 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -4069,7 +4069,7 @@ static int raid_preresume(struct dm_target *ti)
 		int chunksize = to_bytes(rs->requested_bitmap_chunk_sectors) ?: mddev->bitmap_info.chunksize;
 
 		r = mddev->bitmap_ops->resize(mddev, mddev->dev_sectors,
-					      chunksize, false);
+					      chunksize);
 		if (r)
 			DMERR("Failed to resize bitmap");
 	}
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 29da10e6f703..c5e86f9b384f 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -2567,15 +2567,14 @@ static int __bitmap_resize(struct bitmap *bitmap, sector_t blocks,
 	return ret;
 }
 
-static int bitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize,
-			 bool init)
+static int bitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
 {
 	struct bitmap *bitmap = mddev->bitmap;
 
 	if (!bitmap)
 		return 0;
 
-	return __bitmap_resize(bitmap, blocks, chunksize, init);
+	return __bitmap_resize(bitmap, blocks, chunksize, false);
 }
 
 static ssize_t
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index 662e6fc141a7..38425bf4a110 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -73,8 +73,7 @@ struct md_bitmap_stats {
 struct bitmap_operations {
 	bool (*enabled)(struct mddev *mddev);
 	int (*create)(struct mddev *mddev, int slot);
-	int (*resize)(struct mddev *mddev, sector_t blocks, int chunksize,
-		      bool init);
+	int (*resize)(struct mddev *mddev, sector_t blocks, int chunksize);
 
 	int (*load)(struct mddev *mddev);
 	void (*destroy)(struct mddev *mddev);
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 6595f89becdb..67898a02bd3a 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -630,7 +630,7 @@ static int process_recvd_msg(struct mddev *mddev, struct cluster_msg *msg)
 		if (le64_to_cpu(msg->high) != mddev->pers->size(mddev, 0, 0))
 			ret = mddev->bitmap_ops->resize(mddev,
 							le64_to_cpu(msg->high),
-							0, false);
+							0);
 		break;
 	default:
 		ret = -1;
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index cd3e94dceabc..683128122d87 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -3311,7 +3311,7 @@ static int raid1_resize(struct mddev *mddev, sector_t sectors)
 	    mddev->array_sectors > newsize)
 		return -EINVAL;
 
-	ret = mddev->bitmap_ops->resize(mddev, newsize, 0, false);
+	ret = mddev->bitmap_ops->resize(mddev, newsize, 0);
 	if (ret)
 		return ret;
 
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index ff73db2f6c41..5bb957946a09 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4206,7 +4206,7 @@ static int raid10_resize(struct mddev *mddev, sector_t sectors)
 	    mddev->array_sectors > size)
 		return -EINVAL;
 
-	ret = mddev->bitmap_ops->resize(mddev, size, 0, false);
+	ret = mddev->bitmap_ops->resize(mddev, size, 0);
 	if (ret)
 		return ret;
 
@@ -4475,7 +4475,7 @@ static int raid10_start_reshape(struct mddev *mddev)
 		newsize = raid10_size(mddev, 0, conf->geo.raid_disks);
 
 		if (!mddev_is_clustered(mddev)) {
-			ret = mddev->bitmap_ops->resize(mddev, newsize, 0, false);
+			ret = mddev->bitmap_ops->resize(mddev, newsize, 0);
 			if (ret)
 				goto abort;
 			else
@@ -4497,13 +4497,13 @@ static int raid10_start_reshape(struct mddev *mddev)
 			    MD_FEATURE_RESHAPE_ACTIVE)) || (oldsize == newsize))
 			goto out;
 
-		ret = mddev->bitmap_ops->resize(mddev, newsize, 0, false);
+		ret = mddev->bitmap_ops->resize(mddev, newsize, 0);
 		if (ret)
 			goto abort;
 
 		ret = md_cluster_ops->resize_bitmaps(mddev, newsize, oldsize);
 		if (ret) {
-			mddev->bitmap_ops->resize(mddev, oldsize, 0, false);
+			mddev->bitmap_ops->resize(mddev, oldsize, 0);
 			goto abort;
 		}
 	}
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index f5ac81dd21b2..58f71c3e1368 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -8327,7 +8327,7 @@ static int raid5_resize(struct mddev *mddev, sector_t sectors)
 	    mddev->array_sectors > newsize)
 		return -EINVAL;
 
-	ret = mddev->bitmap_ops->resize(mddev, sectors, 0, false);
+	ret = mddev->bitmap_ops->resize(mddev, sectors, 0);
 	if (ret)
 		return ret;
 
-- 
2.39.2

[PATCH RFC 2/4] md/md-bitmap: merge md_bitmap_group into bitmap_operations

[Yu Kuai]

From: Yu Kuai <yukuai3@huawei.com>

Now that all bitmap implementations are internal, it doesn't make sense
to export md_bitmap_group anymore.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/md/md-bitmap.c | 5 ++++-
 drivers/md/md-bitmap.h | 2 ++
 drivers/md/md.c        | 6 +++++-
 drivers/md/md.h        | 1 -
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index c5e86f9b384f..f68eb79e739d 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -2963,7 +2963,8 @@ static struct attribute *md_bitmap_attrs[] = {
 	&max_backlog_used.attr,
 	NULL
 };
-const struct attribute_group md_bitmap_group = {
+
+static struct attribute_group md_bitmap_group = {
 	.name = "bitmap",
 	.attrs = md_bitmap_attrs,
 };
@@ -2996,6 +2997,8 @@ static struct bitmap_operations bitmap_ops = {
 	.copy_from_slot		= bitmap_copy_from_slot,
 	.set_pages		= bitmap_set_pages,
 	.free			= md_bitmap_free,
+
+	.group			= &md_bitmap_group,
 };
 
 void mddev_set_bitmap_ops(struct mddev *mddev)
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index 38425bf4a110..0c19983453c7 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -106,6 +106,8 @@ struct bitmap_operations {
 			      sector_t *hi, bool clear_bits);
 	void (*set_pages)(void *data, unsigned long pages);
 	void (*free)(void *data);
+
+	struct attribute_group *group;
 };
 
 /* the bitmap API */
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 35c2e1e761aa..aad9b87cafa0 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5660,7 +5660,6 @@ static const struct attribute_group md_redundancy_group = {
 
 static const struct attribute_group *md_attr_groups[] = {
 	&md_default_group,
-	&md_bitmap_group,
 	NULL,
 };
 
@@ -5902,6 +5901,11 @@ struct mddev *md_alloc(dev_t dev, char *name)
 		return ERR_PTR(error);
 	}
 
+	if (mddev->bitmap_ops && 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));
+
 	kobject_uevent(&mddev->kobj, KOBJ_ADD);
 	mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd, "array_state");
 	mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd, "level");
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 4ba93af36126..a53af8818923 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -753,7 +753,6 @@ struct md_sysfs_entry {
 	ssize_t (*show)(struct mddev *, char *);
 	ssize_t (*store)(struct mddev *, const char *, size_t);
 };
-extern const struct attribute_group md_bitmap_group;
 
 static inline struct kernfs_node *sysfs_get_dirent_safe(struct kernfs_node *sd, char *name)
 {
-- 
2.39.2

[PATCH RFC 3/4] md: export some helpers

[Yu Kuai]

From: Yu Kuai <yukuai3@huawei.com>

These helpers are used inside md-bitmap.c, prepare to build it as kernel
module first, and perhaps mark it as deprecated after new lockless
bitmap is introduced.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/md/md.c | 15 ++++++---------
 drivers/md/md.h | 10 +++++++++-
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index aad9b87cafa0..d16a3d0f2b90 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -252,6 +252,7 @@ void mddev_create_serial_pool(struct mddev *mddev, struct md_rdev *rdev)
 		}
 	}
 }
+EXPORT_SYMBOL_GPL(mddev_create_serial_pool);
 
 /*
  * Free resource from rdev(s), and destroy serial_info_pool under conditions:
@@ -291,6 +292,7 @@ void mddev_destroy_serial_pool(struct mddev *mddev, struct md_rdev *rdev)
 		}
 	}
 }
+EXPORT_SYMBOL_GPL(mddev_destroy_serial_pool);
 
 static struct ctl_table_header *raid_table_header;
 
@@ -972,15 +974,7 @@ void md_super_write(struct mddev *mddev, struct md_rdev *rdev,
 	atomic_inc(&mddev->pending_writes);
 	submit_bio(bio);
 }
-
-int md_super_wait(struct mddev *mddev)
-{
-	/* wait for all superblock writes that were scheduled to complete */
-	wait_event(mddev->sb_wait, atomic_read(&mddev->pending_writes)==0);
-	if (test_and_clear_bit(MD_SB_NEED_REWRITE, &mddev->sb_flags))
-		return -EAGAIN;
-	return 0;
-}
+EXPORT_SYMBOL_GPL(md_super_write);
 
 int sync_page_io(struct md_rdev *rdev, sector_t sector, int size,
 		 struct page *page, blk_opf_t opf, bool metadata_op)
@@ -3793,6 +3787,7 @@ int strict_strtoul_scaled(const char *cp, unsigned long *res, int scale)
 	*res = result * int_pow(10, scale - decimals);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(strict_strtoul_scaled);
 
 static ssize_t
 safe_delay_show(struct mddev *mddev, char *page)
@@ -8556,6 +8551,7 @@ int md_setup_cluster(struct mddev *mddev, int nodes)
 		mddev->safemode_delay = 0;
 	return ret;
 }
+EXPORT_SYMBOL_GPL(md_setup_cluster);
 
 void md_cluster_stop(struct mddev *mddev)
 {
@@ -8564,6 +8560,7 @@ void md_cluster_stop(struct mddev *mddev)
 	md_cluster_ops->leave(mddev);
 	module_put(md_cluster_mod);
 }
+EXPORT_SYMBOL_GPL(md_cluster_stop);
 
 static int is_mddev_idle(struct mddev *mddev, int init)
 {
diff --git a/drivers/md/md.h b/drivers/md/md.h
index a53af8818923..5eaac1d84523 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -869,7 +869,6 @@ void md_free_cloned_bio(struct bio *bio);
 extern bool __must_check md_flush_request(struct mddev *mddev, struct bio *bio);
 extern void md_super_write(struct mddev *mddev, struct md_rdev *rdev,
 			   sector_t sector, int size, struct page *page);
-extern int md_super_wait(struct mddev *mddev);
 extern int sync_page_io(struct md_rdev *rdev, sector_t sector, int size,
 		struct page *page, blk_opf_t opf, bool metadata_op);
 extern void md_do_sync(struct md_thread *thread);
@@ -994,6 +993,15 @@ static inline bool mddev_is_dm(struct mddev *mddev)
 	return !mddev->gendisk;
 }
 
+static inline int md_super_wait(struct mddev *mddev)
+{
+	/* wait for all superblock writes that were scheduled to complete */
+	wait_event(mddev->sb_wait, atomic_read(&mddev->pending_writes) == 0);
+	if (test_and_clear_bit(MD_SB_NEED_REWRITE, &mddev->sb_flags))
+		return -EAGAIN;
+	return 0;
+}
+
 static inline void mddev_trace_remap(struct mddev *mddev, struct bio *bio,
 		sector_t sector)
 {
-- 
2.39.2

[PATCH RFC 4/4] md/md-bitmap: support to build md-bitmap as kernel module

[Yu Kuai]

From: Yu Kuai <yukuai3@huawei.com>

Now that all implementations are internal, it's sensible to build it as
kernel module now.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/md/Kconfig     | 15 +++++++
 drivers/md/Makefile    |  4 +-
 drivers/md/md-bitmap.c | 28 +++++++++++-
 drivers/md/md-bitmap.h |  8 +++-
 drivers/md/md.c        | 97 +++++++++++++++++++++++++++++++++++++-----
 drivers/md/md.h        |  1 -
 6 files changed, 135 insertions(+), 18 deletions(-)

diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 1e9db8e4acdf..452d7292b617 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -37,6 +37,21 @@ config BLK_DEV_MD
 
 	  If unsure, say N.
 
+config MD_BITMAP
+	tristate "RAID bitmap support"
+	default y
+	depends on BLK_DEV_MD
+	help
+	  If you say Y here, support for the write intent bitmap will be
+	  enabled. The bitmap will be used to record the data regions that need
+	  to be resynced after a power failure, preventing a full disk resync.
+	  The bitmap size ranges from 4K to 132K, depending on the array size.
+	  Each bit corresponds to 2 bytes of data and is managed in
+	  self-maintained memory. All bits are protected by a disk-level
+	  spinlock.
+
+	  If unsure, say Y.
+
 config MD_AUTODETECT
 	bool "Autodetect RAID arrays during kernel boot"
 	depends on BLK_DEV_MD=y
diff --git a/drivers/md/Makefile b/drivers/md/Makefile
index 476a214e4bdc..387670f766b7 100644
--- a/drivers/md/Makefile
+++ b/drivers/md/Makefile
@@ -27,14 +27,14 @@ dm-clone-y	+= dm-clone-target.o dm-clone-metadata.o
 dm-verity-y	+= dm-verity-target.o
 dm-zoned-y	+= dm-zoned-target.o dm-zoned-metadata.o dm-zoned-reclaim.o
 
-md-mod-y	+= md.o md-bitmap.o
+md-mod-y	+= md.o
 raid456-y	+= raid5.o raid5-cache.o raid5-ppl.o
 
 # Note: link order is important.  All raid personalities
 # and must come before md.o, as they each initialise
 # themselves, and md.o may use the personalities when it
 # auto-initialised.
-
+obj-$(CONFIG_MD_BITMAP)		+= md-bitmap.o
 obj-$(CONFIG_MD_RAID0)		+= raid0.o
 obj-$(CONFIG_MD_RAID1)		+= raid1.o
 obj-$(CONFIG_MD_RAID10)		+= raid10.o
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index f68eb79e739d..148a479d32c0 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -212,6 +212,8 @@ struct bitmap {
 	int cluster_slot;
 };
 
+static struct workqueue_struct *md_bitmap_wq;
+
 static int __bitmap_resize(struct bitmap *bitmap, sector_t blocks,
 			   int chunksize, bool init);
 
@@ -2970,6 +2972,9 @@ static struct attribute_group md_bitmap_group = {
 };
 
 static struct bitmap_operations bitmap_ops = {
+	.version		= 1,
+	.owner			= THIS_MODULE,
+
 	.enabled		= bitmap_enabled,
 	.create			= bitmap_create,
 	.resize			= bitmap_resize,
@@ -3001,7 +3006,26 @@ static struct bitmap_operations bitmap_ops = {
 	.group			= &md_bitmap_group,
 };
 
-void mddev_set_bitmap_ops(struct mddev *mddev)
+static int __init bitmap_init(void)
+{
+	md_bitmap_wq = alloc_workqueue("md_bitmap", WQ_MEM_RECLAIM | WQ_UNBOUND,
+				       0);
+	if (!md_bitmap_wq)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&bitmap_ops.list);
+	register_md_bitmap(&bitmap_ops);
+	return 0;
+}
+
+static void __exit bitmap_exit(void)
 {
-	mddev->bitmap_ops = &bitmap_ops;
+	destroy_workqueue(md_bitmap_wq);
+	unregister_md_bitmap(&bitmap_ops);
 }
+
+module_init(bitmap_init);
+module_exit(bitmap_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Bitmap for MD");
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index 0c19983453c7..9d1bf3c43125 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -71,6 +71,10 @@ struct md_bitmap_stats {
 };
 
 struct bitmap_operations {
+	int version;
+	struct module *owner;
+	struct list_head list;
+
 	bool (*enabled)(struct mddev *mddev);
 	int (*create)(struct mddev *mddev, int slot);
 	int (*resize)(struct mddev *mddev, sector_t blocks, int chunksize);
@@ -110,7 +114,7 @@ struct bitmap_operations {
 	struct attribute_group *group;
 };
 
-/* the bitmap API */
-void mddev_set_bitmap_ops(struct mddev *mddev);
+void register_md_bitmap(struct bitmap_operations *op);
+void unregister_md_bitmap(struct bitmap_operations *op);
 
 #endif
diff --git a/drivers/md/md.c b/drivers/md/md.c
index d16a3d0f2b90..09fac65b83b8 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -83,6 +83,9 @@ static const char *action_name[NR_SYNC_ACTIONS] = {
 static LIST_HEAD(pers_list);
 static DEFINE_SPINLOCK(pers_lock);
 
+static LIST_HEAD(bitmap_list);
+static DEFINE_SPINLOCK(bitmap_lock);
+
 static const struct kobj_type md_ktype;
 
 const struct md_cluster_operations *md_cluster_ops;
@@ -100,7 +103,6 @@ static struct workqueue_struct *md_wq;
  * workqueue whith reconfig_mutex grabbed.
  */
 static struct workqueue_struct *md_misc_wq;
-struct workqueue_struct *md_bitmap_wq;
 
 static int remove_and_add_spares(struct mddev *mddev,
 				 struct md_rdev *this);
@@ -630,15 +632,96 @@ static void active_io_release(struct percpu_ref *ref)
 
 static void no_op(struct percpu_ref *r) {}
 
+void register_md_bitmap(struct bitmap_operations *op)
+{
+	pr_info("md: bitmap version %d registered\n", op->version);
+
+	spin_lock(&bitmap_lock);
+	list_add_tail(&op->list, &bitmap_list);
+	spin_unlock(&bitmap_lock);
+}
+EXPORT_SYMBOL_GPL(register_md_bitmap);
+
+void unregister_md_bitmap(struct bitmap_operations *op)
+{
+	pr_info("md: bitmap version %d unregistered\n", op->version);
+
+	spin_lock(&bitmap_lock);
+	list_del_init(&op->list);
+	spin_unlock(&bitmap_lock);
+}
+EXPORT_SYMBOL_GPL(unregister_md_bitmap);
+
+static struct bitmap_operations *__find_bitmap(int version)
+{
+	struct bitmap_operations *op;
+
+	list_for_each_entry(op, &bitmap_list, list)
+		if (op->version == version) {
+			if (try_module_get(op->owner))
+				return op;
+			else
+				return NULL;
+		}
+
+	return NULL;
+}
+
+static struct bitmap_operations *find_bitmap(int version)
+{
+	struct bitmap_operations *op = NULL;
+
+	spin_lock(&bitmap_lock);
+	op = __find_bitmap(version);
+	spin_unlock(&bitmap_lock);
+
+	if (op)
+		return op;
+
+	if (request_module("md-bitmap") != 0)
+		return NULL;
+
+	spin_lock(&bitmap_lock);
+	op = __find_bitmap(version);
+	spin_unlock(&bitmap_lock);
+
+	return op;
+}
+
+/* TODO: support more versions */
+static int mddev_set_bitmap_ops(struct mddev *mddev)
+{
+	struct bitmap_operations *op = find_bitmap(1);
+
+	if (!op)
+		return -ENODEV;
+
+	mddev->bitmap_ops = op;
+	return 0;
+}
+
+static void mddev_clear_bitmap_ops(struct mddev *mddev)
+{
+	module_put(mddev->bitmap_ops->owner);
+	mddev->bitmap_ops = NULL;
+}
+
 int mddev_init(struct mddev *mddev)
 {
+	int ret = mddev_set_bitmap_ops(mddev);
+
+	if (ret)
+		return ret;
 
 	if (percpu_ref_init(&mddev->active_io, active_io_release,
-			    PERCPU_REF_ALLOW_REINIT, GFP_KERNEL))
+			    PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
+		mddev_clear_bitmap_ops(mddev);
 		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;
 	}
@@ -666,7 +749,6 @@ int mddev_init(struct mddev *mddev)
 	mddev->resync_min = 0;
 	mddev->resync_max = MaxSector;
 	mddev->level = LEVEL_NONE;
-	mddev_set_bitmap_ops(mddev);
 
 	INIT_WORK(&mddev->sync_work, md_start_sync);
 	INIT_WORK(&mddev->del_work, mddev_delayed_delete);
@@ -677,6 +759,7 @@ EXPORT_SYMBOL_GPL(mddev_init);
 
 void mddev_destroy(struct mddev *mddev)
 {
+	mddev_clear_bitmap_ops(mddev);
 	percpu_ref_exit(&mddev->active_io);
 	percpu_ref_exit(&mddev->writes_pending);
 }
@@ -9898,11 +9981,6 @@ static int __init md_init(void)
 	if (!md_misc_wq)
 		goto err_misc_wq;
 
-	md_bitmap_wq = alloc_workqueue("md_bitmap", WQ_MEM_RECLAIM | WQ_UNBOUND,
-				       0);
-	if (!md_bitmap_wq)
-		goto err_bitmap_wq;
-
 	ret = __register_blkdev(MD_MAJOR, "md", md_probe);
 	if (ret < 0)
 		goto err_md;
@@ -9921,8 +9999,6 @@ static int __init md_init(void)
 err_mdp:
 	unregister_blkdev(MD_MAJOR, "md");
 err_md:
-	destroy_workqueue(md_bitmap_wq);
-err_bitmap_wq:
 	destroy_workqueue(md_misc_wq);
 err_misc_wq:
 	destroy_workqueue(md_wq);
@@ -10229,7 +10305,6 @@ static __exit void md_exit(void)
 	spin_unlock(&all_mddevs_lock);
 
 	destroy_workqueue(md_misc_wq);
-	destroy_workqueue(md_bitmap_wq);
 	destroy_workqueue(md_wq);
 }
 
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 5eaac1d84523..28347fb3af18 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -972,7 +972,6 @@ struct mdu_array_info_s;
 struct mdu_disk_info_s;
 
 extern int mdp_major;
-extern struct workqueue_struct *md_bitmap_wq;
 void md_autostart_arrays(int part);
 int md_set_array_info(struct mddev *mddev, struct mdu_array_info_s *info);
 int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info);
-- 
2.39.2

Re: [PATCH RFC 4/4] md/md-bitmap: support to build md-bitmap as kernel module

[Mariusz Tkaczyk]

On Thu, 24 Oct 2024 21:13:25 +0800
Yu Kuai <yukuai1@huaweicloud.com> wrote:

> From: Yu Kuai <yukuai3@huawei.com>
> 
> Now that all implementations are internal, it's sensible to build it as
> kernel module now.
> 
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  drivers/md/Kconfig     | 15 +++++++
>  drivers/md/Makefile    |  4 +-
>  drivers/md/md-bitmap.c | 28 +++++++++++-
>  drivers/md/md-bitmap.h |  8 +++-
>  drivers/md/md.c        | 97 +++++++++++++++++++++++++++++++++++++-----
>  drivers/md/md.h        |  1 -
>  6 files changed, 135 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index 1e9db8e4acdf..452d7292b617 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -37,6 +37,21 @@ config BLK_DEV_MD
>  
>  	  If unsure, say N.
>  
> +config MD_BITMAP
> +	tristate "RAID bitmap support"

Maybe "MD RAID bitmap support"? From kernel config GUI description is
presented, it seems better to highlight that it is MD internal.

> +	default y
> +	depends on BLK_DEV_MD
> +	help
> +	  If you say Y here, support for the write intent bitmap will be
> +	  enabled. The bitmap will be used to record the data regions that

"If you say Y here" is confusing because it could be "Y" or "M".
Maybe:

"MD write intent bitmap support. The bitmap can be used to
optimize resync speed after power failure, limiting it to recorded dirty
sectors in bitmap. This feature can be added to existing MD array or MD array
can be created with bitmap via mdadm(8).
If unsure, say M."

"M" because MD is in real life is often compiled as module- shouldn't it be
always same?
We should not allow "Y" if MD is "M", perhaps we need to do more in Kconfig to
prevent this?

It is always good to refer how it can be configured and who uses it in
userspace. You can eventually add that it is MD internal module if you don't
see it enough clear.

On other think- what about recovery? Isn't it used to improve recovery speed? We
have checkpointing for recovery. If I remember correctly it is always 7
checkpoints for the array (at least for IMSM). Isn't bitmap used to improve
this? If yes, please add it here.

The part below should go to the Documentation because these are implementation
details that are not needed to take a decision about enabling/disabling the
module.
Please consider adding Documentation entry.

For the code- lgtm.

Great job Kuai! I love your contribution. 
Thanks,
Mariusz

> need
> +	  to be resynced after a power failure, preventing a full disk
> resync.
> +	  The bitmap size ranges from 4K to 132K, depending on the array
> size.
> +	  Each bit corresponds to 2 bytes of data and is managed in
> +	  self-maintained memory. All bits are protected by a disk-level
> +	  spinlock.
> +
> +	  If unsure, say Y.
> +
>  config MD_AUTODETECT
>  	bool "Autodetect RAID arrays during kernel boot"
>  	depends on BLK_DEV_MD=y
> diff --git a/drivers/md/Makefile b/drivers/md/Makefile
> index 476a214e4bdc..387670f766b7 100644
> --- a/drivers/md/Makefile
> +++ b/drivers/md/Makefile
> @@ -27,14 +27,14 @@ dm-clone-y	+= dm-clone-target.o dm-clone-metadata.o
>  dm-verity-y	+= dm-verity-target.o
>  dm-zoned-y	+= dm-zoned-target.o dm-zoned-metadata.o dm-zoned-reclaim.o
>  
> -md-mod-y	+= md.o md-bitmap.o
> +md-mod-y	+= md.o
>  raid456-y	+= raid5.o raid5-cache.o raid5-ppl.o
>  
>  # Note: link order is important.  All raid personalities
>  # and must come before md.o, as they each initialise
>  # themselves, and md.o may use the personalities when it
>  # auto-initialised.
> -
> +obj-$(CONFIG_MD_BITMAP)		+= md-bitmap.o
>  obj-$(CONFIG_MD_RAID0)		+= raid0.o
>  obj-$(CONFIG_MD_RAID1)		+= raid1.o
>  obj-$(CONFIG_MD_RAID10)		+= raid10.o
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index f68eb79e739d..148a479d32c0 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -212,6 +212,8 @@ struct bitmap {
>  	int cluster_slot;
>  };
>  
> +static struct workqueue_struct *md_bitmap_wq;
> +
>  static int __bitmap_resize(struct bitmap *bitmap, sector_t blocks,
>  			   int chunksize, bool init);
>  
> @@ -2970,6 +2972,9 @@ static struct attribute_group md_bitmap_group = {
>  };
>  
>  static struct bitmap_operations bitmap_ops = {
> +	.version		= 1,
> +	.owner			= THIS_MODULE,
> +
>  	.enabled		= bitmap_enabled,
>  	.create			= bitmap_create,
>  	.resize			= bitmap_resize,
> @@ -3001,7 +3006,26 @@ static struct bitmap_operations bitmap_ops = {
>  	.group			= &md_bitmap_group,
>  };
>  
> -void mddev_set_bitmap_ops(struct mddev *mddev)
> +static int __init bitmap_init(void)
> +{
> +	md_bitmap_wq = alloc_workqueue("md_bitmap", WQ_MEM_RECLAIM |
> WQ_UNBOUND,
> +				       0);
> +	if (!md_bitmap_wq)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&bitmap_ops.list);
> +	register_md_bitmap(&bitmap_ops);
> +	return 0;
> +}
> +
> +static void __exit bitmap_exit(void)
>  {
> -	mddev->bitmap_ops = &bitmap_ops;
> +	destroy_workqueue(md_bitmap_wq);
> +	unregister_md_bitmap(&bitmap_ops);
>  }
> +
> +module_init(bitmap_init);
> +module_exit(bitmap_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Bitmap for MD");
> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
> index 0c19983453c7..9d1bf3c43125 100644
> --- a/drivers/md/md-bitmap.h
> +++ b/drivers/md/md-bitmap.h
> @@ -71,6 +71,10 @@ struct md_bitmap_stats {
>  };
>  
>  struct bitmap_operations {
> +	int version;
> +	struct module *owner;
> +	struct list_head list;
> +
>  	bool (*enabled)(struct mddev *mddev);
>  	int (*create)(struct mddev *mddev, int slot);
>  	int (*resize)(struct mddev *mddev, sector_t blocks, int chunksize);
> @@ -110,7 +114,7 @@ struct bitmap_operations {
>  	struct attribute_group *group;
>  };
>  
> -/* the bitmap API */
> -void mddev_set_bitmap_ops(struct mddev *mddev);
> +void register_md_bitmap(struct bitmap_operations *op);
> +void unregister_md_bitmap(struct bitmap_operations *op);
>  
>  #endif
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d16a3d0f2b90..09fac65b83b8 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -83,6 +83,9 @@ static const char *action_name[NR_SYNC_ACTIONS] = {
>  static LIST_HEAD(pers_list);
>  static DEFINE_SPINLOCK(pers_lock);
>  
> +static LIST_HEAD(bitmap_list);
> +static DEFINE_SPINLOCK(bitmap_lock);
> +
>  static const struct kobj_type md_ktype;
>  
>  const struct md_cluster_operations *md_cluster_ops;
> @@ -100,7 +103,6 @@ static struct workqueue_struct *md_wq;
>   * workqueue whith reconfig_mutex grabbed.
>   */
>  static struct workqueue_struct *md_misc_wq;
> -struct workqueue_struct *md_bitmap_wq;
>  
>  static int remove_and_add_spares(struct mddev *mddev,
>  				 struct md_rdev *this);
> @@ -630,15 +632,96 @@ static void active_io_release(struct percpu_ref *ref)
>  
>  static void no_op(struct percpu_ref *r) {}
>  
> +void register_md_bitmap(struct bitmap_operations *op)
> +{
> +	pr_info("md: bitmap version %d registered\n", op->version);
> +
> +	spin_lock(&bitmap_lock);
> +	list_add_tail(&op->list, &bitmap_list);
> +	spin_unlock(&bitmap_lock);
> +}
> +EXPORT_SYMBOL_GPL(register_md_bitmap);
> +
> +void unregister_md_bitmap(struct bitmap_operations *op)
> +{
> +	pr_info("md: bitmap version %d unregistered\n", op->version);
> +
> +	spin_lock(&bitmap_lock);
> +	list_del_init(&op->list);
> +	spin_unlock(&bitmap_lock);
> +}
> +EXPORT_SYMBOL_GPL(unregister_md_bitmap);
> +
> +static struct bitmap_operations *__find_bitmap(int version)
> +{
> +	struct bitmap_operations *op;
> +
> +	list_for_each_entry(op, &bitmap_list, list)
> +		if (op->version == version) {
> +			if (try_module_get(op->owner))
> +				return op;
> +			else
> +				return NULL;
> +		}
> +
> +	return NULL;
> +}
> +
> +static struct bitmap_operations *find_bitmap(int version)
> +{
> +	struct bitmap_operations *op = NULL;
> +
> +	spin_lock(&bitmap_lock);
> +	op = __find_bitmap(version);
> +	spin_unlock(&bitmap_lock);
> +
> +	if (op)
> +		return op;
> +
> +	if (request_module("md-bitmap") != 0)
> +		return NULL;
> +
> +	spin_lock(&bitmap_lock);
> +	op = __find_bitmap(version);
> +	spin_unlock(&bitmap_lock);
> +
> +	return op;
> +}
> +
> +/* TODO: support more versions */
> +static int mddev_set_bitmap_ops(struct mddev *mddev)
> +{
> +	struct bitmap_operations *op = find_bitmap(1);
> +
> +	if (!op)
> +		return -ENODEV;
> +
> +	mddev->bitmap_ops = op;
> +	return 0;
> +}
> +
> +static void mddev_clear_bitmap_ops(struct mddev *mddev)
> +{
> +	module_put(mddev->bitmap_ops->owner);
> +	mddev->bitmap_ops = NULL;
> +}
> +
>  int mddev_init(struct mddev *mddev)
>  {
> +	int ret = mddev_set_bitmap_ops(mddev);
> +
> +	if (ret)
> +		return ret;
>  
>  	if (percpu_ref_init(&mddev->active_io, active_io_release,
> -			    PERCPU_REF_ALLOW_REINIT, GFP_KERNEL))
> +			    PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
> +		mddev_clear_bitmap_ops(mddev);
>  		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;
>  	}
> @@ -666,7 +749,6 @@ int mddev_init(struct mddev *mddev)
>  	mddev->resync_min = 0;
>  	mddev->resync_max = MaxSector;
>  	mddev->level = LEVEL_NONE;
> -	mddev_set_bitmap_ops(mddev);
>  
>  	INIT_WORK(&mddev->sync_work, md_start_sync);
>  	INIT_WORK(&mddev->del_work, mddev_delayed_delete);
> @@ -677,6 +759,7 @@ EXPORT_SYMBOL_GPL(mddev_init);
>  
>  void mddev_destroy(struct mddev *mddev)
>  {
> +	mddev_clear_bitmap_ops(mddev);
>  	percpu_ref_exit(&mddev->active_io);
>  	percpu_ref_exit(&mddev->writes_pending);
>  }
> @@ -9898,11 +9981,6 @@ static int __init md_init(void)
>  	if (!md_misc_wq)
>  		goto err_misc_wq;
>  
> -	md_bitmap_wq = alloc_workqueue("md_bitmap", WQ_MEM_RECLAIM |
> WQ_UNBOUND,
> -				       0);
> -	if (!md_bitmap_wq)
> -		goto err_bitmap_wq;
> -
>  	ret = __register_blkdev(MD_MAJOR, "md", md_probe);
>  	if (ret < 0)
>  		goto err_md;
> @@ -9921,8 +9999,6 @@ static int __init md_init(void)
>  err_mdp:
>  	unregister_blkdev(MD_MAJOR, "md");
>  err_md:
> -	destroy_workqueue(md_bitmap_wq);
> -err_bitmap_wq:
>  	destroy_workqueue(md_misc_wq);
>  err_misc_wq:
>  	destroy_workqueue(md_wq);
> @@ -10229,7 +10305,6 @@ static __exit void md_exit(void)
>  	spin_unlock(&all_mddevs_lock);
>  
>  	destroy_workqueue(md_misc_wq);
> -	destroy_workqueue(md_bitmap_wq);
>  	destroy_workqueue(md_wq);
>  }
>  
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 5eaac1d84523..28347fb3af18 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -972,7 +972,6 @@ struct mdu_array_info_s;
>  struct mdu_disk_info_s;
>  
>  extern int mdp_major;
> -extern struct workqueue_struct *md_bitmap_wq;
>  void md_autostart_arrays(int part);
>  int md_set_array_info(struct mddev *mddev, struct mdu_array_info_s *info);
>  int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info);

Re: [PATCH RFC 4/4] md/md-bitmap: support to build md-bitmap as kernel module

[Yu Kuai]

Hi,

在 2024/10/25 15:02, Mariusz Tkaczyk 写道:
> On Thu, 24 Oct 2024 21:13:25 +0800
> Yu Kuai <yukuai1@huaweicloud.com> wrote:
> 
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> Now that all implementations are internal, it's sensible to build it as
>> kernel module now.
>>
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> ---
>>   drivers/md/Kconfig     | 15 +++++++
>>   drivers/md/Makefile    |  4 +-
>>   drivers/md/md-bitmap.c | 28 +++++++++++-
>>   drivers/md/md-bitmap.h |  8 +++-
>>   drivers/md/md.c        | 97 +++++++++++++++++++++++++++++++++++++-----
>>   drivers/md/md.h        |  1 -
>>   6 files changed, 135 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
>> index 1e9db8e4acdf..452d7292b617 100644
>> --- a/drivers/md/Kconfig
>> +++ b/drivers/md/Kconfig
>> @@ -37,6 +37,21 @@ config BLK_DEV_MD
>>   
>>   	  If unsure, say N.
>>   
>> +config MD_BITMAP
>> +	tristate "RAID bitmap support"
> 
> Maybe "MD RAID bitmap support"? From kernel config GUI description is
> presented, it seems better to highlight that it is MD internal.
> 
>> +	default y
>> +	depends on BLK_DEV_MD
>> +	help
>> +	  If you say Y here, support for the write intent bitmap will be
>> +	  enabled. The bitmap will be used to record the data regions that
> 
> "If you say Y here" is confusing because it could be "Y" or "M".
> Maybe:

Yeah, I just copy this from other configs.
> 
> "MD write intent bitmap support. The bitmap can be used to
> optimize resync speed after power failure, limiting it to recorded dirty
> sectors in bitmap. This feature can be added to existing MD array or MD array
> can be created with bitmap via mdadm(8).
> If unsure, say M."
> 
> "M" because MD is in real life is often compiled as module- shouldn't it be
> always same?
> We should not allow "Y" if MD is "M", perhaps we need to do more in Kconfig to
> prevent this?

Kconfig already do this, if the depends config is 'M', this config can't
be set to 'Y'.

> 
> It is always good to refer how it can be configured and who uses it in
> userspace. You can eventually add that it is MD internal module if you don't
> see it enough clear.
> 
> On other think- what about recovery? Isn't it used to improve recovery speed? We
> have checkpointing for recovery. If I remember correctly it is always 7
> checkpoints for the array (at least for IMSM). Isn't bitmap used to improve
> this? If yes, please add it here.

For recovery means add a new disk to the array? If so, bitmap is useless
in this case, if you means readd a hot removed disk, then yes, however,
I think this is actually 'resync'.

Anyway, I'll add both power failure and readd a disk in the next
version.
> 
> The part below should go to the Documentation because these are implementation
> details that are not needed to take a decision about enabling/disabling the
> module.
> Please consider adding Documentation entry.

I probably will delay te Documentation untill the new bitmap, I'll just
remove the below.

Thanks,
Kuai

> 
> For the code- lgtm.
> 
> Great job Kuai! I love your contribution.
> Thanks,
> Mariusz
> 
>> need
>> +	  to be resynced after a power failure, preventing a full disk
>> resync.
>> +	  The bitmap size ranges from 4K to 132K, depending on the array
>> size.
>> +	  Each bit corresponds to 2 bytes of data and is managed in
>> +	  self-maintained memory. All bits are protected by a disk-level
>> +	  spinlock.
>> +
>> +	  If unsure, say Y.
>> +
>>   config MD_AUTODETECT
>>   	bool "Autodetect RAID arrays during kernel boot"
>>   	depends on BLK_DEV_MD=y
>> diff --git a/drivers/md/Makefile b/drivers/md/Makefile
>> index 476a214e4bdc..387670f766b7 100644
>> --- a/drivers/md/Makefile
>> +++ b/drivers/md/Makefile
>> @@ -27,14 +27,14 @@ dm-clone-y	+= dm-clone-target.o dm-clone-metadata.o
>>   dm-verity-y	+= dm-verity-target.o
>>   dm-zoned-y	+= dm-zoned-target.o dm-zoned-metadata.o dm-zoned-reclaim.o
>>   
>> -md-mod-y	+= md.o md-bitmap.o
>> +md-mod-y	+= md.o
>>   raid456-y	+= raid5.o raid5-cache.o raid5-ppl.o
>>   
>>   # Note: link order is important.  All raid personalities
>>   # and must come before md.o, as they each initialise
>>   # themselves, and md.o may use the personalities when it
>>   # auto-initialised.
>> -
>> +obj-$(CONFIG_MD_BITMAP)		+= md-bitmap.o
>>   obj-$(CONFIG_MD_RAID0)		+= raid0.o
>>   obj-$(CONFIG_MD_RAID1)		+= raid1.o
>>   obj-$(CONFIG_MD_RAID10)		+= raid10.o
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index f68eb79e739d..148a479d32c0 100644
>> --- a/drivers/md/md-bitmap.c
>> +++ b/drivers/md/md-bitmap.c
>> @@ -212,6 +212,8 @@ struct bitmap {
>>   	int cluster_slot;
>>   };
>>   
>> +static struct workqueue_struct *md_bitmap_wq;
>> +
>>   static int __bitmap_resize(struct bitmap *bitmap, sector_t blocks,
>>   			   int chunksize, bool init);
>>   
>> @@ -2970,6 +2972,9 @@ static struct attribute_group md_bitmap_group = {
>>   };
>>   
>>   static struct bitmap_operations bitmap_ops = {
>> +	.version		= 1,
>> +	.owner			= THIS_MODULE,
>> +
>>   	.enabled		= bitmap_enabled,
>>   	.create			= bitmap_create,
>>   	.resize			= bitmap_resize,
>> @@ -3001,7 +3006,26 @@ static struct bitmap_operations bitmap_ops = {
>>   	.group			= &md_bitmap_group,
>>   };
>>   
>> -void mddev_set_bitmap_ops(struct mddev *mddev)
>> +static int __init bitmap_init(void)
>> +{
>> +	md_bitmap_wq = alloc_workqueue("md_bitmap", WQ_MEM_RECLAIM |
>> WQ_UNBOUND,
>> +				       0);
>> +	if (!md_bitmap_wq)
>> +		return -ENOMEM;
>> +
>> +	INIT_LIST_HEAD(&bitmap_ops.list);
>> +	register_md_bitmap(&bitmap_ops);
>> +	return 0;
>> +}
>> +
>> +static void __exit bitmap_exit(void)
>>   {
>> -	mddev->bitmap_ops = &bitmap_ops;
>> +	destroy_workqueue(md_bitmap_wq);
>> +	unregister_md_bitmap(&bitmap_ops);
>>   }
>> +
>> +module_init(bitmap_init);
>> +module_exit(bitmap_exit);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("Bitmap for MD");
>> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
>> index 0c19983453c7..9d1bf3c43125 100644
>> --- a/drivers/md/md-bitmap.h
>> +++ b/drivers/md/md-bitmap.h
>> @@ -71,6 +71,10 @@ struct md_bitmap_stats {
>>   };
>>   
>>   struct bitmap_operations {
>> +	int version;
>> +	struct module *owner;
>> +	struct list_head list;
>> +
>>   	bool (*enabled)(struct mddev *mddev);
>>   	int (*create)(struct mddev *mddev, int slot);
>>   	int (*resize)(struct mddev *mddev, sector_t blocks, int chunksize);
>> @@ -110,7 +114,7 @@ struct bitmap_operations {
>>   	struct attribute_group *group;
>>   };
>>   
>> -/* the bitmap API */
>> -void mddev_set_bitmap_ops(struct mddev *mddev);
>> +void register_md_bitmap(struct bitmap_operations *op);
>> +void unregister_md_bitmap(struct bitmap_operations *op);
>>   
>>   #endif
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index d16a3d0f2b90..09fac65b83b8 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -83,6 +83,9 @@ static const char *action_name[NR_SYNC_ACTIONS] = {
>>   static LIST_HEAD(pers_list);
>>   static DEFINE_SPINLOCK(pers_lock);
>>   
>> +static LIST_HEAD(bitmap_list);
>> +static DEFINE_SPINLOCK(bitmap_lock);
>> +
>>   static const struct kobj_type md_ktype;
>>   
>>   const struct md_cluster_operations *md_cluster_ops;
>> @@ -100,7 +103,6 @@ static struct workqueue_struct *md_wq;
>>    * workqueue whith reconfig_mutex grabbed.
>>    */
>>   static struct workqueue_struct *md_misc_wq;
>> -struct workqueue_struct *md_bitmap_wq;
>>   
>>   static int remove_and_add_spares(struct mddev *mddev,
>>   				 struct md_rdev *this);
>> @@ -630,15 +632,96 @@ static void active_io_release(struct percpu_ref *ref)
>>   
>>   static void no_op(struct percpu_ref *r) {}
>>   
>> +void register_md_bitmap(struct bitmap_operations *op)
>> +{
>> +	pr_info("md: bitmap version %d registered\n", op->version);
>> +
>> +	spin_lock(&bitmap_lock);
>> +	list_add_tail(&op->list, &bitmap_list);
>> +	spin_unlock(&bitmap_lock);
>> +}
>> +EXPORT_SYMBOL_GPL(register_md_bitmap);
>> +
>> +void unregister_md_bitmap(struct bitmap_operations *op)
>> +{
>> +	pr_info("md: bitmap version %d unregistered\n", op->version);
>> +
>> +	spin_lock(&bitmap_lock);
>> +	list_del_init(&op->list);
>> +	spin_unlock(&bitmap_lock);
>> +}
>> +EXPORT_SYMBOL_GPL(unregister_md_bitmap);
>> +
>> +static struct bitmap_operations *__find_bitmap(int version)
>> +{
>> +	struct bitmap_operations *op;
>> +
>> +	list_for_each_entry(op, &bitmap_list, list)
>> +		if (op->version == version) {
>> +			if (try_module_get(op->owner))
>> +				return op;
>> +			else
>> +				return NULL;
>> +		}
>> +
>> +	return NULL;
>> +}
>> +
>> +static struct bitmap_operations *find_bitmap(int version)
>> +{
>> +	struct bitmap_operations *op = NULL;
>> +
>> +	spin_lock(&bitmap_lock);
>> +	op = __find_bitmap(version);
>> +	spin_unlock(&bitmap_lock);
>> +
>> +	if (op)
>> +		return op;
>> +
>> +	if (request_module("md-bitmap") != 0)
>> +		return NULL;
>> +
>> +	spin_lock(&bitmap_lock);
>> +	op = __find_bitmap(version);
>> +	spin_unlock(&bitmap_lock);
>> +
>> +	return op;
>> +}
>> +
>> +/* TODO: support more versions */
>> +static int mddev_set_bitmap_ops(struct mddev *mddev)
>> +{
>> +	struct bitmap_operations *op = find_bitmap(1);
>> +
>> +	if (!op)
>> +		return -ENODEV;
>> +
>> +	mddev->bitmap_ops = op;
>> +	return 0;
>> +}
>> +
>> +static void mddev_clear_bitmap_ops(struct mddev *mddev)
>> +{
>> +	module_put(mddev->bitmap_ops->owner);
>> +	mddev->bitmap_ops = NULL;
>> +}
>> +
>>   int mddev_init(struct mddev *mddev)
>>   {
>> +	int ret = mddev_set_bitmap_ops(mddev);
>> +
>> +	if (ret)
>> +		return ret;
>>   
>>   	if (percpu_ref_init(&mddev->active_io, active_io_release,
>> -			    PERCPU_REF_ALLOW_REINIT, GFP_KERNEL))
>> +			    PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
>> +		mddev_clear_bitmap_ops(mddev);
>>   		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;
>>   	}
>> @@ -666,7 +749,6 @@ int mddev_init(struct mddev *mddev)
>>   	mddev->resync_min = 0;
>>   	mddev->resync_max = MaxSector;
>>   	mddev->level = LEVEL_NONE;
>> -	mddev_set_bitmap_ops(mddev);
>>   
>>   	INIT_WORK(&mddev->sync_work, md_start_sync);
>>   	INIT_WORK(&mddev->del_work, mddev_delayed_delete);
>> @@ -677,6 +759,7 @@ EXPORT_SYMBOL_GPL(mddev_init);
>>   
>>   void mddev_destroy(struct mddev *mddev)
>>   {
>> +	mddev_clear_bitmap_ops(mddev);
>>   	percpu_ref_exit(&mddev->active_io);
>>   	percpu_ref_exit(&mddev->writes_pending);
>>   }
>> @@ -9898,11 +9981,6 @@ static int __init md_init(void)
>>   	if (!md_misc_wq)
>>   		goto err_misc_wq;
>>   
>> -	md_bitmap_wq = alloc_workqueue("md_bitmap", WQ_MEM_RECLAIM |
>> WQ_UNBOUND,
>> -				       0);
>> -	if (!md_bitmap_wq)
>> -		goto err_bitmap_wq;
>> -
>>   	ret = __register_blkdev(MD_MAJOR, "md", md_probe);
>>   	if (ret < 0)
>>   		goto err_md;
>> @@ -9921,8 +9999,6 @@ static int __init md_init(void)
>>   err_mdp:
>>   	unregister_blkdev(MD_MAJOR, "md");
>>   err_md:
>> -	destroy_workqueue(md_bitmap_wq);
>> -err_bitmap_wq:
>>   	destroy_workqueue(md_misc_wq);
>>   err_misc_wq:
>>   	destroy_workqueue(md_wq);
>> @@ -10229,7 +10305,6 @@ static __exit void md_exit(void)
>>   	spin_unlock(&all_mddevs_lock);
>>   
>>   	destroy_workqueue(md_misc_wq);
>> -	destroy_workqueue(md_bitmap_wq);
>>   	destroy_workqueue(md_wq);
>>   }
>>   
>> diff --git a/drivers/md/md.h b/drivers/md/md.h
>> index 5eaac1d84523..28347fb3af18 100644
>> --- a/drivers/md/md.h
>> +++ b/drivers/md/md.h
>> @@ -972,7 +972,6 @@ struct mdu_array_info_s;
>>   struct mdu_disk_info_s;
>>   
>>   extern int mdp_major;
>> -extern struct workqueue_struct *md_bitmap_wq;
>>   void md_autostart_arrays(int part);
>>   int md_set_array_info(struct mddev *mddev, struct mdu_array_info_s *info);
>>   int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info);
> 
> 
> .
>