diff mbox series

[RFC,mdadm/master] mdadm: add support for new lockless bitmap

Message ID 20250126082714.1588025-1-yukuai1@huaweicloud.com (mailing list archive)
State New
Headers show
Series [RFC,mdadm/master] mdadm: add support for new lockless bitmap | expand

Commit Message

Yu Kuai Jan. 26, 2025, 8:27 a.m. UTC 
From: Yu Kuai <yukuai3@huawei.com>

A new major number 6 is used for the new bitmap.

Noted that for the kernel that doesn't support lockless bitmap, create
such array will fail:

md0: invalid bitmap file superblock: unrecognized superblock version.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 Create.c | 5 ++++-
 Grow.c   | 3 ++-
 bitmap.h | 1 +
 mdadm.c  | 9 ++++++++-
 mdadm.h  | 1 +
 super1.c | 9 +++++++++
 6 files changed, 25 insertions(+), 3 deletions(-)

Comments

Mariusz Tkaczyk Jan. 27, 2025, 9:25 a.m. UTC | #1 
On Sun, 26 Jan 2025 16:27:14 +0800
Yu Kuai <yukuai1@huaweicloud.com> wrote:

> From: Yu Kuai <yukuai3@huawei.com>
> 
> A new major number 6 is used for the new bitmap.
> 
> Noted that for the kernel that doesn't support lockless bitmap, create
> such array will fail:
> 
> md0: invalid bitmap file superblock: unrecognized superblock version.
Hi Kuai,

Please go ahead and create branch on mdadm repo for lockness bitmap
implementation and keep your changes there. This is for sure not ready
and cannot be merged yet to main so sending it is not needed.

What do you think?
> 
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  Create.c | 5 ++++-
>  Grow.c   | 3 ++-
>  bitmap.h | 1 +
>  mdadm.c  | 9 ++++++++-
>  mdadm.h  | 1 +
>  super1.c | 9 +++++++++
>  6 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/Create.c b/Create.c
> index fd6c9215..105d15e0 100644
> --- a/Create.c
> +++ b/Create.c
> @@ -541,6 +541,8 @@ int Create(struct supertype *st, struct
> mddev_ident *ident, int subdevs, pr_err("At least 2 nodes are needed
> for cluster-md\n"); return 1;
>  		}
> +	} else if (s->btype == BitmapLockless) {
> +		major_num = BITMAP_MAJOR_LOCKLESS;
>  	}
>  
>  	memset(&info, 0, sizeof(info));
> @@ -1182,7 +1184,8 @@ int Create(struct supertype *st, struct
> mddev_ident *ident, int subdevs,
>  	 * to stop another mdadm from finding and using those
> devices. */
>  
> -	if (s->btype == BitmapInternal || s->btype == BitmapCluster)
> {
> +	if (s->btype == BitmapInternal || s->btype == BitmapCluster
> ||
> +	    s->btype == BitmapLockless) {

This is asking to be moved to common helper function. Is is repeated 3
times at least so please consider (not sure about naming):

bool is_bitmap_supported(int btype) {
	if (btype == BitmapInternal || btype == BitmapCluster ||
	    btype == BitmapLockless)
		return true;
	return false;
}
Just a nit.
Yu Kuai Feb. 6, 2025, 1:35 a.m. UTC | #2 
Hi,

在 2025/01/27 17:25, Mariusz Tkaczyk 写道:
> On Sun, 26 Jan 2025 16:27:14 +0800
> Yu Kuai <yukuai1@huaweicloud.com> wrote:
> 
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> A new major number 6 is used for the new bitmap.
>>
>> Noted that for the kernel that doesn't support lockless bitmap, create
>> such array will fail:
>>
>> md0: invalid bitmap file superblock: unrecognized superblock version.
> Hi Kuai,
> 
> Please go ahead and create branch on mdadm repo for lockness bitmap
> implementation and keep your changes there. This is for sure not ready
> and cannot be merged yet to main so sending it is not needed.
> 
> What do you think?

Yes, of course, this is just an early RFC version, I'm sending this
for people if they want to test.

I think fork a mdadm repo in my git tree is enough for now. :)

BTW, I still need to send a patch at company before I apply the patch
at home. I can't access my git tree at company, and our company policy
only allow me to send patch to open source community. :( Things will be
easier later after I apply a laptop and skip company's network.

Thanks,
Kuai

>>
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> ---
>>   Create.c | 5 ++++-
>>   Grow.c   | 3 ++-
>>   bitmap.h | 1 +
>>   mdadm.c  | 9 ++++++++-
>>   mdadm.h  | 1 +
>>   super1.c | 9 +++++++++
>>   6 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/Create.c b/Create.c
>> index fd6c9215..105d15e0 100644
>> --- a/Create.c
>> +++ b/Create.c
>> @@ -541,6 +541,8 @@ int Create(struct supertype *st, struct
>> mddev_ident *ident, int subdevs, pr_err("At least 2 nodes are needed
>> for cluster-md\n"); return 1;
>>   		}
>> +	} else if (s->btype == BitmapLockless) {
>> +		major_num = BITMAP_MAJOR_LOCKLESS;
>>   	}
>>   
>>   	memset(&info, 0, sizeof(info));
>> @@ -1182,7 +1184,8 @@ int Create(struct supertype *st, struct
>> mddev_ident *ident, int subdevs,
>>   	 * to stop another mdadm from finding and using those
>> devices. */
>>   
>> -	if (s->btype == BitmapInternal || s->btype == BitmapCluster)
>> {
>> +	if (s->btype == BitmapInternal || s->btype == BitmapCluster
>> ||
>> +	    s->btype == BitmapLockless) {
> 
> This is asking to be moved to common helper function. Is is repeated 3
> times at least so please consider (not sure about naming):
> 
> bool is_bitmap_supported(int btype) {
> 	if (btype == BitmapInternal || btype == BitmapCluster ||
> 	    btype == BitmapLockless)
> 		return true;
> 	return false;
> }
> Just a nit.
> .
>
diff mbox series

Patch

diff --git a/Create.c b/Create.c
index fd6c9215..105d15e0 100644
--- a/Create.c
+++ b/Create.c
@@ -541,6 +541,8 @@  int Create(struct supertype *st, struct mddev_ident *ident, int subdevs,
 			pr_err("At least 2 nodes are needed for cluster-md\n");
 			return 1;
 		}
+	} else if (s->btype == BitmapLockless) {
+		major_num = BITMAP_MAJOR_LOCKLESS;
 	}
 
 	memset(&info, 0, sizeof(info));
@@ -1182,7 +1184,8 @@  int Create(struct supertype *st, struct mddev_ident *ident, int subdevs,
 	 * to stop another mdadm from finding and using those devices.
 	 */
 
-	if (s->btype == BitmapInternal || s->btype == BitmapCluster) {
+	if (s->btype == BitmapInternal || s->btype == BitmapCluster ||
+	    s->btype == BitmapLockless) {
 		if (!st->ss->add_internal_bitmap) {
 			pr_err("internal bitmaps not supported with %s metadata\n",
 				st->ss->name);
diff --git a/Grow.c b/Grow.c
index cc1be6cc..3905f64c 100644
--- a/Grow.c
+++ b/Grow.c
@@ -383,7 +383,8 @@  int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s)
 		free(mdi);
 	}
 
-	if (s->btype == BitmapInternal || s->btype == BitmapCluster) {
+	if (s->btype == BitmapInternal || s->btype == BitmapCluster ||
+	    s->btype == BitmapLockless) {
 		int rv;
 		int d;
 		int offset_setable = 0;
diff --git a/bitmap.h b/bitmap.h
index 7b1f80f2..4b5d2d93 100644
--- a/bitmap.h
+++ b/bitmap.h
@@ -13,6 +13,7 @@ 
 #define BITMAP_MAJOR_HI 4
 #define	BITMAP_MAJOR_HOSTENDIAN 3
 #define	BITMAP_MAJOR_CLUSTERED 5
+#define	BITMAP_MAJOR_LOCKLESS 6
 
 #define BITMAP_MINOR 39
 
diff --git a/mdadm.c b/mdadm.c
index 1fd4dcba..7a64fba2 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -56,6 +56,12 @@  static mdadm_status_t set_bitmap_value(struct shape *s, struct context *c, char
 		return MDADM_STATUS_SUCCESS;
 	}
 
+	if (strcmp(val, "lockless") == 0) {
+		s->btype = BitmapLockless;
+		pr_info("Experimental lockless bitmap, use at your own disk!\n");
+		return MDADM_STATUS_SUCCESS;
+	}
+
 	if (strcmp(val, "clustered") == 0) {
 		s->btype = BitmapCluster;
 		/* Set the default number of cluster nodes
@@ -1251,7 +1257,8 @@  int main(int argc, char *argv[])
 			pr_err("--bitmap is required for consistency policy: %s\n",
 			       map_num_s(consistency_policies, s.consistency_policy));
 			exit(2);
-		} else if ((s.btype == BitmapInternal || s.btype == BitmapCluster) &&
+		} else if ((s.btype == BitmapInternal || s.btype == BitmapCluster ||
+			    s.btype == BitmapLockless) &&
 			   s.consistency_policy != CONSISTENCY_POLICY_BITMAP &&
 			   s.consistency_policy != CONSISTENCY_POLICY_JOURNAL) {
 			pr_err("--bitmap is not compatible with consistency policy: %s\n",
diff --git a/mdadm.h b/mdadm.h
index 77705b11..5985a5bd 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -607,6 +607,7 @@  enum bitmap_type {
 	BitmapNone,
 	BitmapInternal,
 	BitmapCluster,
+	BitmapLockless,
 	BitmapUnknown,
 };
 
diff --git a/super1.c b/super1.c
index fe3c4c64..016ce36c 100644
--- a/super1.c
+++ b/super1.c
@@ -2487,6 +2487,12 @@  static __u64 avail_size1(struct supertype *st, __u64 devsize,
 	return 0;
 }
 
+enum llbitmap_flags {
+	LLB_STALE = 0,
+	LLB_ERROR,
+	LLB_FIRST_USE,
+};
+
 static int
 add_internal_bitmap1(struct supertype *st,
 		     int *chunkp, int delay, int write_behind,
@@ -2650,6 +2656,9 @@  add_internal_bitmap1(struct supertype *st,
 		bms->cluster_name[len - 1] = '\0';
 	}
 
+	/* kernel will initialize bitmap */
+	if (major == BITMAP_MAJOR_LOCKLESS)
+		bms->state = __cpu_to_le32(1 << LLB_FIRST_USE);
 	*chunkp = chunk;
 	return 0;
 }