[PATCH 12/23] md/md-bitmap: add macros for lockless bitmap

Yu Kuai posted 23 patches 6 months, 4 weeks ago
There is a newer version of this series
[PATCH 12/23] md/md-bitmap: add macros for lockless bitmap
Posted by Yu Kuai 6 months, 4 weeks ago
From: Yu Kuai <yukuai3@huawei.com>

Also move other values to md-bitmap.h and update comments.

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

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 17d41a7b30ce..689d5dba9328 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -36,15 +36,6 @@
 #include "md-bitmap.h"
 #include "md-cluster.h"
 
-#define BITMAP_MAJOR_LO 3
-/* version 4 insists the bitmap is in little-endian order
- * with version 3, it is host-endian which is non-portable
- * Version 5 is currently set only for clustered devices
- */
-#define BITMAP_MAJOR_HI 4
-#define BITMAP_MAJOR_CLUSTERED 5
-#define	BITMAP_MAJOR_HOSTENDIAN 3
-
 /*
  * in-memory bitmap:
  *
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index f2d79c8a23b7..d2cdf831ef1a 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -18,10 +18,27 @@ typedef __u16 bitmap_counter_t;
 #define RESYNC_MASK ((bitmap_counter_t) (1 << (COUNTER_BITS - 2)))
 #define COUNTER_MAX ((bitmap_counter_t) RESYNC_MASK - 1)
 
+/*
+ * version 3 is host-endian order, this is deprecated and not used for new
+ * array
+ */
+#define BITMAP_MAJOR_LO		3
+#define BITMAP_MAJOR_HOSTENDIAN	3
+/* version 4 is little-endian order, the default value */
+#define BITMAP_MAJOR_HI		4
+/* version 5 is only used for cluster */
+#define BITMAP_MAJOR_CLUSTERED	5
+/* version 6 is only used for lockless bitmap */
+#define BITMAP_MAJOR_LOCKLESS	6
+
+#define BITMAP_SB_SIZE 1024
 /* use these for bitmap->flags and bitmap->sb->state bit-fields */
 enum bitmap_state {
 	BITMAP_STALE	   = 1,  /* the bitmap file is out of date or had -EIO */
 	BITMAP_WRITE_ERROR = 2, /* A write error has occurred */
+	BITMAP_FIRST_USE   = 3, /* llbitmap is just created */
+	BITMAP_CLEAN       = 4, /* llbitmap is created with assume_clean */
+	BITMAP_DAEMON_BUSY = 5, /* llbitmap daemon is not finished after daemon_sleep */
 	BITMAP_HOSTENDIAN  =15,
 };
 
-- 
2.39.2
Re: [PATCH 12/23] md/md-bitmap: add macros for lockless bitmap
Posted by Xiao Ni 6 months, 3 weeks ago
On Sat, May 24, 2025 at 2:18 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@huawei.com>
>
> Also move other values to md-bitmap.h and update comments.
>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  drivers/md/md-bitmap.c |  9 ---------
>  drivers/md/md-bitmap.h | 17 +++++++++++++++++
>  2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 17d41a7b30ce..689d5dba9328 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -36,15 +36,6 @@
>  #include "md-bitmap.h"
>  #include "md-cluster.h"
>
> -#define BITMAP_MAJOR_LO 3
> -/* version 4 insists the bitmap is in little-endian order
> - * with version 3, it is host-endian which is non-portable
> - * Version 5 is currently set only for clustered devices
> - */
> -#define BITMAP_MAJOR_HI 4
> -#define BITMAP_MAJOR_CLUSTERED 5
> -#define        BITMAP_MAJOR_HOSTENDIAN 3
> -
>  /*
>   * in-memory bitmap:
>   *
> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
> index f2d79c8a23b7..d2cdf831ef1a 100644
> --- a/drivers/md/md-bitmap.h
> +++ b/drivers/md/md-bitmap.h
> @@ -18,10 +18,27 @@ typedef __u16 bitmap_counter_t;
>  #define RESYNC_MASK ((bitmap_counter_t) (1 << (COUNTER_BITS - 2)))
>  #define COUNTER_MAX ((bitmap_counter_t) RESYNC_MASK - 1)
>
> +/*
> + * version 3 is host-endian order, this is deprecated and not used for new
> + * array
> + */
> +#define BITMAP_MAJOR_LO                3
> +#define BITMAP_MAJOR_HOSTENDIAN        3
> +/* version 4 is little-endian order, the default value */
> +#define BITMAP_MAJOR_HI                4
> +/* version 5 is only used for cluster */
> +#define BITMAP_MAJOR_CLUSTERED 5
> +/* version 6 is only used for lockless bitmap */
> +#define BITMAP_MAJOR_LOCKLESS  6
> +
> +#define BITMAP_SB_SIZE 1024

Hi

For super1, the bitmap bits are next to bitmap superblock.
BITMAP_SB_SIZE is only used by md-llbitmap, is it better to define it
in md-llbitmap.c?

Regards
Xiao

>  /* use these for bitmap->flags and bitmap->sb->state bit-fields */
>  enum bitmap_state {
>         BITMAP_STALE       = 1,  /* the bitmap file is out of date or had -EIO */
>         BITMAP_WRITE_ERROR = 2, /* A write error has occurred */
> +       BITMAP_FIRST_USE   = 3, /* llbitmap is just created */
> +       BITMAP_CLEAN       = 4, /* llbitmap is created with assume_clean */
> +       BITMAP_DAEMON_BUSY = 5, /* llbitmap daemon is not finished after daemon_sleep */
>         BITMAP_HOSTENDIAN  =15,
>  };
>
> --
> 2.39.2
>
Re: [PATCH 12/23] md/md-bitmap: add macros for lockless bitmap
Posted by Hannes Reinecke 6 months, 3 weeks ago
On 5/24/25 08:13, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> Also move other values to md-bitmap.h and update comments.
> 
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>   drivers/md/md-bitmap.c |  9 ---------
>   drivers/md/md-bitmap.h | 17 +++++++++++++++++
>   2 files changed, 17 insertions(+), 9 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH 12/23] md/md-bitmap: add macros for lockless bitmap
Posted by Christoph Hellwig 6 months, 3 weeks ago
On Sat, May 24, 2025 at 02:13:09PM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> Also move other values to md-bitmap.h and update comments.

Hmm.  The commit message looks very confusing to me.

I think this should be two patches:

 1) move defines relevant to the disk format from md-bitmap.c to md-bitmap.h
 2) add new bits for llbitmap (and explain what they are).

> +#define BITMAP_SB_SIZE 1024

And while we're at it: this is still duplicated in llbitmap.c later.
But shouldn't it simply be replaced with a sizeof on struct bitmap_super_s?

(and when cleaning thing up, rename that to bitmap_super without
the _s and use it instead of the typedef at least for all new code)?
Re: [PATCH 12/23] md/md-bitmap: add macros for lockless bitmap
Posted by Yu Kuai 6 months, 3 weeks ago
Hi,

在 2025/05/26 14:40, Christoph Hellwig 写道:
> On Sat, May 24, 2025 at 02:13:09PM +0800, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> Also move other values to md-bitmap.h and update comments.
> 
> Hmm.  The commit message looks very confusing to me.
> 
> I think this should be two patches:
> 
>   1) move defines relevant to the disk format from md-bitmap.c to md-bitmap.h
>   2) add new bits for llbitmap (and explain what they are).

OK.

> 
>> +#define BITMAP_SB_SIZE 1024
> 
> And while we're at it: this is still duplicated in llbitmap.c later.
> But shouldn't it simply be replaced with a sizeof on struct bitmap_super_s?

Sorry that I forgot to explain why it's still in .c

sizeof(struct bitmap_super_s) is actually 256 bytes, while by default,
1k is reserved, perhaps I can name it as BITMAP_DATA_OFFSET ?

0-255B		bitmap_super_s
256-1023B	hole
1024-[space]	bitmap
[space] - 128k	hole

BTW, the new bitmap only support the default offset and space, user
can't configure it manually.

Thanks,
Kuai

> 
> (and when cleaning thing up, rename that to bitmap_super without
> the _s and use it instead of the typedef at least for all new code)?
> .
> 

Re: [PATCH 12/23] md/md-bitmap: add macros for lockless bitmap
Posted by Christoph Hellwig 6 months, 3 weeks ago
On Mon, May 26, 2025 at 04:12:06PM +0800, Yu Kuai wrote:
>>> +#define BITMAP_SB_SIZE 1024
>>
>> And while we're at it: this is still duplicated in llbitmap.c later.
>> But shouldn't it simply be replaced with a sizeof on struct bitmap_super_s?
>
> Sorry that I forgot to explain why it's still in .c
>
> sizeof(struct bitmap_super_s) is actually 256 bytes, while by default,
> 1k is reserved, perhaps I can name it as BITMAP_DATA_OFFSET ?

Sounds good.