* Re: [PATCH 2/8] um: Check for missing AVX and AVX-512 xstate bits
From: David Laight @ 2026-06-26 7:41 UTC (permalink / raw)
To: Eric Biggers
Cc: x86, linux-um, linux-raid, linux-crypto, linux-kernel,
Christoph Hellwig, Andrew Morton
In-Reply-To: <20260626043731.319287-3-ebiggers@kernel.org>
On Thu, 25 Jun 2026 21:37:25 -0700
Eric Biggers <ebiggers@kernel.org> wrote:
> If the CPU declares AVX or AVX-512 support, verify that all the
> corresponding xstate bits are also set. If any are missing, warn and
> don't set the corresponding X86_FEATURE_* flags.
>
> This eliminates the perceived need for UML-supporting AVX and AVX-512
> optimized code in the kernel (that is, lib/raid/ currently) to start
> checking the xstate bits in addition to X86_FEATURE_AVX*.
>
...
> static void __init parse_host_cpu_flags(char *line)
> {
> + u64 xcr0 = read_xcr0();
> int i;
> +
> for (i = 0; i < 32*NCAPINTS; i++) {
> if ((x86_cap_flags[i] != NULL) && strstr(line, x86_cap_flags[i]))
'line' comes from /proc/cpuinfo
Surely something would be terribly wrong if that included something the kernel
had disabled (or didn't support).
David
> - set_cpu_cap(&boot_cpu_data, i);
> + validate_and_set_cpu_cap(i, xcr0);
> }
> }
>
> static void __init parse_cache_line(char *line)
> {
^ permalink raw reply
* Re: [PATCH 01/18] raid6: turn the userspace test harness into a kunit test
From: patchwork-bot+linux-riscv @ 2026-06-26 8:20 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-riscv, akpm, catalin.marinas, will, ardb, chenhuacai,
kernel, maddy, mpe, npiggin, chleroy, pjw, palmer, aou, alex, hca,
gor, agordeev, borntraeger, svens, tglx, mingo, bp, dave.hansen,
x86, hpa, herbert, dan.j.williams, clm, dsterba, arnd, song,
yukuai, linan122, linux-kernel, linux-arm-kernel, loongarch,
linuxppc-dev, linux-s390, linux-crypto, linux-btrfs, linux-arch,
linux-raid
In-Reply-To: <20260518051804.462141-2-hch@lst.de>
Hello:
This series was applied to riscv/linux.git (fixes)
by Andrew Morton <akpm@linux-foundation.org>:
On Mon, 18 May 2026 07:17:44 +0200 you wrote:
> Currently the raid6 code can be compiled as userspace code to run the
> test suite. Convert that to be a kunit case with minimal changes to
> avoid mutating global state so that we can drop this requirement.
>
> Note that this is not a good kunit test case yet and will need a lot more
> work, but that is deferred until the raid6 code is moved to it's new
> place, which is easier if the userspace makefile doesn't need adjustments
> for the new location first.
>
> [...]
Here is the summary with links:
- [01/18] raid6: turn the userspace test harness into a kunit test
https://git.kernel.org/riscv/c/c4697486fc23
- [02/18] raid6: remove __KERNEL__ ifdefs
https://git.kernel.org/riscv/c/3d6beb659ddf
- [03/18] raid6: move to lib/raid/
https://git.kernel.org/riscv/c/3626738bc714
- [04/18] raid6: remove unused defines in pq.h
https://git.kernel.org/riscv/c/06d2a66fb7c0
- [05/18] raid6: remove raid6_get_zero_page
https://git.kernel.org/riscv/c/885d31423183
- [06/18] raid6: use named initializers for struct raid6_calls
https://git.kernel.org/riscv/c/7e91f76a9668
- [07/18] raid6: improve the public interface
https://git.kernel.org/riscv/c/35472bc6f31b
- [08/18] raid6: warn when using less than four devices
https://git.kernel.org/riscv/c/2790045a62eb
- [09/18] raid6: hide internals
(no matching commit)
- [10/18] raid6: rework registration of optimized algorithms
(no matching commit)
- [11/18] raid6: use static_call for gen_syndrom and xor_syndrom
https://git.kernel.org/riscv/c/10f4b8e2a164
- [12/18] raid6: use static_call for raid6_recov_2data and raid6_recov_datap
https://git.kernel.org/riscv/c/dd83de0341da
- [13/18] raid6: update top of file comments
https://git.kernel.org/riscv/c/30bf04bd13a5
- [14/18] raid6_kunit: use KUNIT_CASE_PARAM
https://git.kernel.org/riscv/c/2175395f76c3
- [15/18] raid6_kunit: dynamically allocate data buffers using vmalloc
https://git.kernel.org/riscv/c/d67c25712fe3
- [16/18] raid6_kunit: cleanup dataptr handling
https://git.kernel.org/riscv/c/562bcbfcb99b
- [17/18] raid6_kunit: randomize parameters and increase limits
https://git.kernel.org/riscv/c/fa0c812c0aa5
- [18/18] raid6_kunit: randomize buffer alignment
https://git.kernel.org/riscv/c/8cf0a6c4bb9e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH 01/19] btrfs: require at least 4 devices for RAID 6
From: patchwork-bot+linux-riscv @ 2026-06-26 8:21 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-riscv, akpm, catalin.marinas, will, ardb, chenhuacai,
kernel, maddy, mpe, npiggin, chleroy, pjw, palmer, aou, alex, hca,
gor, agordeev, borntraeger, svens, tglx, mingo, bp, dave.hansen,
x86, hpa, herbert, dan.j.williams, clm, dsterba, arnd, song,
yukuai, linan122, linux-kernel, linux-arm-kernel, loongarch,
linuxppc-dev, linux-s390, linux-crypto, linux-btrfs, linux-arch,
linux-raid
In-Reply-To: <20260512052230.2947683-2-hch@lst.de>
Hello:
This series was applied to riscv/linux.git (fixes)
by Andrew Morton <akpm@linux-foundation.org>:
On Tue, 12 May 2026 07:20:41 +0200 you wrote:
> While the RAID6 algorithm could in theory support 3 devices by just
> copying the data disk to the two parity disks, this version is not only
> useless because it is a suboptimal version of 3-way mirroring, but also
> broken with various crashes and incorrect parity generation in various
> architecture-optimized implementations. Disallow it similar to mdraid
> which requires at least 4 devices for RAID 6.
>
> [...]
Here is the summary with links:
- [01/19] btrfs: require at least 4 devices for RAID 6
(no matching commit)
- [02/19] raid6: turn the userspace test harness into a kunit test
(no matching commit)
- [03/19] raid6: remove __KERNEL__ ifdefs
https://git.kernel.org/riscv/c/3d6beb659ddf
- [04/19] raid6: move to lib/raid/
(no matching commit)
- [05/19] raid6: remove unused defines in pq.h
https://git.kernel.org/riscv/c/06d2a66fb7c0
- [06/19] raid6: remove raid6_get_zero_page
https://git.kernel.org/riscv/c/885d31423183
- [07/19] raid6: use named initializers for struct raid6_calls
https://git.kernel.org/riscv/c/7e91f76a9668
- [08/19] raid6: improve the public interface
(no matching commit)
- [09/19] raid6: hide internals
(no matching commit)
- [10/19] raid6: rework the init helpers
(no matching commit)
- [11/19] raid6: use static_call for gen_syndrom and xor_syndrom
https://git.kernel.org/riscv/c/10f4b8e2a164
- [12/19] raid6: use static_call for raid6_recov_2data and raid6_recov_datap
(no matching commit)
- [13/19] raid6: update top of file comments
https://git.kernel.org/riscv/c/30bf04bd13a5
- [14/19] raid6_kunit: use KUNIT_CASE_PARAM
https://git.kernel.org/riscv/c/2175395f76c3
- [15/19] raid6_kunit: dynamically allocate data buffers using vmalloc
https://git.kernel.org/riscv/c/d67c25712fe3
- [16/19] raid6_kunit: cleanup dataptr handling
https://git.kernel.org/riscv/c/562bcbfcb99b
- [17/19] raid6_kunit: randomize parameters and increase limits
(no matching commit)
- [18/19] raid6_kunit: randomize parameters and increase limits
(no matching commit)
- [19/19] raid6_kunit: randomize buffer alignment
https://git.kernel.org/riscv/c/8cf0a6c4bb9e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH v2 next] drivers/md/md: Use bounded strings ops when setting disk->disk_name
From: David Laight @ 2026-06-26 8:43 UTC (permalink / raw)
To: Kees Cook, linux-hardening, linux-kernel, linux-raid
Cc: Arnd Bergmann, Song Liu, Yu Kuai, David Laight
Replace strcpy() with strscpy() and sprintf() with snprintf() for safety.
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
v2: Make sizeof look like a function call for consistency.
This is one of a group of patches that remove potentially unbounded
strcpy() calls.
They are mostly replaced by strscpy() or, when strlen() has just been
called, with memcpy() (usually including the '\0').
Calls with copy string literals into arrays are left unchanged.
They are safe and easily detected as such.
The changes were made by getting the compiler to detect the calls and
then fixing the code by hand.
Note that all the changes are only compile tested.
Some Makefiles were changed to allow files to contain strcpy().
As well as 'difficult to fix' files, this included 'show' functions
as they really need to use sysfs_emit() or seq_printf().
All the patches are being sent individually to avoid very long cc lists.
Apologies for the terse commit messages and likely unexpected tags.
(There are about 100 patches in total.)
drivers/md/md.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 8b568eee8743..4b659d10fa4d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6346,11 +6346,11 @@ struct mddev *md_alloc(dev_t dev, char *name)
disk->first_minor = unit << shift;
disk->minors = 1 << shift;
if (name)
- strcpy(disk->disk_name, name);
+ strscpy(disk->disk_name, name);
else if (partitioned)
- sprintf(disk->disk_name, "md_d%d", unit);
+ snprintf(disk->disk_name, sizeof(disk->disk_name), "md_d%d", unit);
else
- sprintf(disk->disk_name, "md%d", unit);
+ snprintf(disk->disk_name, sizeof (disk->disk_name), "md%d", unit);
disk->fops = &md_fops;
disk->private_data = mddev;
--
2.39.5
^ permalink raw reply related
* Re: [PATCH v2 next] drivers/md/md: Use bounded strings ops when setting disk->disk_name
From: sashiko-bot @ 2026-06-26 8:51 UTC (permalink / raw)
To: David Laight; +Cc: yukuai, linux-raid
In-Reply-To: <20260626084334.3012-1-david.laight.linux@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Inconsistent coding style for the sizeof operator in newly added code.
--
commit 663613700e83a5bb9c1749d80b4fae68752645fb
Author: David Laight <david.laight.linux@gmail.com>
drivers/md/md: Use bounded strings ops when setting disk->disk_name
This commit replaces unbounded strcpy() and sprintf() with bounded strscpy()
and snprintf() operations when setting the disk_name in md_alloc() to improve
safety.
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c81..afbdc52a11270 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6354,11 +6354,11 @@ struct mddev *md_alloc(dev_t dev, char *name)
[ ... ]
> if (name)
> - strcpy(disk->disk_name, name);
> + strscpy(disk->disk_name, name);
> else if (partitioned)
> - sprintf(disk->disk_name, "md_d%d", unit);
> + snprintf(disk->disk_name, sizeof(disk->disk_name), "md_d%d", unit);
> else
> - sprintf(disk->disk_name, "md%d", unit);
> + snprintf(disk->disk_name, sizeof (disk->disk_name), "md%d", unit);
[Severity: Low]
This isn't a bug, but there is an inconsistent coding style for the sizeof
operator here in md_alloc(). The adjacent branch uses sizeof(disk->disk_name)
without a space, while this newly added branch includes a space. The kernel
coding style prefers no space after sizeof.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626084334.3012-1-david.laight.linux@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 2/8] um: Check for missing AVX and AVX-512 xstate bits
From: Anton Ivanov @ 2026-06-26 8:21 UTC (permalink / raw)
To: David Laight, Eric Biggers
Cc: x86, linux-um, linux-raid, linux-crypto, linux-kernel,
Christoph Hellwig, Andrew Morton
In-Reply-To: <20260626084113.42eae31c@pumpkin>
On 26/06/2026 08:41, David Laight wrote:
> On Thu, 25 Jun 2026 21:37:25 -0700
> Eric Biggers <ebiggers@kernel.org> wrote:
>
>> If the CPU declares AVX or AVX-512 support, verify that all the
>> corresponding xstate bits are also set. If any are missing, warn and
>> don't set the corresponding X86_FEATURE_* flags.
>>
>> This eliminates the perceived need for UML-supporting AVX and AVX-512
>> optimized code in the kernel (that is, lib/raid/ currently) to start
>> checking the xstate bits in addition to X86_FEATURE_AVX*.
>>
> ...
>> static void __init parse_host_cpu_flags(char *line)
>> {
>> + u64 xcr0 = read_xcr0();
>> int i;
>> +
>> for (i = 0; i < 32*NCAPINTS; i++) {
>> if ((x86_cap_flags[i] != NULL) && strstr(line, x86_cap_flags[i]))
>
> 'line' comes from /proc/cpuinfo
> Surely something would be terribly wrong if that included something the kernel
> had disabled (or didn't support).
>
> David
>
>
>> - set_cpu_cap(&boot_cpu_data, i);
>> + validate_and_set_cpu_cap(i, xcr0);
>> }
>> }
>>
>> static void __init parse_cache_line(char *line)
>> {
>
>
>
>
Lots of other stuff will go wrong before that. Glibc, things compiled with LLVM, python, perl, etc.
Half of the userland will go belly up, because AVX is used in string operations and hashing if it is available.
UML is just another userland application from this perspective, so there is no reason for it to behave any different from the rest of the userland.
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
^ permalink raw reply
* Re: [PATCH 4/7] md/raid10: raid10_write_request() drops the barrier before calling
From: Abd-Alrhman Masalkhi @ 2026-06-26 9:35 UTC (permalink / raw)
To: yu kuai, song, magiclinan, xiao, axboe, john.g.garry,
martin.petersen, yukuai
Cc: linux-raid, linux-kernel
In-Reply-To: <draft-m2h5ms1i5p.fsf@gmail.com>
Hi Kuai,
On Wed, Jun 24, 2026 at 10:01 +0200, Abd-Alrhman Masalkhi wrote:
> On Wed, Jun 24, 2026 at 15:23 +0800, yu kuai wrote:
>> Hi,
>>
>> 在 2026/6/23 15:24, Abd-Alrhman Masalkhi 写道:
>>> bio_submit_split_bioset() and reacquires it afterwards. This is
>>> unnecessary because bio_submit_split_bioset() does not require
>>> releasing the barrier protection.
>>
>> Why is this safe?
>>
>> It's possible plug is not enabled in this context, and the allocated new
>> aplit bio will be submitted and enter wait_barrier() again. wait_barrier()
>> is not supposed to be called recursively. Otherwise deadlock can happend:
>>
>> t1: sync_thread grab barrier and waiting for nr_pending to be 0 from raise_barrier()
>> t2: wait_barrier() first inc nr_pending, then recursive wait_barrier() waiting
>> for barrier to be released.
>>
>
> raid10_write_request() is only called from the bio submission path and
> not from raid10d thread, so we do not re-enter it while already holding
> barrier. Therefore, the split bio submitted by bio_submit_split_bioset()
> cannot recurse back into wait_barrier() from the same execution path.
I looked at commit e820d55cb99d. At the time, it addressed this issue
because submit_flushes() called md_handle_request() directly. Since
v5.2, submit_flushes() has gone through submit_bio() instead before
submit_flushes() was eventually removed.
Today, md_handle_request() is called from md_submit_bio() and dm-raid.
I don't have much experience with dm, but at first glance it seems safe
to remove these calls. However, if md_handle_request() were ever invoked
from a worker thread or kthread, we could run into the same kind of
issues, not only for RAID10, but also for RAID1.
What do you think? Should I drop this patch and guard RAID1 as well, or
is it safe to assume that dm-raid cannot hit this path from a kthread
or a workqueue worker?
>
> The situation is different for reads, where raid10_read_request() will
> be called both from the submission path and from raid10d. There,
> dropping and reacquiring the barrier protection remains necessary.
>
>>>
>>> Remove the redundant allow_barrier()/wait_barrier() pair around
>>> bio_submit_split_bioset().
>>>
>>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>>> ---
>>> drivers/md/raid10.c | 2 --
>>> 1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>>> index 840f0446c231..4bc1d5553ec7 100644
>>> --- a/drivers/md/raid10.c
>>> +++ b/drivers/md/raid10.c
>>> @@ -1493,10 +1493,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
>>> if (atomic)
>>> goto err_handle;
>>>
>>> - allow_barrier(conf);
>>> bio = bio_submit_split_bioset(bio, r10_bio->sectors,
>>> &conf->bio_split);
>>> - wait_barrier(conf, false);
>>> if (!bio) {
>>> set_bit(R10BIO_Returned, &r10_bio->state);
>>> goto err_handle;
>>
>> --
>> Thanks,
>> Kuai
>
> --
> Best Regards,
> Abd-Alrhman
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH 2/8] um: Check for missing AVX and AVX-512 xstate bits
From: David Laight @ 2026-06-26 10:49 UTC (permalink / raw)
To: Anton Ivanov
Cc: Eric Biggers, x86, linux-um, linux-raid, linux-crypto,
linux-kernel, Christoph Hellwig, Andrew Morton
In-Reply-To: <6a20b442-b97f-4cae-9168-30201d5ef82c@cambridgegreys.com>
On Fri, 26 Jun 2026 09:21:49 +0100
Anton Ivanov <anton.ivanov@cambridgegreys.com> wrote:
> On 26/06/2026 08:41, David Laight wrote:
> > On Thu, 25 Jun 2026 21:37:25 -0700
> > Eric Biggers <ebiggers@kernel.org> wrote:
> >
> >> If the CPU declares AVX or AVX-512 support, verify that all the
> >> corresponding xstate bits are also set. If any are missing, warn and
> >> don't set the corresponding X86_FEATURE_* flags.
> >>
> >> This eliminates the perceived need for UML-supporting AVX and AVX-512
> >> optimized code in the kernel (that is, lib/raid/ currently) to start
> >> checking the xstate bits in addition to X86_FEATURE_AVX*.
> >>
> > ...
> >> static void __init parse_host_cpu_flags(char *line)
> >> {
> >> + u64 xcr0 = read_xcr0();
> >> int i;
> >> +
> >> for (i = 0; i < 32*NCAPINTS; i++) {
> >> if ((x86_cap_flags[i] != NULL) && strstr(line, x86_cap_flags[i]))
> >
> > 'line' comes from /proc/cpuinfo
> > Surely something would be terribly wrong if that included something the kernel
> > had disabled (or didn't support).
> >
> > David
> >
> >
> >> - set_cpu_cap(&boot_cpu_data, i);
> >> + validate_and_set_cpu_cap(i, xcr0);
> >> }
> >> }
> >>
> >> static void __init parse_cache_line(char *line)
> >> {
> >
> >
> >
> >
> Lots of other stuff will go wrong before that. Glibc, things compiled with LLVM, python, perl, etc.
>
> Half of the userland will go belly up, because AVX is used in string operations and hashing if it is available.
And glibc will check xcr0.
>
> UML is just another userland application from this perspective, so there is no reason for it to behave any different from the rest of the userland.
^ permalink raw reply
* Re: [PATCH v2 next] drivers/md/md: Use bounded strings ops when setting disk->disk_name
From: David Laight @ 2026-06-26 10:52 UTC (permalink / raw)
To: sashiko-bot; +Cc: sashiko-reviews, yukuai, linux-raid
In-Reply-To: <20260626085141.3AEBC1F000E9@smtp.kernel.org>
On Fri, 26 Jun 2026 08:51:40 +0000
sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Inconsistent coding style for the sizeof operator in newly added code.
> --
>
> commit 663613700e83a5bb9c1749d80b4fae68752645fb
> Author: David Laight <david.laight.linux@gmail.com>
>
> drivers/md/md: Use bounded strings ops when setting disk->disk_name
>
> This commit replaces unbounded strcpy() and sprintf() with bounded strscpy()
> and snprintf() operations when setting the disk_name in md_alloc() to improve
> safety.
>
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index d1465bcd86c81..afbdc52a11270 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -6354,11 +6354,11 @@ struct mddev *md_alloc(dev_t dev, char *name)
> [ ... ]
> > if (name)
> > - strcpy(disk->disk_name, name);
> > + strscpy(disk->disk_name, name);
> > else if (partitioned)
> > - sprintf(disk->disk_name, "md_d%d", unit);
> > + snprintf(disk->disk_name, sizeof(disk->disk_name), "md_d%d", unit);
> > else
> > - sprintf(disk->disk_name, "md%d", unit);
> > + snprintf(disk->disk_name, sizeof (disk->disk_name), "md%d", unit);
>
> [Severity: Low]
> This isn't a bug, but there is an inconsistent coding style for the sizeof
> operator here in md_alloc(). The adjacent branch uses sizeof(disk->disk_name)
> without a space, while this newly added branch includes a space. The kernel
> coding style prefers no space after sizeof.
>
B****r I only fixed one of them :-(
David
^ permalink raw reply
* Re: [PATCH 2/8] um: Check for missing AVX and AVX-512 xstate bits
From: Eric Biggers @ 2026-06-26 20:55 UTC (permalink / raw)
To: David Laight
Cc: Anton Ivanov, x86, linux-um, linux-raid, linux-crypto,
linux-kernel, Christoph Hellwig, Andrew Morton
In-Reply-To: <20260626114957.1a2b7e5b@pumpkin>
On Fri, Jun 26, 2026 at 11:49:57AM +0100, David Laight wrote:
> > UML is just another userland application from this perspective, so
> > there is no reason for it to behave any different from the rest of
> > the userland.
Which is why it should do the XCR0 check that the vast majority of
userspace programs do, right? It has always been part of the documented
way to detect AVX and AVX-512 support.
(I think this helps explain why LLMs notice this too. They've been
trained on lots of code that does it correctly.)
That being said, it does seem likely that it's basically obsolete now.
So maybe we could take a shortcut and omit it.
The important thing is really that we make a definitive decision *once*
for each of UML and native x86. The status quo is that the decision is
instead punted to every individual AVX optimized function in the kernel,
which isn't working well.
- Eric
^ permalink raw reply
* Re: [PATCH 2/8] um: Check for missing AVX and AVX-512 xstate bits
From: David Laight @ 2026-06-26 21:33 UTC (permalink / raw)
To: Eric Biggers
Cc: Anton Ivanov, x86, linux-um, linux-raid, linux-crypto,
linux-kernel, Christoph Hellwig, Andrew Morton
In-Reply-To: <20260626205544.GB2368695@google.com>
On Fri, 26 Jun 2026 20:55:44 +0000
Eric Biggers <ebiggers@kernel.org> wrote:
> On Fri, Jun 26, 2026 at 11:49:57AM +0100, David Laight wrote:
> > > UML is just another userland application from this perspective, so
> > > there is no reason for it to behave any different from the rest of
> > > the userland.
>
> Which is why it should do the XCR0 check that the vast majority of
> userspace programs do, right? It has always been part of the documented
> way to detect AVX and AVX-512 support.
That and looking at the other 'cpuid' registers rather than reading
/proc/cpuinfo.
If you want to run 'um' on (say) NetBSD you'd need to do it differently.
> (I think this helps explain why LLMs notice this too. They've been
> trained on lots of code that does it correctly.)
>
> That being said, it does seem likely that it's basically obsolete now.
> So maybe we could take a shortcut and omit it.
It would need the linux kernel to report a cpu feature that needed
kernel support, but that the kernel didn't support.
But for things like popcnt um should probably check the relevant cpuid bit
rather than scanning /proc/cpuinfo.
>
> The important thing is really that we make a definitive decision *once*
> for each of UML and native x86. The status quo is that the decision is
> instead punted to every individual AVX optimized function in the kernel,
> which isn't working well.
Indeed.
David
>
> - Eric
^ permalink raw reply
* Re: cleanup the RAID6 P/Q library v3
From: H. Peter Anvin @ 2026-06-27 0:52 UTC (permalink / raw)
To: Christoph Hellwig, Andrew Morton
Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, Huacai Chen,
WANG Xuerui, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, Herbert Xu, Dan Williams, Chris Mason,
David Sterba, Arnd Bergmann, Song Liu, Yu Kuai, Li Nan,
linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
linux-riscv, linux-s390, linux-crypto, linux-btrfs, linux-arch,
linux-raid
In-Reply-To: <20260519082432.GA14956@lst.de>
On 2026-05-19 01:24, Christoph Hellwig wrote:
>
> raid6: rework registration of optimized algorithms
>
> - avx2 instead of avx512 is probably the right thing for no
> benchmarking, but if it was intentional (it wasn't), that should
> be document. So I'll just switch back to the previous version to
> keep the state of the art
It is unlikely to be the right thing *going forward*, though.
The very unfortunate performance inversion is likely model-specific. It is one
of those things where you largely would have to have a list of quirks :(
-hpa
^ permalink raw reply
* Re: [PATCH 08/18] raid6: warn when using less than four devices
From: H. Peter Anvin @ 2026-06-27 1:14 UTC (permalink / raw)
To: Christoph Hellwig, Andrew Morton
Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, Huacai Chen,
WANG Xuerui, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, Herbert Xu, Dan Williams, Chris Mason,
David Sterba, Arnd Bergmann, Song Liu, Yu Kuai, Li Nan,
linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
linux-riscv, linux-s390, linux-crypto, linux-btrfs, linux-arch,
linux-raid
In-Reply-To: <20260518051804.462141-9-hch@lst.de>
On 2026-05-17 22:17, Christoph Hellwig wrote:
> Quoting H. Peter Anvin who came up with the RAID6 P/Q algorithm, and
> who wrote the initial implementation, then still part of the md driver:
>
> The RAID-6 code has *never* supported only 3 units, and if it ever
> worked for *any* of the implementations it was purely by accident.
> Speaking as the original author I should know; this was deliberate as
> in some cases the degenerate case (3) would have required extra trays
Stupid autocorrect. That was of course supposed to be "tests" (as in extra
code paths) not "trays" :)
> in the code to no user benefit.
>
> While md never allowed less than 4 devices, btrfs does. This new
> warning will trigger for such file systems, but given how it already
> causes havoc that is a good thing. If btrfs wants to fix third, it
> should switch to transparently use three-way mirroring underneath,
> which will work as P and Q are copies of the single data device by
> the definition of the Linux RAID 6 P/Q algorithm.
For what it's worth, this is also true in the degenerate two-drive RAID-4|5
case (D = P).
-hpa
^ permalink raw reply
* [PATCH] md/raid5: protect lockless recovery_offset accesses during reshape
From: Chen Cheng @ 2026-06-27 10:25 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
During reshape:
- reshape_request() advances rdev->recovery_offset for non-In_sync
devices locklessly.
- analyse_stripe() reads rdev->recovery_offset locklessly to decide:
a. use a replacement device to read ?
b. a device can already be treated as in-sync for the current
stripe ?
one possible scenario is:
CPU1 CPU2
reshape_request()
-> mddev->curr_resync_completed = sector_nr
-> if (!mddev->reshape_backwards)
-> rdev->recovery_offset = sector_nr
analyse_stripe(sh)
-> rdev = conf->disks[i].replacement
-> if (rdev->recovery_offset >=
sh->sector + stripe_sectors)
set_bit(R5_ReadRepl)
-> or
-> if (sh->sector + stripe_sectors <=
rdev->recovery_offset)
set_bit(R5_Insync)
And it could be:
- reading from a replacement before it is recovered far enough; or
- treating a not-yet-recovered device as in-sync for the current stripe.
Fixes: db0505d32066 ("md: be cautious about using ->curr_resync_completed for ->recovery_offset")
The race report:
==================================================================
BUG: KCSAN: data-race in ops_run_io / reshape_request
write to 0xffff8bdee168b270 of 8 bytes by task 1704 on cpu 10:
reshape_request+0x1292/0x17b0
raid5_sync_request+0x815/0xa00
md_do_sync.cold+0xf8d/0x1516
[......]
read to 0xffff8bdee168b270 of 8 bytes by task 1696 on cpu 9:
ops_run_io+0xc25/0x1960
handle_stripe+0x2273/0x4570
handle_active_stripes.isra.0+0x6e0/0xa50
raid5d+0x7d5/0xb90
[......]
value changed: 0x0000000000091a00 -> 0x0000000000091b00
==================================================================
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/raid5.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index cacdf4211628..eaee7f206ab8 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3808,11 +3808,11 @@ static int want_replace(struct stripe_head *sh, int disk_idx)
rdev = sh->raid_conf->disks[disk_idx].replacement;
if (rdev
&& !test_bit(Faulty, &rdev->flags)
&& !test_bit(In_sync, &rdev->flags)
- && (rdev->recovery_offset <= sh->sector
+ && (READ_ONCE(rdev->recovery_offset) <= sh->sector
|| rdev->mddev->resync_offset <= sh->sector))
rv = 1;
return rv;
}
@@ -4726,11 +4726,11 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
/* Prefer to use the replacement for reads, but only
* if it is recovered enough and has no bad blocks.
*/
rdev = conf->disks[i].replacement;
if (rdev && !test_bit(Faulty, &rdev->flags) &&
- rdev->recovery_offset >= sh->sector + RAID5_STRIPE_SECTORS(conf) &&
+ READ_ONCE(rdev->recovery_offset) >= sh->sector + RAID5_STRIPE_SECTORS(conf) &&
!rdev_has_badblock(rdev, sh->sector,
RAID5_STRIPE_SECTORS(conf)))
set_bit(R5_ReadRepl, &dev->flags);
else {
if (rdev && !test_bit(Faulty, &rdev->flags))
@@ -4768,11 +4768,11 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
set_bit(R5_ReadError, &dev->flags);
}
} else if (test_bit(In_sync, &rdev->flags))
set_bit(R5_Insync, &dev->flags);
else if (sh->sector + RAID5_STRIPE_SECTORS(conf) <=
- rdev->recovery_offset) {
+ READ_ONCE(rdev->recovery_offset)) {
/*
* in sync if:
* - normal IO, or
* - resync IO that is not lazy recovery
*
@@ -5514,17 +5514,17 @@ static int raid5_read_one_chunk(struct mddev *mddev, struct bio *raid_bio)
if (r5c_big_stripe_cached(conf, sector))
return 0;
rdev = conf->disks[dd_idx].replacement;
if (!rdev || test_bit(Faulty, &rdev->flags) ||
- rdev->recovery_offset < end_sector) {
+ READ_ONCE(rdev->recovery_offset) < end_sector) {
rdev = conf->disks[dd_idx].rdev;
if (!rdev)
return 0;
if (test_bit(Faulty, &rdev->flags) ||
!(test_bit(In_sync, &rdev->flags) ||
- rdev->recovery_offset >= end_sector))
+ READ_ONCE(rdev->recovery_offset) >= end_sector))
return 0;
}
atomic_inc(&rdev->nr_pending);
@@ -6463,12 +6463,12 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
/* Can update recovery_offset */
rdev_for_each(rdev, mddev)
if (rdev->raid_disk >= 0 &&
!test_bit(Journal, &rdev->flags) &&
!test_bit(In_sync, &rdev->flags) &&
- rdev->recovery_offset < sector_nr)
- rdev->recovery_offset = sector_nr;
+ READ_ONCE(rdev->recovery_offset) < sector_nr)
+ WRITE_ONCE(rdev->recovery_offset, sector_nr);
conf->reshape_checkpoint = jiffies;
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
md_wakeup_thread(mddev->thread);
wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
@@ -6572,12 +6572,12 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
/* Can update recovery_offset */
rdev_for_each(rdev, mddev)
if (rdev->raid_disk >= 0 &&
!test_bit(Journal, &rdev->flags) &&
!test_bit(In_sync, &rdev->flags) &&
- rdev->recovery_offset < sector_nr)
- rdev->recovery_offset = sector_nr;
+ READ_ONCE(rdev->recovery_offset) < sector_nr)
+ WRITE_ONCE(rdev->recovery_offset, sector_nr);
conf->reshape_checkpoint = jiffies;
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
md_wakeup_thread(mddev->thread);
wait_event(mddev->sb_wait,
!test_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags)
@@ -8106,13 +8106,13 @@ static int raid5_run(struct mddev *mddev)
* to worry about reshape going forwards.
*/
/* Hack because v0.91 doesn't store recovery_offset properly. */
if (mddev->major_version == 0 &&
mddev->minor_version > 90)
- rdev->recovery_offset = reshape_offset;
+ WRITE_ONCE(rdev->recovery_offset, reshape_offset);
- if (rdev->recovery_offset < reshape_offset) {
+ if (READ_ONCE(rdev->recovery_offset) < reshape_offset) {
/* We need to check old and new layout */
if (!only_parity(rdev->raid_disk,
conf->algorithm,
conf->raid_disks,
conf->max_degraded))
@@ -8264,11 +8264,11 @@ static int raid5_spare_active(struct mddev *mddev)
for (i = 0; i < conf->raid_disks; i++) {
rdev = conf->disks[i].rdev;
replacement = conf->disks[i].replacement;
if (replacement
- && replacement->recovery_offset == MaxSector
+ && READ_ONCE(replacement->recovery_offset) == MaxSector
&& !test_bit(Faulty, &replacement->flags)
&& !test_and_set_bit(In_sync, &replacement->flags)) {
/* Replacement has just become active. */
if (!rdev
|| !test_and_clear_bit(In_sync, &rdev->flags))
@@ -8282,13 +8282,13 @@ static int raid5_spare_active(struct mddev *mddev)
sysfs_notify_dirent_safe(
rdev->sysfs_state);
}
sysfs_notify_dirent_safe(replacement->sysfs_state);
} else if (rdev
- && rdev->recovery_offset == MaxSector
- && !test_bit(Faulty, &rdev->flags)
- && !test_and_set_bit(In_sync, &rdev->flags)) {
+ && READ_ONCE(rdev->recovery_offset) == MaxSector
+ && !test_bit(Faulty, &rdev->flags)
+ && !test_and_set_bit(In_sync, &rdev->flags)) {
count++;
sysfs_notify_dirent_safe(rdev->sysfs_state);
}
}
spin_lock_irqsave(&conf->device_lock, flags);
@@ -8653,11 +8653,11 @@ static int raid5_start_reshape(struct mddev *mddev)
if (raid5_add_disk(mddev, rdev) == 0) {
if (rdev->raid_disk
>= conf->previous_raid_disks)
set_bit(In_sync, &rdev->flags);
else
- rdev->recovery_offset = 0;
+ WRITE_ONCE(rdev->recovery_offset, 0);
/* Failure here is OK */
sysfs_link_rdev(mddev, rdev);
}
} else if (rdev->raid_disk >= conf->previous_raid_disks
@@ -8705,11 +8705,11 @@ static void end_reshape(struct r5conf *conf)
conf->mddev->reshape_position = MaxSector;
rdev_for_each(rdev, conf->mddev)
if (rdev->raid_disk >= 0 &&
!test_bit(Journal, &rdev->flags) &&
!test_bit(In_sync, &rdev->flags))
- rdev->recovery_offset = MaxSector;
+ WRITE_ONCE(rdev->recovery_offset, MaxSector);
spin_unlock_irq(&conf->device_lock);
wake_up(&conf->wait_for_reshape);
mddev_update_io_opt(conf->mddev,
conf->raid_disks - conf->max_degraded);
--
2.54.0
^ permalink raw reply related
* [PATCH] md: protect read mddev->recovery in md_sync_action()
From: Chen Cheng @ 2026-06-27 10:29 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
md_sync_action() read mddev->recovery in lockless path, use READ_ONCE()
instead of u64 plain read.
Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
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 c5c50640b684..f4415c1a79d9 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5074,11 +5074,11 @@ static enum sync_action md_get_active_sync_action(struct mddev *mddev)
return is_recover ? ACTION_RECOVER : ACTION_IDLE;
}
enum sync_action md_sync_action(struct mddev *mddev)
{
- unsigned long recovery = mddev->recovery;
+ unsigned long recovery = READ_ONCE(mddev->recovery);
enum sync_action active_action;
/*
* frozen has the highest priority, means running sync_thread will be
* stopped immediately, and no new sync_thread can start.
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] md: protect read mddev->recovery in md_sync_action()
From: Abd-Alrhman Masalkhi @ 2026-06-27 10:46 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: chencheng, linux-kernel
In-Reply-To: <20260627102909.141036-1-chencheng@fnnas.com>
On Sat, Jun 27, 2026 at 18:29 +0800, Chen Cheng wrote:
> From: Chen Cheng <chencheng@fnnas.com>
>
> md_sync_action() read mddev->recovery in lockless path, use READ_ONCE()
> instead of u64 plain read.
>
unisgned long is not always u64...
I can not see what it fixes, is this just to silence KCSAN? If so,
please say so in the commit message.
> Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> 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 c5c50640b684..f4415c1a79d9 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -5074,11 +5074,11 @@ static enum sync_action md_get_active_sync_action(struct mddev *mddev)
> return is_recover ? ACTION_RECOVER : ACTION_IDLE;
> }
>
> enum sync_action md_sync_action(struct mddev *mddev)
> {
> - unsigned long recovery = mddev->recovery;
> + unsigned long recovery = READ_ONCE(mddev->recovery);
> enum sync_action active_action;
>
> /*
> * frozen has the highest priority, means running sync_thread will be
> * stopped immediately, and no new sync_thread can start.
> --
> 2.54.0
>
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH] md: protect read mddev->recovery in md_sync_action()
From: Chen Cheng @ 2026-06-27 10:52 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <m25x341cro.fsf@gmail.com>
在 2026/6/27 18:46, Abd-Alrhman Masalkhi 写道:
> On Sat, Jun 27, 2026 at 18:29 +0800, Chen Cheng wrote:
>> From: Chen Cheng <chencheng@fnnas.com>
>>
>> md_sync_action() read mddev->recovery in lockless path, use READ_ONCE()
>> instead of u64 plain read.
>>
> unisgned long is not always u64...
> I can not see what it fixes, is this just to silence KCSAN? If so,
> please say so in the commit message.
I need to find KCSAN report , I lost this one, but I think it's easy to
find locklessly path to call md_sync_action(), so. READ_ONCE is need.
>
>> Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
>>
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>> 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 c5c50640b684..f4415c1a79d9 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -5074,11 +5074,11 @@ static enum sync_action md_get_active_sync_action(struct mddev *mddev)
>> return is_recover ? ACTION_RECOVER : ACTION_IDLE;
>> }
>>
>> enum sync_action md_sync_action(struct mddev *mddev)
>> {
>> - unsigned long recovery = mddev->recovery;
>> + unsigned long recovery = READ_ONCE(mddev->recovery);
>> enum sync_action active_action;
>>
>> /*
>> * frozen has the highest priority, means running sync_thread will be
>> * stopped immediately, and no new sync_thread can start.
>> --
>> 2.54.0
>>
>
^ permalink raw reply
* [PATCH] md/raid5: protect lockless reshape_progress accesses
From: Chen Cheng @ 2026-06-27 10:55 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
During reshape:
- reshape_request() advances conf->reshape_progress.
- reshape_request() itself also reads conf->reshape_progress several times
while calculating sector_nr, writepos, and readpos for the current
reshape step.
- use_new_offset() reads conf->reshape_progress locklessly to decide
whether the current stripe should use data_offset or new_data_offset.
one possible scenario is:
CPU1 CPU2
reshape_request()
-> conf->reshape_progress += delta
ops_run_io()
-> use_new_offset(conf, sh)
-> progress = conf->reshape_progress
-> decide old layout or new layout
-> use data_offset or new_data_offset
reshape_progress is the boundary between old and new layout mapping.
If CPU2 observes an unprotected reshape_progress value, it can choose
the wrong layout offset for the current stripe.
if reshape_request() reads reshape_progress multiple times without
a stable snapshot, one iteration of reshape can calculate its range from
inconsistent progress values.
Fixes: 7a6613810785 ("md/raid5: reshape using largest of old and new chunk size")
The race report:
==================================================================
BUG: KCSAN: data-race in ops_run_io / reshape_request
write to 0xffff89a8d3ef2270 of 8 bytes by task 1299 on cpu 7:
reshape_request+0x1292/0x17b0
raid5_sync_request+0x815/0xa00
md_do_sync.cold+0xf8d/0x1516
md_thread+0x15a/0x2d0
[....]
read to 0xffff89a8d3ef2270 of 8 bytes by task 1292 on cpu 9:
ops_run_io+0xc25/0x1960
handle_stripe+0x2273/0x4570
handle_active_stripes.isra.0+0x6e0/0xa50
raid5d+0x7d5/0xb90
[....]
value changed: 0x0000000000173700 -> 0x0000000000173800
==================================================================
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/raid5.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index eaee7f206ab8..b6809f1a3ab4 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -1011,11 +1011,11 @@ static void stripe_add_to_batch_list(struct r5conf *conf,
/* Determine if 'data_offset' or 'new_data_offset' should be used
* in this stripe_head.
*/
static int use_new_offset(struct r5conf *conf, struct stripe_head *sh)
{
- sector_t progress = conf->reshape_progress;
+ sector_t progress = READ_ONCE(conf->reshape_progress);
/* Need a memory barrier to make sure we see the value
* of conf->generation, or ->data_offset that was set before
* reshape_progress was updated.
*/
smp_rmb();
@@ -6340,24 +6340,25 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
sector_t writepos, readpos, safepos;
sector_t stripe_addr;
int reshape_sectors;
struct list_head stripes;
sector_t retn;
+ sector_t reshape_progress = READ_ONCE(conf->reshape_progress);
if (sector_nr == 0) {
/* If restarting in the middle, skip the initial sectors */
if (mddev->reshape_backwards &&
- conf->reshape_progress < raid5_size(mddev, 0, 0)) {
+ reshape_progress < raid5_size(mddev, 0, 0)) {
sector_nr = raid5_size(mddev, 0, 0)
- - conf->reshape_progress;
+ - reshape_progress;
} else if (mddev->reshape_backwards &&
- conf->reshape_progress == MaxSector) {
+ reshape_progress == MaxSector) {
/* shouldn't happen, but just in case, finish up.*/
sector_nr = MaxSector;
} else if (!mddev->reshape_backwards &&
- conf->reshape_progress > 0)
- sector_nr = conf->reshape_progress;
+ reshape_progress > 0)
+ sector_nr = reshape_progress;
sector_div(sector_nr, new_data_disks);
if (sector_nr) {
mddev->curr_resync_completed = sector_nr;
sysfs_notify_dirent_safe(mddev->sysfs_completed);
*skipped = 1;
@@ -6377,13 +6378,13 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
* the data about to be copied would over-write the source of
* the data at the front of the range. i.e. one new_stripe
* along from reshape_progress new_maps to after where
* reshape_safe old_maps to
*/
- writepos = conf->reshape_progress;
+ writepos = reshape_progress;
sector_div(writepos, new_data_disks);
- readpos = conf->reshape_progress;
+ readpos = reshape_progress;
sector_div(readpos, data_disks);
safepos = conf->reshape_safe;
sector_div(safepos, data_disks);
if (mddev->reshape_backwards) {
if (WARN_ON(writepos < reshape_sectors))
@@ -6404,11 +6405,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
/* Having calculated the 'writepos' possibly use it
* to set 'stripe_addr' which is where we will write to.
*/
if (mddev->reshape_backwards) {
- if (WARN_ON(conf->reshape_progress == 0))
+ if (WARN_ON(reshape_progress == 0))
return MaxSector;
stripe_addr = writepos;
if (WARN_ON((mddev->dev_sectors &
~((sector_t)reshape_sectors - 1)) -
@@ -6514,14 +6515,21 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
set_bit(STRIPE_HANDLE, &sh->state);
}
list_add(&sh->lru, &stripes);
}
spin_lock_irq(&conf->device_lock);
- if (mddev->reshape_backwards)
- conf->reshape_progress -= reshape_sectors * new_data_disks;
- else
- conf->reshape_progress += reshape_sectors * new_data_disks;
+ if (mddev->reshape_backwards) {
+ sector_t progress = conf->reshape_progress;
+
+ progress -= reshape_sectors * new_data_disks;
+ WRITE_ONCE(conf->reshape_progress, progress);
+ } else {
+ sector_t progress = conf->reshape_progress;
+
+ progress += reshape_sectors * new_data_disks;
+ WRITE_ONCE(conf->reshape_progress, progress);
+ }
spin_unlock_irq(&conf->device_lock);
/* Ok, those stripe are ready. We can start scheduling
* reads on the source stripes.
* The source stripes are determined by mapping the first and last
* block on the destination stripes.
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] md: protect read mddev->recovery in md_sync_action()
From: Abd-Alrhman Masalkhi @ 2026-06-27 11:25 UTC (permalink / raw)
To: Chen Cheng, Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <da95c106-6416-4abd-8fef-3b71986c17f8@gmail.com>
Hi Chen,
On Sat, Jun 27, 2026 at 18:52 +0800, Chen Cheng wrote:
> 在 2026/6/27 18:46, Abd-Alrhman Masalkhi 写道:
>> On Sat, Jun 27, 2026 at 18:29 +0800, Chen Cheng wrote:
>>> From: Chen Cheng <chencheng@fnnas.com>
>>>
>>> md_sync_action() read mddev->recovery in lockless path, use READ_ONCE()
>>> instead of u64 plain read.
>>>
>> unisgned long is not always u64...
>> I can not see what it fixes, is this just to silence KCSAN? If so,
>> please say so in the commit message.
>
> I need to find KCSAN report , I lost this one, but I think it's easy to
> find locklessly path to call md_sync_action(), so. READ_ONCE is need.
>
Thanks. Adding the KCSAN report would be helpful.
>>
>>> Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
>>>
>>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>>> ---
>>> 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 c5c50640b684..f4415c1a79d9 100644
>>> --- a/drivers/md/md.c
>>> +++ b/drivers/md/md.c
>>> @@ -5074,11 +5074,11 @@ static enum sync_action md_get_active_sync_action(struct mddev *mddev)
>>> return is_recover ? ACTION_RECOVER : ACTION_IDLE;
>>> }
>>>
>>> enum sync_action md_sync_action(struct mddev *mddev)
>>> {
>>> - unsigned long recovery = mddev->recovery;
>>> + unsigned long recovery = READ_ONCE(mddev->recovery);
>>> enum sync_action active_action;
>>>
>>> /*
>>> * frozen has the highest priority, means running sync_thread will be
>>> * stopped immediately, and no new sync_thread can start.
>>> --
>>> 2.54.0
>>>
>>
>
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* [PATCH v2 0/7] md/raid10: fixes, atomic write handling, and error-path cleanup
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
Hi,
This v2 of series contains a mix of bug fixes and cleanups for RAID10,
along with a related atomic write fix for RAID1.
Changes in v2:
- Expand the commit message to explain why the
allow_barrier()/wait_barrier() pair is no longer needed.
- Drop the early atomic write split check from raid1_write_request().
- Advertise the atomic write size limit via queue limits.
- Disable write-behind instead of failing atomic writes when the
BIO_MAX_VECS limit is encountered.
- Drop the early atomic write split check from raid10_write_request()
and rely on queue limits instead.
- Fix a compilation error (bi -> bio).
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-1-abd.masalkhi@gmail.com/
Thanks,
Abd-alrhman,
Abd-Alrhman Masalkhi (7):
md/raid10: fix r10bio leak in raid10_write_request() error paths
md/raid1: advertise atomic write limits and handle runtime constraints
md/raid10: consistently fail atomic writes that require splitting
md/raid10: remove unnecessary barrier around bio_submit_split_bioset()
md/raid10: replace wait loop with wait_event_idle()
md/raid10: simplify write request error handling
md/raid10: simplify read request error handling
drivers/md/raid1.c | 36 +++++++-------
drivers/md/raid10.c | 118 +++++++++++++++++++++-----------------------
2 files changed, 74 insertions(+), 80 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v2 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid, sashiko-bot
In-Reply-To: <20260628142420.1051027-1-abd.masalkhi@gmail.com>
When raid10_write_request() fails because REQ_NOWAIT is set, the
allocated r10_bio is not freed before returning, resulting in a memory
leak. Free r10_bio before returning from the REQ_NOWAIT error paths.
Fixes: c9aa889b035f ("md: raid10 add nowait support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-raid/20260613184042.BCEC01F000E9@smtp.kernel.org/
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- No changes.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-2-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0a3cfdd3f5df..bd322eccdc3f 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1365,6 +1365,7 @@ static bool 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);
+ free_r10bio(r10_bio);
return false;
}
for (;;) {
@@ -1398,6 +1399,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
if (bio->bi_opf & REQ_NOWAIT) {
allow_barrier(conf);
bio_wouldblock_error(bio);
+ free_r10bio(r10_bio);
return false;
}
mddev_add_trace_msg(conf->mddev,
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260628142420.1051027-1-abd.masalkhi@gmail.com>
Atomic writes in RAID1 must fit within a single barrier unit. Advertise
this restriction through the queue limits by setting
atomic_write_hw_unit_max to BARRIER_UNIT_SECTOR_SIZE so that bios which
would cross a barrier-unit boundary are rejected by the block layer
before reaching MD.
A bio that passes block-layer validation may still become unserviceable
within RAID1 due to bad blocks or write-behind constraints. In the former
case, complete the bio with EIO. In the latter case, disable
write-behind rather than failing the bio with EIO.
Fixes: f2a38abf5f1c ("md/raid1: Atomic write support")
Fixes: a4c55c902670 ("md/raid1: simplify raid1_write_request() error handling")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- Drop the early atomic write split check from raid1_write_request().
- Advertise the atomic write size limit via queue limits.
- Disable write-behind instead of failing atomic writes when the
BIO_MAX_VECS limit is encountered.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-3-abd.masalkhi@gmail.com/
---
drivers/md/raid1.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index afe2ca96ad8c..f322048ab3c2 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1522,6 +1522,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
int first_clone;
bool write_behind = false;
bool nowait = bio->bi_opf & REQ_NOWAIT;
+ bool atomic = bio->bi_opf & REQ_ATOMIC;
bool is_discard = op_is_discard(bio->bi_opf);
sector_t sector = bio->bi_iter.bi_sector;
@@ -1603,20 +1604,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
}
if (is_bad) {
int good_sectors;
-
- /*
- * We cannot atomically write this, so just
- * error in that case. It could be possible to
- * atomically write other mirrors, but the
- * complexity of supporting that is not worth
- * the benefit.
- */
- if (bio->bi_opf & REQ_ATOMIC) {
- bio->bi_status = BLK_STS_NOTSUPP;
- bio_endio(bio);
- goto err_dec_pending;
- }
-
good_sectors = first_bad - sector;
if (good_sectors < max_sectors)
max_sectors = good_sectors;
@@ -1633,10 +1620,24 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
* at a time and thus needs a new bio that can fit the whole payload
* this bio in page sized chunks.
*/
- if (write_behind && mddev->bitmap)
- max_sectors = min_t(int, max_sectors,
- BIO_MAX_VECS * (PAGE_SIZE >> 9));
+ if (write_behind && mddev->bitmap) {
+ if (atomic && max_sectors > BIO_MAX_VECS * (PAGE_SIZE >> 9))
+ /*
+ * Atomic writes cannot be split, so disable
+ * write-behind.
+ */
+ write_behind = false;
+ else
+ max_sectors = min_t(int, max_sectors,
+ BIO_MAX_VECS * (PAGE_SIZE >> 9));
+ }
+
if (max_sectors < bio_sectors(bio)) {
+ if (atomic) {
+ bio_io_error(bio);
+ goto err_dec_pending;
+ }
+
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
if (!bio)
@@ -3229,6 +3230,7 @@ static int raid1_set_limits(struct mddev *mddev)
lim.max_write_zeroes_sectors = 0;
lim.max_hw_wzeroes_unmap_sectors = 0;
lim.logical_block_size = mddev->logical_block_size;
+ lim.atomic_write_hw_unit_max = BARRIER_UNIT_SECTOR_SIZE;
lim.features |= BLK_FEAT_ATOMIC_WRITES;
lim.features |= BLK_FEAT_PCI_P2PDMA;
err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
--
2.43.0
^ permalink raw reply related
* [PATCH v2 3/7] md/raid10: consistently fail atomic writes that require splitting
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260628142420.1051027-1-abd.masalkhi@gmail.com>
RAID10 currently handles one badblock path explicitly by failing atomic
writes with EIO. However, another badblock path can also reduce the
writable range and force the bio through bio_submit_split_bioset(),
which implicitly completes the bio with EINVAL.
Fix this by handling atomic writes in the common split check. If RAID10
determines that an atomic write would require splitting, complete the
bio with EIO.
Fixes: a1d9b4fd42d9 ("md/raid10: Atomic write support")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- Drop the early atomic write split check from raid10_write_request()
and rely on queue limits instead.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-4-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index bd322eccdc3f..3480fc7907f0 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1356,6 +1356,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
int i, k;
sector_t sectors;
int max_sectors;
+ bool atomic = bio->bi_opf & REQ_ATOMIC;
if ((mddev_is_clustered(mddev) &&
mddev->cluster_ops->area_resyncing(mddev, WRITE,
@@ -1464,16 +1465,6 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
if (is_bad) {
int good_sectors;
- /*
- * We cannot atomically write this, so just
- * error in that case. It could be possible to
- * atomically write other mirrors, but the
- * complexity of supporting that is not worth
- * the benefit.
- */
- if (bio->bi_opf & REQ_ATOMIC)
- goto err_handle;
-
good_sectors = first_bad - dev_sector;
if (good_sectors < max_sectors)
max_sectors = good_sectors;
@@ -1493,6 +1484,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
r10_bio->sectors = max_sectors;
if (r10_bio->sectors < bio_sectors(bio)) {
+ if (atomic)
+ goto err_handle;
+
allow_barrier(conf);
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
--
2.43.0
^ permalink raw reply related
* [PATCH v2 4/7] md/raid10: remove unnecessary barrier around bio_submit_split_bioset()
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260628142420.1051027-1-abd.masalkhi@gmail.com>
raid10_write_request() drops the barrier before calling
bio_submit_split_bioset() and reacquires it afterwards. This is no
longer necessary because the split bio cannot re-enter
raid10_write_request() while the barrier is held.
The allow_barrier()/wait_barrier() pair was introduced by commit
e820d55cb99d ("md: fix raid10 hang issue caused by barrier") when
submit_flushes() called md_handle_request() directly, allowing re-entry
into raid10_write_request(). Since v5.2, submit_flushes() has instead
gone through submit_bio(), eliminating that recursion. submit_flushes()
was later removed entirely by commit b75197e86e6d ("md: Remove flush
handling").
Currently, raid10_write_request() is only entered from the bio
submission path, so the split bio submitted by bio_submit_split_bioset()
cannot recurse back into wait_barrier().
Remove the redundant allow_barrier()/wait_barrier() pair around
bio_submit_split_bioset().
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- Expand the commit message to explain why the
allow_barrier()/wait_barrier() pair is no longer needed.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-5-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 3480fc7907f0..2574f60dd771 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1487,10 +1487,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
if (atomic)
goto err_handle;
- allow_barrier(conf);
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
- wait_barrier(conf, false);
if (!bio) {
set_bit(R10BIO_Returned, &r10_bio->state);
goto err_handle;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 5/7] md/raid10: replace wait loop with wait_event_idle()
From: Abd-Alrhman Masalkhi @ 2026-06-28 14:24 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, axboe, vverma, john.g.garry,
martin.petersen, abd.masalkhi, linux-kernel
Cc: linux-raid
In-Reply-To: <20260628142420.1051027-1-abd.masalkhi@gmail.com>
The wait loop is equivalent to wait_event_idle() and can be simplified
by usaing it for improving readability.
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
Changes in v2:
- No changes.
- Link to v1: https://lore.kernel.org/linux-raid/20260623072456.333437-6-abd.masalkhi@gmail.com/
---
drivers/md/raid10.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 2574f60dd771..57813f249578 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1362,22 +1362,17 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
mddev->cluster_ops->area_resyncing(mddev, WRITE,
bio->bi_iter.bi_sector,
bio_end_sector(bio)))) {
- DEFINE_WAIT(w);
/* Bail out if REQ_NOWAIT is set for the bio */
if (bio->bi_opf & REQ_NOWAIT) {
bio_wouldblock_error(bio);
free_r10bio(r10_bio);
return false;
}
- for (;;) {
- prepare_to_wait(&conf->wait_barrier,
- &w, TASK_IDLE);
- if (!mddev->cluster_ops->area_resyncing(mddev, WRITE,
- bio->bi_iter.bi_sector, bio_end_sector(bio)))
- break;
- schedule();
- }
- finish_wait(&conf->wait_barrier, &w);
+
+ wait_event_idle(conf->wait_barrier,
+ !mddev->cluster_ops->area_resyncing(mddev, WRITE,
+ bio->bi_iter.bi_sector,
+ bio_end_sector(bio)));
}
sectors = r10_bio->sectors;
--
2.43.0
^ permalink raw reply related
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