Linux RAID subsystem development
 help / color / mirror / Atom feed
* [PATCH 8/8] ARM: Remove hacked-up asm/types.h header
From: Ard Biesheuvel @ 2026-04-22 17:17 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-crypto, linux-raid, Ard Biesheuvel, Christoph Hellwig,
	Russell King, Arnd Bergmann, Eric Biggers
In-Reply-To: <20260422171655.3437334-10-ardb+git@google.com>

From: Ard Biesheuvel <ardb@kernel.org>

ARM has a special version of asm/types.h which contains overrides for
certain #define's related to the C types used to back C99 types such as
uint32_t and uintptr_t.

This is only needed when pulling in system headers such as stdint.h
during the build, and this only happens when using NEON intrinsics,
for which there is now a dedicated header file.

So drop this header entirely, and revert to the asm-generic one.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/include/uapi/asm/types.h | 41 --------------------
 1 file changed, 41 deletions(-)

diff --git a/arch/arm/include/uapi/asm/types.h b/arch/arm/include/uapi/asm/types.h
deleted file mode 100644
index 1a667bc26510..000000000000
--- a/arch/arm/include/uapi/asm/types.h
+++ /dev/null
@@ -1,41 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _UAPI_ASM_TYPES_H
-#define _UAPI_ASM_TYPES_H
-
-#include <asm-generic/int-ll64.h>
-
-/*
- * The C99 types uintXX_t that are usually defined in 'stdint.h' are not as
- * unambiguous on ARM as you would expect. For the types below, there is a
- * difference on ARM between GCC built for bare metal ARM, GCC built for glibc
- * and the kernel itself, which results in build errors if you try to build with
- * -ffreestanding and include 'stdint.h' (such as when you include 'arm_neon.h'
- * in order to use NEON intrinsics)
- *
- * As the typedefs for these types in 'stdint.h' are based on builtin defines
- * supplied by GCC, we can tweak these to align with the kernel's idea of those
- * types, so 'linux/types.h' and 'stdint.h' can be safely included from the same
- * source file (provided that -ffreestanding is used).
- *
- *                    int32_t         uint32_t               uintptr_t
- * bare metal GCC     long            unsigned long          unsigned int
- * glibc GCC          int             unsigned int           unsigned int
- * kernel             int             unsigned int           unsigned long
- */
-
-#ifdef __INT32_TYPE__
-#undef __INT32_TYPE__
-#define __INT32_TYPE__		int
-#endif
-
-#ifdef __UINT32_TYPE__
-#undef __UINT32_TYPE__
-#define __UINT32_TYPE__	unsigned int
-#endif
-
-#ifdef __UINTPTR_TYPE__
-#undef __UINTPTR_TYPE__
-#define __UINTPTR_TYPE__	unsigned long
-#endif
-
-#endif /* _UAPI_ASM_TYPES_H */
-- 
2.54.0.rc1.555.g9c883467ad-goog


^ permalink raw reply related

* Re: [PATCH 2/8] xor/arm: Replace vectorized implementation with arm64's intrinsics
From: Josh Law @ 2026-04-22 18:07 UTC (permalink / raw)
  To: ardb+git
  Cc: ardb, arnd, ebiggers, hch, linux-arm-kernel, linux-crypto,
	linux-raid, linux
In-Reply-To: <20260422171655.3437334-12-ardb+git@google.com>

Hi ard.

I like this patch.

So, I'd be crazy not to say what I love here.

+		/* p1 ^= p2 */
+		v0 = veorq_u64(vld1q_u64(dp1 +  0), vld1q_u64(dp2 +  0));
+		v1 = veorq_u64(vld1q_u64(dp1 +  2), vld1q_u64(dp2 +  2));
+		v2 = veorq_u64(vld1q_u64(dp1 +  4), vld1q_u64(dp2 +  4));
+		v3 = veorq_u64(vld1q_u64(dp1 +  6), vld1q_u64(dp2 +  6));
+
+		/* p1 ^= p3 */
+		v0 = veorq_u64(v0, vld1q_u64(dp3 +  0));
+		v1 = veorq_u64(v1, vld1q_u64(dp3 +  2));
+		v2 = veorq_u64(v2, vld1q_u64(dp3 +  4));
+		v3 = veorq_u64(v3, vld1q_u64(dp3 +  6));
+
+		/* store */
+		vst1q_u64(dp1 +  0, v0);
+		vst1q_u64(dp1 +  2, v1);
+		vst1q_u64(dp1 +  4, v2);
+		vst1q_u64(dp1 +  6, v3);
+
+		dp1 += 8;
+		dp2 += 8;
+		dp3 += 8;
+	} while (--lines > 0);
+}

I really like how clean this is, I'm personally nodding my head here

Taking the "bad" guesswork of the compiler here is also amazing, it also
guarantees we won't get stupid regressions in the future.

Also, that performance boost is even better ;) 

I'm not the biggest expert of this subdirectory, but I understand it well.

So well,

Reviewed-by: Josh Law <joshlaw48@gmail.com>


Thanks! (I will review your lib patches) :)

^ permalink raw reply

* Re: [PATCH 3/8] xor/arm64: Use shared NEON intrinsics implementation from 32-bit ARM
From: Josh Law @ 2026-04-22 18:11 UTC (permalink / raw)
  To: ardb+git
  Cc: ardb, arnd, ebiggers, hch, linux-arm-kernel, linux-crypto,
	linux-raid, linux
In-Reply-To: <20260422171655.3437334-13-ardb+git@google.com>

Hi ard.

>+#ifdef CONFIG_ARM64
>+extern typeof(__xor_neon_2) >__xor_eor3_2 >__alias(__xor_neon_2);
>+#endif

Creative. A  reduction of about 150 lines of duplicate code while
maintaining
the __alias for the 2 input case is great.


Reviewed-by: Josh Law <joshlaw48@gmail.com>

Thanks!

^ permalink raw reply

* Re: [PATCH 4/8] lib/crc: Turn NEON intrinsics crc64 implementation into common code
From: Josh Law @ 2026-04-22 18:13 UTC (permalink / raw)
  To: ardb+git
  Cc: ardb, arnd, ebiggers, hch, linux-arm-kernel, linux-crypto,
	linux-raid, linux
In-Reply-To: <20260422171655.3437334-14-ardb+git@google.com>

Hi ard.


diff --git a/lib/crc/arm64/crc64-neon.h b/lib/crc/arm64/crc64-neon.h
new file mode 100644
index 000000000000..fcd5b1e6f812
--- /dev/null
+++ b/lib/crc/arm64/crc64-neon.h
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+static inline uint64x2_t pmull64(uint64x2_t a, uint64x2_t b)
+{
return vreinterpretq_u64_p128(vmull_p64(vgetq_lane_u64(a, 0),
vgetq_lane_u64(b, 0)));
+}
+static inline uint64x2_t pmull64_high(uint64x2_t a, uint64x2_t b)
+{
poly64x2_t l = vreinterpretq_p64_u64(a);
poly64x2_t m = vreinterpretq_p64_u64(b);
return vreinterpretq_u64_p128(vmull_high_p64(l, m));
+}

Makes sense.

Moving these polynomial multiplication wrappers into their own header is
good. 

Reviewed-by: Josh Law <joshlaw48@gmail.com>


Thanks!

^ permalink raw reply related

* Re: [PATCH 5/8] lib/crc: arm: Enable arm64's NEON intrinsics implementation of crc64
From: Josh Law @ 2026-04-22 18:16 UTC (permalink / raw)
  To: ardb+git
  Cc: ardb, arnd, ebiggers, hch, linux-arm-kernel, linux-crypto,
	linux-raid, linux
In-Reply-To: <20260422171655.3437334-15-ardb+git@google.com>

Hi Ard,

Wow, 20x improvement is nuts.

I like how you handle this change *safely*

Like.

+static inline u64 crc64_nvme_arch(u64 crc, const u8 *p, size_t len)
+{
+	if (len >= 128 && static_branch_likely(&have_pmull) &&
+	    likely(may_use_simd())) {
+		do {
+			size_t chunk = min_t(size_t, len & ~15, SZ_4K);
+
+			scoped_ksimd()
+				crc = crc64_nvme_neon(crc, p, chunk);
+
+			p += chunk;
+			len -= chunk;
+		} while (len >= 128);
+	}

chunking the SIMD work at SZ_4K to avoid hogging the CPU and allowing
softirqs/preemption to process is a great detail. 

It’s easy to just wing it and throw
the entire buffer at the FPU, but respecting the kernel's latency
requirements is better!


Reviewed-by: Josh Law <joshlaw48@gmail.com>

Thanks!

^ permalink raw reply

* Re: [PATCH 6/8] crypto: aegis128 - Use neon-intrinsics.h on ARM too
From: Josh Law @ 2026-04-22 18:19 UTC (permalink / raw)
  To: ardb+git
  Cc: ardb, arnd, ebiggers, hch, linux-arm-kernel, linux-crypto,
	linux-raid, linux
In-Reply-To: <20260422171655.3437334-16-ardb+git@google.com>

Hi ard, this is a good cleanup!

Being able to drop <arm_neon.h> and just using
<asm/neon-intrinsics.h> across both architectures makes the C code much
cleaner.

-# Enable <arm_neon.h>
-CFLAGS_aegis128-neon-inner.o += -isystem $(shell $(CC) -print-file-name=include)

Getting rid of the isystem is good. iirc that was a hack anyway, feel free
to correct me on that

Reviewed-by: Josh Law <joshlaw48@gmail.com>


^ permalink raw reply

* Re: [PATCH 7/8] lib/raid6: Include asm/neon-intrinsics.h rather than arm_neon.h
From: Josh Law @ 2026-04-22 18:20 UTC (permalink / raw)
  To: ardb+git
  Cc: ardb, arnd, ebiggers, hch, linux-arm-kernel, linux-crypto,
	linux-raid, linux
In-Reply-To: <20260422171655.3437334-17-ardb+git@google.com>

Hi ard.

Makes sense here

-#include <arm_neon.h>
+#include <asm/neon-intrinsics.h>

Reviewed-by: Josh Law <joshlaw48@gmail.com>

This series is a good (and deserved series)


That's me done! I've reviewed your lib patches for you, have a great day!

^ permalink raw reply

* Re: [PATCH 2/8] xor/arm: Replace vectorized implementation with arm64's intrinsics
From: Christoph Hellwig @ 2026-04-23  7:44 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, linux-crypto, linux-raid, Ard Biesheuvel,
	Christoph Hellwig, Russell King, Arnd Bergmann, Eric Biggers
In-Reply-To: <20260422171655.3437334-12-ardb+git@google.com>

Nice!

Acked-by: Christoph Hellwig <hch@lst.de>


^ permalink raw reply

* Re: [PATCH 3/8] xor/arm64: Use shared NEON intrinsics implementation from 32-bit ARM
From: Christoph Hellwig @ 2026-04-23  7:46 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, linux-crypto, linux-raid, Ard Biesheuvel,
	Christoph Hellwig, Russell King, Arnd Bergmann, Eric Biggers
In-Reply-To: <20260422171655.3437334-13-ardb+git@google.com>

> +extern void __xor_eor3_2(unsigned long bytes, unsigned long * __restrict p1,
> +		const unsigned long * __restrict p2);

Does the alias magic prevent this from being in a header?  If so a comment
would be nice, otherwise moving it to a header would be even better.


^ permalink raw reply

* Re: [PATCH 7/8] lib/raid6: Include asm/neon-intrinsics.h rather than arm_neon.h
From: Christoph Hellwig @ 2026-04-23  7:47 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, linux-crypto, linux-raid, Ard Biesheuvel,
	Christoph Hellwig, Russell King, Arnd Bergmann, Eric Biggers
In-Reply-To: <20260422171655.3437334-17-ardb+git@google.com>

On Wed, Apr 22, 2026 at 07:17:03PM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> arm_neon.h is a compiler header which needs some scaffolding to work
> correctly in the linux context, and so it is better not to include it
> directly. Both ARM and arm64 now provide asm/neon-intrinsics.h which
> takes care of this.


This could potentially clash with the raid6 library rework I'm doing
for 7.2. Although git has become pretty good about renamed files, so
maybe it won't be so bad.


^ permalink raw reply

* Re: [PATCH 3/8] xor/arm64: Use shared NEON intrinsics implementation from 32-bit ARM
From: Ard Biesheuvel @ 2026-04-23  7:48 UTC (permalink / raw)
  To: Christoph Hellwig, Ard Biesheuvel
  Cc: linux-arm-kernel, linux-crypto, linux-raid, Russell King,
	Arnd Bergmann, Eric Biggers
In-Reply-To: <20260423074614.GB31018@lst.de>



On Thu, 23 Apr 2026, at 09:46, Christoph Hellwig wrote:
>> +extern void __xor_eor3_2(unsigned long bytes, unsigned long * __restrict p1,
>> +		const unsigned long * __restrict p2);
>
> Does the alias magic prevent this from being in a header?


Yes, it emits the ELF symbol for the alias, and this is only permitted
in the compilation unit that defines the original.

> If so a comment
> would be nice, otherwise moving it to a header would be even better.

Ack.

^ permalink raw reply

* [PATCH 0/3] md/raid1: small cleanups in MD and raid1 drivers
From: Abd-Alrhman Masalkhi @ 2026-04-23 10:13 UTC (permalink / raw)
  To: song, yukuai; +Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi

Hi,

This small series contains three cleanups in the MD and raid1 drivers.

Thanks,
Abd-Alrhman

Abd-Alrhman Masalkhi (3):
  md/raid1: replace wait loop with wait_event_idle() in
    raid1_write_request()
  md: use mddev_is_dm() instead of open-coding gendisk checks
  md: use ATTRIBUTE_GROUPS() for md default sysfs attributes

 drivers/md/md.c    | 16 ++++------------
 drivers/md/raid1.c | 15 ++++-----------
 2 files changed, 8 insertions(+), 23 deletions(-)

-- 
2.43.0


^ permalink raw reply

* [PATCH 1/3] md/raid1: replace wait loop with wait_event_idle() in raid1_write_request()
From: Abd-Alrhman Masalkhi @ 2026-04-23 10:13 UTC (permalink / raw)
  To: song, yukuai; +Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
In-Reply-To: <20260423101303.48196-1-abd.masalkhi@gmail.com>

The wait loop is equivalent to wait_event_idle() and can be simplified
by usaing it for improving readability.

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 drivers/md/raid1.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index cc9914bd15c1..b549be9174bb 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1488,21 +1488,14 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
 	    mddev->cluster_ops->area_resyncing(mddev, WRITE,
 		     bio->bi_iter.bi_sector, bio_end_sector(bio))) {
 
-		DEFINE_WAIT(w);
 		if (bio->bi_opf & REQ_NOWAIT) {
 			bio_wouldblock_error(bio);
 			return;
 		}
-		for (;;) {
-			prepare_to_wait(&conf->wait_barrier,
-					&w, TASK_IDLE);
-			if (!mddev->cluster_ops->area_resyncing(mddev, WRITE,
-							bio->bi_iter.bi_sector,
-							bio_end_sector(bio)))
-				break;
-			schedule();
-		}
-		finish_wait(&conf->wait_barrier, &w);
+		wait_event_idle(conf->wait_barrier,
+				!mddev->cluster_ops->area_resyncing(mddev, WRITE,
+								    bio->bi_iter.bi_sector,
+								    bio_end_sector(bio)));
 	}
 
 	/*
-- 
2.43.0


^ permalink raw reply related

* [PATCH 2/3] md: use mddev_is_dm() instead of open-coding gendisk checks
From: Abd-Alrhman Masalkhi @ 2026-04-23 10:13 UTC (permalink / raw)
  To: song, yukuai; +Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
In-Reply-To: <20260423101303.48196-1-abd.masalkhi@gmail.com>

Replace direct checks on mddev->gendisk with mddev_is_dm() in
md_handle_request() and md_run().

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 drivers/md/md.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index ac71640ff3a8..346d071c1b8e 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -417,7 +417,7 @@ bool md_handle_request(struct mddev *mddev, struct bio *bio)
 
 	if (!mddev->pers->make_request(mddev, bio)) {
 		percpu_ref_put(&mddev->active_io);
-		if (!mddev->gendisk && mddev->pers->prepare_suspend)
+		if (mddev_is_dm(mddev) && mddev->pers->prepare_suspend)
 			return false;
 		goto check_suspended;
 	}
@@ -6584,7 +6584,7 @@ int md_run(struct mddev *mddev)
 	}
 
 	/* dm-raid expect sync_thread to be frozen until resume */
-	if (mddev->gendisk)
+	if (!mddev_is_dm(mddev))
 		mddev->recovery = 0;
 
 	/* may be over-ridden by personality */
-- 
2.43.0


^ permalink raw reply related

* [PATCH 3/3] md: use ATTRIBUTE_GROUPS() for md default sysfs attributes
From: Abd-Alrhman Masalkhi @ 2026-04-23 10:13 UTC (permalink / raw)
  To: song, yukuai; +Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
In-Reply-To: <20260423101303.48196-1-abd.masalkhi@gmail.com>

Replace the md_default_group and md_attr_groups with
ATTRIBUTE_GROUPS().

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 drivers/md/md.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 346d071c1b8e..0e55639211f2 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6045,10 +6045,7 @@ static struct attribute *md_default_attrs[] = {
 	&md_logical_block_size.attr,
 	NULL,
 };
-
-static const struct attribute_group md_default_group = {
-	.attrs = md_default_attrs,
-};
+ATTRIBUTE_GROUPS(md_default);
 
 static struct attribute *md_redundancy_attrs[] = {
 	&md_scan_mode.attr,
@@ -6073,11 +6070,6 @@ static const struct attribute_group md_redundancy_group = {
 	.attrs = md_redundancy_attrs,
 };
 
-static const struct attribute_group *md_attr_groups[] = {
-	&md_default_group,
-	NULL,
-};
-
 static ssize_t
 md_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
 {
@@ -6154,7 +6146,7 @@ static const struct sysfs_ops md_sysfs_ops = {
 static const struct kobj_type md_ktype = {
 	.release	= md_kobj_release,
 	.sysfs_ops	= &md_sysfs_ops,
-	.default_groups	= md_attr_groups,
+	.default_groups	= md_default_groups,
 };
 
 int mdp_major = 0;
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH 1/4] md/raid10: prepare per-r10bio dev slot tracking
From: Chen Cheng @ 2026-04-24  2:11 UTC (permalink / raw)
  To: Paul Menzel; +Cc: linux-raid, yukuai, chenchneg33
In-Reply-To: <6e6e4340-2181-4a79-9284-7ed167aab807@molgen.mpg.de>

On Wed, Apr 22, 2026 at 08:40:42AM +0200, Paul Menzel wrote:

Hi Paul,

> Dear Cheng,
>
>
> Am 22.04.26 um 04:33 schrieb Chen Cheng:
> > From: Chen Cheng <chencheng@fnnas.com>
> >
> > raid10 reuses r10bio objects from both r10bio_pool and r10buf_pool. Track
> > the number of devs[] slots used by each request in the r10bio itself and
> > initialize it whenever one of these objects is reused.
> >
> > No functional change yet. A later patch will use this width when reshape
> > changes conf->geo.raid_disks.
>
> Your Signed-off-by: line is missing.

Yes, i missed it, thanks for point-out;

>
> > ---
> >   drivers/md/raid10.c | 4 ++++
> >   drivers/md/raid10.h | 1 +
> >   2 files changed, 5 insertions(+)
> >
> > diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> > index 0653b5d8545a..e93933632893 100644
> > --- a/drivers/md/raid10.c
> > +++ b/drivers/md/raid10.c
> > @@ -1540,6 +1540,7 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
> >     r10_bio->sector = bio->bi_iter.bi_sector;
> >     r10_bio->state = 0;
> >     r10_bio->read_slot = -1;
> > +   r10_bio->used_nr_devs = conf->geo.raid_disks;
> >     memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) *
> >                     conf->geo.raid_disks);
> > @@ -1727,6 +1728,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
> >     r10_bio->mddev = mddev;
> >     r10_bio->state = 0;
> >     r10_bio->sectors = 0;
> > +   r10_bio->used_nr_devs = geo->raid_disks;
> >     memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * geo->raid_disks);
> >     wait_blocked_dev(mddev, r10_bio);
> > @@ -3061,6 +3063,8 @@ static struct r10bio *raid10_alloc_init_r10buf(struct r10conf *conf)
> >     else
> >             nalloc = 2; /* recovery */
> > +   r10bio->used_nr_devs = nalloc;
> > +
> >     for (i = 0; i < nalloc; i++) {
> >             bio = r10bio->devs[i].bio;
> >             rp = bio->bi_private;
> > diff --git a/drivers/md/raid10.h b/drivers/md/raid10.h
> > index ec79d87fb92f..92e8743023e6 100644
> > --- a/drivers/md/raid10.h
> > +++ b/drivers/md/raid10.h
> > @@ -127,6 +127,7 @@ struct r10bio {
> >      * if the IO is in READ direction, then this is where we read
> >      */
> >     int                     read_slot;
> > +   unsigned int            used_nr_devs;
>
> Most entries have a comment describing the use. Maybe add one too, or at
> least a blank line, so it’s clear that the existing comment is just for
> `read_slot`?

Agreed.

>
> >     struct list_head        retry_list;
> >     /*
>
> From a performance and resource usage point of view, will increasing the
> struct have a negative impact?

On 64-bit platform, doesn't have negative resource usage impact,
the new field fits into the existing padding after read_slot, so
offsetof(struct r10bio, devs) stays unchanged;

On 32-bit platform, may increase by 4 bytes per r10bio, but that's
negligible compared with the bios/pages allocated for each request;


No negative performance impact, cause bottleneck is IO, and
the IO path has no changed;

>
> The diff looks good.
>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>

Thanks for review;

>
> Kind regards,
>
> Paul


Thanks,
Cheng

^ permalink raw reply

* Re: [PATCH 1/4] md/raid10: prepare per-r10bio dev slot tracking
From: Yu Kuai @ 2026-04-24  7:04 UTC (permalink / raw)
  To: Chen Cheng, linux-raid, yukuai; +Cc: chenchneg33
In-Reply-To: <20260422023317.796326-1-chencheng@fnnas.com>

Hi,

在 2026/4/22 10:33, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> raid10 reuses r10bio objects from both r10bio_pool and r10buf_pool. Track
> the number of devs[] slots used by each request in the r10bio itself and
> initialize it whenever one of these objects is reused.
>
> No functional change yet. A later patch will use this width when reshape
> changes conf->geo.raid_disks.
> ---
>   drivers/md/raid10.c | 4 ++++
>   drivers/md/raid10.h | 1 +
>   2 files changed, 5 insertions(+)

For patchset please also add a patch 0.

This solution looks incorrect. The usage of r10bio_pool() is wrong in the first
place. Noted for mempool, it will preallocate elements and such elements can be
reused in following mempool allocation. Which means:

1) preallocate elements with old raid disks;
2) rehsape update raid disks;
3) allocate new r10bio, elements from 1) can be used.

The solution can refer to raid1.

1) convert mempool to fixed size;
2) during reshape, suspend/quiesce the array first to wait for all prallocated
r10bios to return first.

>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0653b5d8545a..e93933632893 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1540,6 +1540,7 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
>   	r10_bio->sector = bio->bi_iter.bi_sector;
>   	r10_bio->state = 0;
>   	r10_bio->read_slot = -1;
> +	r10_bio->used_nr_devs = conf->geo.raid_disks;
>   	memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) *
>   			conf->geo.raid_disks);
>   
> @@ -1727,6 +1728,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
>   	r10_bio->mddev = mddev;
>   	r10_bio->state = 0;
>   	r10_bio->sectors = 0;
> +	r10_bio->used_nr_devs = geo->raid_disks;
>   	memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * geo->raid_disks);
>   	wait_blocked_dev(mddev, r10_bio);
>   
> @@ -3061,6 +3063,8 @@ static struct r10bio *raid10_alloc_init_r10buf(struct r10conf *conf)
>   	else
>   		nalloc = 2; /* recovery */
>   
> +	r10bio->used_nr_devs = nalloc;
> +
>   	for (i = 0; i < nalloc; i++) {
>   		bio = r10bio->devs[i].bio;
>   		rp = bio->bi_private;
> diff --git a/drivers/md/raid10.h b/drivers/md/raid10.h
> index ec79d87fb92f..92e8743023e6 100644
> --- a/drivers/md/raid10.h
> +++ b/drivers/md/raid10.h
> @@ -127,6 +127,7 @@ struct r10bio {
>   	 * if the IO is in READ direction, then this is where we read
>   	 */
>   	int			read_slot;
> +	unsigned int		used_nr_devs;
>   
>   	struct list_head	retry_list;
>   	/*

-- 
Thansk,
Kuai

^ permalink raw reply

* [PATCH v13 0/3] md/md-bitmap: restore bitmap grow through sysfs
From: Yu Kuai @ 2026-04-25  2:46 UTC (permalink / raw)
  To: Song Liu; +Cc: glass.su, Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel

mdadm --grow adds an internal bitmap by writing bitmap/location for an
array that currently has no bitmap. That requires the bitmap directory
and location attribute to exist before the classic bitmap backend is
created.

This series separates bitmap backend lifetime from bitmap sysfs lifetime,
splits the sysfs layout into common and backend-specific groups, and adds
a small "none" bitmap backend. The none backend keeps bitmap/location
available while no real bitmap is active, and the location store path can
then switch between the none backend and the classic bitmap backend
without tearing down the common bitmap sysfs directory.

Patch 1 factors bitmap creation and destruction into helpers that do not
touch sysfs registration.

Patch 2 splits the classic bitmap sysfs files into a common group and an
internal-bitmap group, and converts bitmap backend operations to use a
sysfs group array.

Patch 3 adds the none backend and uses it to restore mdadm --grow bitmap
addition through bitmap/location.

Changes since v12:
- Keep the factoring patch focused on no-sysfs bitmap lifetime helpers.
- Make bitmap operation lookup depend only on the current bitmap id
  matching the installed backend.
- Trim the none backend to only the operations required by the active
  call paths.
- Rework bitmap/location error handling with explicit cleanup labels.
- Restore the none backend after bitmap removal and creation/load
  failures so bitmap/location stays available.

Validation:
  - create a RAID1 array with --bitmap=none
  - verify /sys/block/md0/md/bitmap/location exists and reports "none"
  - mdadm --grow /dev/md0 --bitmap=internal
  - verify location switches to "+8", mdadm reports "Intent Bitmap:
    Internal", and /proc/mdstat reports a bitmap
  - mdadm --grow /dev/md0 --bitmap=none
  - verify location switches back to "none" and only the common location
    attribute remains under md/bitmap
  - repeat the internal/none switch once more
- Checked the QEMU serial log for panic, Oops, BUG, WARNING, Call Trace,
  RCU stall, and hung-task patterns; none were found.

Yu Kuai (3):
  md: factor bitmap creation away from sysfs handling
  md/md-bitmap: split bitmap sysfs groups
  md/md-bitmap: add a none backend for bitmap grow

 drivers/md/md-bitmap.c   | 131 +++++++++++++++++++++++++++++++++++----
 drivers/md/md-bitmap.h   |   2 +-
 drivers/md/md-llbitmap.c |   7 ++-
 drivers/md/md.c          | 125 ++++++++++++++++++++++++++-----------
 drivers/md/md.h          |   3 +
 5 files changed, 218 insertions(+), 50 deletions(-)

base-commit: c85d314b135ff569c1031f2ef8e40368bcfe72ac
-- 
2.51.0

^ permalink raw reply

* [PATCH v13 1/3] md: factor bitmap creation away from sysfs handling
From: Yu Kuai @ 2026-04-25  2:46 UTC (permalink / raw)
  To: Song Liu; +Cc: glass.su, Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260425024615.1696892-1-yukuai@fnnas.com>

Factor bitmap creation and destruction into helpers that do not touch
bitmap sysfs registration.

This prepares the bitmap sysfs rework so callers such as the sysfs
bitmap location path can create or destroy a bitmap backend without
coupling that to sysfs group lifetime management.

Signed-off-by: Yu Kuai <yukuai@fnnas.com>
---
 drivers/md/md.c | 78 +++++++++++++++++++++++++++++++------------------
 1 file changed, 49 insertions(+), 29 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index ebaf47fb9de6..99aa1367c991 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -679,7 +679,25 @@ static void active_io_release(struct percpu_ref *ref)
 
 static void no_op(struct percpu_ref *r) {}
 
-static bool mddev_set_bitmap_ops(struct mddev *mddev)
+static void md_bitmap_sysfs_add(struct mddev *mddev)
+{
+	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);
+}
+
+static void md_bitmap_sysfs_del(struct mddev *mddev)
+{
+	sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
+}
+
+static bool mddev_set_bitmap_ops_nosysfs(struct mddev *mddev)
 {
 	struct bitmap_operations *old = mddev->bitmap_ops;
 	struct md_submodule_head *head;
@@ -703,18 +721,6 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
 
 	mddev->bitmap_ops = (void *)head;
 	xa_unlock(&md_submodule);
-
-	if (!mddev_is_dm(mddev) && 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:
@@ -722,15 +728,6 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
 	return false;
 }
 
-static void mddev_clear_bitmap_ops(struct mddev *mddev)
-{
-	if (!mddev_is_dm(mddev) && 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)
 {
 	int err = 0;
@@ -6531,7 +6528,7 @@ static enum md_submodule_id md_bitmap_get_id_from_sb(struct mddev *mddev)
 	return id;
 }
 
-static int md_bitmap_create(struct mddev *mddev)
+static int md_bitmap_create_nosysfs(struct mddev *mddev)
 {
 	enum md_submodule_id orig_id = mddev->bitmap_id;
 	enum md_submodule_id sb_id;
@@ -6540,7 +6537,7 @@ static int md_bitmap_create(struct mddev *mddev)
 	if (mddev->bitmap_id == ID_BITMAP_NONE)
 		return -EINVAL;
 
-	if (!mddev_set_bitmap_ops(mddev))
+	if (!mddev_set_bitmap_ops_nosysfs(mddev))
 		return -ENOENT;
 
 	err = mddev->bitmap_ops->create(mddev);
@@ -6552,7 +6549,7 @@ static int md_bitmap_create(struct mddev *mddev)
 	 * doesn't match, and mdadm is not the latest version to set
 	 * bitmap_type, set bitmap_ops based on the disk version.
 	 */
-	mddev_clear_bitmap_ops(mddev);
+	mddev->bitmap_ops = NULL;
 
 	sb_id = md_bitmap_get_id_from_sb(mddev);
 	if (sb_id == ID_BITMAP_NONE || sb_id == orig_id)
@@ -6562,27 +6559,50 @@ static int md_bitmap_create(struct mddev *mddev)
 		mdname(mddev), orig_id, sb_id);
 
 	mddev->bitmap_id = sb_id;
-	if (!mddev_set_bitmap_ops(mddev)) {
+	if (!mddev_set_bitmap_ops_nosysfs(mddev)) {
 		mddev->bitmap_id = orig_id;
 		return -ENOENT;
 	}
 
 	err = mddev->bitmap_ops->create(mddev);
 	if (err) {
-		mddev_clear_bitmap_ops(mddev);
+		mddev->bitmap_ops = NULL;
 		mddev->bitmap_id = orig_id;
 	}
 
 	return err;
 }
 
-static void md_bitmap_destroy(struct mddev *mddev)
+static int md_bitmap_create(struct mddev *mddev)
+{
+	int err;
+
+	err = md_bitmap_create_nosysfs(mddev);
+	if (err)
+		return err;
+
+	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group)
+		md_bitmap_sysfs_add(mddev);
+
+	return 0;
+}
+
+static void md_bitmap_destroy_nosysfs(struct mddev *mddev)
 {
 	if (!md_bitmap_registered(mddev))
 		return;
 
 	mddev->bitmap_ops->destroy(mddev);
-	mddev_clear_bitmap_ops(mddev);
+	mddev->bitmap_ops = NULL;
+}
+
+static void md_bitmap_destroy(struct mddev *mddev)
+{
+	if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
+	    mddev->bitmap_ops->group)
+		md_bitmap_sysfs_del(mddev);
+
+	md_bitmap_destroy_nosysfs(mddev);
 }
 
 int md_run(struct mddev *mddev)
-- 
2.51.0


^ permalink raw reply related

* [PATCH v13 2/3] md/md-bitmap: split bitmap sysfs groups
From: Yu Kuai @ 2026-04-25  2:46 UTC (permalink / raw)
  To: Song Liu; +Cc: glass.su, Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260425024615.1696892-1-yukuai@fnnas.com>

Split the classic bitmap sysfs files into a common bitmap group with
the location attribute and a separate internal bitmap group for the
remaining files.

At the same time, convert bitmap operations from a single sysfs group
to a sysfs group array so backends can share part of their sysfs
layout while adding backend-specific attributes separately.

Switch the bitmap sysfs helpers to use sysfs_update_groups() for the
add and update path, and remove groups in reverse order so shared named
groups are unmerged before the last group removes the directory.

Also make bitmap operation lookup depend only on the currently selected
bitmap id matching the installed backend. This prepares the lookup path
for a later registered none backend.

Signed-off-by: Yu Kuai <yukuai@fnnas.com>
---
 drivers/md/md-bitmap.c   | 23 +++++++++++++++++++----
 drivers/md/md-bitmap.h   |  2 +-
 drivers/md/md-llbitmap.c |  7 ++++++-
 drivers/md/md.c          | 21 ++++++++++++++-------
 4 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 83378c033c72..eba649703a1c 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -2955,8 +2955,12 @@ static struct md_sysfs_entry max_backlog_used =
 __ATTR(max_backlog_used, S_IRUGO | S_IWUSR,
        behind_writes_used_show, behind_writes_used_reset);
 
-static struct attribute *md_bitmap_attrs[] = {
+static struct attribute *md_bitmap_common_attrs[] = {
 	&bitmap_location.attr,
+	NULL
+};
+
+static struct attribute *md_bitmap_internal_attrs[] = {
 	&bitmap_space.attr,
 	&bitmap_timeout.attr,
 	&bitmap_backlog.attr,
@@ -2967,9 +2971,20 @@ static struct attribute *md_bitmap_attrs[] = {
 	NULL
 };
 
-static struct attribute_group md_bitmap_group = {
+static struct attribute_group md_bitmap_common_group = {
+	.name = "bitmap",
+	.attrs = md_bitmap_common_attrs,
+};
+
+static struct attribute_group md_bitmap_internal_group = {
 	.name = "bitmap",
-	.attrs = md_bitmap_attrs,
+	.attrs = md_bitmap_internal_attrs,
+};
+
+static const struct attribute_group *bitmap_groups[] = {
+	&md_bitmap_common_group,
+	&md_bitmap_internal_group,
+	NULL,
 };
 
 static struct bitmap_operations bitmap_ops = {
@@ -3013,7 +3028,7 @@ static struct bitmap_operations bitmap_ops = {
 	.set_pages		= bitmap_set_pages,
 	.free			= md_bitmap_free,
 
-	.group			= &md_bitmap_group,
+	.groups			= bitmap_groups,
 };
 
 int md_bitmap_init(void)
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index b42a28fa83a0..214f623c7e79 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -125,7 +125,7 @@ struct bitmap_operations {
 	void (*set_pages)(void *data, unsigned long pages);
 	void (*free)(void *data);
 
-	struct attribute_group *group;
+	const struct attribute_group **groups;
 };
 
 /* the bitmap API */
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 9e7e6b1a6f15..1adc5b117821 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1738,6 +1738,11 @@ static struct attribute_group md_llbitmap_group = {
 	.attrs = md_llbitmap_attrs,
 };
 
+static const struct attribute_group *md_llbitmap_groups[] = {
+	&md_llbitmap_group,
+	NULL,
+};
+
 static struct bitmap_operations llbitmap_ops = {
 	.head = {
 		.type	= MD_BITMAP,
@@ -1774,7 +1779,7 @@ static struct bitmap_operations llbitmap_ops = {
 	.dirty_bits		= llbitmap_dirty_bits,
 	.write_all		= llbitmap_write_all,
 
-	.group			= &md_llbitmap_group,
+	.groups			= md_llbitmap_groups,
 };
 
 int md_llbitmap_init(void)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 99aa1367c991..0ef81d116191 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -681,7 +681,7 @@ static void no_op(struct percpu_ref *r) {}
 
 static void md_bitmap_sysfs_add(struct mddev *mddev)
 {
-	if (sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group))
+	if (sysfs_update_groups(&mddev->kobj, mddev->bitmap_ops->groups))
 		pr_warn("md: cannot register extra bitmap attributes for %s\n",
 			mdname(mddev));
 	else
@@ -694,16 +694,23 @@ static void md_bitmap_sysfs_add(struct mddev *mddev)
 
 static void md_bitmap_sysfs_del(struct mddev *mddev)
 {
-	sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
+	int nr_groups = 0;
+
+	for (nr_groups = 0; mddev->bitmap_ops->groups[nr_groups]; nr_groups++)
+		;
+
+	while (--nr_groups >= 1)
+		sysfs_unmerge_group(&mddev->kobj,
+				    mddev->bitmap_ops->groups[nr_groups]);
+	sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->groups[0]);
 }
 
 static bool mddev_set_bitmap_ops_nosysfs(struct mddev *mddev)
 {
-	struct bitmap_operations *old = mddev->bitmap_ops;
 	struct md_submodule_head *head;
 
-	if (mddev->bitmap_id == ID_BITMAP_NONE ||
-	    (old && old->head.id == mddev->bitmap_id))
+	if (mddev->bitmap_ops &&
+	    mddev->bitmap_ops->head.id == mddev->bitmap_id)
 		return true;
 
 	xa_lock(&md_submodule);
@@ -6581,7 +6588,7 @@ static int md_bitmap_create(struct mddev *mddev)
 	if (err)
 		return err;
 
-	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group)
+	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->groups)
 		md_bitmap_sysfs_add(mddev);
 
 	return 0;
@@ -6599,7 +6606,7 @@ static void md_bitmap_destroy_nosysfs(struct mddev *mddev)
 static void md_bitmap_destroy(struct mddev *mddev)
 {
 	if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
-	    mddev->bitmap_ops->group)
+	    mddev->bitmap_ops->groups)
 		md_bitmap_sysfs_del(mddev);
 
 	md_bitmap_destroy_nosysfs(mddev);
-- 
2.51.0


^ permalink raw reply related

* [PATCH v13 3/3] md/md-bitmap: add a none backend for bitmap grow
From: Yu Kuai @ 2026-04-25  2:46 UTC (permalink / raw)
  To: Song Liu; +Cc: glass.su, Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260425024615.1696892-1-yukuai@fnnas.com>

Add a real none bitmap backend that exposes the common bitmap sysfs
group and use it to keep bitmap/location available when an array has no
bitmap.

Then switch the bitmap location sysfs path to move only between none
and the classic bitmap backend, using the no-sysfs bitmap helpers while
merging or unmerging the internal bitmap sysfs group.

This restores mdadm --grow bitmap addition through bitmap/location.

Signed-off-by: Yu Kuai <yukuai@fnnas.com>
---
 drivers/md/md-bitmap.c | 108 ++++++++++++++++++++++++++++++++++++++---
 drivers/md/md.c        |  42 +++++++++++++---
 drivers/md/md.h        |   3 ++
 3 files changed, 137 insertions(+), 16 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index eba649703a1c..028b9ca8ce52 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -216,6 +216,7 @@ struct bitmap {
 };
 
 static struct workqueue_struct *md_bitmap_wq;
+static struct attribute_group md_bitmap_internal_group;
 
 static int __bitmap_resize(struct bitmap *bitmap, sector_t blocks,
 			   int chunksize, bool init);
@@ -2580,6 +2581,30 @@ static int bitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
 	return __bitmap_resize(bitmap, blocks, chunksize, false);
 }
 
+static bool bitmap_none_enabled(void *data, bool flush)
+{
+	return false;
+}
+
+static int bitmap_none_create(struct mddev *mddev)
+{
+	return 0;
+}
+
+static int bitmap_none_load(struct mddev *mddev)
+{
+	return 0;
+}
+
+static void bitmap_none_destroy(struct mddev *mddev)
+{
+}
+
+static int bitmap_none_get_stats(void *data, struct md_bitmap_stats *stats)
+{
+	return -ENOENT;
+}
+
 static ssize_t
 location_show(struct mddev *mddev, char *page)
 {
@@ -2618,7 +2643,11 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
 			goto out;
 		}
 
-		bitmap_destroy(mddev);
+		sysfs_unmerge_group(&mddev->kobj, &md_bitmap_internal_group);
+		md_bitmap_destroy_nosysfs(mddev);
+		mddev->bitmap_id = ID_BITMAP_NONE;
+		if (!mddev_set_bitmap_ops_nosysfs(mddev))
+			goto none_err;
 		mddev->bitmap_info.offset = 0;
 		if (mddev->bitmap_info.file) {
 			struct file *f = mddev->bitmap_info.file;
@@ -2654,16 +2683,25 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
 			}
 
 			mddev->bitmap_info.offset = offset;
-			rv = bitmap_create(mddev);
+			md_bitmap_destroy_nosysfs(mddev);
+			mddev->bitmap_id = ID_BITMAP;
+			if (!mddev_set_bitmap_ops_nosysfs(mddev))
+				goto bitmap_err;
+
+			rv = md_bitmap_create_nosysfs(mddev);
 			if (rv)
-				goto out;
+				goto create_err;
 
-			rv = bitmap_load(mddev);
+			rv = mddev->bitmap_ops->load(mddev);
 			if (rv) {
 				mddev->bitmap_info.offset = 0;
-				bitmap_destroy(mddev);
-				goto out;
+				goto load_err;
 			}
+
+			rv = sysfs_merge_group(&mddev->kobj,
+					       &md_bitmap_internal_group);
+			if (rv)
+				goto merge_err;
 		}
 	}
 	if (!mddev->external) {
@@ -2679,6 +2717,22 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
 	if (rv)
 		return rv;
 	return len;
+
+merge_err:
+	mddev->bitmap_info.offset = 0;
+load_err:
+	md_bitmap_destroy_nosysfs(mddev);
+create_err:
+	mddev->bitmap_info.offset = 0;
+	mddev->bitmap_id = ID_BITMAP_NONE;
+	if (!mddev_set_bitmap_ops_nosysfs(mddev))
+		rv = -ENOENT;
+	goto out;
+bitmap_err:
+	rv = -ENOENT;
+none_err:
+	mddev->bitmap_info.offset = 0;
+	goto out;
 }
 
 static struct md_sysfs_entry bitmap_location =
@@ -2987,6 +3041,27 @@ static const struct attribute_group *bitmap_groups[] = {
 	NULL,
 };
 
+static const struct attribute_group *bitmap_none_groups[] = {
+	&md_bitmap_common_group,
+	NULL,
+};
+
+static struct bitmap_operations bitmap_none_ops = {
+	.head = {
+		.type	= MD_BITMAP,
+		.id	= ID_BITMAP_NONE,
+		.name	= "none",
+	},
+
+	.enabled		= bitmap_none_enabled,
+	.create			= bitmap_none_create,
+	.load			= bitmap_none_load,
+	.destroy		= bitmap_none_destroy,
+	.get_stats		= bitmap_none_get_stats,
+
+	.groups			= bitmap_none_groups,
+};
+
 static struct bitmap_operations bitmap_ops = {
 	.head = {
 		.type	= MD_BITMAP,
@@ -3033,16 +3108,33 @@ static struct bitmap_operations bitmap_ops = {
 
 int md_bitmap_init(void)
 {
+	int err;
+
 	md_bitmap_wq = alloc_workqueue("md_bitmap", WQ_MEM_RECLAIM | WQ_UNBOUND,
 				       0);
 	if (!md_bitmap_wq)
 		return -ENOMEM;
 
-	return register_md_submodule(&bitmap_ops.head);
+	err = register_md_submodule(&bitmap_none_ops.head);
+	if (err)
+		goto err_wq;
+
+	err = register_md_submodule(&bitmap_ops.head);
+	if (err)
+		goto err_none;
+
+	return 0;
+
+err_none:
+	unregister_md_submodule(&bitmap_none_ops.head);
+err_wq:
+	destroy_workqueue(md_bitmap_wq);
+	return err;
 }
 
 void md_bitmap_exit(void)
 {
-	destroy_workqueue(md_bitmap_wq);
 	unregister_md_submodule(&bitmap_ops.head);
+	unregister_md_submodule(&bitmap_none_ops.head);
+	destroy_workqueue(md_bitmap_wq);
 }
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 0ef81d116191..7937b927d923 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -705,7 +705,7 @@ static void md_bitmap_sysfs_del(struct mddev *mddev)
 	sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->groups[0]);
 }
 
-static bool mddev_set_bitmap_ops_nosysfs(struct mddev *mddev)
+bool mddev_set_bitmap_ops_nosysfs(struct mddev *mddev)
 {
 	struct md_submodule_head *head;
 
@@ -4275,7 +4275,7 @@ bitmap_type_show(struct mddev *mddev, char *page)
 
 	xa_lock(&md_submodule);
 	xa_for_each(&md_submodule, i, head) {
-		if (head->type != MD_BITMAP)
+		if (head->type != MD_BITMAP || head->id == ID_BITMAP_NONE)
 			continue;
 
 		if (mddev->bitmap_id == head->id)
@@ -6535,7 +6535,7 @@ static enum md_submodule_id md_bitmap_get_id_from_sb(struct mddev *mddev)
 	return id;
 }
 
-static int md_bitmap_create_nosysfs(struct mddev *mddev)
+int md_bitmap_create_nosysfs(struct mddev *mddev)
 {
 	enum md_submodule_id orig_id = mddev->bitmap_id;
 	enum md_submodule_id sb_id;
@@ -6544,8 +6544,10 @@ static int md_bitmap_create_nosysfs(struct mddev *mddev)
 	if (mddev->bitmap_id == ID_BITMAP_NONE)
 		return -EINVAL;
 
-	if (!mddev_set_bitmap_ops_nosysfs(mddev))
+	if (!mddev_set_bitmap_ops_nosysfs(mddev)) {
+		mddev->bitmap_id = orig_id;
 		return -ENOENT;
+	}
 
 	err = mddev->bitmap_ops->create(mddev);
 	if (!err)
@@ -6559,8 +6561,10 @@ static int md_bitmap_create_nosysfs(struct mddev *mddev)
 	mddev->bitmap_ops = NULL;
 
 	sb_id = md_bitmap_get_id_from_sb(mddev);
-	if (sb_id == ID_BITMAP_NONE || sb_id == orig_id)
+	if (sb_id == ID_BITMAP_NONE || sb_id == orig_id) {
+		mddev->bitmap_id = orig_id;
 		return err;
+	}
 
 	pr_info("md: %s: bitmap version mismatch, switching from %d to %d\n",
 		mdname(mddev), orig_id, sb_id);
@@ -6594,7 +6598,7 @@ static int md_bitmap_create(struct mddev *mddev)
 	return 0;
 }
 
-static void md_bitmap_destroy_nosysfs(struct mddev *mddev)
+void md_bitmap_destroy_nosysfs(struct mddev *mddev)
 {
 	if (!md_bitmap_registered(mddev))
 		return;
@@ -6612,6 +6616,16 @@ static void md_bitmap_destroy(struct mddev *mddev)
 	md_bitmap_destroy_nosysfs(mddev);
 }
 
+static void md_bitmap_set_none(struct mddev *mddev)
+{
+	mddev->bitmap_id = ID_BITMAP_NONE;
+	if (!mddev_set_bitmap_ops_nosysfs(mddev))
+		return;
+
+	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->groups)
+		md_bitmap_sysfs_add(mddev);
+}
+
 int md_run(struct mddev *mddev)
 {
 	int err;
@@ -6821,6 +6835,10 @@ int md_run(struct mddev *mddev)
 	if (mddev->sb_flags)
 		md_update_sb(mddev, 0);
 
+	if (IS_ENABLED(CONFIG_MD_BITMAP) && !mddev->bitmap_info.file &&
+	    !mddev->bitmap_info.offset)
+		md_bitmap_set_none(mddev);
+
 	md_new_event();
 	return 0;
 
@@ -7766,7 +7784,8 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
 {
 	int err = 0;
 
-	if (!md_bitmap_registered(mddev))
+	if (!md_bitmap_registered(mddev) ||
+	    mddev->bitmap_id == ID_BITMAP_NONE)
 		return -EINVAL;
 
 	if (mddev->pers) {
@@ -7831,10 +7850,12 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
 
 			if (err) {
 				md_bitmap_destroy(mddev);
+				md_bitmap_set_none(mddev);
 				fd = -1;
 			}
 		} else if (fd < 0) {
 			md_bitmap_destroy(mddev);
+			md_bitmap_set_none(mddev);
 		}
 	}
 
@@ -8141,12 +8162,16 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
 				mddev->bitmap_info.default_offset;
 			mddev->bitmap_info.space =
 				mddev->bitmap_info.default_space;
+			mddev->bitmap_id = ID_BITMAP;
 			rv = md_bitmap_create(mddev);
 			if (!rv)
 				rv = mddev->bitmap_ops->load(mddev);
 
-			if (rv)
+			if (rv) {
 				md_bitmap_destroy(mddev);
+				mddev->bitmap_info.offset = 0;
+				md_bitmap_set_none(mddev);
+			}
 		} else {
 			struct md_bitmap_stats stats;
 
@@ -8174,6 +8199,7 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
 			}
 			md_bitmap_destroy(mddev);
 			mddev->bitmap_info.offset = 0;
+			md_bitmap_set_none(mddev);
 		}
 	}
 	md_update_sb(mddev, 1);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index d3d4e2150dc8..52c378086046 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -934,6 +934,9 @@ extern void md_allow_write(struct mddev *mddev);
 extern void md_wait_for_blocked_rdev(struct md_rdev *rdev, struct mddev *mddev);
 extern void md_set_array_sectors(struct mddev *mddev, sector_t array_sectors);
 extern int md_check_no_bitmap(struct mddev *mddev);
+bool mddev_set_bitmap_ops_nosysfs(struct mddev *mddev);
+int md_bitmap_create_nosysfs(struct mddev *mddev);
+void md_bitmap_destroy_nosysfs(struct mddev *mddev);
 extern int md_integrity_register(struct mddev *mddev);
 extern int strict_strtoul_scaled(const char *cp, unsigned long *res, int scale);
 
-- 
2.51.0


^ permalink raw reply related

* Re: [PATCH v13 0/3] md/md-bitmap: restore bitmap grow through sysfs
From: Yu Kuai @ 2026-04-25  2:49 UTC (permalink / raw)
  To: Song Liu; +Cc: glass.su, Li Nan, Xiao Ni, linux-raid, linux-kernel, yukuai
In-Reply-To: <20260425024615.1696892-1-yukuai@fnnas.com>

Hi,

Please just ignore the v13, it's local version by ai that I forgot to remove.

在 2026/4/25 10:46, Yu Kuai 写道:
> mdadm --grow adds an internal bitmap by writing bitmap/location for an
> array that currently has no bitmap. That requires the bitmap directory
> and location attribute to exist before the classic bitmap backend is
> created.
>
> This series separates bitmap backend lifetime from bitmap sysfs lifetime,
> splits the sysfs layout into common and backend-specific groups, and adds
> a small "none" bitmap backend. The none backend keeps bitmap/location
> available while no real bitmap is active, and the location store path can
> then switch between the none backend and the classic bitmap backend
> without tearing down the common bitmap sysfs directory.
>
> Patch 1 factors bitmap creation and destruction into helpers that do not
> touch sysfs registration.
>
> Patch 2 splits the classic bitmap sysfs files into a common group and an
> internal-bitmap group, and converts bitmap backend operations to use a
> sysfs group array.
>
> Patch 3 adds the none backend and uses it to restore mdadm --grow bitmap
> addition through bitmap/location.
>
> Changes since v12:
> - Keep the factoring patch focused on no-sysfs bitmap lifetime helpers.
> - Make bitmap operation lookup depend only on the current bitmap id
>    matching the installed backend.
> - Trim the none backend to only the operations required by the active
>    call paths.
> - Rework bitmap/location error handling with explicit cleanup labels.
> - Restore the none backend after bitmap removal and creation/load
>    failures so bitmap/location stays available.
>
> Validation:
>    - create a RAID1 array with --bitmap=none
>    - verify /sys/block/md0/md/bitmap/location exists and reports "none"
>    - mdadm --grow /dev/md0 --bitmap=internal
>    - verify location switches to "+8", mdadm reports "Intent Bitmap:
>      Internal", and /proc/mdstat reports a bitmap
>    - mdadm --grow /dev/md0 --bitmap=none
>    - verify location switches back to "none" and only the common location
>      attribute remains under md/bitmap
>    - repeat the internal/none switch once more
> - Checked the QEMU serial log for panic, Oops, BUG, WARNING, Call Trace,
>    RCU stall, and hung-task patterns; none were found.
>
> Yu Kuai (3):
>    md: factor bitmap creation away from sysfs handling
>    md/md-bitmap: split bitmap sysfs groups
>    md/md-bitmap: add a none backend for bitmap grow
>
>   drivers/md/md-bitmap.c   | 131 +++++++++++++++++++++++++++++++++++----
>   drivers/md/md-bitmap.h   |   2 +-
>   drivers/md/md-llbitmap.c |   7 ++-
>   drivers/md/md.c          | 125 ++++++++++++++++++++++++++-----------
>   drivers/md/md.h          |   3 +
>   5 files changed, 218 insertions(+), 50 deletions(-)
>
> base-commit: c85d314b135ff569c1031f2ef8e40368bcfe72ac

-- 
Thansk,
Kuai

^ permalink raw reply

* Re: [PATCH v13 1/3] md: factor bitmap creation away from sysfs handling
From: Su Yue @ 2026-04-25  8:30 UTC (permalink / raw)
  To: Yu Kuai; +Cc: Song Liu, glass.su, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260425024615.1696892-2-yukuai@fnnas.com>

On Sat 25 Apr 2026 at 10:46, Yu Kuai <yukuai@fnnas.com> wrote:

> Factor bitmap creation and destruction into helpers that do not 
> touch
> bitmap sysfs registration.
>
> This prepares the bitmap sysfs rework so callers such as the 
> sysfs
> bitmap location path can create or destroy a bitmap backend 
> without
> coupling that to sysfs group lifetime management.
>
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
>
Reviewed-by: Su Yue <glass.su@suse.com>

> ---
>  drivers/md/md.c | 78 
>  +++++++++++++++++++++++++++++++------------------
>  1 file changed, 49 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index ebaf47fb9de6..99aa1367c991 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -679,7 +679,25 @@ static void active_io_release(struct 
> percpu_ref *ref)
>
>  static void no_op(struct percpu_ref *r) {}
>
> -static bool mddev_set_bitmap_ops(struct mddev *mddev)
> +static void md_bitmap_sysfs_add(struct mddev *mddev)
> +{
> +	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);
> +}
> +
> +static void md_bitmap_sysfs_del(struct mddev *mddev)
> +{
> +	sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
> +}
> +
> +static bool mddev_set_bitmap_ops_nosysfs(struct mddev *mddev)
>  {
>  	struct bitmap_operations *old = mddev->bitmap_ops;
>  	struct md_submodule_head *head;
> @@ -703,18 +721,6 @@ static bool mddev_set_bitmap_ops(struct 
> mddev *mddev)
>
>  	mddev->bitmap_ops = (void *)head;
>  	xa_unlock(&md_submodule);
> -
> -	if (!mddev_is_dm(mddev) && 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:
> @@ -722,15 +728,6 @@ static bool mddev_set_bitmap_ops(struct 
> mddev *mddev)
>  	return false;
>  }
>
> -static void mddev_clear_bitmap_ops(struct mddev *mddev)
> -{
> -	if (!mddev_is_dm(mddev) && 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)
>  {
>  	int err = 0;
> @@ -6531,7 +6528,7 @@ static enum md_submodule_id 
> md_bitmap_get_id_from_sb(struct mddev *mddev)
>  	return id;
>  }
>
> -static int md_bitmap_create(struct mddev *mddev)
> +static int md_bitmap_create_nosysfs(struct mddev *mddev)
>  {
>  	enum md_submodule_id orig_id = mddev->bitmap_id;
>  	enum md_submodule_id sb_id;
> @@ -6540,7 +6537,7 @@ static int md_bitmap_create(struct mddev 
> *mddev)
>  	if (mddev->bitmap_id == ID_BITMAP_NONE)
>  		return -EINVAL;
>
> -	if (!mddev_set_bitmap_ops(mddev))
> +	if (!mddev_set_bitmap_ops_nosysfs(mddev))
>  		return -ENOENT;
>
>  	err = mddev->bitmap_ops->create(mddev);
> @@ -6552,7 +6549,7 @@ static int md_bitmap_create(struct mddev 
> *mddev)
>  	 * doesn't match, and mdadm is not the latest version to set
>  	 * bitmap_type, set bitmap_ops based on the disk version.
>  	 */
> -	mddev_clear_bitmap_ops(mddev);
> +	mddev->bitmap_ops = NULL;
>
>  	sb_id = md_bitmap_get_id_from_sb(mddev);
>  	if (sb_id == ID_BITMAP_NONE || sb_id == orig_id)
> @@ -6562,27 +6559,50 @@ static int md_bitmap_create(struct mddev 
> *mddev)
>  		mdname(mddev), orig_id, sb_id);
>
>  	mddev->bitmap_id = sb_id;
> -	if (!mddev_set_bitmap_ops(mddev)) {
> +	if (!mddev_set_bitmap_ops_nosysfs(mddev)) {
>  		mddev->bitmap_id = orig_id;
>  		return -ENOENT;
>  	}
>
>  	err = mddev->bitmap_ops->create(mddev);
>  	if (err) {
> -		mddev_clear_bitmap_ops(mddev);
> +		mddev->bitmap_ops = NULL;
>  		mddev->bitmap_id = orig_id;
>  	}
>
>  	return err;
>  }
>
> -static void md_bitmap_destroy(struct mddev *mddev)
> +static int md_bitmap_create(struct mddev *mddev)
> +{
> +	int err;
> +
> +	err = md_bitmap_create_nosysfs(mddev);
> +	if (err)
> +		return err;
> +
> +	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group)
> +		md_bitmap_sysfs_add(mddev);
> +
> +	return 0;
> +}
> +
> +static void md_bitmap_destroy_nosysfs(struct mddev *mddev)
>  {
>  	if (!md_bitmap_registered(mddev))
>  		return;
>
>  	mddev->bitmap_ops->destroy(mddev);
> -	mddev_clear_bitmap_ops(mddev);
> +	mddev->bitmap_ops = NULL;
> +}
> +
> +static void md_bitmap_destroy(struct mddev *mddev)
> +{
> +	if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
> +	    mddev->bitmap_ops->group)
> +		md_bitmap_sysfs_del(mddev);
> +
> +	md_bitmap_destroy_nosysfs(mddev);
>  }
>
>  int md_run(struct mddev *mddev)

^ permalink raw reply

* Re: [PATCH v13 2/3] md/md-bitmap: split bitmap sysfs groups
From: Su Yue @ 2026-04-25  8:36 UTC (permalink / raw)
  To: Yu Kuai; +Cc: Song Liu, glass.su, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260425024615.1696892-3-yukuai@fnnas.com>

On Sat 25 Apr 2026 at 10:46, Yu Kuai <yukuai@fnnas.com> wrote:

> Split the classic bitmap sysfs files into a common bitmap group 
> with
> the location attribute and a separate internal bitmap group for 
> the
> remaining files.
>
> At the same time, convert bitmap operations from a single sysfs 
> group
> to a sysfs group array so backends can share part of their sysfs
> layout while adding backend-specific attributes separately.
>
> Switch the bitmap sysfs helpers to use sysfs_update_groups() for 
> the
> add and update path, and remove groups in reverse order so 
> shared named
> groups are unmerged before the last group removes the directory.
>
> Also make bitmap operation lookup depend only on the currently 
> selected
> bitmap id matching the installed backend. This prepares the 
> lookup path
> for a later registered none backend.
>
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
> ---
>  drivers/md/md-bitmap.c   | 23 +++++++++++++++++++----
>  drivers/md/md-bitmap.h   |  2 +-
>  drivers/md/md-llbitmap.c |  7 ++++++-
>  drivers/md/md.c          | 21 ++++++++++++++-------
>  4 files changed, 40 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 83378c033c72..eba649703a1c 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -2955,8 +2955,12 @@ static struct md_sysfs_entry 
> max_backlog_used =
>  __ATTR(max_backlog_used, S_IRUGO | S_IWUSR,
>         behind_writes_used_show, behind_writes_used_reset);
>
> -static struct attribute *md_bitmap_attrs[] = {
> +static struct attribute *md_bitmap_common_attrs[] = {
>  	&bitmap_location.attr,
> +	NULL
> +};
> +
> +static struct attribute *md_bitmap_internal_attrs[] = {
>  	&bitmap_space.attr,
>  	&bitmap_timeout.attr,
>  	&bitmap_backlog.attr,
> @@ -2967,9 +2971,20 @@ static struct attribute 
> *md_bitmap_attrs[] = {
>  	NULL
>  };
>
> -static struct attribute_group md_bitmap_group = {
> +static struct attribute_group md_bitmap_common_group = {
> +	.name = "bitmap",
> +	.attrs = md_bitmap_common_attrs,
> +};
> +
> +static struct attribute_group md_bitmap_internal_group = {
>  	.name = "bitmap",
> -	.attrs = md_bitmap_attrs,
> +	.attrs = md_bitmap_internal_attrs,
> +};
> +
> +static const struct attribute_group *bitmap_groups[] = {
> +	&md_bitmap_common_group,
> +	&md_bitmap_internal_group,
> +	NULL,
>  };
>
>  static struct bitmap_operations bitmap_ops = {
> @@ -3013,7 +3028,7 @@ static struct bitmap_operations bitmap_ops 
> = {
>  	.set_pages		= bitmap_set_pages,
>  	.free			= md_bitmap_free,
>
> -	.group			= &md_bitmap_group,
> +	.groups			= bitmap_groups,
>  };
>
>  int md_bitmap_init(void)
> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
> index b42a28fa83a0..214f623c7e79 100644
> --- a/drivers/md/md-bitmap.h
> +++ b/drivers/md/md-bitmap.h
> @@ -125,7 +125,7 @@ struct bitmap_operations {
>  	void (*set_pages)(void *data, unsigned long pages);
>  	void (*free)(void *data);
>
> -	struct attribute_group *group;
> +	const struct attribute_group **groups;
>  };
>
>  /* the bitmap API */
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 9e7e6b1a6f15..1adc5b117821 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1738,6 +1738,11 @@ static struct attribute_group 
> md_llbitmap_group = {
>  	.attrs = md_llbitmap_attrs,
>  };
>
> +static const struct attribute_group *md_llbitmap_groups[] = {
> +	&md_llbitmap_group,
> +	NULL,
> +};
> +
>  static struct bitmap_operations llbitmap_ops = {
>  	.head = {
>  		.type	= MD_BITMAP,
> @@ -1774,7 +1779,7 @@ static struct bitmap_operations 
> llbitmap_ops = {
>  	.dirty_bits		= llbitmap_dirty_bits,
>  	.write_all		= llbitmap_write_all,
>
> -	.group			= &md_llbitmap_group,
> +	.groups			= md_llbitmap_groups,
>  };
>
>  int md_llbitmap_init(void)
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 99aa1367c991..0ef81d116191 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -681,7 +681,7 @@ static void no_op(struct percpu_ref *r) {}
>
>  static void md_bitmap_sysfs_add(struct mddev *mddev)
>  {
> -	if (sysfs_create_group(&mddev->kobj, 
> mddev->bitmap_ops->group))
> +	if (sysfs_update_groups(&mddev->kobj, 
> mddev->bitmap_ops->groups))
>  		pr_warn("md: cannot register extra bitmap attributes for 
>  %s\n",
>  			mdname(mddev));
>  	else
> @@ -694,16 +694,23 @@ static void md_bitmap_sysfs_add(struct 
> mddev *mddev)
>
>  static void md_bitmap_sysfs_del(struct mddev *mddev)
>  {
> -	sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
> +	int nr_groups = 0;
> +
> +	for (nr_groups = 0; mddev->bitmap_ops->groups[nr_groups]; 
> nr_groups++)
> +		;
> +
> +	while (--nr_groups >= 1)
> +		sysfs_unmerge_group(&mddev->kobj,
> +				    mddev->bitmap_ops->groups[nr_groups]);
>
Amazing magic here!

Reviewed-by: Su Yue <glass.su@suse.com>

> +	sysfs_remove_group(&mddev->kobj, 
> mddev->bitmap_ops->groups[0]);
>  }
>
>  static bool mddev_set_bitmap_ops_nosysfs(struct mddev *mddev)
>  {
> -	struct bitmap_operations *old = mddev->bitmap_ops;
>  	struct md_submodule_head *head;
>
> -	if (mddev->bitmap_id == ID_BITMAP_NONE ||
> -	    (old && old->head.id == mddev->bitmap_id))
> +	if (mddev->bitmap_ops &&
> +	    mddev->bitmap_ops->head.id == mddev->bitmap_id)
>  		return true;
>
>  	xa_lock(&md_submodule);
> @@ -6581,7 +6588,7 @@ static int md_bitmap_create(struct mddev 
> *mddev)
>  	if (err)
>  		return err;
>
> -	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group)
> +	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->groups)
>  		md_bitmap_sysfs_add(mddev);
>
>  	return 0;
> @@ -6599,7 +6606,7 @@ static void 
> md_bitmap_destroy_nosysfs(struct mddev *mddev)
>  static void md_bitmap_destroy(struct mddev *mddev)
>  {
>  	if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
> -	    mddev->bitmap_ops->group)
> +	    mddev->bitmap_ops->groups)
>  		md_bitmap_sysfs_del(mddev);
>
>  	md_bitmap_destroy_nosysfs(mddev);

^ permalink raw reply

* Re: [PATCH v13 3/3] md/md-bitmap: add a none backend for bitmap grow
From: Su Yue @ 2026-04-25  8:39 UTC (permalink / raw)
  To: Yu Kuai; +Cc: Song Liu, glass.su, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260425024615.1696892-4-yukuai@fnnas.com>

On Sat 25 Apr 2026 at 10:46, Yu Kuai <yukuai@fnnas.com> wrote:

> Add a real none bitmap backend that exposes the common bitmap 
> sysfs
> group and use it to keep bitmap/location available when an array 
> has no
> bitmap.
>
> Then switch the bitmap location sysfs path to move only between 
> none
> and the classic bitmap backend, using the no-sysfs bitmap 
> helpers while
> merging or unmerging the internal bitmap sysfs group.
>
> This restores mdadm --grow bitmap addition through 
> bitmap/location.
>
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
>

Reviewed-by: Su Yue <glass.su@suse.com>

> ---
>  drivers/md/md-bitmap.c | 108 
>  ++++++++++++++++++++++++++++++++++++++---
>  drivers/md/md.c        |  42 +++++++++++++---
>  drivers/md/md.h        |   3 ++
>  3 files changed, 137 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index eba649703a1c..028b9ca8ce52 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -216,6 +216,7 @@ struct bitmap {
>  };
>
>  static struct workqueue_struct *md_bitmap_wq;
> +static struct attribute_group md_bitmap_internal_group;
>
>  static int __bitmap_resize(struct bitmap *bitmap, sector_t 
>  blocks,
>  			   int chunksize, bool init);
> @@ -2580,6 +2581,30 @@ static int bitmap_resize(struct mddev 
> *mddev, sector_t blocks, int chunksize)
>  	return __bitmap_resize(bitmap, blocks, chunksize, false);
>  }
>
> +static bool bitmap_none_enabled(void *data, bool flush)
> +{
> +	return false;
> +}
> +
> +static int bitmap_none_create(struct mddev *mddev)
> +{
> +	return 0;
> +}
> +
> +static int bitmap_none_load(struct mddev *mddev)
> +{
> +	return 0;
> +}
> +
> +static void bitmap_none_destroy(struct mddev *mddev)
> +{
> +}
> +
> +static int bitmap_none_get_stats(void *data, struct 
> md_bitmap_stats *stats)
> +{
> +	return -ENOENT;
> +}
> +
>  static ssize_t
>  location_show(struct mddev *mddev, char *page)
>  {
> @@ -2618,7 +2643,11 @@ location_store(struct mddev *mddev, const 
> char *buf, size_t len)
>  			goto out;
>  		}
>
> -		bitmap_destroy(mddev);
> +		sysfs_unmerge_group(&mddev->kobj, 
> &md_bitmap_internal_group);
> +		md_bitmap_destroy_nosysfs(mddev);
> +		mddev->bitmap_id = ID_BITMAP_NONE;
> +		if (!mddev_set_bitmap_ops_nosysfs(mddev))
> +			goto none_err;
>  		mddev->bitmap_info.offset = 0;
>  		if (mddev->bitmap_info.file) {
>  			struct file *f = mddev->bitmap_info.file;
> @@ -2654,16 +2683,25 @@ location_store(struct mddev *mddev, 
> const char *buf, size_t len)
>  			}
>
>  			mddev->bitmap_info.offset = offset;
> -			rv = bitmap_create(mddev);
> +			md_bitmap_destroy_nosysfs(mddev);
> +			mddev->bitmap_id = ID_BITMAP;
> +			if (!mddev_set_bitmap_ops_nosysfs(mddev))
> +				goto bitmap_err;
> +
> +			rv = md_bitmap_create_nosysfs(mddev);
>  			if (rv)
> -				goto out;
> +				goto create_err;
>
> -			rv = bitmap_load(mddev);
> +			rv = mddev->bitmap_ops->load(mddev);
>  			if (rv) {
>  				mddev->bitmap_info.offset = 0;
> -				bitmap_destroy(mddev);
> -				goto out;
> +				goto load_err;
>  			}
> +
> +			rv = sysfs_merge_group(&mddev->kobj,
> +					       &md_bitmap_internal_group);
> +			if (rv)
> +				goto merge_err;
>  		}
>  	}
>  	if (!mddev->external) {
> @@ -2679,6 +2717,22 @@ location_store(struct mddev *mddev, const 
> char *buf, size_t len)
>  	if (rv)
>  		return rv;
>  	return len;
> +
> +merge_err:
> +	mddev->bitmap_info.offset = 0;
> +load_err:
> +	md_bitmap_destroy_nosysfs(mddev);
> +create_err:
> +	mddev->bitmap_info.offset = 0;
> +	mddev->bitmap_id = ID_BITMAP_NONE;
> +	if (!mddev_set_bitmap_ops_nosysfs(mddev))
> +		rv = -ENOENT;
> +	goto out;
> +bitmap_err:
> +	rv = -ENOENT;
> +none_err:
> +	mddev->bitmap_info.offset = 0;
> +	goto out;
>  }
>
>  static struct md_sysfs_entry bitmap_location =
> @@ -2987,6 +3041,27 @@ static const struct attribute_group 
> *bitmap_groups[] = {
>  	NULL,
>  };
>
> +static const struct attribute_group *bitmap_none_groups[] = {
> +	&md_bitmap_common_group,
> +	NULL,
> +};
> +
> +static struct bitmap_operations bitmap_none_ops = {
> +	.head = {
> +		.type	= MD_BITMAP,
> +		.id	= ID_BITMAP_NONE,
> +		.name	= "none",
> +	},
> +
> +	.enabled		= bitmap_none_enabled,
> +	.create			= bitmap_none_create,
> +	.load			= bitmap_none_load,
> +	.destroy		= bitmap_none_destroy,
> +	.get_stats		= bitmap_none_get_stats,
> +
> +	.groups			= bitmap_none_groups,
> +};
> +
>  static struct bitmap_operations bitmap_ops = {
>  	.head = {
>  		.type	= MD_BITMAP,
> @@ -3033,16 +3108,33 @@ static struct bitmap_operations 
> bitmap_ops = {
>
>  int md_bitmap_init(void)
>  {
> +	int err;
> +
>  	md_bitmap_wq = alloc_workqueue("md_bitmap", WQ_MEM_RECLAIM | 
>  WQ_UNBOUND,
>  				       0);
>  	if (!md_bitmap_wq)
>  		return -ENOMEM;
>
> -	return register_md_submodule(&bitmap_ops.head);
> +	err = register_md_submodule(&bitmap_none_ops.head);
> +	if (err)
> +		goto err_wq;
> +
> +	err = register_md_submodule(&bitmap_ops.head);
> +	if (err)
> +		goto err_none;
> +
> +	return 0;
> +
> +err_none:
> +	unregister_md_submodule(&bitmap_none_ops.head);
> +err_wq:
> +	destroy_workqueue(md_bitmap_wq);
> +	return err;
>  }
>
>  void md_bitmap_exit(void)
>  {
> -	destroy_workqueue(md_bitmap_wq);
>  	unregister_md_submodule(&bitmap_ops.head);
> +	unregister_md_submodule(&bitmap_none_ops.head);
> +	destroy_workqueue(md_bitmap_wq);
>  }
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 0ef81d116191..7937b927d923 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -705,7 +705,7 @@ static void md_bitmap_sysfs_del(struct mddev 
> *mddev)
>  	sysfs_remove_group(&mddev->kobj, 
>  mddev->bitmap_ops->groups[0]);
>  }
>
> -static bool mddev_set_bitmap_ops_nosysfs(struct mddev *mddev)
> +bool mddev_set_bitmap_ops_nosysfs(struct mddev *mddev)
>  {
>  	struct md_submodule_head *head;
>
> @@ -4275,7 +4275,7 @@ bitmap_type_show(struct mddev *mddev, char 
> *page)
>
>  	xa_lock(&md_submodule);
>  	xa_for_each(&md_submodule, i, head) {
> -		if (head->type != MD_BITMAP)
> +		if (head->type != MD_BITMAP || head->id == ID_BITMAP_NONE)
>  			continue;
>
>  		if (mddev->bitmap_id == head->id)
> @@ -6535,7 +6535,7 @@ static enum md_submodule_id 
> md_bitmap_get_id_from_sb(struct mddev *mddev)
>  	return id;
>  }
>
> -static int md_bitmap_create_nosysfs(struct mddev *mddev)
> +int md_bitmap_create_nosysfs(struct mddev *mddev)
>  {
>  	enum md_submodule_id orig_id = mddev->bitmap_id;
>  	enum md_submodule_id sb_id;
> @@ -6544,8 +6544,10 @@ static int 
> md_bitmap_create_nosysfs(struct mddev *mddev)
>  	if (mddev->bitmap_id == ID_BITMAP_NONE)
>  		return -EINVAL;
>
> -	if (!mddev_set_bitmap_ops_nosysfs(mddev))
> +	if (!mddev_set_bitmap_ops_nosysfs(mddev)) {
> +		mddev->bitmap_id = orig_id;
>  		return -ENOENT;
> +	}
>
>  	err = mddev->bitmap_ops->create(mddev);
>  	if (!err)
> @@ -6559,8 +6561,10 @@ static int 
> md_bitmap_create_nosysfs(struct mddev *mddev)
>  	mddev->bitmap_ops = NULL;
>
>  	sb_id = md_bitmap_get_id_from_sb(mddev);
> -	if (sb_id == ID_BITMAP_NONE || sb_id == orig_id)
> +	if (sb_id == ID_BITMAP_NONE || sb_id == orig_id) {
> +		mddev->bitmap_id = orig_id;
>  		return err;
> +	}
>
>  	pr_info("md: %s: bitmap version mismatch, switching from %d to 
>  %d\n",
>  		mdname(mddev), orig_id, sb_id);
> @@ -6594,7 +6598,7 @@ static int md_bitmap_create(struct mddev 
> *mddev)
>  	return 0;
>  }
>
> -static void md_bitmap_destroy_nosysfs(struct mddev *mddev)
> +void md_bitmap_destroy_nosysfs(struct mddev *mddev)
>  {
>  	if (!md_bitmap_registered(mddev))
>  		return;
> @@ -6612,6 +6616,16 @@ static void md_bitmap_destroy(struct 
> mddev *mddev)
>  	md_bitmap_destroy_nosysfs(mddev);
>  }
>
> +static void md_bitmap_set_none(struct mddev *mddev)
> +{
> +	mddev->bitmap_id = ID_BITMAP_NONE;
> +	if (!mddev_set_bitmap_ops_nosysfs(mddev))
> +		return;
> +
> +	if (!mddev_is_dm(mddev) && mddev->bitmap_ops->groups)
> +		md_bitmap_sysfs_add(mddev);
> +}
> +
>  int md_run(struct mddev *mddev)
>  {
>  	int err;
> @@ -6821,6 +6835,10 @@ int md_run(struct mddev *mddev)
>  	if (mddev->sb_flags)
>  		md_update_sb(mddev, 0);
>
> +	if (IS_ENABLED(CONFIG_MD_BITMAP) && !mddev->bitmap_info.file 
> &&
> +	    !mddev->bitmap_info.offset)
> +		md_bitmap_set_none(mddev);
> +
>  	md_new_event();
>  	return 0;
>
> @@ -7766,7 +7784,8 @@ static int set_bitmap_file(struct mddev 
> *mddev, int fd)
>  {
>  	int err = 0;
>
> -	if (!md_bitmap_registered(mddev))
> +	if (!md_bitmap_registered(mddev) ||
> +	    mddev->bitmap_id == ID_BITMAP_NONE)
>  		return -EINVAL;
>
>  	if (mddev->pers) {
> @@ -7831,10 +7850,12 @@ static int set_bitmap_file(struct mddev 
> *mddev, int fd)
>
>  			if (err) {
>  				md_bitmap_destroy(mddev);
> +				md_bitmap_set_none(mddev);
>  				fd = -1;
>  			}
>  		} else if (fd < 0) {
>  			md_bitmap_destroy(mddev);
> +			md_bitmap_set_none(mddev);
>  		}
>  	}
>
> @@ -8141,12 +8162,16 @@ static int update_array_info(struct 
> mddev *mddev, mdu_array_info_t *info)
>  				mddev->bitmap_info.default_offset;
>  			mddev->bitmap_info.space =
>  				mddev->bitmap_info.default_space;
> +			mddev->bitmap_id = ID_BITMAP;
>  			rv = md_bitmap_create(mddev);
>  			if (!rv)
>  				rv = mddev->bitmap_ops->load(mddev);
>
> -			if (rv)
> +			if (rv) {
>  				md_bitmap_destroy(mddev);
> +				mddev->bitmap_info.offset = 0;
> +				md_bitmap_set_none(mddev);
> +			}
>  		} else {
>  			struct md_bitmap_stats stats;
>
> @@ -8174,6 +8199,7 @@ static int update_array_info(struct mddev 
> *mddev, mdu_array_info_t *info)
>  			}
>  			md_bitmap_destroy(mddev);
>  			mddev->bitmap_info.offset = 0;
> +			md_bitmap_set_none(mddev);
>  		}
>  	}
>  	md_update_sb(mddev, 1);
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index d3d4e2150dc8..52c378086046 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -934,6 +934,9 @@ extern void md_allow_write(struct mddev 
> *mddev);
>  extern void md_wait_for_blocked_rdev(struct md_rdev *rdev, 
>  struct mddev *mddev);
>  extern void md_set_array_sectors(struct mddev *mddev, sector_t 
>  array_sectors);
>  extern int md_check_no_bitmap(struct mddev *mddev);
> +bool mddev_set_bitmap_ops_nosysfs(struct mddev *mddev);
> +int md_bitmap_create_nosysfs(struct mddev *mddev);
> +void md_bitmap_destroy_nosysfs(struct mddev *mddev);
>  extern int md_integrity_register(struct mddev *mddev);
>  extern int strict_strtoul_scaled(const char *cp, unsigned long 
>  *res, int scale);

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox