* Re: [PATCH v3] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: Eric Biggers @ 2026-06-15 20:10 UTC (permalink / raw)
To: x86
Cc: Christoph Hellwig, linux-crypto, David Laight, linux-raid,
Andrew Morton, linux-kernel
In-Reply-To: <20260615190338.26581-1-ebiggers@kernel.org>
On Mon, Jun 15, 2026 at 12:03:38PM -0700, Eric Biggers wrote:
> Note: for now I omitted the cpu_has_xfeatures() check that the AVX-512
> optimized crypto and CRC code does, since it's not implemented on
> User-Mode Linux and it's never been present in the RAID6 code either.
By the way, Sashiko keeps complaining about this decision.
Maybe the x86 maintainers have some advice here?
For context: on x86 processors, executing AVX or AVX512 instructions
requires not just that the CPU supports the feature, but also that the
operating system has set certain bits in XCR0. For example all EVEX
coded instructions (i.e. AVX-512) require XCR0=111xx111b. (See Intel
manual "2.6.11.1 State Dependent #UD".)
Therefore most of the kernel's AVX and AVX512 optimized code checks not
just X86_FEATURE_AVX* but also calls cpu_has_xfeatures() to check XCR0.
But "most" isn't all. The RAID6 code for example doesn't check
cpu_has_xfeatures(). So if you e.g. boot a kernel in QEMU using
"-cpu max,xsave=off", it already crashes when the RAID6 code does its
boot-time benchmark.
Part of the reason for that omission probably is that UML doesn't
provide an implementation of cpu_has_xfeatures(). And the x86 RAID (XOR
and RAID6) code is enabled on UML.
It could be implemented for UML by using the xgetbv instruction, like
what userspace programs do. (We'd also need to copy the XFEATURE_MASK_*
constants, as UML can't include arch/x86/include/asm/fpu/types.h)
But I wanted to ask: do we really care about the case where features are
"supported" but their XCR0 bits aren't set? Perhaps the kernel just
doesn't/shouldn't support weird cases like "-cpu max,xsave=off"?
If this case indeed needs to be handled, could we make things easier for
the kernel's AVX and AVX-512 optimized code? Currently AVX-512 needs:
if (boot_cpu_has(X86_FEATURE_AVX512F) &&
cpu_has_xfeatures(XFEATURE_MASK_FP | XFEATURE_MASK_SSE |
XFEATURE_MASK_YMM | XFEATURE_MASK_AVX512, NULL))
How about we make X86_FEATURE_AVX512F depend on XCR0=111xx111, and
X86_FEATURE_AVX depend on XCR0=xxxxx111? Then the cpu_has_xfeatures()
check wouldn't be needed. Is there any reason not to do that?
- Eric
^ permalink raw reply
* Re: [PATCH v3] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: sashiko-bot @ 2026-06-15 19:25 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-raid, yukuai
In-Reply-To: <20260615190338.26581-1-ebiggers@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The AVX-512 XOR implementation is forced based solely on `boot_cpu_has(X86_FEATURE_AVX512F)` without checking if the OS actually manages ZMM registers via `cpu_has_xfeatures()`. This introduces an Undefined Opcode (#UD) kernel panic vulnerability on systems where AVX-512 is supported by the CPU but restricted in XCR0.
--
commit b63aaca8e8b386ef1820d1caac3bce2cb328e8c3
Author: Eric Biggers <ebiggers@kernel.org>
lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
This commit adds an AVX-512 implementation of xor_gen() using 512-bit ZMM
registers and the vpternlogq instruction. It enables this implementation on
x86_64 CPUs that support AVX512F but do not prefer YMM registers, avoiding
known downclocking issues on older architectures. The implementation
explicitly omits the cpu_has_xfeatures() check.
> Note: for now I omitted the cpu_has_xfeatures() check that the AVX-512
> optimized crypto and CRC code does, since it's not implemented on
> User-Mode Linux and it's never been present in the RAID6 code either.
[Severity: High]
Does omitting the cpu_has_xfeatures() check introduce a potential Invalid
Opcode exception in virtualized environments?
If a hypervisor advertises AVX512F in CPUID but omits the AVX-512 FPU
extended state from XSTATE, boot_cpu_has(X86_FEATURE_AVX512F) will evaluate
to true, but the OS will not enable AVX-512 in XCR0. Executing 512-bit
EVEX instructions without XCR0[7:5] enabled causes an Invalid Opcode
exception and kernel panic.
Would it be safer to preserve this check, perhaps providing a stub or
using ifdef for User-Mode Linux compatibility instead?
> diff --git a/lib/raid/xor/x86/xor_arch.h b/lib/raid/xor/x86/xor_arch.h
> index 99fe85a213c66..b5d49376fc974 100644
> --- a/lib/raid/xor/x86/xor_arch.h
> +++ b/lib/raid/xor/x86/xor_arch.h
[ ... ]
> @@ -6,22 +6,26 @@ extern struct xor_block_template xor_block_p5_mmx;
> extern struct xor_block_template xor_block_sse;
> extern struct xor_block_template xor_block_sse_pf64;
> extern struct xor_block_template xor_block_avx;
> +extern struct xor_block_template xor_block_avx512;
>
> static __always_inline void __init arch_xor_init(void)
> {
> - if (boot_cpu_has(X86_FEATURE_AVX) &&
> - boot_cpu_has(X86_FEATURE_OSXSAVE)) {
> + if (IS_ENABLED(CONFIG_X86_64) && boot_cpu_has(X86_FEATURE_AVX512F) &&
> + boot_cpu_has(X86_FEATURE_OSXSAVE) &&
> + !boot_cpu_has(X86_FEATURE_PREFER_YMM)) {
> + /* AVX-512 will be the best; no need to try others. */
> + /* !PREFER_YMM excludes CPUs with overly-eager downclocking. */
> + xor_force(&xor_block_avx512);
[Severity: High]
Can this force the AVX-512 implementation on systems that do not actually
support ZMM registers in the OS?
During boot, fpu__init_system_xstate() limits fpu_kernel_cfg.max_features
based on the XSTATE leaf, but does not clear X86_FEATURE_AVX512F from
boot_cpu_data.x86_capability. As a result, this check passes and forces
xor_block_avx512, which then attempts to execute AVX-512 instructions on
an unsupported FPU context.
Is it necessary to include cpu_has_xfeatures(XFEATURE_MASK_AVX512, NULL)
here to verify the FPU context supports these instructions before forcing
the block?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615190338.26581-1-ebiggers@kernel.org?part=1
^ permalink raw reply
* [PATCH v3] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: Eric Biggers @ 2026-06-15 19:03 UTC (permalink / raw)
To: Andrew Morton, linux-kernel
Cc: Christoph Hellwig, linux-crypto, x86, Eric Biggers, David Laight,
linux-raid
Add an implementation of xor_gen() using AVX-512.
It uses 512-bit vectors, i.e. ZMM registers. It also uses the
vpternlogq instruction to do three-input XORs when applicable.
It's enabled on x86_64 CPUs that have AVX512F && !PREFER_YMM. In
practice that means:
- AMD Zen 4 and later (client and server)
- Intel Sapphire Rapids and later (server)
- Intel Rocket Lake (client)
- Intel Nova Lake and later (client)
The !PREFER_YMM condition excludes the older AVX-512 implementations in
Intel Skylake Server and Intel Ice Lake. They could run this code, but
they're known to have overly-eager downclocking when ZMM registers are
used. This is the same policy that the crypto and CRC code uses.
Benchmark on AMD Ryzen 9 9950X (Zen 5):
src_cnt avx avx512 Improvement
======= ========== ========== ===========
1 56353 MB/s 75388 MB/s 33%
2 54274 MB/s 68409 MB/s 26%
3 44649 MB/s 64042 MB/s 43%
4 41315 MB/s 55002 MB/s 33%
Note: for now I omitted the cpu_has_xfeatures() check that the AVX-512
optimized crypto and CRC code does, since it's not implemented on
User-Mode Linux and it's never been present in the RAID6 code either.
Reviewed-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
Changed in v3:
- Renamed p0-p4 to p1-p5 to match the inline asm indices
- Swapped the base and index registers to the logical order
- Added David's Reviewed-by
Changed in v2:
- Fixed build on UML
- Reworked the implementation
lib/raid/xor/Makefile | 2 +-
lib/raid/xor/x86/xor-avx512.c | 121 ++++++++++++++++++++++++++++++++++
lib/raid/xor/x86/xor_arch.h | 26 ++++----
3 files changed, 137 insertions(+), 12 deletions(-)
create mode 100644 lib/raid/xor/x86/xor-avx512.c
diff --git a/lib/raid/xor/Makefile b/lib/raid/xor/Makefile
index 4d633dfd5b90..4af945861a51 100644
--- a/lib/raid/xor/Makefile
+++ b/lib/raid/xor/Makefile
@@ -26,11 +26,11 @@ xor-$(CONFIG_ALTIVEC) += powerpc/xor_vmx.o powerpc/xor_vmx_glue.o
xor-$(CONFIG_RISCV_ISA_V) += riscv/xor.o riscv/xor-glue.o
xor-$(CONFIG_SPARC32) += sparc/xor-sparc32.o
xor-$(CONFIG_SPARC64) += sparc/xor-sparc64.o sparc/xor-sparc64-glue.o
xor-$(CONFIG_S390) += s390/xor.o
xor-$(CONFIG_X86_32) += x86/xor-avx.o x86/xor-sse.o x86/xor-mmx.o
-xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o
+xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o x86/xor-avx512.o
obj-y += tests/
CFLAGS_arm/xor-neon.o += $(CC_FLAGS_FPU)
CFLAGS_REMOVE_arm/xor-neon.o += $(CC_FLAGS_NO_FPU)
diff --git a/lib/raid/xor/x86/xor-avx512.c b/lib/raid/xor/x86/xor-avx512.c
new file mode 100644
index 000000000000..17f57900d827
--- /dev/null
+++ b/lib/raid/xor/x86/xor-avx512.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AVX-512 optimized implementation of xor_gen()
+ *
+ * Copyright 2026 Google LLC
+ */
+
+#include <linux/types.h>
+#include <asm/fpu/api.h>
+#include "xor_impl.h"
+#include "xor_arch.h"
+
+/*
+ * Implementation notes:
+ *
+ * Unrolling by the number of buffers (2-5) is very important.
+ *
+ * Unrolling by length is less important, especially when using register-indexed
+ * addressing with negative indices from the end of the buffers. That approach
+ * results in just two loop control instructions being needed per iteration,
+ * regardless of the number of buffers.
+ *
+ * In fact, benchmarks showed that the 2 and 3 buffer cases require only 2x
+ * unrolling by length, while the 4 and 5 buffer cases don't require any
+ * unrolling by length. Benchmarks also showed that the register-indexed
+ * addressing isn't a bottleneck either; i.e., we can't do any better by
+ * incrementing the pointers as we go along, even with more unrolling.
+ */
+
+static void xor_avx512_2(long bytes, u8 *p1, const u8 *p2)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%1,%0), %%zmm0\n"
+ "vmovdqa64 64(%1,%0), %%zmm1\n"
+ "vpxorq (%2,%0), %%zmm0, %%zmm0\n"
+ "vpxorq 64(%2,%0), %%zmm1, %%zmm1\n"
+ "vmovdqa64 %%zmm0, (%1,%0)\n"
+ "vmovdqa64 %%zmm1, 64(%1,%0)\n"
+ "add $128, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p1 + bytes), "r"(p2 + bytes)
+ : "memory", "cc");
+}
+
+static void xor_avx512_3(long bytes, u8 *p1, const u8 *p2, const u8 *p3)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%1,%0), %%zmm0\n"
+ "vmovdqa64 64(%1,%0), %%zmm1\n"
+ "vmovdqa64 (%2,%0), %%zmm2\n"
+ "vmovdqa64 64(%2,%0), %%zmm3\n"
+ "vpternlogq $0x96, (%3,%0), %%zmm2, %%zmm0\n"
+ "vpternlogq $0x96, 64(%3,%0), %%zmm3, %%zmm1\n"
+ "vmovdqa64 %%zmm0, (%1,%0)\n"
+ "vmovdqa64 %%zmm1, 64(%1,%0)\n"
+ "add $128, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p1 + bytes), "r"(p2 + bytes), "r"(p3 + bytes)
+ : "memory", "cc");
+}
+
+static void xor_avx512_4(long bytes, u8 *p1, const u8 *p2, const u8 *p3,
+ const u8 *p4)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%1,%0), %%zmm0\n"
+ "vmovdqa64 (%2,%0), %%zmm1\n"
+ "vpxorq (%3,%0), %%zmm0, %%zmm0\n"
+ "vpternlogq $0x96, (%4,%0), %%zmm1, %%zmm0\n"
+ "vmovdqa64 %%zmm0, (%1,%0)\n"
+ "add $64, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p1 + bytes), "r"(p2 + bytes), "r"(p3 + bytes),
+ "r"(p4 + bytes)
+ : "memory", "cc");
+}
+
+static void xor_avx512_5(long bytes, u8 *p1, const u8 *p2, const u8 *p3,
+ const u8 *p4, const u8 *p5)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%1,%0), %%zmm0\n"
+ "vmovdqa64 (%2,%0), %%zmm1\n"
+ "vpternlogq $0x96, (%3,%0), %%zmm1, %%zmm0\n"
+ "vmovdqa64 (%4,%0), %%zmm1\n"
+ "vpternlogq $0x96, (%5,%0), %%zmm1, %%zmm0\n"
+ "vmovdqa64 %%zmm0, (%1,%0)\n"
+ "add $64, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p1 + bytes), "r"(p2 + bytes), "r"(p3 + bytes),
+ "r"(p4 + bytes), "r"(p5 + bytes)
+ : "memory", "cc");
+}
+
+DO_XOR_BLOCKS(avx512_inner, xor_avx512_2, xor_avx512_3, xor_avx512_4,
+ xor_avx512_5);
+
+/*
+ * Preconditions: bytes is a nonzero multiple of 512, and all buffers are
+ * 64-byte aligned.
+ */
+static void xor_gen_avx512(void *dest, void **srcs, unsigned int src_cnt,
+ unsigned int bytes)
+{
+ kernel_fpu_begin();
+ xor_gen_avx512_inner(dest, srcs, src_cnt, bytes);
+ kernel_fpu_end();
+}
+
+struct xor_block_template xor_block_avx512 = {
+ .name = "avx512",
+ .xor_gen = xor_gen_avx512,
+};
diff --git a/lib/raid/xor/x86/xor_arch.h b/lib/raid/xor/x86/xor_arch.h
index 99fe85a213c6..b5d49376fc97 100644
--- a/lib/raid/xor/x86/xor_arch.h
+++ b/lib/raid/xor/x86/xor_arch.h
@@ -4,26 +4,30 @@
extern struct xor_block_template xor_block_pII_mmx;
extern struct xor_block_template xor_block_p5_mmx;
extern struct xor_block_template xor_block_sse;
extern struct xor_block_template xor_block_sse_pf64;
extern struct xor_block_template xor_block_avx;
+extern struct xor_block_template xor_block_avx512;
-/*
- * When SSE is available, use it as it can write around L2. We may also be able
- * to load into the L1 only depending on how the cpu deals with a load to a line
- * that is being prefetched.
- *
- * When AVX2 is available, force using it as it is better by all measures.
- *
- * 32-bit without MMX can fall back to the generic routines.
- */
static __always_inline void __init arch_xor_init(void)
{
- if (boot_cpu_has(X86_FEATURE_AVX) &&
- boot_cpu_has(X86_FEATURE_OSXSAVE)) {
+ if (IS_ENABLED(CONFIG_X86_64) && boot_cpu_has(X86_FEATURE_AVX512F) &&
+ boot_cpu_has(X86_FEATURE_OSXSAVE) &&
+ !boot_cpu_has(X86_FEATURE_PREFER_YMM)) {
+ /* AVX-512 will be the best; no need to try others. */
+ /* !PREFER_YMM excludes CPUs with overly-eager downclocking. */
+ xor_force(&xor_block_avx512);
+ } else if (boot_cpu_has(X86_FEATURE_AVX) &&
+ boot_cpu_has(X86_FEATURE_OSXSAVE)) {
+ /* AVX will be the best; no need to try others. */
xor_force(&xor_block_avx);
} else if (IS_ENABLED(CONFIG_X86_64) || boot_cpu_has(X86_FEATURE_XMM)) {
+ /*
+ * When SSE is available, use it as it can write around L2. We
+ * may also be able to load into the L1 only depending on how
+ * the cpu deals with a load to a line that is being prefetched.
+ */
xor_register(&xor_block_sse);
xor_register(&xor_block_sse_pf64);
} else if (boot_cpu_has(X86_FEATURE_MMX)) {
xor_register(&xor_block_pII_mmx);
xor_register(&xor_block_p5_mmx);
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: Eric Biggers @ 2026-06-15 18:44 UTC (permalink / raw)
To: David Laight
Cc: Andrew Morton, linux-kernel, Christoph Hellwig, linux-crypto, x86,
linux-raid
In-Reply-To: <20260614111628.00af46b9@pumpkin>
On Sun, Jun 14, 2026 at 11:16:28AM +0100, David Laight wrote:
> On Sat, 13 Jun 2026 18:03:57 -0700
> Eric Biggers <ebiggers@kernel.org> wrote:
>
> > Add an implementation of xor_gen() using AVX-512.
> >
> > It uses 512-bit vectors, i.e. ZMM registers. It also uses the
> > vpternlogq instruction to do three-input XORs when applicable.
> >
> > It's enabled on x86_64 CPUs that have AVX512F && !PREFER_YMM. In
> > practice that means:
> >
> > - AMD Zen 4 and later (client and server)
>
> Doesn't zen4 only have a 256bit bus between the cpu and cache?
> So avx512 reads take two clocks.
> Since this is memory limited it is unlikely to run faster than the
> avx256 version.
On AMD Genoa (Zen 4 server processor), the AVX-512 code added by this
patch is indeed about the same speed as the existing AVX-2 code.
> OTOH if it doesn't cause down-clocking as well then it won't be slower.
Yes, as far as I know that's not an issue on AMD processors, even Zen 4.
The "avoid AVX-512 due to downclocking" rule is historical guidance for
Intel processors that had a bad implementation of AVX-512. There's no
reason to exclude Zen 4 from executing AVX-512 optimized code. At worst
it will just be the same, as we're seeing here.
> Since I suggested it :-)
>
> Reviewed-By: David Laight <david.laight.linux@gmail.com>
>
> Some 'not very important' comments:
>
> I did wonder whether moving the loop into the asm() would help.
> gcc has a nasty habit of pessimising loops when you try to be clever.
> It is certainly safer for tight loops like these.
I originally tried leaving the loops to the compiler, but gcc unrolled
the 1x ones by 2x, despite it having no visibility into the asm block.
That broke the intent with the indexed addressing, since to achieve the
unrolling it generated code that incremented the pointers.
So I just ended up moving the loop to the asm, which reliably gives us
the code we want.
> That does have the side effect of making p0 be %1 which doesn't improve
> readability. Either used named parameters or possibly just change p0 to p1 (etc)
> so they match.
>
> The code should be limited by the memory reads, so the 3-argument xor and
> the interleave of the unroll may make no difference.
The unroll by 2x in the 2 and 3-buffer cases helped a little bit on
Sapphire Rapids. I don't know exactly why, but it makes sense that
those cases are where the loop overhead is most likely to matter.
> Some cpu do have constraints on the cache alignment in order to do two
> reads per clock, but I've forgotten them and they got better before AVX-512.
> If that were affecting this code (on the tested cpu) then I'd expect the
> interleaved unroll would improve the _4 and -5 functions.
> So it probably doesn't affect this code.
The buffers are always 64-byte aligned here, as documented.
> Using the same loop for the avx-256 and sse (and even smaller) functions could
> well generate code that runs 'pretty much as fast as possible' on older cpu.
> Intel cpu (going back to Sandy bridge) are likely to execute the loop in the
> same number of clocks - but clearly copying half or a quarter of the data.
> But I've no experience of zen1.
>
> Might be worth doing for avx-256, does any care about anything older :-)
Yes, the existing AVX code is probably excessively unrolled. It
generates almost 4 KiB of code.
- Eric
^ permalink raw reply
* Re: [PATCH] md/md-llbitmap: grow the page cache in place for reshape
From: yu kuai @ 2026-06-15 16:19 UTC (permalink / raw)
To: Su Yue, Yu Kuai
Cc: Song Liu, Li Nan, Xiao Ni, linux-raid, linux-kernel, yukuai
In-Reply-To: <8q8gawaz.fsf@damenly.org>
Hi,
在 2026/6/15 19:16, Su Yue 写道:
> On Fri 05 Jun 2026 at 17:15, Yu Kuai <yukuai@kernel.org> wrote:
>
>> From: Yu Kuai <yukuai@fygo.io>
>>
>> Use the page-control helpers to grow llbitmap's cached pages in place
>> for resize and later reshape preparation, instead of rebuilding the
>> whole cache.
>>
>> Signed-off-by: Yu Kuai <yukuai@fygo.io>
>> ---
>> drivers/md/md-llbitmap.c | 139 +++++++++++++++++++++++++++++++++++----
>> 1 file changed, 127 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
>> index 2f2896fe4d6f..91d3dec43d48 100644
>> --- a/drivers/md/md-llbitmap.c
>> +++ b/drivers/md/md-llbitmap.c
>> @@ -414,10 +414,23 @@ static char
>> state_machine[BitStateCount][BitmapActionCount] = {
>> [BitmapActionClearUnwritten] = BitUnwritten,
>> },
>> };
>>
>> static void __llbitmap_flush(struct mddev *mddev);
>> +static void llbitmap_flush(struct mddev *mddev);
>> +static void llbitmap_update_sb(void *data);
>> +
>> +static void llbitmap_resize_chunks(struct mddev *mddev, sector_t
>> blocks,
>> + unsigned long *chunksize,
>> + unsigned long *chunks)
>>
> NIT: I would like call it llbitmap_calculate_chunks.
Agreed this is better, will change it.
>
>> +{
>> + *chunks = DIV_ROUND_UP_SECTOR_T(blocks, *chunksize);
>> + while (*chunks > mddev->bitmap_info.space << SECTOR_SHIFT) {
>> + *chunksize = *chunksize << 1;
>> + *chunks = DIV_ROUND_UP_SECTOR_T(blocks, *chunksize);
>> + }
>> +}
>>
>> static enum llbitmap_state llbitmap_read(struct llbitmap *llbitmap,
>> loff_t pos)
>> {
>> unsigned int idx;
>> unsigned int offset;
>> @@ -653,10 +666,52 @@ static unsigned int
>> llbitmap_reserved_pages(struct llbitmap *llbitmap)
>> {
>> return DIV_ROUND_UP(llbitmap->mddev->bitmap_info.space <<
>> SECTOR_SHIFT,
>> PAGE_SIZE);
>> }
>>
>> +static int llbitmap_expand_pages(struct llbitmap *llbitmap,
>> + unsigned long chunks)
>> +{
>> + struct llbitmap_page_ctl **pctl;
>> + unsigned int old_nr_pages = llbitmap->nr_pages;
>> + unsigned int nr_pages = llbitmap_used_pages(llbitmap, chunks);
>> + int i;
>> + int ret;
>> +
>> + if (nr_pages <= old_nr_pages)
>> + return 0;
>> +
>> + pctl = kcalloc(nr_pages, sizeof(*pctl), GFP_KERNEL);
>> + if (!pctl)
>> + return -ENOMEM;
>> +
>> + if (llbitmap->pctl)
>> + memcpy(pctl, llbitmap->pctl,
>> + array_size(old_nr_pages, sizeof(*pctl)));
>> +
>> + for (i = old_nr_pages; i < nr_pages; i++) {
>> + pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i);
>> + if (IS_ERR(pctl[i]))
>> + goto err_alloc_ptr;
>> + }
>> +
>> + kfree(llbitmap->pctl);
>> + llbitmap->pctl = pctl;
>> + llbitmap->nr_pages = nr_pages;
>> + return 0;
>> +
>> +err_alloc_ptr:
>> + ret = PTR_ERR(pctl[i]);
>> + for (i--; i >= (int)old_nr_pages; i--) {
>>
> Confused about why not just declare i as an unsigned int?
>
Yes, this looks better as well.
>
> --
> Su
>> + __free_page(pctl[i]->page);
>> + percpu_ref_exit(&pctl[i]->active);
>> + kfree(pctl[i]);
>> + }
>> + kfree(pctl);
>> + return ret;
>> +}
>> +
>> static int llbitmap_alloc_pages(struct llbitmap *llbitmap)
>> {
>> unsigned int used_pages = llbitmap_used_pages(llbitmap,
>> llbitmap->chunks);
>> unsigned int nr_pages = max(used_pages,
>> llbitmap_reserved_pages(llbitmap));
>> int i;
>> @@ -728,10 +783,38 @@ static bool llbitmap_zero_all_disks(struct
>> llbitmap *llbitmap)
>> }
>>
>> return true;
>> }
>>
>> +static void llbitmap_mark_range(struct llbitmap *llbitmap,
>> + unsigned long start,
>> + unsigned long end,
>> + enum llbitmap_state state)
>> +{
>> + while (start <= end) {
>> + llbitmap_write(llbitmap, state, start);
>> + start++;
>> + }
>> +}
>> +
>> +static int llbitmap_prepare_resize(struct llbitmap *llbitmap,
>> + unsigned long old_chunks,
>> + unsigned long new_chunks,
>> + unsigned long cache_chunks)
>> +{
>> + int ret;
>> +
>> + llbitmap_flush(llbitmap->mddev);
>> + ret = llbitmap_expand_pages(llbitmap, cache_chunks);
>> + if (ret)
>> + return ret;
>> + if (new_chunks > old_chunks)
>> + llbitmap_mark_range(llbitmap, old_chunks, new_chunks - 1,
>> + BitUnwritten);
>> + return 0;
>> +}
>> +
>> static void llbitmap_init_state(struct llbitmap *llbitmap)
>> {
>> struct mddev *mddev = llbitmap->mddev;
>> enum llbitmap_state state = BitUnwritten;
>> unsigned long i;
>> @@ -1024,14 +1107,14 @@ static int llbitmap_read_sb(struct llbitmap
>> *llbitmap)
>> pr_err("md/llbitmap: %s: chunksize not a power of 2",
>> mdname(mddev));
>> goto out_put_page;
>> }
>>
>> - if (chunksize < DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors,
>> + if (chunksize < DIV_ROUND_UP_SECTOR_T(sync_size,
>> mddev->bitmap_info.space << SECTOR_SHIFT)) {
>> pr_err("md/llbitmap: %s: chunksize too small %lu < %llu /
>> %lu",
>> - mdname(mddev), chunksize, mddev->resync_max_sectors,
>> + mdname(mddev), chunksize, sync_size,
>> mddev->bitmap_info.space);
>> goto out_put_page;
>> }
>>
>> daemon_sleep = le32_to_cpu(sb->daemon_sleep);
>> @@ -1169,28 +1252,60 @@ static int llbitmap_create(struct mddev *mddev)
>> }
>>
>> static int llbitmap_resize(struct mddev *mddev, sector_t blocks,
>> int chunksize)
>> {
>> struct llbitmap *llbitmap = mddev->bitmap;
>> + sector_t old_blocks = llbitmap->sync_size;
>> + unsigned long old_chunks = llbitmap->chunks;
>> unsigned long chunks;
>> + unsigned long cache_chunks;
>> + int ret = 0;
>> + unsigned long bitmap_chunksize;
>> + bool reshape;
>>
>> if (chunksize == 0)
>> chunksize = llbitmap->chunksize;
>>
>> - /* If there is enough space, leave the chunksize unchanged. */
>> - chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
>> - while (chunks > mddev->bitmap_info.space << SECTOR_SHIFT) {
>> - chunksize = chunksize << 1;
>> - chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
>> - }
>> + bitmap_chunksize = chunksize;
>> + llbitmap_resize_chunks(mddev, blocks, &bitmap_chunksize, &chunks);
>>
>> - llbitmap->chunkshift = ffz(~chunksize);
>> - llbitmap->chunksize = chunksize;
>> - llbitmap->chunks = chunks;
>> - llbitmap->sync_size = blocks;
>> + reshape = mddev->delta_disks || mddev->new_level != mddev->level ||
>> + mddev->new_layout != mddev->layout ||
>> + mddev->new_chunk_sectors != mddev->chunk_sectors;
>> + if (!reshape && bitmap_chunksize != llbitmap->chunksize)
>> + return -EOPNOTSUPP;
>> + if (blocks == old_blocks && chunks == llbitmap->chunks)
>> + return 0;
>> +
>> + mutex_lock(&mddev->bitmap_info.mutex);
>>
>> + cache_chunks = reshape ? max(old_chunks, chunks) : chunks;
>> + ret = llbitmap_prepare_resize(llbitmap, old_chunks, chunks,
>> cache_chunks);
>> + if (ret)
>> + goto out;
>> +
>> + if (reshape) {
>> + llbitmap->reshape_sync_size = blocks;
>> + llbitmap->reshape_chunksize = bitmap_chunksize;
>> + llbitmap->reshape_chunks = chunks;
>> + llbitmap->chunks = max(old_chunks, chunks);
>> + } else {
>> + if (blocks < old_blocks && chunks < old_chunks)
>> + llbitmap_mark_range(llbitmap, chunks, old_chunks - 1,
>> + BitUnwritten);
>> + mddev->bitmap_info.chunksize = bitmap_chunksize;
>> + llbitmap->chunks = chunks;
>> + llbitmap->sync_size = blocks;
>> + llbitmap_update_sb(llbitmap);
>> + }
>> + __llbitmap_flush(mddev);
>> + mutex_unlock(&mddev->bitmap_info.mutex);
>> return 0;
>> +
>> +out:
>> + mutex_unlock(&mddev->bitmap_info.mutex);
>> + return ret;
>> }
>>
>> static int llbitmap_load(struct mddev *mddev)
>> {
>> enum llbitmap_action action = BitmapActionReload;
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2] md/raid5: fix reshape deadlock while failed devices more than max degraded
From: sashiko-bot @ 2026-06-15 11:49 UTC (permalink / raw)
To: Chen Cheng; +Cc: yukuai, linux-raid
In-Reply-To: <20260615113450.2088877-1-chencheng@fnnas.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Clearing STRIPE_EXPANDING prematurely in handle_failed_reshape() allows a destination stripe to be freed and reused while asynchronous DMA is still writing to its pages, leading to DMA-after-free memory corruption.
--
commit ec55b78c975b4f9f35e287b83bee72d76b92b33a
Author: Chen Cheng <chencheng@fnnas.com>
md/raid5: fix reshape deadlock while failed devices more than max degraded
This patch fixes a deadlock in raid5 reshape by cleaning up stripe states
(including destination and source stripes) when failed devices exceed the
max degraded count.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index d0d7f96fd7cd9..2f4d82bd7c2e6 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3730,6 +3730,78 @@ handle_failed_sync(struct r5conf *conf, struct stripe_head *sh,
> md_sync_error(conf->mddev);
> }
>
> +/*
> + * handle_failed_reshape - handl failed stripes when reshape failed and
> + * degraded devices >= max_degraded
[ ... ]
> +static void handle_failed_reshape(struct r5conf *conf, struct stripe_head *sh,
> + struct stripe_head_state *s)
> +{
> + int i;
[ ... ]
> + /* release the destination stripes that are waiting to be filled */
> + if (test_and_clear_bit(STRIPE_EXPAND_SOURCE, &sh->state)) {
> + for (i = 0; i < sh->disks; i++) {
> + int dd_idx;
> + struct stripe_head *sh2;
> + sector_t bn, sec;
> +
> + if (i == sh->pd_idx)
> + continue;
> + if (conf->level == 6 && i == sh->qd_idx)
> + continue;
> +
> + bn = raid5_compute_blocknr(sh, i, 1);
> + sec = raid5_compute_sector(conf, bn, 0, &dd_idx, NULL);
> + sh2 = raid5_get_active_stripe(conf, NULL, sec,
> + R5_GAS_NOBLOCK | R5_GAS_NOQUIESCE);
> + if (!sh2)
> + continue;
> +
> + if (test_and_clear_bit(STRIPE_EXPANDING, &sh2->state)) {
[Severity: Critical]
Could clearing STRIPE_EXPANDING prematurely here lead to a DMA-after-free?
During a reshape, a destination stripe (sh2) might require data from multiple
source stripes. STRIPE_EXPANDING normally prevents the destination stripe from
being returned to the free list before all data is copied.
If Worker 1 is handling a source stripe via handle_stripe_expansion(), it
submits an asynchronous DMA copy to sh2's pages and then calls
raid5_release_stripe(sh2) to drop its reference BEFORE waiting for the DMA to
complete:
drivers/md/raid5.c:handle_stripe_expansion() {
...
raid5_release_stripe(sh2);
}
/* done submitting copies, wait for them to complete */
async_tx_quiesce(&tx);
...
}
If the array concurrently fails, Worker 2 could enter handle_failed_reshape()
for a different source stripe, look up sh2, clear its STRIPE_EXPANDING bit,
and release it.
If sh2's refcount drops to 0 without STRIPE_EXPANDING set, could it be
added to the inactive_list and reallocated (or have its pages freed) while
the DMA from Worker 1 is still actively writing to it?
> + atomic_dec(&conf->reshape_stripes);
> + wake_up(&conf->wait_for_reshape);
> + md_done_sync(conf->mddev,
> + RAID5_STRIPE_SECTORS(conf));
> + }
> +
> + clear_bit(STRIPE_EXPAND_READY, &sh2->state);
> +
> + raid5_release_stripe(sh2);
> + }
> + }
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615113450.2088877-1-chencheng@fnnas.com?part=1
^ permalink raw reply
* [PATCH v2] md/raid5: fix reshape deadlock while failed devices more than max degraded
From: Chen Cheng @ 2026-06-15 11:34 UTC (permalink / raw)
To: linux-raid, yukuai, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
reshape stripe lifetime:
- start reshape ==> reshape_request():
* get destination stripe,
- if need to copy source data chunks, set STRIPE_EXPANDING;
- or, if new regions past the old end of the array, zero-filled,
no need source data, set STRIPE_EXPANDING | STRIPE_READY
* get source stripe,
- set STRIPE_EXPAND_SOURCE
- handle expand stripe ==> handle_stripe():
reshape use reconstruct-write to construct stripe,
four stages:
1. prepare source data chunks for old geometry stripe
- fill source stripe data by read or compute
2. move data from old geometry source stripe to new geometry
destination stripe
- source stripe clear STRIPE_EXPAND_SOURCE
- drain data from source to destination stripe
- mark stripe chunk as R5_Expanded|R5_UPTODATE when the
drain from source chunk to destionation chunk is completed
- all stripe chunks drain are completed, then mark
STRIPE_EXPAND_READY
3. calculate p/q chunks for destination stripe
- if destination stripe does't depends on source dstripe,
then we can clear STRIPE_EXPANDING
4. write-out to disks and release
- set R5_Wantwrite|R5_Locked, writeout to disk
- if write-out successed, clear STRIPE_EXPAND_READY, and
decrement reshape_stripe, call md_done_sync() to report
reshape progress.
1. cleanup the following kinds of **destination stripe**
when failed device more than max degraded:
- new regions past the old end of the array, zero-filled in place,
requires no source data.
(STRIPE_EXPANDING | STRIPE_EXPAND_READY)
- prepare source data chunks already done, and writeout failed
(STRIPE_EXPAND_READY)
2. destination stripes that need source data
(STRIPE_EXPANDING, no STRIPE_HANDLE)
- these kind of stripes sit idle in the stripe cache and are never seen
by handle_stripe(). So clean up indirectly when thier source stripe
(type 3) is processed.
3. source stripes (STRIPE_EXPAND_SOURCE)
- hit handle_stripe() after thier member disks are markded Faulty.
- clear STRIPE_EXPAND_SOURCE, finds and cleanup all dependent destination
stripes that were waiting for data.
- walks the source's data disks, compute the corresponding destination
sector, looks up the destination stripe, and do cleanup(clear flags,
dec counters, call md_done_sync())
Reproducer:
- Create a 4-disk RAID5 with mdadm on top of 5 disposable test disks
wrapped by dm targets.
- Add the 5th device as a spare and start a 4 -> 5 reshape.
- Wait until /sys/block/mdX/md/sync_action reports "reshape".
- Inject failures on two members so reshape exceeds max_degraded.
- After a few seconds, write "frozen" to /sys/block/mdX/md/sync_action.
Before this fix, the write blocks indefinitely.
Read-error variant:
- Use dm-dust on /dev/sd[b-f].
- Preload bad blocks on two source members, e.g. dust0 and dust1:
dmsetup message dust0 0 addbadblock <range>
dmsetup message dust1 0 addbadblock <range>
- Start reshape:
mdadm -C /dev/mdX -e 1.2 -l 5 -n 4 -c 64 --assume-clean /dev/mapper/dust{0..3}
mdadm --manage /dev/mdX --add /dev/mapper/dust4
mdadm --grow /dev/mdX -n 5 --backup-file=/tmp/grow.backup &
- Once reshape starts, enable the injected read failures:
dmsetup message dust0 0 enable
dmsetup message dust1 0 enable
- Then:
echo frozen > /sys/block/mdX/md/sync_action
hangs forever before the fix.
Write-error variant:
- Use dm-flakey on /dev/sd[b-f].
- Start the same 4 -> 5 reshape on flakey0..flakey4.
- Once reshape starts, switch two members, e.g. flakey3 and flakey4,
to error_writes.
- Then:
echo frozen > /sys/block/mdX/md/sync_action
hangs forever before the fix.
md_do_sync() exits its main loop on MD_RECOVERY_INTR but then blocks
forever at:
wait_event(mddev->recovery_wait,
!atomic_read(&mddev->recovery_active));
After the fix recovery_active drains to zero, md_do_sync() prints
md/raid:md0: Cannot continue operation (2/5 failed).
md: md0: reshape interrupted.
changes v1 -> v2:
- handle reshape write deadlock while failed devices more than max degraded
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/raid5.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 65ae7d8930fc..a320b71d7117 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3728,10 +3728,82 @@ handle_failed_sync(struct r5conf *conf, struct stripe_head *sh,
if (abort)
md_sync_error(conf->mddev);
}
+/*
+ * handle_failed_reshape - handl failed stripes when reshape failed and
+ * degraded devices >= max_degraded
+ *
+ * handle following kinds of stripe:
+ * 1. cleanup the following kinds of destination stripe:
+ * - new regions past the old end of the array, zero-filled in place,
+ * requires no source data.
+ * (STRIPE_EXPANDING | STRIPE_EXPAND_READY)
+ * - prepare source data chunks already done, and writeout failed
+ * (STRIPE_EXPAND_READY)
+ * 2. dest stripes that need source data (STRIPE_EXPANDING, no STRIPE_HANDLE)
+ * - these kind of stripes sit idle in the stripe cache and are never seen
+ * by handle_stripe(). So clean up indirectly when thier source stripe
+ * (type 3) is processed.
+ * 3. src stripes (STRIPE_EXPAND_SOURCE)
+ * - hit handle_stripe() after thier member disks are markded Faulty.
+ * - clear STRIPE_EXPAND_SOURCE, finds and cleanup all dependent destination
+ * stripes that were waiting for data.
+ * - walks the source's data disks, compute the corresponding destination
+ * sector, looks up the destination stripe, and do cleanup(clear flags,
+ * dec counters, call md_done_sync())
+ */
+static void handle_failed_reshape(struct r5conf *conf, struct stripe_head *sh,
+ struct stripe_head_state *s)
+{
+ int i;
+ bool was_expanding = test_and_clear_bit(STRIPE_EXPANDING, &sh->state);
+ bool was_ready = test_and_clear_bit(STRIPE_EXPAND_READY, &sh->state);
+
+ if (was_expanding || was_ready) {
+ atomic_dec(&conf->reshape_stripes);
+ wake_up(&conf->wait_for_reshape);
+ md_done_sync(conf->mddev, RAID5_STRIPE_SECTORS(conf));
+ }
+
+ s->expanded = 0;
+ s->expanding = 0;
+
+ /* release the destination stripes that are waiting to be filled */
+ if (test_and_clear_bit(STRIPE_EXPAND_SOURCE, &sh->state)) {
+ for (i = 0; i < sh->disks; i++) {
+ int dd_idx;
+ struct stripe_head *sh2;
+ sector_t bn, sec;
+
+ if (i == sh->pd_idx)
+ continue;
+ if (conf->level == 6 && i == sh->qd_idx)
+ continue;
+
+ bn = raid5_compute_blocknr(sh, i, 1);
+ sec = raid5_compute_sector(conf, bn, 0, &dd_idx, NULL);
+ sh2 = raid5_get_active_stripe(conf, NULL, sec,
+ R5_GAS_NOBLOCK | R5_GAS_NOQUIESCE);
+ if (!sh2)
+ continue;
+
+ if (test_and_clear_bit(STRIPE_EXPANDING, &sh2->state)) {
+ atomic_dec(&conf->reshape_stripes);
+ wake_up(&conf->wait_for_reshape);
+ md_done_sync(conf->mddev,
+ RAID5_STRIPE_SECTORS(conf));
+ }
+
+ clear_bit(STRIPE_EXPAND_READY, &sh2->state);
+
+ raid5_release_stripe(sh2);
+ }
+ }
+}
+
static int want_replace(struct stripe_head *sh, int disk_idx)
{
struct md_rdev *rdev;
int rv = 0;
@@ -5001,10 +5073,12 @@ static void handle_stripe(struct stripe_head *sh)
break_stripe_batch_list(sh, 0);
if (s.to_read+s.to_write+s.written)
handle_failed_stripe(conf, sh, &s, disks);
if (s.syncing + s.replacing)
handle_failed_sync(conf, sh, &s);
+ if (s.expanding + s.expanded)
+ handle_failed_reshape(conf, sh, &s);
}
/* Now we check to see if any write operations have recently
* completed
*/
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] md/md-llbitmap: grow the page cache in place for reshape
From: Su Yue @ 2026-06-15 11:16 UTC (permalink / raw)
To: Yu Kuai; +Cc: Song Liu, Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260605091527.2463539-7-yukuai@kernel.org>
On Fri 05 Jun 2026 at 17:15, Yu Kuai <yukuai@kernel.org> wrote:
> From: Yu Kuai <yukuai@fygo.io>
>
> Use the page-control helpers to grow llbitmap's cached pages in
> place
> for resize and later reshape preparation, instead of rebuilding
> the
> whole cache.
>
> Signed-off-by: Yu Kuai <yukuai@fygo.io>
> ---
> drivers/md/md-llbitmap.c | 139
> +++++++++++++++++++++++++++++++++++----
> 1 file changed, 127 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 2f2896fe4d6f..91d3dec43d48 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -414,10 +414,23 @@ static char
> state_machine[BitStateCount][BitmapActionCount] = {
> [BitmapActionClearUnwritten] = BitUnwritten,
> },
> };
>
> static void __llbitmap_flush(struct mddev *mddev);
> +static void llbitmap_flush(struct mddev *mddev);
> +static void llbitmap_update_sb(void *data);
> +
> +static void llbitmap_resize_chunks(struct mddev *mddev,
> sector_t blocks,
> + unsigned long *chunksize,
> + unsigned long *chunks)
>
NIT: I would like call it llbitmap_calculate_chunks.
> +{
> + *chunks = DIV_ROUND_UP_SECTOR_T(blocks, *chunksize);
> + while (*chunks > mddev->bitmap_info.space << SECTOR_SHIFT) {
> + *chunksize = *chunksize << 1;
> + *chunks = DIV_ROUND_UP_SECTOR_T(blocks, *chunksize);
> + }
> +}
>
> static enum llbitmap_state llbitmap_read(struct llbitmap
> *llbitmap, loff_t pos)
> {
> unsigned int idx;
> unsigned int offset;
> @@ -653,10 +666,52 @@ static unsigned int
> llbitmap_reserved_pages(struct llbitmap *llbitmap)
> {
> return DIV_ROUND_UP(llbitmap->mddev->bitmap_info.space <<
> SECTOR_SHIFT,
> PAGE_SIZE);
> }
>
> +static int llbitmap_expand_pages(struct llbitmap *llbitmap,
> + unsigned long chunks)
> +{
> + struct llbitmap_page_ctl **pctl;
> + unsigned int old_nr_pages = llbitmap->nr_pages;
> + unsigned int nr_pages = llbitmap_used_pages(llbitmap, chunks);
> + int i;
> + int ret;
> +
> + if (nr_pages <= old_nr_pages)
> + return 0;
> +
> + pctl = kcalloc(nr_pages, sizeof(*pctl), GFP_KERNEL);
> + if (!pctl)
> + return -ENOMEM;
> +
> + if (llbitmap->pctl)
> + memcpy(pctl, llbitmap->pctl,
> + array_size(old_nr_pages, sizeof(*pctl)));
> +
> + for (i = old_nr_pages; i < nr_pages; i++) {
> + pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i);
> + if (IS_ERR(pctl[i]))
> + goto err_alloc_ptr;
> + }
> +
> + kfree(llbitmap->pctl);
> + llbitmap->pctl = pctl;
> + llbitmap->nr_pages = nr_pages;
> + return 0;
> +
> +err_alloc_ptr:
> + ret = PTR_ERR(pctl[i]);
> + for (i--; i >= (int)old_nr_pages; i--) {
>
Confused about why not just declare i as an unsigned int?
--
Su
> + __free_page(pctl[i]->page);
> + percpu_ref_exit(&pctl[i]->active);
> + kfree(pctl[i]);
> + }
> + kfree(pctl);
> + return ret;
> +}
> +
> static int llbitmap_alloc_pages(struct llbitmap *llbitmap)
> {
> unsigned int used_pages = llbitmap_used_pages(llbitmap,
> llbitmap->chunks);
> unsigned int nr_pages = max(used_pages,
> llbitmap_reserved_pages(llbitmap));
> int i;
> @@ -728,10 +783,38 @@ static bool llbitmap_zero_all_disks(struct
> llbitmap *llbitmap)
> }
>
> return true;
> }
>
> +static void llbitmap_mark_range(struct llbitmap *llbitmap,
> + unsigned long start,
> + unsigned long end,
> + enum llbitmap_state state)
> +{
> + while (start <= end) {
> + llbitmap_write(llbitmap, state, start);
> + start++;
> + }
> +}
> +
> +static int llbitmap_prepare_resize(struct llbitmap *llbitmap,
> + unsigned long old_chunks,
> + unsigned long new_chunks,
> + unsigned long cache_chunks)
> +{
> + int ret;
> +
> + llbitmap_flush(llbitmap->mddev);
> + ret = llbitmap_expand_pages(llbitmap, cache_chunks);
> + if (ret)
> + return ret;
> + if (new_chunks > old_chunks)
> + llbitmap_mark_range(llbitmap, old_chunks, new_chunks - 1,
> + BitUnwritten);
> + return 0;
> +}
> +
> static void llbitmap_init_state(struct llbitmap *llbitmap)
> {
> struct mddev *mddev = llbitmap->mddev;
> enum llbitmap_state state = BitUnwritten;
> unsigned long i;
> @@ -1024,14 +1107,14 @@ static int llbitmap_read_sb(struct
> llbitmap *llbitmap)
> pr_err("md/llbitmap: %s: chunksize not a power of 2",
> mdname(mddev));
> goto out_put_page;
> }
>
> - if (chunksize <
> DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors,
> + if (chunksize < DIV_ROUND_UP_SECTOR_T(sync_size,
> mddev->bitmap_info.space <<
> SECTOR_SHIFT)) {
> pr_err("md/llbitmap: %s: chunksize too small %lu < %llu /
> %lu",
> - mdname(mddev), chunksize,
> mddev->resync_max_sectors,
> + mdname(mddev), chunksize, sync_size,
> mddev->bitmap_info.space);
> goto out_put_page;
> }
>
> daemon_sleep = le32_to_cpu(sb->daemon_sleep);
> @@ -1169,28 +1252,60 @@ static int llbitmap_create(struct mddev
> *mddev)
> }
>
> static int llbitmap_resize(struct mddev *mddev, sector_t
> blocks, int chunksize)
> {
> struct llbitmap *llbitmap = mddev->bitmap;
> + sector_t old_blocks = llbitmap->sync_size;
> + unsigned long old_chunks = llbitmap->chunks;
> unsigned long chunks;
> + unsigned long cache_chunks;
> + int ret = 0;
> + unsigned long bitmap_chunksize;
> + bool reshape;
>
> if (chunksize == 0)
> chunksize = llbitmap->chunksize;
>
> - /* If there is enough space, leave the chunksize unchanged. */
> - chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
> - while (chunks > mddev->bitmap_info.space << SECTOR_SHIFT) {
> - chunksize = chunksize << 1;
> - chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
> - }
> + bitmap_chunksize = chunksize;
> + llbitmap_resize_chunks(mddev, blocks, &bitmap_chunksize,
> &chunks);
>
> - llbitmap->chunkshift = ffz(~chunksize);
> - llbitmap->chunksize = chunksize;
> - llbitmap->chunks = chunks;
> - llbitmap->sync_size = blocks;
> + reshape = mddev->delta_disks || mddev->new_level !=
> mddev->level ||
> + mddev->new_layout != mddev->layout ||
> + mddev->new_chunk_sectors != mddev->chunk_sectors;
> + if (!reshape && bitmap_chunksize != llbitmap->chunksize)
> + return -EOPNOTSUPP;
> + if (blocks == old_blocks && chunks == llbitmap->chunks)
> + return 0;
> +
> + mutex_lock(&mddev->bitmap_info.mutex);
>
> + cache_chunks = reshape ? max(old_chunks, chunks) : chunks;
> + ret = llbitmap_prepare_resize(llbitmap, old_chunks, chunks,
> cache_chunks);
> + if (ret)
> + goto out;
> +
> + if (reshape) {
> + llbitmap->reshape_sync_size = blocks;
> + llbitmap->reshape_chunksize = bitmap_chunksize;
> + llbitmap->reshape_chunks = chunks;
> + llbitmap->chunks = max(old_chunks, chunks);
> + } else {
> + if (blocks < old_blocks && chunks < old_chunks)
> + llbitmap_mark_range(llbitmap, chunks, old_chunks - 1,
> + BitUnwritten);
> + mddev->bitmap_info.chunksize = bitmap_chunksize;
> + llbitmap->chunks = chunks;
> + llbitmap->sync_size = blocks;
> + llbitmap_update_sb(llbitmap);
> + }
> + __llbitmap_flush(mddev);
> + mutex_unlock(&mddev->bitmap_info.mutex);
> return 0;
> +
> +out:
> + mutex_unlock(&mddev->bitmap_info.mutex);
> + return ret;
> }
>
> static int llbitmap_load(struct mddev *mddev)
> {
> enum llbitmap_action action = BitmapActionReload;
^ permalink raw reply
* Re: [PATCH] md/md-llbitmap: allocate page controls independently
From: Su Yue @ 2026-06-15 11:06 UTC (permalink / raw)
To: Yu Kuai; +Cc: Song Liu, Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260605091527.2463539-6-yukuai@kernel.org>
On Fri 05 Jun 2026 at 17:15, Yu Kuai <yukuai@kernel.org> wrote:
> From: Yu Kuai <yukuai@fygo.io>
>
> Allocate one llbitmap page-control object at a time and free
> each
> object through the same model.
>
> Let llbitmap_read_page() return a zeroed page without reading
> disk when
> the page index is beyond the current bitmap size, so
> page-control
> allocation no longer needs a separate read_existing flag.
>
> This keeps the llbitmap page-control lifetime self-consistent
> and
> prepares the page-cache code for later in-place growth.
>
> Signed-off-by: Yu Kuai <yukuai@fygo.io>
>
Straight enough.
Reviewed-by: Su Yue <glass.su@suse.com>
> ---
> drivers/md/md-llbitmap.c | 99
> +++++++++++++++++++++++++---------------
> 1 file changed, 62 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index ecf3ed712315..2f2896fe4d6f 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -510,24 +510,32 @@ static void llbitmap_write(struct llbitmap
> *llbitmap, enum llbitmap_state state,
> llbitmap_set_page_dirty(llbitmap, idx, bit, true);
> else if (state == BitNeedSyncUnwritten)
> llbitmap_set_page_dirty(llbitmap, idx, bit, false);
> }
>
> +static unsigned int llbitmap_used_pages(struct llbitmap
> *llbitmap,
> + unsigned long chunks)
> +{
> + return DIV_ROUND_UP(chunks + BITMAP_DATA_OFFSET, PAGE_SIZE);
> +}
> +
> static struct page *llbitmap_read_page(struct llbitmap
> *llbitmap, int idx)
> {
> struct mddev *mddev = llbitmap->mddev;
> struct page *page = NULL;
> struct md_rdev *rdev;
>
> - if (llbitmap->pctl && llbitmap->pctl[idx])
> + if (llbitmap->pctl && idx < llbitmap->nr_pages &&
> llbitmap->pctl[idx])
> page = llbitmap->pctl[idx]->page;
> if (page)
> return page;
>
> page = alloc_page(GFP_KERNEL | __GFP_ZERO);
> if (!page)
> return ERR_PTR(-ENOMEM);
> + if (idx >= llbitmap_used_pages(llbitmap, llbitmap->chunks))
> + return page;
>
> rdev_for_each(rdev, mddev) {
> sector_t sector;
>
> if (rdev->raid_disk < 0 || test_bit(Faulty, &rdev->flags)
> ||
> @@ -594,65 +602,82 @@ static void llbitmap_free_pages(struct
> llbitmap *llbitmap)
> return;
>
> for (i = 0; i < llbitmap->nr_pages; i++) {
> struct llbitmap_page_ctl *pctl = llbitmap->pctl[i];
>
> - if (!pctl || !pctl->page)
> - break;
> -
> - __free_page(pctl->page);
> + if (!pctl)
> + continue;
> + if (pctl->page)
> + __free_page(pctl->page);
> percpu_ref_exit(&pctl->active);
> + kfree(pctl);
> }
>
> - kfree(llbitmap->pctl[0]);
> kfree(llbitmap->pctl);
> llbitmap->pctl = NULL;
> }
>
> -static int llbitmap_cache_pages(struct llbitmap *llbitmap)
> +static struct llbitmap_page_ctl *
> +llbitmap_alloc_page_ctl(struct llbitmap *llbitmap, int idx)
> {
> struct llbitmap_page_ctl *pctl;
> - unsigned int nr_pages = DIV_ROUND_UP(llbitmap->chunks +
> - BITMAP_DATA_OFFSET, PAGE_SIZE);
> + struct page *page;
> unsigned int size = struct_size(pctl, dirty, BITS_TO_LONGS(
> llbitmap->blocks_per_page));
> - int i;
> -
> - llbitmap->pctl = kmalloc_array(nr_pages, sizeof(void *),
> - GFP_KERNEL | __GFP_ZERO);
> - if (!llbitmap->pctl)
> - return -ENOMEM;
>
> size = round_up(size, cache_line_size());
> - pctl = kmalloc_array(nr_pages, size, GFP_KERNEL | __GFP_ZERO);
> - if (!pctl) {
> - kfree(llbitmap->pctl);
> - return -ENOMEM;
> + pctl = kzalloc(size, GFP_KERNEL);
> + if (!pctl)
> + return ERR_PTR(-ENOMEM);
> +
> + page = llbitmap_read_page(llbitmap, idx);
> +
> + if (IS_ERR(page)) {
> + kfree(pctl);
> + return ERR_CAST(page);
> }
>
> - llbitmap->nr_pages = nr_pages;
> + if (percpu_ref_init(&pctl->active, active_release,
> + PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
> + __free_page(page);
> + kfree(pctl);
> + return ERR_PTR(-ENOMEM);
> + }
>
> - for (i = 0; i < nr_pages; i++, pctl = (void *)pctl + size) {
> - struct page *page = llbitmap_read_page(llbitmap, i);
> + pctl->page = page;
> + pctl->state = page_address(page);
> + init_waitqueue_head(&pctl->wait);
> + return pctl;
> +}
>
> - llbitmap->pctl[i] = pctl;
> +static unsigned int llbitmap_reserved_pages(struct llbitmap
> *llbitmap)
> +{
> + return DIV_ROUND_UP(llbitmap->mddev->bitmap_info.space <<
> SECTOR_SHIFT,
> + PAGE_SIZE);
> +}
>
> - if (IS_ERR(page)) {
> - llbitmap_free_pages(llbitmap);
> - return PTR_ERR(page);
> - }
> +static int llbitmap_alloc_pages(struct llbitmap *llbitmap)
> +{
> + unsigned int used_pages = llbitmap_used_pages(llbitmap,
> llbitmap->chunks);
> + unsigned int nr_pages = max(used_pages,
> llbitmap_reserved_pages(llbitmap));
> + int i;
> +
> + llbitmap->pctl = kcalloc(nr_pages, sizeof(*llbitmap->pctl),
> GFP_KERNEL);
> + if (!llbitmap->pctl)
> + return -ENOMEM;
>
> - if (percpu_ref_init(&pctl->active, active_release,
> - PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
> - __free_page(page);
> + llbitmap->nr_pages = nr_pages;
> +
> + for (i = 0; i < nr_pages; i++) {
> + llbitmap->pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i);
> + if (IS_ERR(llbitmap->pctl[i])) {
> + int ret = PTR_ERR(llbitmap->pctl[i]);
> +
> + llbitmap->pctl[i] = NULL;
> llbitmap_free_pages(llbitmap);
> - return -ENOMEM;
> + return ret;
> }
> -
> - pctl->page = page;
> - pctl->state = page_address(page);
> - init_waitqueue_head(&pctl->wait);
> }
>
> return 0;
> }
>
> @@ -921,11 +946,11 @@ static int llbitmap_init(struct llbitmap
> *llbitmap)
> llbitmap->chunksize = chunksize;
> llbitmap->chunks = chunks;
> llbitmap->sync_size = blocks;
> mddev->bitmap_info.daemon_sleep = DEFAULT_DAEMON_SLEEP;
>
> - ret = llbitmap_cache_pages(llbitmap);
> + ret = llbitmap_alloc_pages(llbitmap);
> if (ret)
> return ret;
>
> llbitmap_state_machine(llbitmap, 0, llbitmap->chunks - 1,
> BitmapActionInit);
> @@ -1030,11 +1055,11 @@ static int llbitmap_read_sb(struct
> llbitmap *llbitmap)
> llbitmap->barrier_idle = DEFAULT_BARRIER_IDLE;
> llbitmap->chunksize = chunksize;
> llbitmap->chunks = DIV_ROUND_UP_SECTOR_T(sync_size,
> chunksize);
> llbitmap->chunkshift = ffz(~chunksize);
> llbitmap->sync_size = sync_size;
> - ret = llbitmap_cache_pages(llbitmap);
> + ret = llbitmap_alloc_pages(llbitmap);
>
> out_put_page:
> __free_page(sb_page);
> kunmap_local(sb);
> return ret;
^ permalink raw reply
* Re: [PATCH] md/md-llbitmap: track bitmap sync_size explicitly
From: Su Yue @ 2026-06-15 10:48 UTC (permalink / raw)
To: Yu Kuai; +Cc: Song Liu, Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260605091527.2463539-5-yukuai@kernel.org>
On Fri 05 Jun 2026 at 17:15, Yu Kuai <yukuai@kernel.org> wrote:
> From: Yu Kuai <yukuai@fygo.io>
>
> Track llbitmap's own sync_size instead of always using
> mddev->resync_max_sectors directly.
>
> This is the minimal bookkeeping needed before llbitmap can track
> old
> and new reshape geometry independently.
>
> Signed-off-by: Yu Kuai <yukuai@fygo.io>
>
Reviewed-by: Su Yue <glass.su@suse.com>
> ---
> drivers/md/md-llbitmap.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index bcf34f0c9af6..ecf3ed712315 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -285,10 +285,12 @@ struct llbitmap {
> unsigned long chunkshift;
> /* size of one chunk in sector */
> unsigned long chunksize;
> /* total number of chunks */
> unsigned long chunks;
> + /* total number of sectors tracked by current bitmap geometry
> */
> + sector_t sync_size;
> unsigned long last_end_sync;
> /*
> * time in seconds that dirty bits will be cleared if the page
> is not
> * accessed.
> */
> @@ -916,10 +918,11 @@ static int llbitmap_init(struct llbitmap
> *llbitmap)
>
> llbitmap->barrier_idle = DEFAULT_BARRIER_IDLE;
> llbitmap->chunkshift = ffz(~chunksize);
> llbitmap->chunksize = chunksize;
> llbitmap->chunks = chunks;
> + llbitmap->sync_size = blocks;
> mddev->bitmap_info.daemon_sleep = DEFAULT_DAEMON_SLEEP;
>
> ret = llbitmap_cache_pages(llbitmap);
> if (ret)
> return ret;
> @@ -936,10 +939,11 @@ static int llbitmap_read_sb(struct
> llbitmap *llbitmap)
> {
> struct mddev *mddev = llbitmap->mddev;
> unsigned long daemon_sleep;
> unsigned long chunksize;
> unsigned long events;
> + sector_t sync_size;
> struct page *sb_page;
> bitmap_super_t *sb;
> int ret = -EINVAL;
>
> if (!mddev->bitmap_info.offset) {
> @@ -985,10 +989,13 @@ static int llbitmap_read_sb(struct
> llbitmap *llbitmap)
> if (test_and_clear_bit(BITMAP_FIRST_USE, &llbitmap->flags)) {
> ret = llbitmap_init(llbitmap);
> goto out_put_page;
> }
>
> + sync_size = le64_to_cpu(sb->sync_size);
> + if (!sync_size)
> + sync_size = mddev->resync_max_sectors;
> chunksize = le32_to_cpu(sb->chunksize);
> if (!is_power_of_2(chunksize)) {
> pr_err("md/llbitmap: %s: chunksize not a power of 2",
> mdname(mddev));
> goto out_put_page;
> @@ -1020,12 +1027,13 @@ static int llbitmap_read_sb(struct
> llbitmap *llbitmap)
> mddev->bitmap_info.chunksize = chunksize;
> mddev->bitmap_info.daemon_sleep = daemon_sleep;
>
> llbitmap->barrier_idle = DEFAULT_BARRIER_IDLE;
> llbitmap->chunksize = chunksize;
> - llbitmap->chunks =
> DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors, chunksize);
> + llbitmap->chunks = DIV_ROUND_UP_SECTOR_T(sync_size,
> chunksize);
> llbitmap->chunkshift = ffz(~chunksize);
> + llbitmap->sync_size = sync_size;
> ret = llbitmap_cache_pages(llbitmap);
>
> out_put_page:
> __free_page(sb_page);
> kunmap_local(sb);
> @@ -1151,10 +1159,11 @@ static int llbitmap_resize(struct mddev
> *mddev, sector_t blocks, int chunksize)
> }
>
> llbitmap->chunkshift = ffz(~chunksize);
> llbitmap->chunksize = chunksize;
> llbitmap->chunks = chunks;
> + llbitmap->sync_size = blocks;
>
> return 0;
> }
>
> static int llbitmap_load(struct mddev *mddev)
> @@ -1524,11 +1533,11 @@ static void llbitmap_update_sb(void
> *data)
>
> sb = kmap_local_page(sb_page);
> sb->events = cpu_to_le64(mddev->events);
> sb->state = cpu_to_le32(llbitmap->flags);
> sb->chunksize = cpu_to_le32(llbitmap->chunksize);
> - sb->sync_size = cpu_to_le64(mddev->resync_max_sectors);
> + sb->sync_size = cpu_to_le64(llbitmap->sync_size);
> sb->events_cleared = cpu_to_le64(llbitmap->events_cleared);
> sb->sectors_reserved = cpu_to_le32(mddev->bitmap_info.space);
> sb->daemon_sleep =
> cpu_to_le32(mddev->bitmap_info.daemon_sleep);
>
> kunmap_local(sb);
> @@ -1542,10 +1551,11 @@ static int llbitmap_get_stats(void
> *data, struct md_bitmap_stats *stats)
> memset(stats, 0, sizeof(*stats));
>
> stats->missing_pages = 0;
> stats->pages = llbitmap->nr_pages;
> stats->file_pages = llbitmap->nr_pages;
> + stats->sync_size = llbitmap->sync_size;
>
> stats->behind_writes = atomic_read(&llbitmap->behind_writes);
> stats->behind_wait = wq_has_sleeper(&llbitmap->behind_wait);
> stats->events_cleared = llbitmap->events_cleared;
^ permalink raw reply
* Re: [PATCH v2 4/4] md/raid1: simplify raid1_write_request() error handling
From: Abd-Alrhman Masalkhi @ 2026-06-15 8:11 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-raid, yukuai
In-Reply-To: <20260613184757.57BC51F000E9@smtp.kernel.org>
Hi Kuai,
On Sat, Jun 13, 2026 at 18:47 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Critical] Moving `atomic_inc(&rdev->nr_pending)` down opens a race window with `hot_remove_disk`, leading to a NULL pointer dereference during error handling or a Use-After-Free in clustered configurations.
>
I don't think this can happen. the array must be suspended before any
disk can be removed.
> Pre-existing issues:
> - [High] A `REQ_ATOMIC` write can be silently split if a bad block overlaps the start of the write, violating atomicity guarantees.
I don't think this is possible. bio_submit_split_bioset() will fail
rather than split the bio.
> --
>
> commit c334f57d91677f32131f21e50bb32b83136e5a60
> Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>
> md/raid1: simplify raid1_write_request() error handling
>
> This commit delays the rdev->nr_pending increment until after badblocks
> are checked to avoid incrementing and immediately decrementing on skipped
> devices. It also consolidates error handling paths and modifies REQ_ATOMIC
> failures to complete with BLK_STS_NOTSUPP instead of I/O errors.
>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index 632d72607e11e..86d4f224ffb11 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
> [ ... ]
>> @@ -1576,23 +1573,21 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>> if (!rdev || test_bit(Faulty, &rdev->flags))
>> continue;
>>
>> - atomic_inc(&rdev->nr_pending);
>> if (test_bit(WriteErrorSeen, &rdev->flags)) {
>> sector_t first_bad;
>> sector_t bad_sectors;
>> int is_bad;
>>
>> - is_bad = is_badblock(rdev, r1_bio->sector, max_sectors,
>> + is_bad = is_badblock(rdev, sector, max_sectors,
>> &first_bad, &bad_sectors);
>> - if (is_bad && first_bad <= r1_bio->sector) {
>> + if (is_bad && first_bad <= sector) {
>> /* Cannot write here at all */
>> - bad_sectors -= (r1_bio->sector - first_bad);
>> + bad_sectors -= (sector - first_bad);
>> if (bad_sectors < max_sectors)
>> /* mustn't write more than bad_sectors
>> * to other devices yet
>> */
>> max_sectors = bad_sectors;
>> - rdev_dec_pending(rdev, mddev);
>> continue;
>
> [Severity: High]
> This is a pre-existing issue, but does this logic skip the rejection of
> REQ_ATOMIC bios if a bad block overlaps the start of the write?
>
> When is_badblock() finds a bad block at or before the write sector, the
> code shrinks max_sectors to force a split at the boundary and then calls
> continue. Because of the continue, the REQ_ATOMIC bio skips the rejection
> logic below.
>
> Could this silently split an atomic write into smaller pieces and submit
> them to the healthy mirrors via bio_submit_split_bioset(), violating
> block-level atomicity guarantees?
>
>> }
>> if (is_bad) {
> [ ... ]
>> if (bio->bi_opf & REQ_ATOMIC) {
>> - rdev_dec_pending(rdev, mddev);
>> - goto err_handle;
>> + bio->bi_status = BLK_STS_NOTSUPP;
>> + bio_endio(bio);
I misunderstood the comment. it should call bio_io_error(bio) instead.
I'll fix this in v3.
>> + goto err_dec_pending;
>> }
>>
>> - good_sectors = first_bad - r1_bio->sector;
>> + good_sectors = first_bad - sector;
>> if (good_sectors < max_sectors)
>> max_sectors = good_sectors;
>> }
>> }
>> +
>> + atomic_inc(&rdev->nr_pending);
>
> [Severity: Critical]
> Does moving atomic_inc(&rdev->nr_pending) down open a race window with
> hot_remove_disk()?
>
> Because nr_pending acts as an existence lock that prevents the device from
> being removed from the array configuration by raid1_remove_conf(),
> incrementing it after the is_badblock() and test_bit(WriteErrorSeen, ...)
> checks means md_check_recovery() could evaluate rdev_removeable() while
> nr_pending is 0 and call hot_remove_disk().
>
> If raid1_remove_conf() unlinks the device by setting
> conf->mirrors[i].rdev = NULL, when this thread resumes and the I/O
> eventually fails, would the error handling path (handle_write_finished()
> calling narrow_write_error()) read conf->mirrors[i].rdev and dereference a
> NULL pointer?
>
> Additionally, in clustered configurations, if the unlinked rdev is
> immediately freed by md_kick_rdev_from_array(), could this lead to a
> use-after-free directly in raid1_write_request() when evaluating
> test_bit(WriteErrorSeen, &rdev->flags) or is_badblock()?
>
>> r1_bio->bios[i] = bio;
>> }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260613182810.1317258-1-abd.masalkhi@gmail.com?part=4
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH v2] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: David Laight @ 2026-06-14 10:16 UTC (permalink / raw)
To: Eric Biggers
Cc: Andrew Morton, linux-kernel, Christoph Hellwig, linux-crypto, x86,
linux-raid
In-Reply-To: <20260614010357.69416-1-ebiggers@kernel.org>
On Sat, 13 Jun 2026 18:03:57 -0700
Eric Biggers <ebiggers@kernel.org> wrote:
> Add an implementation of xor_gen() using AVX-512.
>
> It uses 512-bit vectors, i.e. ZMM registers. It also uses the
> vpternlogq instruction to do three-input XORs when applicable.
>
> It's enabled on x86_64 CPUs that have AVX512F && !PREFER_YMM. In
> practice that means:
>
> - AMD Zen 4 and later (client and server)
Doesn't zen4 only have a 256bit bus between the cpu and cache?
So avx512 reads take two clocks.
Since this is memory limited it is unlikely to run faster than the
avx256 version.
OTOH if it doesn't cause down-clocking as well then it won't be slower.
> - Intel Sapphire Rapids and later (server)
> - Intel Rocket Lake (client)
> - Intel Nova Lake and later (client)
>
> The !PREFER_YMM condition excludes the older AVX-512 implementations in
> Intel Skylake Server and Intel Ice Lake. They could run this code, but
> they're known to have overly-eager downclocking when ZMM registers are
> used. This is the same policy that the crypto and CRC code uses.
>
> Benchmark on AMD Ryzen 9 9950X (Zen 5):
>
> src_cnt avx avx512 Improvement
> ======= ========== ========== ===========
> 1 56353 MB/s 75388 MB/s 33%
> 2 54274 MB/s 68409 MB/s 26%
> 3 44649 MB/s 64042 MB/s 43%
> 4 41315 MB/s 55002 MB/s 33%
>
> Note: for now I omitted the cpu_has_xfeatures() check that the AVX-512
> optimized crypto and CRC code does, since it's not implemented on
> User-Mode Linux and it's never been present in the RAID6 code either.
>
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Since I suggested it :-)
Reviewed-By: David Laight <david.laight.linux@gmail.com>
Some 'not very important' comments:
I did wonder whether moving the loop into the asm() would help.
gcc has a nasty habit of pessimising loops when you try to be clever.
It is certainly safer for tight loops like these.
That does have the side effect of making p0 be %1 which doesn't improve
readability. Either used named parameters or possibly just change p0 to p1 (etc)
so they match.
The code should be limited by the memory reads, so the 3-argument xor and
the interleave of the unroll may make no difference.
Some cpu do have constraints on the cache alignment in order to do two
reads per clock, but I've forgotten them and they got better before AVX-512.
If that were affecting this code (on the tested cpu) then I'd expect the
interleaved unroll would improve the _4 and -5 functions.
So it probably doesn't affect this code.
Using the same loop for the avx-256 and sse (and even smaller) functions could
well generate code that runs 'pretty much as fast as possible' on older cpu.
Intel cpu (going back to Sandy bridge) are likely to execute the loop in the
same number of clocks - but clearly copying half or a quarter of the data.
But I've no experience of zen1.
Might be worth doing for avx-256, does any care about anything older :-)
David
> ---
>
> Changed in v2:
> - Fixed build on UML
> - Reworked the implementation
>
> lib/raid/xor/Makefile | 2 +-
> lib/raid/xor/x86/xor-avx512.c | 121 ++++++++++++++++++++++++++++++++++
> lib/raid/xor/x86/xor_arch.h | 26 ++++----
> 3 files changed, 137 insertions(+), 12 deletions(-)
> create mode 100644 lib/raid/xor/x86/xor-avx512.c
>
> diff --git a/lib/raid/xor/Makefile b/lib/raid/xor/Makefile
> index 4d633dfd5b90..4af945861a51 100644
> --- a/lib/raid/xor/Makefile
> +++ b/lib/raid/xor/Makefile
> @@ -26,11 +26,11 @@ xor-$(CONFIG_ALTIVEC) += powerpc/xor_vmx.o powerpc/xor_vmx_glue.o
> xor-$(CONFIG_RISCV_ISA_V) += riscv/xor.o riscv/xor-glue.o
> xor-$(CONFIG_SPARC32) += sparc/xor-sparc32.o
> xor-$(CONFIG_SPARC64) += sparc/xor-sparc64.o sparc/xor-sparc64-glue.o
> xor-$(CONFIG_S390) += s390/xor.o
> xor-$(CONFIG_X86_32) += x86/xor-avx.o x86/xor-sse.o x86/xor-mmx.o
> -xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o
> +xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o x86/xor-avx512.o
> obj-y += tests/
>
> CFLAGS_arm/xor-neon.o += $(CC_FLAGS_FPU)
> CFLAGS_REMOVE_arm/xor-neon.o += $(CC_FLAGS_NO_FPU)
>
> diff --git a/lib/raid/xor/x86/xor-avx512.c b/lib/raid/xor/x86/xor-avx512.c
> new file mode 100644
> index 000000000000..87b981d74c90
> --- /dev/null
> +++ b/lib/raid/xor/x86/xor-avx512.c
> @@ -0,0 +1,121 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * AVX-512 optimized implementation of xor_gen()
> + *
> + * Copyright 2026 Google LLC
> + */
> +
> +#include <linux/types.h>
> +#include <asm/fpu/api.h>
> +#include "xor_impl.h"
> +#include "xor_arch.h"
> +
> +/*
> + * Implementation notes:
> + *
> + * Unrolling by the number of buffers (2-5) is very important.
> + *
> + * Unrolling by length is less important, especially when using register-indexed
> + * addressing with negative indices from the end of the buffers. That approach
> + * results in just two loop control instructions being needed per iteration,
> + * regardless of the number of buffers.
> + *
> + * In fact, benchmarks showed that the 2 and 3 buffer cases require only 2x
> + * unrolling by length, while the 4 and 5 buffer cases don't require any
> + * unrolling by length. Benchmarks also showed that the register-indexed
> + * addressing isn't a bottleneck either; i.e., we can't do any better by
> + * incrementing the pointers as we go along, even with more unrolling.
> + */
> +
> +static void xor_avx512_2(long bytes, u8 *p0, const u8 *p1)
> +{
> + long i = -bytes;
> +
> + asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
> + "vmovdqa64 64(%0,%1), %%zmm1\n"
> + "vpxorq (%0,%2), %%zmm0, %%zmm0\n"
> + "vpxorq 64(%0,%2), %%zmm1, %%zmm1\n"
> + "vmovdqa64 %%zmm0, (%0,%1)\n"
> + "vmovdqa64 %%zmm1, 64(%0,%1)\n"
> + "add $128, %0\n"
> + "jnz 1b\n"
> + : "+&r"(i)
> + : "r"(p0 + bytes), "r"(p1 + bytes)
> + : "memory", "cc");
> +}
> +
> +static void xor_avx512_3(long bytes, u8 *p0, const u8 *p1, const u8 *p2)
> +{
> + long i = -bytes;
> +
> + asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
> + "vmovdqa64 64(%0,%1), %%zmm1\n"
> + "vmovdqa64 (%0,%2), %%zmm2\n"
> + "vmovdqa64 64(%0,%2), %%zmm3\n"
> + "vpternlogq $0x96, (%0,%3), %%zmm2, %%zmm0\n"
> + "vpternlogq $0x96, 64(%0,%3), %%zmm3, %%zmm1\n"
> + "vmovdqa64 %%zmm0, (%0,%1)\n"
> + "vmovdqa64 %%zmm1, 64(%0,%1)\n"
> + "add $128, %0\n"
> + "jnz 1b\n"
> + : "+&r"(i)
> + : "r"(p0 + bytes), "r"(p1 + bytes), "r"(p2 + bytes)
> + : "memory", "cc");
> +}
> +
> +static void xor_avx512_4(long bytes, u8 *p0, const u8 *p1, const u8 *p2,
> + const u8 *p3)
> +{
> + long i = -bytes;
> +
> + asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
> + "vmovdqa64 (%0,%2), %%zmm1\n"
> + "vpxorq (%0,%3), %%zmm0, %%zmm0\n"
> + "vpternlogq $0x96, (%0,%4), %%zmm1, %%zmm0\n"
> + "vmovdqa64 %%zmm0, (%0,%1)\n"
> + "add $64, %0\n"
> + "jnz 1b\n"
> + : "+&r"(i)
> + : "r"(p0 + bytes), "r"(p1 + bytes), "r"(p2 + bytes),
> + "r"(p3 + bytes)
> + : "memory", "cc");
> +}
> +
> +static void xor_avx512_5(long bytes, u8 *p0, const u8 *p1, const u8 *p2,
> + const u8 *p3, const u8 *p4)
> +{
> + long i = -bytes;
> +
> + asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
> + "vmovdqa64 (%0,%2), %%zmm1\n"
> + "vpternlogq $0x96, (%0,%3), %%zmm1, %%zmm0\n"
> + "vmovdqa64 (%0,%4), %%zmm1\n"
> + "vpternlogq $0x96, (%0,%5), %%zmm1, %%zmm0\n"
> + "vmovdqa64 %%zmm0, (%0,%1)\n"
> + "add $64, %0\n"
> + "jnz 1b\n"
> + : "+&r"(i)
> + : "r"(p0 + bytes), "r"(p1 + bytes), "r"(p2 + bytes),
> + "r"(p3 + bytes), "r"(p4 + bytes)
> + : "memory", "cc");
> +}
> +
> +DO_XOR_BLOCKS(avx512_inner, xor_avx512_2, xor_avx512_3, xor_avx512_4,
> + xor_avx512_5);
> +
> +/*
> + * Preconditions: bytes is a nonzero multiple of 512, and all buffers are
> + * 64-byte aligned.
> + */
> +static void xor_gen_avx512(void *dest, void **srcs, unsigned int src_cnt,
> + unsigned int bytes)
> +{
> + kernel_fpu_begin();
> + xor_gen_avx512_inner(dest, srcs, src_cnt, bytes);
> + kernel_fpu_end();
> +}
> +
> +struct xor_block_template xor_block_avx512 = {
> + .name = "avx512",
> + .xor_gen = xor_gen_avx512,
> +};
> diff --git a/lib/raid/xor/x86/xor_arch.h b/lib/raid/xor/x86/xor_arch.h
> index 99fe85a213c6..b5d49376fc97 100644
> --- a/lib/raid/xor/x86/xor_arch.h
> +++ b/lib/raid/xor/x86/xor_arch.h
> @@ -4,26 +4,30 @@
> extern struct xor_block_template xor_block_pII_mmx;
> extern struct xor_block_template xor_block_p5_mmx;
> extern struct xor_block_template xor_block_sse;
> extern struct xor_block_template xor_block_sse_pf64;
> extern struct xor_block_template xor_block_avx;
> +extern struct xor_block_template xor_block_avx512;
>
> -/*
> - * When SSE is available, use it as it can write around L2. We may also be able
> - * to load into the L1 only depending on how the cpu deals with a load to a line
> - * that is being prefetched.
> - *
> - * When AVX2 is available, force using it as it is better by all measures.
> - *
> - * 32-bit without MMX can fall back to the generic routines.
> - */
> static __always_inline void __init arch_xor_init(void)
> {
> - if (boot_cpu_has(X86_FEATURE_AVX) &&
> - boot_cpu_has(X86_FEATURE_OSXSAVE)) {
> + if (IS_ENABLED(CONFIG_X86_64) && boot_cpu_has(X86_FEATURE_AVX512F) &&
> + boot_cpu_has(X86_FEATURE_OSXSAVE) &&
> + !boot_cpu_has(X86_FEATURE_PREFER_YMM)) {
> + /* AVX-512 will be the best; no need to try others. */
> + /* !PREFER_YMM excludes CPUs with overly-eager downclocking. */
> + xor_force(&xor_block_avx512);
> + } else if (boot_cpu_has(X86_FEATURE_AVX) &&
> + boot_cpu_has(X86_FEATURE_OSXSAVE)) {
> + /* AVX will be the best; no need to try others. */
> xor_force(&xor_block_avx);
> } else if (IS_ENABLED(CONFIG_X86_64) || boot_cpu_has(X86_FEATURE_XMM)) {
> + /*
> + * When SSE is available, use it as it can write around L2. We
> + * may also be able to load into the L1 only depending on how
> + * the cpu deals with a load to a line that is being prefetched.
> + */
> xor_register(&xor_block_sse);
> xor_register(&xor_block_sse_pf64);
> } else if (boot_cpu_has(X86_FEATURE_MMX)) {
> xor_register(&xor_block_pII_mmx);
> xor_register(&xor_block_p5_mmx);
>
> base-commit: 2b07ea76fd28989bde5993532d7a943a6f90e246
^ permalink raw reply
* Re: [PATCH v2] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: sashiko-bot @ 2026-06-14 1:23 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-raid, yukuai
In-Reply-To: <20260614010357.69416-1-ebiggers@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The AVX-512 XOR implementation relies solely on CPUID hardware checks (`boot_cpu_has(X86_FEATURE_AVX512F)`) and skips verifying if the OS actually enabled AVX-512 state management in the XCR0 register, risking a #UD exception.
--
commit d9094ea6909535dc1fa7dcdb6976401cdf1f90e8
Author: Eric Biggers <ebiggers@kernel.org>
lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
This commit adds an AVX-512 optimized implementation of xor_gen() using
512-bit vectors and the vpternlogq instruction. The optimization is enabled
on x86_64 CPUs with AVX512F support that do not have overly-eager
downclocking (!PREFER_YMM). The author intentionally omitted the
cpu_has_xfeatures() check to avoid build issues on User-Mode Linux.
> diff --git a/lib/raid/xor/x86/xor_arch.h b/lib/raid/xor/x86/xor_arch.h
> index 99fe85a213c66..b5d49376fc974 100644
> --- a/lib/raid/xor/x86/xor_arch.h
> +++ b/lib/raid/xor/x86/xor_arch.h
[ ... ]
> static __always_inline void __init arch_xor_init(void)
> {
> - if (boot_cpu_has(X86_FEATURE_AVX) &&
> - boot_cpu_has(X86_FEATURE_OSXSAVE)) {
> + if (IS_ENABLED(CONFIG_X86_64) && boot_cpu_has(X86_FEATURE_AVX512F) &&
> + boot_cpu_has(X86_FEATURE_OSXSAVE) &&
> + !boot_cpu_has(X86_FEATURE_PREFER_YMM)) {
[Severity: High]
Could omitting the cpu_has_xfeatures() check in arch_xor_init() result in a
#UD exception?
While CR4.OSXSAVE indicates that XCR0 is enabled, it does not guarantee that
the OS or hypervisor actually enabled the AVX-512 state components (OPMASK,
ZMM_Hi256, Hi16_ZMM) in XCR0.
If a hypervisor advertises AVX-512 CPUID bits but masks out the corresponding
XCR0 bits, executing EVEX-encoded AVX-512 instructions could trigger an
Invalid Opcode exception and cause a kernel panic.
> + /* AVX-512 will be the best; no need to try others. */
> + /* !PREFER_YMM excludes CPUs with overly-eager downclocking. */
> + xor_force(&xor_block_avx512);
> + } else if (boot_cpu_has(X86_FEATURE_AVX) &&
> + boot_cpu_has(X86_FEATURE_OSXSAVE)) {
> + /* AVX will be the best; no need to try others. */
> xor_force(&xor_block_avx);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614010357.69416-1-ebiggers@kernel.org?part=1
^ permalink raw reply
* [PATCH v2] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: Eric Biggers @ 2026-06-14 1:03 UTC (permalink / raw)
To: Andrew Morton, linux-kernel
Cc: Christoph Hellwig, linux-crypto, x86, Eric Biggers, David Laight,
linux-raid
Add an implementation of xor_gen() using AVX-512.
It uses 512-bit vectors, i.e. ZMM registers. It also uses the
vpternlogq instruction to do three-input XORs when applicable.
It's enabled on x86_64 CPUs that have AVX512F && !PREFER_YMM. In
practice that means:
- AMD Zen 4 and later (client and server)
- Intel Sapphire Rapids and later (server)
- Intel Rocket Lake (client)
- Intel Nova Lake and later (client)
The !PREFER_YMM condition excludes the older AVX-512 implementations in
Intel Skylake Server and Intel Ice Lake. They could run this code, but
they're known to have overly-eager downclocking when ZMM registers are
used. This is the same policy that the crypto and CRC code uses.
Benchmark on AMD Ryzen 9 9950X (Zen 5):
src_cnt avx avx512 Improvement
======= ========== ========== ===========
1 56353 MB/s 75388 MB/s 33%
2 54274 MB/s 68409 MB/s 26%
3 44649 MB/s 64042 MB/s 43%
4 41315 MB/s 55002 MB/s 33%
Note: for now I omitted the cpu_has_xfeatures() check that the AVX-512
optimized crypto and CRC code does, since it's not implemented on
User-Mode Linux and it's never been present in the RAID6 code either.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
Changed in v2:
- Fixed build on UML
- Reworked the implementation
lib/raid/xor/Makefile | 2 +-
lib/raid/xor/x86/xor-avx512.c | 121 ++++++++++++++++++++++++++++++++++
lib/raid/xor/x86/xor_arch.h | 26 ++++----
3 files changed, 137 insertions(+), 12 deletions(-)
create mode 100644 lib/raid/xor/x86/xor-avx512.c
diff --git a/lib/raid/xor/Makefile b/lib/raid/xor/Makefile
index 4d633dfd5b90..4af945861a51 100644
--- a/lib/raid/xor/Makefile
+++ b/lib/raid/xor/Makefile
@@ -26,11 +26,11 @@ xor-$(CONFIG_ALTIVEC) += powerpc/xor_vmx.o powerpc/xor_vmx_glue.o
xor-$(CONFIG_RISCV_ISA_V) += riscv/xor.o riscv/xor-glue.o
xor-$(CONFIG_SPARC32) += sparc/xor-sparc32.o
xor-$(CONFIG_SPARC64) += sparc/xor-sparc64.o sparc/xor-sparc64-glue.o
xor-$(CONFIG_S390) += s390/xor.o
xor-$(CONFIG_X86_32) += x86/xor-avx.o x86/xor-sse.o x86/xor-mmx.o
-xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o
+xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o x86/xor-avx512.o
obj-y += tests/
CFLAGS_arm/xor-neon.o += $(CC_FLAGS_FPU)
CFLAGS_REMOVE_arm/xor-neon.o += $(CC_FLAGS_NO_FPU)
diff --git a/lib/raid/xor/x86/xor-avx512.c b/lib/raid/xor/x86/xor-avx512.c
new file mode 100644
index 000000000000..87b981d74c90
--- /dev/null
+++ b/lib/raid/xor/x86/xor-avx512.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AVX-512 optimized implementation of xor_gen()
+ *
+ * Copyright 2026 Google LLC
+ */
+
+#include <linux/types.h>
+#include <asm/fpu/api.h>
+#include "xor_impl.h"
+#include "xor_arch.h"
+
+/*
+ * Implementation notes:
+ *
+ * Unrolling by the number of buffers (2-5) is very important.
+ *
+ * Unrolling by length is less important, especially when using register-indexed
+ * addressing with negative indices from the end of the buffers. That approach
+ * results in just two loop control instructions being needed per iteration,
+ * regardless of the number of buffers.
+ *
+ * In fact, benchmarks showed that the 2 and 3 buffer cases require only 2x
+ * unrolling by length, while the 4 and 5 buffer cases don't require any
+ * unrolling by length. Benchmarks also showed that the register-indexed
+ * addressing isn't a bottleneck either; i.e., we can't do any better by
+ * incrementing the pointers as we go along, even with more unrolling.
+ */
+
+static void xor_avx512_2(long bytes, u8 *p0, const u8 *p1)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
+ "vmovdqa64 64(%0,%1), %%zmm1\n"
+ "vpxorq (%0,%2), %%zmm0, %%zmm0\n"
+ "vpxorq 64(%0,%2), %%zmm1, %%zmm1\n"
+ "vmovdqa64 %%zmm0, (%0,%1)\n"
+ "vmovdqa64 %%zmm1, 64(%0,%1)\n"
+ "add $128, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p0 + bytes), "r"(p1 + bytes)
+ : "memory", "cc");
+}
+
+static void xor_avx512_3(long bytes, u8 *p0, const u8 *p1, const u8 *p2)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
+ "vmovdqa64 64(%0,%1), %%zmm1\n"
+ "vmovdqa64 (%0,%2), %%zmm2\n"
+ "vmovdqa64 64(%0,%2), %%zmm3\n"
+ "vpternlogq $0x96, (%0,%3), %%zmm2, %%zmm0\n"
+ "vpternlogq $0x96, 64(%0,%3), %%zmm3, %%zmm1\n"
+ "vmovdqa64 %%zmm0, (%0,%1)\n"
+ "vmovdqa64 %%zmm1, 64(%0,%1)\n"
+ "add $128, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p0 + bytes), "r"(p1 + bytes), "r"(p2 + bytes)
+ : "memory", "cc");
+}
+
+static void xor_avx512_4(long bytes, u8 *p0, const u8 *p1, const u8 *p2,
+ const u8 *p3)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
+ "vmovdqa64 (%0,%2), %%zmm1\n"
+ "vpxorq (%0,%3), %%zmm0, %%zmm0\n"
+ "vpternlogq $0x96, (%0,%4), %%zmm1, %%zmm0\n"
+ "vmovdqa64 %%zmm0, (%0,%1)\n"
+ "add $64, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p0 + bytes), "r"(p1 + bytes), "r"(p2 + bytes),
+ "r"(p3 + bytes)
+ : "memory", "cc");
+}
+
+static void xor_avx512_5(long bytes, u8 *p0, const u8 *p1, const u8 *p2,
+ const u8 *p3, const u8 *p4)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%0,%1), %%zmm0\n"
+ "vmovdqa64 (%0,%2), %%zmm1\n"
+ "vpternlogq $0x96, (%0,%3), %%zmm1, %%zmm0\n"
+ "vmovdqa64 (%0,%4), %%zmm1\n"
+ "vpternlogq $0x96, (%0,%5), %%zmm1, %%zmm0\n"
+ "vmovdqa64 %%zmm0, (%0,%1)\n"
+ "add $64, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p0 + bytes), "r"(p1 + bytes), "r"(p2 + bytes),
+ "r"(p3 + bytes), "r"(p4 + bytes)
+ : "memory", "cc");
+}
+
+DO_XOR_BLOCKS(avx512_inner, xor_avx512_2, xor_avx512_3, xor_avx512_4,
+ xor_avx512_5);
+
+/*
+ * Preconditions: bytes is a nonzero multiple of 512, and all buffers are
+ * 64-byte aligned.
+ */
+static void xor_gen_avx512(void *dest, void **srcs, unsigned int src_cnt,
+ unsigned int bytes)
+{
+ kernel_fpu_begin();
+ xor_gen_avx512_inner(dest, srcs, src_cnt, bytes);
+ kernel_fpu_end();
+}
+
+struct xor_block_template xor_block_avx512 = {
+ .name = "avx512",
+ .xor_gen = xor_gen_avx512,
+};
diff --git a/lib/raid/xor/x86/xor_arch.h b/lib/raid/xor/x86/xor_arch.h
index 99fe85a213c6..b5d49376fc97 100644
--- a/lib/raid/xor/x86/xor_arch.h
+++ b/lib/raid/xor/x86/xor_arch.h
@@ -4,26 +4,30 @@
extern struct xor_block_template xor_block_pII_mmx;
extern struct xor_block_template xor_block_p5_mmx;
extern struct xor_block_template xor_block_sse;
extern struct xor_block_template xor_block_sse_pf64;
extern struct xor_block_template xor_block_avx;
+extern struct xor_block_template xor_block_avx512;
-/*
- * When SSE is available, use it as it can write around L2. We may also be able
- * to load into the L1 only depending on how the cpu deals with a load to a line
- * that is being prefetched.
- *
- * When AVX2 is available, force using it as it is better by all measures.
- *
- * 32-bit without MMX can fall back to the generic routines.
- */
static __always_inline void __init arch_xor_init(void)
{
- if (boot_cpu_has(X86_FEATURE_AVX) &&
- boot_cpu_has(X86_FEATURE_OSXSAVE)) {
+ if (IS_ENABLED(CONFIG_X86_64) && boot_cpu_has(X86_FEATURE_AVX512F) &&
+ boot_cpu_has(X86_FEATURE_OSXSAVE) &&
+ !boot_cpu_has(X86_FEATURE_PREFER_YMM)) {
+ /* AVX-512 will be the best; no need to try others. */
+ /* !PREFER_YMM excludes CPUs with overly-eager downclocking. */
+ xor_force(&xor_block_avx512);
+ } else if (boot_cpu_has(X86_FEATURE_AVX) &&
+ boot_cpu_has(X86_FEATURE_OSXSAVE)) {
+ /* AVX will be the best; no need to try others. */
xor_force(&xor_block_avx);
} else if (IS_ENABLED(CONFIG_X86_64) || boot_cpu_has(X86_FEATURE_XMM)) {
+ /*
+ * When SSE is available, use it as it can write around L2. We
+ * may also be able to load into the L1 only depending on how
+ * the cpu deals with a load to a line that is being prefetched.
+ */
xor_register(&xor_block_sse);
xor_register(&xor_block_sse_pf64);
} else if (boot_cpu_has(X86_FEATURE_MMX)) {
xor_register(&xor_block_pII_mmx);
xor_register(&xor_block_p5_mmx);
base-commit: 2b07ea76fd28989bde5993532d7a943a6f90e246
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2 4/4] md/raid1: simplify raid1_write_request() error handling
From: sashiko-bot @ 2026-06-13 18:47 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: linux-raid, yukuai
In-Reply-To: <20260613182810.1317258-5-abd.masalkhi@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] Moving `atomic_inc(&rdev->nr_pending)` down opens a race window with `hot_remove_disk`, leading to a NULL pointer dereference during error handling or a Use-After-Free in clustered configurations.
Pre-existing issues:
- [High] A `REQ_ATOMIC` write can be silently split if a bad block overlaps the start of the write, violating atomicity guarantees.
--
commit c334f57d91677f32131f21e50bb32b83136e5a60
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
md/raid1: simplify raid1_write_request() error handling
This commit delays the rdev->nr_pending increment until after badblocks
are checked to avoid incrementing and immediately decrementing on skipped
devices. It also consolidates error handling paths and modifies REQ_ATOMIC
failures to complete with BLK_STS_NOTSUPP instead of I/O errors.
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 632d72607e11e..86d4f224ffb11 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
[ ... ]
> @@ -1576,23 +1573,21 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
> if (!rdev || test_bit(Faulty, &rdev->flags))
> continue;
>
> - atomic_inc(&rdev->nr_pending);
> if (test_bit(WriteErrorSeen, &rdev->flags)) {
> sector_t first_bad;
> sector_t bad_sectors;
> int is_bad;
>
> - is_bad = is_badblock(rdev, r1_bio->sector, max_sectors,
> + is_bad = is_badblock(rdev, sector, max_sectors,
> &first_bad, &bad_sectors);
> - if (is_bad && first_bad <= r1_bio->sector) {
> + if (is_bad && first_bad <= sector) {
> /* Cannot write here at all */
> - bad_sectors -= (r1_bio->sector - first_bad);
> + bad_sectors -= (sector - first_bad);
> if (bad_sectors < max_sectors)
> /* mustn't write more than bad_sectors
> * to other devices yet
> */
> max_sectors = bad_sectors;
> - rdev_dec_pending(rdev, mddev);
> continue;
[Severity: High]
This is a pre-existing issue, but does this logic skip the rejection of
REQ_ATOMIC bios if a bad block overlaps the start of the write?
When is_badblock() finds a bad block at or before the write sector, the
code shrinks max_sectors to force a split at the boundary and then calls
continue. Because of the continue, the REQ_ATOMIC bio skips the rejection
logic below.
Could this silently split an atomic write into smaller pieces and submit
them to the healthy mirrors via bio_submit_split_bioset(), violating
block-level atomicity guarantees?
> }
> if (is_bad) {
[ ... ]
> if (bio->bi_opf & REQ_ATOMIC) {
> - rdev_dec_pending(rdev, mddev);
> - goto err_handle;
> + bio->bi_status = BLK_STS_NOTSUPP;
> + bio_endio(bio);
> + goto err_dec_pending;
> }
>
> - good_sectors = first_bad - r1_bio->sector;
> + good_sectors = first_bad - sector;
> if (good_sectors < max_sectors)
> max_sectors = good_sectors;
> }
> }
> +
> + atomic_inc(&rdev->nr_pending);
[Severity: Critical]
Does moving atomic_inc(&rdev->nr_pending) down open a race window with
hot_remove_disk()?
Because nr_pending acts as an existence lock that prevents the device from
being removed from the array configuration by raid1_remove_conf(),
incrementing it after the is_badblock() and test_bit(WriteErrorSeen, ...)
checks means md_check_recovery() could evaluate rdev_removeable() while
nr_pending is 0 and call hot_remove_disk().
If raid1_remove_conf() unlinks the device by setting
conf->mirrors[i].rdev = NULL, when this thread resumes and the I/O
eventually fails, would the error handling path (handle_write_finished()
calling narrow_write_error()) read conf->mirrors[i].rdev and dereference a
NULL pointer?
Additionally, in clustered configurations, if the unlinked rdev is
immediately freed by md_kick_rdev_from_array(), could this lead to a
use-after-free directly in raid1_write_request() when evaluating
test_bit(WriteErrorSeen, &rdev->flags) or is_badblock()?
> r1_bio->bios[i] = bio;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613182810.1317258-1-abd.masalkhi@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH v2 2/4] md/raid10: fix writes_pending leak on write request failures
From: sashiko-bot @ 2026-06-13 18:40 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi; +Cc: linux-raid, yukuai
In-Reply-To: <20260613182810.1317258-3-abd.masalkhi@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The `r10_bio` structure allocated from the mempool is leaked when `raid10_write_request()` bails out early due to `REQ_NOWAIT`.
--
commit 3940ccdb4c7c7e19c20f9b5e2e1d12545c2558b5
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
md/raid10: fix writes_pending leak on write request failures
raid10_make_request() acquires a writes_pending reference with
md_write_start() before dispatching write requests. Several failure
paths in raid10_write_request() complete the bio and return without
reaching the normal write completion path, causing the corresponding
md_write_end() to be skipped.
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index cee5a253a281d..c123a8c76ddcd 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
[ ... ]
> @@ -1365,7 +1365,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> /* Bail out if REQ_NOWAIT is set for the bio */
> if (bio->bi_opf & REQ_NOWAIT) {
> bio_wouldblock_error(bio);
> - return;
> + return false;
[Severity: High]
This isn't a bug introduced by this patch, but does this early return leak
the r10_bio structure?
__make_request() allocates r10_bio using:
r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
It then passes it to raid10_write_request(). When the bio has REQ_NOWAIT set
during a clustered area resync, we complete the bio with an error and return
false without freeing the r10_bio. Could repeatedly triggering this exhaust
the r10bio_pool, causing deadlocks on subsequent operations?
Note that the regular_request_wait() error path right below correctly calls
free_r10bio(r10_bio) before returning.
> }
> for (;;) {
[ ... ]
> @@ -1398,7 +1398,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> if (bio->bi_opf & REQ_NOWAIT) {
> allow_barrier(conf);
> bio_wouldblock_error(bio);
> - return;
> + return false;
[Severity: High]
This is a pre-existing issue, but does this path also leak the r10_bio
mempool allocation when waiting for a reshape metadata update?
Similar to the clustered area resync case above, the r10_bio allocated in
__make_request() does not appear to be freed before returning here.
> }
> mddev_add_trace_msg(conf->mddev,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613182810.1317258-1-abd.masalkhi@gmail.com?part=2
^ permalink raw reply
* [PATCH v2 4/4] md/raid1: simplify raid1_write_request() error handling
From: Abd-Alrhman Masalkhi @ 2026-06-13 18:28 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, hare, john.g.garry,
martin.petersen, vverma
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
In-Reply-To: <20260613182810.1317258-1-abd.masalkhi@gmail.com>
raid1_write_request() increments rdev->nr_pending before checking the
badblocks and then immediately decrements it again when a device is
skipped. Move the increment until after the checks succeed so the
reference accounting is easier to follow.
Consolidate the failure paths so that each error label releases exactly
the resources acquired up to that point. err_dec_pending drops pending
references and frees the r1bio, while err_allow_barrier handles the
barrier release before returning.
When a REQ_ATOMIC write cannot be satisfied due to a badblock range,
complete the bio with BLK_STS_NOTSUPP rather than reporting an I/O
error, since the operation is unsupported rather than having failed
during I/O.
Rename max_write_sectors to max_sectors and remove the redundant local
copy.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- new patch, depends on patch 1.
---
drivers/md/raid1.c | 59 +++++++++++++++++++++++-----------------------
1 file changed, 30 insertions(+), 29 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index f0e1c7125972..dc0b7b8bc2f8 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1502,29 +1502,29 @@ static void raid1_start_write_behind(struct mddev *mddev, struct r1bio *r1_bio,
}
static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
- int max_write_sectors)
+ int max_sectors)
{
struct r1conf *conf = mddev->private;
struct r1bio *r1_bio;
int i, disks, k;
unsigned long flags;
int first_clone;
- int max_sectors;
bool write_behind = false;
- bool is_discard = (bio_op(bio) == REQ_OP_DISCARD);
+ bool nowait = bio->bi_opf & REQ_NOWAIT;
+ bool is_discard = op_is_discard(bio->bi_opf);
sector_t sector = bio->bi_iter.bi_sector;
if (mddev_is_clustered(mddev) &&
- mddev->cluster_ops->area_resyncing(mddev, WRITE,
- bio->bi_iter.bi_sector, bio_end_sector(bio))) {
+ mddev->cluster_ops->area_resyncing(mddev, WRITE, sector,
+ bio_end_sector(bio))) {
- if (bio->bi_opf & REQ_NOWAIT) {
+ if (nowait) {
bio_wouldblock_error(bio);
return false;
}
wait_event_idle(conf->wait_barrier,
!mddev->cluster_ops->area_resyncing(mddev, WRITE,
- bio->bi_iter.bi_sector,
+ sector,
bio_end_sector(bio)));
}
@@ -1533,20 +1533,18 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
* thread has put up a bar for new requests.
* Continue immediately if no resync is active currently.
*/
- if (!wait_barrier(conf, bio->bi_iter.bi_sector,
- bio->bi_opf & REQ_NOWAIT)) {
+ if (!wait_barrier(conf, sector, nowait)) {
bio_wouldblock_error(bio);
return false;
}
if (!wait_blocked_rdev(mddev, bio)) {
bio_wouldblock_error(bio);
- allow_barrier(conf, sector);
- return false;
+ goto err_allow_barrier;
}
r1_bio = alloc_r1bio(mddev, bio);
- r1_bio->sectors = max_write_sectors;
+ r1_bio->sectors = max_sectors;
/* first select target devices under rcu_lock and
* inc refcount on their rdev. Record them by setting
@@ -1560,7 +1558,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
*/
disks = conf->raid_disks * 2;
- max_sectors = r1_bio->sectors;
for (i = 0; i < disks; i++) {
struct md_rdev *rdev = conf->mirrors[i].rdev;
@@ -1576,23 +1573,21 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
if (!rdev || test_bit(Faulty, &rdev->flags))
continue;
- atomic_inc(&rdev->nr_pending);
if (test_bit(WriteErrorSeen, &rdev->flags)) {
sector_t first_bad;
sector_t bad_sectors;
int is_bad;
- is_bad = is_badblock(rdev, r1_bio->sector, max_sectors,
+ is_bad = is_badblock(rdev, sector, max_sectors,
&first_bad, &bad_sectors);
- if (is_bad && first_bad <= r1_bio->sector) {
+ if (is_bad && first_bad <= sector) {
/* Cannot write here at all */
- bad_sectors -= (r1_bio->sector - first_bad);
+ bad_sectors -= (sector - first_bad);
if (bad_sectors < max_sectors)
/* mustn't write more than bad_sectors
* to other devices yet
*/
max_sectors = bad_sectors;
- rdev_dec_pending(rdev, mddev);
continue;
}
if (is_bad) {
@@ -1606,15 +1601,18 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
* the benefit.
*/
if (bio->bi_opf & REQ_ATOMIC) {
- rdev_dec_pending(rdev, mddev);
- goto err_handle;
+ bio->bi_status = BLK_STS_NOTSUPP;
+ bio_endio(bio);
+ goto err_dec_pending;
}
- good_sectors = first_bad - r1_bio->sector;
+ good_sectors = first_bad - sector;
if (good_sectors < max_sectors)
max_sectors = good_sectors;
}
}
+
+ atomic_inc(&rdev->nr_pending);
r1_bio->bios[i] = bio;
}
@@ -1630,10 +1628,8 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
if (max_sectors < bio_sectors(bio)) {
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
- if (!bio) {
- set_bit(R1BIO_Returned, &r1_bio->state);
- goto err_handle;
- }
+ if (!bio)
+ goto err_dec_pending;
r1_bio->master_bio = bio;
r1_bio->sectors = max_sectors;
@@ -1677,7 +1673,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
mbio->bi_opf &= ~REQ_NOWAIT;
r1_bio->bios[i] = mbio;
- mbio->bi_iter.bi_sector = (r1_bio->sector + rdev->data_offset);
+ mbio->bi_iter.bi_sector = sector + rdev->data_offset;
mbio->bi_end_io = raid1_end_write_request;
if (test_bit(FailFast, &rdev->flags) &&
!test_bit(WriteMostly, &rdev->flags) &&
@@ -1686,7 +1682,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
mbio->bi_private = r1_bio;
atomic_inc(&r1_bio->remaining);
- mddev_trace_remap(mddev, mbio, r1_bio->sector);
+ mddev_trace_remap(mddev, mbio, sector);
/* flush_pending_writes() needs access to the rdev so...*/
mbio->bi_bdev = (void *)rdev;
if (!raid1_add_bio_to_plug(mddev, mbio, raid1_unplug, disks)) {
@@ -1701,9 +1697,10 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
/* In case raid1d snuck in to freeze_array */
wake_up_barrier(conf);
+
return true;
-err_handle:
+err_dec_pending:
for (k = 0; k < i; k++) {
if (r1_bio->bios[k]) {
rdev_dec_pending(conf->mirrors[k].rdev, mddev);
@@ -1711,7 +1708,11 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
}
}
- raid_end_bio_io(r1_bio);
+ free_r1bio(r1_bio);
+
+err_allow_barrier:
+ allow_barrier(conf, sector);
+
return false;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 3/4] md/raid10: fix writes_pending and barrier reference leaks on discard failures
From: Abd-Alrhman Masalkhi @ 2026-06-13 18:28 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, hare, john.g.garry,
martin.petersen, vverma
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
In-Reply-To: <20260613182810.1317258-1-abd.masalkhi@gmail.com>
raid10_make_request() acquires a writes_pending reference with
md_write_start() before calling raid10_handle_discard(). Several failure
paths in raid10_handle_discard() complete the bio and return without
releasing the corresponding reference, causing md_write_end() to be
skipped.
Call md_write_end() before returning from these failure paths to keep
writes_pending accounting balanced.
Additionally, discard split allocation failures can occur after
wait_barrier() succeeds. Those paths return without calling
allow_barrier(), leaking the associated barrier reference.
Release the barrier before returning from those paths.
Fixes: c9aa889b035f ("md: raid10 add nowait support")
Fixes: 4cf58d952909 ("md/raid10: Handle bio_split() errors")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- new patch.
---
drivers/md/raid10.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 5ad1b0c6207a..aacf160ee9f2 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1639,6 +1639,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT)) {
bio_wouldblock_error(bio);
+ md_write_end(mddev);
return 0;
}
@@ -1681,6 +1682,8 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
if (IS_ERR(split)) {
bio->bi_status = errno_to_blk_status(PTR_ERR(split));
bio_endio(bio);
+ md_write_end(mddev);
+ allow_barrier(conf);
return 0;
}
@@ -1698,6 +1701,8 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
if (IS_ERR(split)) {
bio->bi_status = errno_to_blk_status(PTR_ERR(split));
bio_endio(bio);
+ md_write_end(mddev);
+ allow_barrier(conf);
return 0;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/4] md/raid10: fix writes_pending leak on write request failures
From: Abd-Alrhman Masalkhi @ 2026-06-13 18:28 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, hare, john.g.garry,
martin.petersen, vverma
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
In-Reply-To: <20260613182810.1317258-1-abd.masalkhi@gmail.com>
raid10_make_request() acquires a writes_pending reference with
md_write_start() before dispatching write requests. Several failure
paths in raid10_write_request() complete the bio and return without
reaching the normal write completion path, causing the corresponding
md_write_end() to be skipped.
Make raid10_write_request() return a status indicating whether the write
request was successfully queued. This allows raid10_make_request() to
release the writes_pending reference with md_write_end() when a write
request fails.
Fixes: 4cf58d952909 ("md/raid10: Handle bio_split() errors")
Fixes: c9aa889b035f ("md: raid10 add nowait support")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- new patch.
---
drivers/md/raid10.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 5bd7698e0a1b..5ad1b0c6207a 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1349,7 +1349,7 @@ static void wait_blocked_dev(struct mddev *mddev, struct r10bio *r10_bio)
}
}
-static void raid10_write_request(struct mddev *mddev, struct bio *bio,
+static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
struct r10bio *r10_bio)
{
struct r10conf *conf = mddev->private;
@@ -1365,7 +1365,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
/* Bail out if REQ_NOWAIT is set for the bio */
if (bio->bi_opf & REQ_NOWAIT) {
bio_wouldblock_error(bio);
- return;
+ return false;
}
for (;;) {
prepare_to_wait(&conf->wait_barrier,
@@ -1381,7 +1381,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
sectors = r10_bio->sectors;
if (!regular_request_wait(mddev, conf, bio, sectors)) {
free_r10bio(r10_bio);
- return;
+ return false;
}
if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
@@ -1398,7 +1398,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
if (bio->bi_opf & REQ_NOWAIT) {
allow_barrier(conf);
bio_wouldblock_error(bio);
- return;
+ return false;
}
mddev_add_trace_msg(conf->mddev,
"raid10 wait reshape metadata");
@@ -1514,7 +1514,8 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
raid10_write_one_disk(mddev, r10_bio, bio, true, i);
}
one_write_done(r10_bio);
- return;
+ return true;
+
err_handle:
for (k = 0; k < i; k++) {
int d = r10_bio->devs[k].devnum;
@@ -1532,10 +1533,12 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
}
raid_end_bio_io(r10_bio);
+ return false;
}
-static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
+static bool __make_request(struct mddev *mddev, struct bio *bio, int sectors)
{
+ bool ret;
struct r10conf *conf = mddev->private;
struct r10bio *r10_bio;
@@ -1551,10 +1554,13 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) *
conf->geo.raid_disks);
+ ret = true;
if (bio_data_dir(bio) == READ)
raid10_read_request(mddev, bio, r10_bio);
else
- raid10_write_request(mddev, bio, r10_bio);
+ ret = raid10_write_request(mddev, bio, r10_bio);
+
+ return ret;
}
static void raid_end_discard_bio(struct r10bio *r10bio)
@@ -1900,7 +1906,8 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
sectors = chunk_sects -
(bio->bi_iter.bi_sector &
(chunk_sects - 1));
- __make_request(mddev, bio, sectors);
+ if (!__make_request(mddev, bio, sectors))
+ md_write_end(mddev);
/* In case raid10d snuck in to freeze_array */
wake_up_barrier(conf);
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/4] md/raid1: fix writes_pending and barrier reference leaks on write failures
From: Abd-Alrhman Masalkhi @ 2026-06-13 18:28 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, hare, john.g.garry,
martin.petersen, vverma
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi, sashiko-bot
In-Reply-To: <20260613182810.1317258-1-abd.masalkhi@gmail.com>
raid1_make_request() acquires a writes_pending reference with
md_write_start() before calling raid1_write_request(). Several failure
paths in raid1_write_request() complete the bio and return without
reaching the normal write completion path, causing the corresponding
md_write_end() to be skipped.
Make raid1_write_request() return a status indicating whether the write
request was successfully queued. This allows raid1_make_request() to
call md_write_end() when raid1_write_request() fails.
Additionally, if wait_blocked_rdev() fails after wait_barrier()
succeeds, the associated barrier reference is not released.
Call allow_barrier() before returning from that path to keep the barrier
accounting balanced.
Fixes: b1a7ad8b5c4f ("md/raid1: Handle bio_split() errors")
Fixes: f2a38abf5f1c ("md/raid1: Atomic write support")
Fixes: 5aa705039c4f ("md: raid1 add nowait support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260611083514.754922-1-abd.masalkhi@gmail.com?part=1
Closes: https://sashiko.dev/#/patchset/20260611132500.763528-1-abd.masalkhi@gmail.com?part=1
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- fix writes_pending leaks in addition to the barrier reference leak.
- make raid1_write_request() return whether the write was successfully
queued so raid1_make_request() can release writes_pending references
on failure paths.
- Link v1: https://lore.kernel.org/linux-raid/20260611132500.763528-1-abd.masalkhi@gmail.com/
---
drivers/md/raid1.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index b1ed4cc6ade4..f0e1c7125972 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1501,7 +1501,7 @@ static void raid1_start_write_behind(struct mddev *mddev, struct r1bio *r1_bio,
}
-static void raid1_write_request(struct mddev *mddev, struct bio *bio,
+static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
int max_write_sectors)
{
struct r1conf *conf = mddev->private;
@@ -1512,6 +1512,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
int max_sectors;
bool write_behind = false;
bool is_discard = (bio_op(bio) == REQ_OP_DISCARD);
+ sector_t sector = bio->bi_iter.bi_sector;
if (mddev_is_clustered(mddev) &&
mddev->cluster_ops->area_resyncing(mddev, WRITE,
@@ -1519,7 +1520,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
if (bio->bi_opf & REQ_NOWAIT) {
bio_wouldblock_error(bio);
- return;
+ return false;
}
wait_event_idle(conf->wait_barrier,
!mddev->cluster_ops->area_resyncing(mddev, WRITE,
@@ -1535,12 +1536,13 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
if (!wait_barrier(conf, bio->bi_iter.bi_sector,
bio->bi_opf & REQ_NOWAIT)) {
bio_wouldblock_error(bio);
- return;
+ return false;
}
if (!wait_blocked_rdev(mddev, bio)) {
bio_wouldblock_error(bio);
- return;
+ allow_barrier(conf, sector);
+ return false;
}
r1_bio = alloc_r1bio(mddev, bio);
@@ -1699,7 +1701,8 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
/* In case raid1d snuck in to freeze_array */
wake_up_barrier(conf);
- return;
+ return true;
+
err_handle:
for (k = 0; k < i; k++) {
if (r1_bio->bios[k]) {
@@ -1709,6 +1712,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
}
raid_end_bio_io(r1_bio);
+ return false;
}
static bool raid1_make_request(struct mddev *mddev, struct bio *bio)
@@ -1732,8 +1736,9 @@ static bool raid1_make_request(struct mddev *mddev, struct bio *bio)
if (bio_data_dir(bio) == READ)
raid1_read_request(mddev, bio, sectors, NULL);
else {
- md_write_start(mddev,bio);
- raid1_write_request(mddev, bio, sectors);
+ md_write_start(mddev, bio);
+ if (!raid1_write_request(mddev, bio, sectors))
+ md_write_end(mddev);
}
return true;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/4] md/raid1,raid10: fix write-path reference leaks and clean up error handling
From: Abd-Alrhman Masalkhi @ 2026-06-13 18:28 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, hare, john.g.garry,
martin.petersen, vverma
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi
Hi,
This series fixes several write-path failure handling issues in raid1 and
raid10 and then follows up with a cleanup of raid1_write_request().
The first two patches fix writes_pending leaks caused by failure paths
that complete bios without reaching the normal write completion path.
The raid1 fix also addresses a barrier reference leak when
wait_blocked_rdev() fails after wait_barrier() succeeds.
The third patch fixes additional writes_pending and barrier reference
leaks in raid10 discard handling.
The final patch simplifies raid1_write_request() error handling.
Patches:
md/raid1: fix writes_pending and barrier reference leaks on write failures
md/raid10: fix writes_pending leak on write request failures
md/raid10: fix writes_pending and barrier reference leaks on discard failures
md/raid1: simplify raid1_write_request() error handling
Changes in v2:
- fix writes_pending leaks in addition to the barrier reference leaks
- add raid10 fixes for analogous write and discard failure paths
- add a follow-up cleanup patch to simplify raid1_write_request()
- simplifies raid1_write_request() error handling.
- Link v1: https://lore.kernel.org/linux-raid/20260611132500.763528-1-abd.masalkhi@gmail.com/
Thanks,
Abd-Alrhman
Abd-Alrhman Masalkhi (4):
md/raid1: fix writes_pending and barrier reference leaks on write
failures
md/raid10: fix writes_pending leak on write request failures
md/raid10: fix writes_pending and barrier reference leaks on discard
failures
md/raid1: simplify raid1_write_request() error handling
drivers/md/raid1.c | 74 ++++++++++++++++++++++++---------------------
drivers/md/raid10.c | 28 ++++++++++++-----
2 files changed, 60 insertions(+), 42 deletions(-)
--
2.43.0
^ permalink raw reply
* AVX-512 Optimization For Linux RAID Showing Up To 41% Improvement
From: Anton Gavriliuk @ 2026-06-12 16:19 UTC (permalink / raw)
To: linux-raid
Hi
https://www.phoronix.com/news/AVX-512-Linux-RAID-Optimization
I'm just want to clarify - will help to improve write performance and
raid56 build/rebuid time on modern hardware - amd zen5 (9005 series)
and pci5.0 nvme ssds ?
Anton
^ permalink raw reply
* Re: [PATCH] raid10: badblock-aware reshape write error handling
From: caogh @ 2026-06-12 8:05 UTC (permalink / raw)
To: Li Nan; +Cc: linux-kernel, linux-raid, song, xiao, yukuai
In-Reply-To: <tencent_1A4057A928F0CDF70F24A070B03ABFB34A05@qq.com>
Nan, thanks for the review.
You're right that the current code chains several mechanisms (badblock
recording, WantReplacement, md_error) together, which hurts readability.
I'll refactor in the next version to make the logic clearer.
However, regarding your suggested logic:
> The correct logic might be:
> 1. If there is a replacement, fail directly with md_error
> 2. If not, mark the badblock and set WantReplacement
I have some questions.
If a replacement device hits a write error, calling md_error directly
will mark it Faulty and kick it out of the array. But it may just be a
few bad sectors — recording the badblock and continuing the reshape is
sufficient. Is it really necessary to call md_error directly in this case?
The current patch does:
1、Try to record the badblock first
2、On success:
a) member device: set WantReplacement to trigger a replacement
b) replacement device: skip WantReplacement to avoid a
replacement loop
3、On failure: rdev_set_badblocks has already called md_error
internally, no further action needed
Does this approach seem reasonable?
在 2026/6/12 11:42, Li Nan 写道:
> On Mon Jun 1, 2026 at 1:46 PM CST, ghuicao wrote:
>> From: Cao Guanghui<caoguanghui@kylinos.cn>
>>
>> Replace the FIXME in end_reshape_write(). Instead of failing the device
>> immediately on write errors during reshape, attempt to record badblocks
>> using new_data_offset with is_new=1.
>>
>> rdev_set_badblocks() returns true on success. On failure (e.g., badblocks
>> table full), it has already called md_error() internally to degrade the
>> device. Queue WantReplacement for member devices regardless of badblock
>> recording success, but skip this for replacement devices to avoid
>> replacement loops.
>>
>> On successful write, clear stale badblock records at the new location
>> since data has migrated.
>>
>> Signed-off-by: Cao Guanghui<caoguanghui@kylinos.cn>
>> ---
>> drivers/md/raid10.c | 27 ++++++++++++++++++++++++---
>> 1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index 4901ebe45c87..08d58a1c680e 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -4991,9 +4991,30 @@ static void end_reshape_write(struct bio *bio)
>> conf->mirrors[d].rdev;
>>
>> if (bio->bi_status) {
>> - /* FIXME should record badblock */
>> - md_error(mddev, rdev);
>> - }
>> + set_bit(WriteErrorSeen, &rdev->flags);
>> +
>> + /* rdev_set_badblocks returns true on success.
>> + * On failure, it has already called md_error() internally.
>> + * Use is_new=1 as reshape writes target the new layout
>> + * (new_data_offset).
>> + */
>> + if (rdev_set_badblocks(rdev, r10_bio->devs[slot].addr,
>> + r10_bio->sectors, 1)) {
>> + /* Queue async replacement for member devices
>> + * For replacement devices, do not trigger WantReplacement
>> + * to avoid circular replacement storms.
>> + */
>> + if (!repl) {
>> + if (!test_and_set_bit(WantReplacement, &rdev->flags))
>> + set_bit(MD_RECOVERY_NEEDED,
>> + &rdev->mddev->recovery);
> The logic here seems a bit odd — several mechanisms are chained together.
>
> The correct logic might be:
> 1. If there is a replacement, fail directly with md_error
> 2. If not, mark the badblock and set WantReplacement
>
>> + }
>> + }
>> + } else {
>> + /* Write succeeded, clear stale badblock records */
>> + rdev_clear_badblocks(rdev, r10_bio->devs[slot].addr,
>> + r10_bio->sectors, 1);
>> + }
>>
>> rdev_dec_pending(rdev, mddev);
>> end_reshape_request(r10_bio);
>>
Thanks, Cao Guanghui
^ permalink raw reply
* Re: [PATCH] md: use READ_ONCE() for rdev_attr_show() mddev check
From: Li Nan @ 2026-06-12 6:22 UTC (permalink / raw)
To: Runyu Xiao, song, yukuai3, linux-raid
Cc: lilingfeng3, linux-kernel, jianhao.xu
In-Reply-To: <20260611083510.476945-1-runyu.xiao@seu.edu.cn>
On Thu Jun 11, 2026 at 4:35 PM CST, Runyu Xiao wrote:
> md_kick_rdev_from_array() clears rdev->mddev with
> WRITE_ONCE(rdev->mddev, NULL), and rdev_attr_store() already snapshots
> that same shared pointer with READ_ONCE(rdev->mddev). rdev_attr_show()
> still tests the pointer with a plain lockless `if (!rdev->mddev)` before
> calling entry->show().
>
> A running system can reach this by reading rdev sysfs attributes while a
> device removal path is tearing the same md_rdev down. In that window,
> rdev_attr_show() can pass a stale plain guard while sibling
> rdev_attr_store() already observes NULL and returns -ENODEV, leaving the
> show path to operate after the shared mddev pointer has been revoked.
>
> Use READ_ONCE() in rdev_attr_show() so this sysfs read side matches the
> existing visibility contract on rdev->mddev.
>
> Fixes: 9cfcf99e7ed6 ("md: get rdev->mddev with READ_ONCE()")
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
> drivers/md/md.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 92ec4be20db8..a021cf4a798d 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -3704,7 +3704,7 @@ rdev_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
>
> if (!entry->show)
> return -EIO;
> - if (!rdev->mddev)
> + if (!READ_ONCE(rdev->mddev))
> return -ENODEV;
> return entry->show(rdev, page);
> }
LGTM
Reviewed-by: Li Nan <magiclinan@didiglobal.com>
--
Thanks,
Nan
^ permalink raw reply
* Re: [PATCH] md/raid1: release barrier when REQ_NOWAIT write would block
From: Abd-Alrhman Masalkhi @ 2026-06-12 5:27 UTC (permalink / raw)
To: sashiko-reviews; +Cc: yukuai, linux-raid
In-Reply-To: <20260611134130.B4E911F00893@smtp.kernel.org>
On Thu, Jun 11, 2026 at 13:41 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Missing RCU read lock around `conf->mirrors[i].rdev` access in `wait_blocked_rdev()`.
I'm not quite sure whether this is a real bug.
> - [High] Early failure paths in `raid1_write_request()` leak the `mddev->writes_pending` reference.
I'll submit a v2 containing this fix, along with the fix for the
mddev->writes_pending
> - [High] Allocating bios and r1bios can sleep even when the `REQ_NOWAIT` flag is set.
@kuai, Should I handle this issue as well, or should I just ignore it
for now?
> --
>
> commit 3db9da0ec804800aa3afb93361ccd18de39ce5f0
> Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>
> md/raid1: release barrier when REQ_NOWAIT write would block
>
> raid1_write_request() leaks the barrier reference taken by wait_barrier()
> if wait_blocked_rdev() fails for a REQ_NOWAIT write. This patch releases
> the barrier on this failure path to prevent array freeze hangs.
>
> Closes: https://sashiko.dev/#/patchset/20260611083514.754922-1-abd.masalkhi@gmail.com?part=1
>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index 5b9368bd9e700..f3cc30932ba2b 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -1512,6 +1512,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>> int max_sectors;
>> bool write_behind = false;
>> bool is_discard = (bio_op(bio) == REQ_OP_DISCARD);
>> + sector_t sector = bio->bi_iter.bi_sector;
>>
>> if (mddev_is_clustered(mddev) &&
>> mddev->cluster_ops->area_resyncing(mddev, WRITE,
>> @@ -1540,6 +1541,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>>
>> if (!wait_blocked_rdev(mddev, bio)) {
>> bio_wouldblock_error(bio);
>> + allow_barrier(conf, sector);
>
> [Severity: High]
> This is a pre-existing issue, but does this early return path also leak the
> mddev->writes_pending reference?
>
> In raid1_make_request(), md_write_start() unconditionally takes a per-cpu
> reference on mddev->writes_pending before calling raid1_write_request().
> Normally, this is released via md_write_end() inside close_write().
>
> By directly returning here after bio_wouldblock_error(bio), we never call
> md_write_end(). Does this permanently leak the writes_pending counter and
> prevent the array from transitioning to a clean state?
>
> It looks like this same leak exists on the clustered area_resyncing and
> wait_barrier REQ_NOWAIT failure paths earlier in this function, as well as
> the err_handle path if bio_submit_split_bioset() fails.
>
>> return;
>> }
>
> [Severity: Critical]
> This is a pre-existing issue, but is it safe to call wait_blocked_rdev()
> here without holding the RCU read lock?
>
> Looking at wait_blocked_rdev():
>
> for (i = 0; i < disks; i++) {
> struct md_rdev *rdev = conf->mirrors[i].rdev;
>
> if (!rdev)
> continue;
>
> /* don't write here until the bad block is acknowledged */
> if (test_bit(WriteErrorSeen, &rdev->flags) &&
>
> It accesses the rdev pointer and its fields without rcu_read_lock().
> Could a concurrent device removal (via md_ioctl -> raid1_remove_conf)
> replace the pointer with NULL and free the rdev structure while we are
> dereferencing it here, resulting in a use-after-free?
>
> [Severity: High]
> This is also a pre-existing issue, but if we proceed past this point for a
> REQ_NOWAIT bio, we call alloc_r1bio() and eventually bio_alloc_clone(),
> both of which use GFP_NOIO.
>
> Since memory pool allocations with GFP_NOIO can block when the mempool is
> exhausted under memory pressure, could this cause REQ_NOWAIT writes to
> sleep, defeating the purpose of the REQ_NOWAIT flag?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260611132500.763528-1-abd.masalkhi@gmail.com?part=1
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox