* Re: [PATCH 01/25] xor: assert that xor_blocks is not called from interrupt context
From: Heiko Carstens @ 2026-03-04 15:08 UTC (permalink / raw)
To: Heiko Carstens
Cc: Eric Biggers, Christoph Hellwig, Peter Zijlstra, Andrew Morton,
Richard Henderson, Matt Turner, Magnus Lindholm, Russell King,
Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, David S. Miller,
Andreas Larsson, Richard Weinberger, Anton Ivanov, Johannes Berg,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Herbert Xu, Dan Williams, Chris Mason,
David Sterba, Arnd Bergmann, Song Liu, Yu Kuai, Li Nan,
linux-alpha, linux-kernel, linux-arm-kernel, loongarch,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-um,
linux-crypto, linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260304150142.10892A0b-hca@linux.ibm.com>
On Wed, Mar 04, 2026 at 04:01:46PM +0100, Heiko Carstens wrote:
> On Tue, Mar 03, 2026 at 11:55:17AM -0800, Eric Biggers wrote:
> > On Tue, Mar 03, 2026 at 05:00:50PM +0100, Christoph Hellwig wrote:
> > > On Fri, Feb 27, 2026 at 03:24:55PM +0100, Peter Zijlstra wrote:
> > Because of that CPU feature check, I don't think
> > "WARN_ON_ONCE(!may_use_simd())" would actually be correct here.
> >
> > How about "WARN_ON_ONCE(!preemptible())"? I think that covers the union
> > of the context restrictions correctly. (Compared to in_task(), it
> > handles the cases where hardirqs or softirqs are disabled.)
>
> I guess, this is not true, since there is at least one architecture which
> allows to run simd code in interrupt context (but which missed to implement
> may_use_simd()).
Oh, just to avoid confusion, which I may have caused: I made only
general comments about s390 simd usage. Our xor() implementation does
not make use of simd, since our normal xc instruction allows to xor up
to 256 bytes. A simd implementation wouldn't be faster.
Also here it would be possible to run it in any context.
^ permalink raw reply
* Re: [PATCH 01/25] xor: assert that xor_blocks is not called from interrupt context
From: Christoph Hellwig @ 2026-03-04 15:06 UTC (permalink / raw)
To: Heiko Carstens
Cc: Eric Biggers, Christoph Hellwig, Peter Zijlstra, Andrew Morton,
Richard Henderson, Matt Turner, Magnus Lindholm, Russell King,
Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, David S. Miller,
Andreas Larsson, Richard Weinberger, Anton Ivanov, Johannes Berg,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Herbert Xu, Dan Williams, Chris Mason,
David Sterba, Arnd Bergmann, Song Liu, Yu Kuai, Li Nan,
linux-alpha, linux-kernel, linux-arm-kernel, loongarch,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-um,
linux-crypto, linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260304150142.10892A0b-hca@linux.ibm.com>
On Wed, Mar 04, 2026 at 04:01:42PM +0100, Heiko Carstens wrote:
> > Because of that CPU feature check, I don't think
> > "WARN_ON_ONCE(!may_use_simd())" would actually be correct here.
> >
> > How about "WARN_ON_ONCE(!preemptible())"? I think that covers the union
> > of the context restrictions correctly. (Compared to in_task(), it
> > handles the cases where hardirqs or softirqs are disabled.)
>
> I guess, this is not true, since there is at least one architecture which
> allows to run simd code in interrupt context (but which missed to implement
> may_use_simd()).
I'd rather have a strict upper limit that is generally applicable.
Currently there is no non-task user of this code, and while maybe doing
XOR recovery for tiny blocks from irq context would be nice, let's defer
that until we need it. There is much bigger fish to fry in terms of
raid performance at the moment.
^ permalink raw reply
* Re: [PATCH 01/25] xor: assert that xor_blocks is not called from interrupt context
From: Heiko Carstens @ 2026-03-04 15:01 UTC (permalink / raw)
To: Eric Biggers
Cc: Christoph Hellwig, Peter Zijlstra, Andrew Morton,
Richard Henderson, Matt Turner, Magnus Lindholm, Russell King,
Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, David S. Miller,
Andreas Larsson, Richard Weinberger, Anton Ivanov, Johannes Berg,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Herbert Xu, Dan Williams, Chris Mason,
David Sterba, Arnd Bergmann, Song Liu, Yu Kuai, Li Nan,
linux-alpha, linux-kernel, linux-arm-kernel, loongarch,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-um,
linux-crypto, linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260303195517.GC2846@sol>
On Tue, Mar 03, 2026 at 11:55:17AM -0800, Eric Biggers wrote:
> On Tue, Mar 03, 2026 at 05:00:50PM +0100, Christoph Hellwig wrote:
> > On Fri, Feb 27, 2026 at 03:24:55PM +0100, Peter Zijlstra wrote:
> > > > unsigned long *p1, *p2, *p3, *p4;
> > > >
> > > > + WARN_ON_ONCE(in_interrupt());
> > >
> > > Your changelog makes it sound like you want:
> > >
> > > WARN_ON_ONCE(!in_task());
> > >
> > > But perhaps something like so:
> > >
> > > lockdep_assert_preempt_enabled();
> > >
> > > Would do? That ensures we are in preemptible context, which is much the
> > > same. That also ensures the cost of this assertion is only paid on debug
> > > kernels.
> >
> > No idea honestly. The kernel FPU/vector helpers generally don't work
> > from irq context, and I want to assert that. Happy to do whatever
> > version works best for that.
>
> may_use_simd() is the "generic" way to check "can the FPU/vector/SIMD
> registers be used". However, what it does varies by architecture, and
> it's kind of a questionable abstraction in the first place. It's used
> mostly by architecture-specific code.
>
> If you union together the context restrictions from all the
> architectures, I think you get: "For may_use_simd() to be guaranteed not
> to return false due to the context, the caller needs to be running in
> task context without hardirqs or softirqs disabled."
>
> However, some architectures also incorporate a CPU feature check in
> may_use_simd() as well, which makes it return false if some
> CPU-dependent SIMD feature is not supported.
Oh, interesting. I wasn't aware of may_use_simd(), and of course this is
missing on s390, and hence we fallback to the generic !in_interrupt()
variant.
In fact the s390 simd implementation allows for usage in any context, also
interrupt context. So the s390 implementation of may_use_simd() would
always return true, _except_ for the feature check you mention.
Let me try to change that and see if anything explodes.
> Because of that CPU feature check, I don't think
> "WARN_ON_ONCE(!may_use_simd())" would actually be correct here.
>
> How about "WARN_ON_ONCE(!preemptible())"? I think that covers the union
> of the context restrictions correctly. (Compared to in_task(), it
> handles the cases where hardirqs or softirqs are disabled.)
I guess, this is not true, since there is at least one architecture which
allows to run simd code in interrupt context (but which missed to implement
may_use_simd()).
^ permalink raw reply
* Re: [PATCH 01/25] xor: assert that xor_blocks is not called from interrupt context
From: Christoph Hellwig @ 2026-03-04 14:51 UTC (permalink / raw)
To: Eric Biggers
Cc: Christoph Hellwig, Peter Zijlstra, Andrew Morton,
Richard Henderson, Matt Turner, Magnus Lindholm, Russell King,
Catalin Marinas, Will Deacon, 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,
David S. Miller, Andreas Larsson, Richard Weinberger,
Anton Ivanov, Johannes Berg, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Herbert Xu,
Dan Williams, Chris Mason, David Sterba, Arnd Bergmann, Song Liu,
Yu Kuai, Li Nan, linux-alpha, linux-kernel, linux-arm-kernel,
loongarch, linuxppc-dev, linux-riscv, linux-s390, sparclinux,
linux-um, linux-crypto, linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260303195517.GC2846@sol>
On Tue, Mar 03, 2026 at 11:55:17AM -0800, Eric Biggers wrote:
> may_use_simd() is the "generic" way to check "can the FPU/vector/SIMD
> registers be used". However, what it does varies by architecture, and
> it's kind of a questionable abstraction in the first place. It's used
> mostly by architecture-specific code.
Yeah, I don't think that is quite right here.
> How about "WARN_ON_ONCE(!preemptible())"? I think that covers the union
> of the context restrictions correctly. (Compared to in_task(), it
> handles the cases where hardirqs or softirqs are disabled.)
Good enough I guess. Peter?
^ permalink raw reply
* Re: [PATCH] md: remove unused mddev argument from export_rdev
From: chencheng @ 2026-03-04 8:26 UTC (permalink / raw)
To: Paul Menzel; +Cc: Song Liu, Yu Kuai, Li Nan, linux-raid, chencheng
In-Reply-To: <891274e7-cdb0-4864-bb0a-bc4ee915ea84@molgen.mpg.de>
Hi Paul,
On 2026/3/4 15:34, Paul Menzel wrote:
> Dear Chen,
>
>
> Am 04.03.26 um 12:14 schrieb Chen Cheng:
>> The mddev argument in export_rdev() is never used. Remove it to
>> simplify callers.
>
> Nice catch. Was it like this from the beginning? Was the compiler
> noticing this itself, so the code stays the same?
As I remember, the compiler didn't catch this, probably because
-Wunused-parameter is not enabled by default for static functions.
Thanks for the review.
Chen Cheng
>
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>> drivers/md/md.c | 28 ++++++++++++++--------------
>> 1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index c2cc2302d727..521d9b34cd9e 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -98,7 +98,7 @@ static struct workqueue_struct *md_misc_wq;
>> static int remove_and_add_spares(struct mddev *mddev,
>> struct md_rdev *this);
>> static void mddev_detach(struct mddev *mddev);
>> -static void export_rdev(struct md_rdev *rdev, struct mddev *mddev);
>> +static void export_rdev(struct md_rdev *rdev);
>> static void md_wakeup_thread_directly(struct md_thread __rcu **thread);
>> /*
>> @@ -959,7 +959,7 @@ void mddev_unlock(struct mddev *mddev)
>> list_for_each_entry_safe(rdev, tmp, &delete, same_set) {
>> list_del_init(&rdev->same_set);
>> kobject_del(&rdev->kobj);
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> }
>> if (!legacy_async_del_gendisk) {
>> @@ -2632,7 +2632,7 @@ void md_autodetect_dev(dev_t dev);
>> /* just for claiming the bdev */
>> static struct md_rdev claim_rdev;
>> -static void export_rdev(struct md_rdev *rdev, struct mddev *mddev)
>> +static void export_rdev(struct md_rdev *rdev)
>> {
>> pr_debug("md: export_rdev(%pg)\n", rdev->bdev);
>> md_rdev_clear(rdev);
>> @@ -4850,7 +4850,7 @@ new_dev_store(struct mddev *mddev, const char
>> *buf, size_t len)
>> err = bind_rdev_to_array(rdev, mddev);
>> out:
>> if (err)
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> mddev_unlock_and_resume(mddev);
>> if (!err)
>> md_new_event();
>> @@ -7142,7 +7142,7 @@ static void autorun_devices(int part)
>> rdev_for_each_list(rdev, tmp, &candidates) {
>> list_del_init(&rdev->same_set);
>> if (bind_rdev_to_array(rdev, mddev))
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> }
>> autorun_array(mddev);
>> mddev_unlock_and_resume(mddev);
>> @@ -7152,7 +7152,7 @@ static void autorun_devices(int part)
>> */
>> rdev_for_each_list(rdev, tmp, &candidates) {
>> list_del_init(&rdev->same_set);
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> }
>> mddev_put(mddev);
>> }
>> @@ -7340,13 +7340,13 @@ int md_add_new_disk(struct mddev *mddev,
>> struct mdu_disk_info_s *info)
>> pr_warn("md: %pg has different UUID to %pg\n",
>> rdev->bdev,
>> rdev0->bdev);
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return -EINVAL;
>> }
>> }
>> err = bind_rdev_to_array(rdev, mddev);
>> if (err)
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return err;
>> }
>> @@ -7389,7 +7389,7 @@ int md_add_new_disk(struct mddev *mddev, struct
>> mdu_disk_info_s *info)
>> /* This was a hot-add request, but events doesn't
>> * match, so reject it.
>> */
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return -EINVAL;
>> }
>> @@ -7415,7 +7415,7 @@ int md_add_new_disk(struct mddev *mddev, struct
>> mdu_disk_info_s *info)
>> }
>> }
>> if (has_journal || mddev->bitmap) {
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return -EBUSY;
>> }
>> set_bit(Journal, &rdev->flags);
>> @@ -7430,7 +7430,7 @@ int md_add_new_disk(struct mddev *mddev, struct
>> mdu_disk_info_s *info)
>> /* --add initiated by this node */
>> err = mddev->cluster_ops->add_new_disk(mddev, rdev);
>> if (err) {
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return err;
>> }
>> }
>> @@ -7440,7 +7440,7 @@ int md_add_new_disk(struct mddev *mddev, struct
>> mdu_disk_info_s *info)
>> err = bind_rdev_to_array(rdev, mddev);
>> if (err)
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> if (mddev_is_clustered(mddev)) {
>> if (info->state & (1 << MD_DISK_CANDIDATE)) {
>> @@ -7503,7 +7503,7 @@ int md_add_new_disk(struct mddev *mddev, struct
>> mdu_disk_info_s *info)
>> err = bind_rdev_to_array(rdev, mddev);
>> if (err) {
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return err;
>> }
>> }
>> @@ -7615,7 +7615,7 @@ static int hot_add_disk(struct mddev *mddev,
>> dev_t dev)
>> return 0;
>> abort_export:
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return err;
>> }
>
> The diff looks good:
>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>
>
> Kind regards,
>
> Paul
^ permalink raw reply
* Re: [PATCH] md: remove unused mddev argument from export_rdev
From: Paul Menzel @ 2026-03-04 7:43 UTC (permalink / raw)
To: Chen Cheng; +Cc: Song Liu, Yu Kuai, Li Nan, linux-raid
In-Reply-To: <891274e7-cdb0-4864-bb0a-bc4ee915ea84@molgen.mpg.de>
Dear Cheng,
Am 04.03.26 um 08:34 schrieb Paul Menzel:
> Am 04.03.26 um 12:14 schrieb Chen Cheng:
>> The mddev argument in export_rdev() is never used. Remove it to
>> simplify callers.
>
> Nice catch. Was it like this from the beginning?
Commit 99892147f028 (md: fix warning for holder mismatch from
export_rdev()) remove the argument usage, as far as I can see.
> Was the compiler noticing this itself, so the code stays the same?
I am still curious about this.
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>> drivers/md/md.c | 28 ++++++++++++++--------------
>> 1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index c2cc2302d727..521d9b34cd9e 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -98,7 +98,7 @@ static struct workqueue_struct *md_misc_wq;
>> static int remove_and_add_spares(struct mddev *mddev,
>> struct md_rdev *this);
>> static void mddev_detach(struct mddev *mddev);
>> -static void export_rdev(struct md_rdev *rdev, struct mddev *mddev);
>> +static void export_rdev(struct md_rdev *rdev);
>> static void md_wakeup_thread_directly(struct md_thread __rcu **thread);
>> /*
>> @@ -959,7 +959,7 @@ void mddev_unlock(struct mddev *mddev)
>> list_for_each_entry_safe(rdev, tmp, &delete, same_set) {
>> list_del_init(&rdev->same_set);
>> kobject_del(&rdev->kobj);
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> }
>> if (!legacy_async_del_gendisk) {
>> @@ -2632,7 +2632,7 @@ void md_autodetect_dev(dev_t dev);
>> /* just for claiming the bdev */
>> static struct md_rdev claim_rdev;
>> -static void export_rdev(struct md_rdev *rdev, struct mddev *mddev)
>> +static void export_rdev(struct md_rdev *rdev)
>> {
>> pr_debug("md: export_rdev(%pg)\n", rdev->bdev);
>> md_rdev_clear(rdev);
>> @@ -4850,7 +4850,7 @@ new_dev_store(struct mddev *mddev, const char
>> *buf, size_t len)
>> err = bind_rdev_to_array(rdev, mddev);
>> out:
>> if (err)
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> mddev_unlock_and_resume(mddev);
>> if (!err)
>> md_new_event();
>> @@ -7142,7 +7142,7 @@ static void autorun_devices(int part)
>> rdev_for_each_list(rdev, tmp, &candidates) {
>> list_del_init(&rdev->same_set);
>> if (bind_rdev_to_array(rdev, mddev))
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> }
>> autorun_array(mddev);
>> mddev_unlock_and_resume(mddev);
>> @@ -7152,7 +7152,7 @@ static void autorun_devices(int part)
>> */
>> rdev_for_each_list(rdev, tmp, &candidates) {
>> list_del_init(&rdev->same_set);
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> }
>> mddev_put(mddev);
>> }
>> @@ -7340,13 +7340,13 @@ int md_add_new_disk(struct mddev *mddev,
>> struct mdu_disk_info_s *info)
>> pr_warn("md: %pg has different UUID to %pg\n",
>> rdev->bdev,
>> rdev0->bdev);
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return -EINVAL;
>> }
>> }
>> err = bind_rdev_to_array(rdev, mddev);
>> if (err)
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return err;
>> }
>> @@ -7389,7 +7389,7 @@ int md_add_new_disk(struct mddev *mddev, struct
>> mdu_disk_info_s *info)
>> /* This was a hot-add request, but events doesn't
>> * match, so reject it.
>> */
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return -EINVAL;
>> }
>> @@ -7415,7 +7415,7 @@ int md_add_new_disk(struct mddev *mddev, struct
>> mdu_disk_info_s *info)
>> }
>> }
>> if (has_journal || mddev->bitmap) {
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return -EBUSY;
>> }
>> set_bit(Journal, &rdev->flags);
>> @@ -7430,7 +7430,7 @@ int md_add_new_disk(struct mddev *mddev, struct
>> mdu_disk_info_s *info)
>> /* --add initiated by this node */
>> err = mddev->cluster_ops->add_new_disk(mddev, rdev);
>> if (err) {
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return err;
>> }
>> }
>> @@ -7440,7 +7440,7 @@ int md_add_new_disk(struct mddev *mddev, struct
>> mdu_disk_info_s *info)
>> err = bind_rdev_to_array(rdev, mddev);
>> if (err)
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> if (mddev_is_clustered(mddev)) {
>> if (info->state & (1 << MD_DISK_CANDIDATE)) {
>> @@ -7503,7 +7503,7 @@ int md_add_new_disk(struct mddev *mddev, struct
>> mdu_disk_info_s *info)
>> err = bind_rdev_to_array(rdev, mddev);
>> if (err) {
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return err;
>> }
>> }
>> @@ -7615,7 +7615,7 @@ static int hot_add_disk(struct mddev *mddev,
>> dev_t dev)
>> return 0;
>> abort_export:
>> - export_rdev(rdev, mddev);
>> + export_rdev(rdev);
>> return err;
>> }
>
> The diff looks good:
>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply
* Re: [PATCH] md: remove unused mddev argument from export_rdev
From: Paul Menzel @ 2026-03-04 7:34 UTC (permalink / raw)
To: Chen Cheng; +Cc: Song Liu, Yu Kuai, Li Nan, linux-raid
In-Reply-To: <20260304111417.20777-1-chencheng@fnnas.com>
Dear Chen,
Am 04.03.26 um 12:14 schrieb Chen Cheng:
> The mddev argument in export_rdev() is never used. Remove it to
> simplify callers.
Nice catch. Was it like this from the beginning? Was the compiler
noticing this itself, so the code stays the same?
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/md.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index c2cc2302d727..521d9b34cd9e 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -98,7 +98,7 @@ static struct workqueue_struct *md_misc_wq;
> static int remove_and_add_spares(struct mddev *mddev,
> struct md_rdev *this);
> static void mddev_detach(struct mddev *mddev);
> -static void export_rdev(struct md_rdev *rdev, struct mddev *mddev);
> +static void export_rdev(struct md_rdev *rdev);
> static void md_wakeup_thread_directly(struct md_thread __rcu **thread);
>
> /*
> @@ -959,7 +959,7 @@ void mddev_unlock(struct mddev *mddev)
> list_for_each_entry_safe(rdev, tmp, &delete, same_set) {
> list_del_init(&rdev->same_set);
> kobject_del(&rdev->kobj);
> - export_rdev(rdev, mddev);
> + export_rdev(rdev);
> }
>
> if (!legacy_async_del_gendisk) {
> @@ -2632,7 +2632,7 @@ void md_autodetect_dev(dev_t dev);
> /* just for claiming the bdev */
> static struct md_rdev claim_rdev;
>
> -static void export_rdev(struct md_rdev *rdev, struct mddev *mddev)
> +static void export_rdev(struct md_rdev *rdev)
> {
> pr_debug("md: export_rdev(%pg)\n", rdev->bdev);
> md_rdev_clear(rdev);
> @@ -4850,7 +4850,7 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
> err = bind_rdev_to_array(rdev, mddev);
> out:
> if (err)
> - export_rdev(rdev, mddev);
> + export_rdev(rdev);
> mddev_unlock_and_resume(mddev);
> if (!err)
> md_new_event();
> @@ -7142,7 +7142,7 @@ static void autorun_devices(int part)
> rdev_for_each_list(rdev, tmp, &candidates) {
> list_del_init(&rdev->same_set);
> if (bind_rdev_to_array(rdev, mddev))
> - export_rdev(rdev, mddev);
> + export_rdev(rdev);
> }
> autorun_array(mddev);
> mddev_unlock_and_resume(mddev);
> @@ -7152,7 +7152,7 @@ static void autorun_devices(int part)
> */
> rdev_for_each_list(rdev, tmp, &candidates) {
> list_del_init(&rdev->same_set);
> - export_rdev(rdev, mddev);
> + export_rdev(rdev);
> }
> mddev_put(mddev);
> }
> @@ -7340,13 +7340,13 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
> pr_warn("md: %pg has different UUID to %pg\n",
> rdev->bdev,
> rdev0->bdev);
> - export_rdev(rdev, mddev);
> + export_rdev(rdev);
> return -EINVAL;
> }
> }
> err = bind_rdev_to_array(rdev, mddev);
> if (err)
> - export_rdev(rdev, mddev);
> + export_rdev(rdev);
> return err;
> }
>
> @@ -7389,7 +7389,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
> /* This was a hot-add request, but events doesn't
> * match, so reject it.
> */
> - export_rdev(rdev, mddev);
> + export_rdev(rdev);
> return -EINVAL;
> }
>
> @@ -7415,7 +7415,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
> }
> }
> if (has_journal || mddev->bitmap) {
> - export_rdev(rdev, mddev);
> + export_rdev(rdev);
> return -EBUSY;
> }
> set_bit(Journal, &rdev->flags);
> @@ -7430,7 +7430,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
> /* --add initiated by this node */
> err = mddev->cluster_ops->add_new_disk(mddev, rdev);
> if (err) {
> - export_rdev(rdev, mddev);
> + export_rdev(rdev);
> return err;
> }
> }
> @@ -7440,7 +7440,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
> err = bind_rdev_to_array(rdev, mddev);
>
> if (err)
> - export_rdev(rdev, mddev);
> + export_rdev(rdev);
>
> if (mddev_is_clustered(mddev)) {
> if (info->state & (1 << MD_DISK_CANDIDATE)) {
> @@ -7503,7 +7503,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
>
> err = bind_rdev_to_array(rdev, mddev);
> if (err) {
> - export_rdev(rdev, mddev);
> + export_rdev(rdev);
> return err;
> }
> }
> @@ -7615,7 +7615,7 @@ static int hot_add_disk(struct mddev *mddev, dev_t dev)
> return 0;
>
> abort_export:
> - export_rdev(rdev, mddev);
> + export_rdev(rdev);
> return err;
> }
The diff looks good:
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply
* [PATCH] md/raid0,linear: move error handling from make_request to IO completion
From: Chen Cheng @ 2026-03-04 14:29 UTC (permalink / raw)
To: Song Liu, Yu Kuai; +Cc: Li Nan, linux-raid, Chen Cheng
raid0 and linear call md_error() in the submission path after checking
is_rdev_broken() (i.e. disk_live() == false). This was intended as a
safety fast-fail for removed disks, but is unnecessary: the block layer
already handles GD_DEAD disks in __bio_queue_enter() and fails IO
immediately without hanging. More importantly, it misses the offline
case where the disk still exists but rejects IO — in that scenario, IO
fails silently and md_error() is never called.
Remove the submission-path check and instead call md_error() from
md_end_clone_io() on any IO failure. Store the target rdev in
md_io_clone so the completion handler knows which device failed. This
uniformly handles disk removal, offline status, and any other IO error.
Since raid0 and linear have no redundancy, any disk IO failure means
data is already lost, so marking the array broken on any error is
correct.
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/md-linear.c | 7 +------
drivers/md/md.c | 5 +++++
drivers/md/md.h | 11 ++++++-----
drivers/md/raid0.c | 7 +------
4 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/md/md-linear.c b/drivers/md/md-linear.c
index fdff250d0d51..2ba181a1297d 100644
--- a/drivers/md/md-linear.c
+++ b/drivers/md/md-linear.c
@@ -251,12 +251,6 @@ static bool linear_make_request(struct mddev *mddev, struct bio *bio)
bio_sector < start_sector))
goto out_of_bounds;
- if (unlikely(is_rdev_broken(tmp_dev->rdev))) {
- md_error(mddev, tmp_dev->rdev);
- bio_io_error(bio);
- return true;
- }
-
if (unlikely(bio_end_sector(bio) > end_sector)) {
/* This bio crosses a device boundary, so we have to split it */
bio = bio_submit_split_bioset(bio, end_sector - bio_sector,
@@ -269,6 +263,7 @@ static bool linear_make_request(struct mddev *mddev, struct bio *bio)
bio_set_dev(bio, tmp_dev->rdev->bdev);
bio->bi_iter.bi_sector = bio->bi_iter.bi_sector -
start_sector + data_offset;
+ md_set_clone_rdev((struct md_io_clone *)bio->bi_private, tmp_dev->rdev);
if (unlikely((bio_op(bio) == REQ_OP_DISCARD) &&
!bdev_max_discard_sectors(bio->bi_bdev))) {
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 3ce6f9e9d38e..7caa24c4919d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9218,6 +9218,7 @@ static void md_end_clone_io(struct bio *bio)
struct md_io_clone *md_io_clone = bio->bi_private;
struct bio *orig_bio = md_io_clone->orig_bio;
struct mddev *mddev = md_io_clone->mddev;
+ enum md_submodule_id id = mddev->pers->head.id;
if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
md_bitmap_end(mddev, md_io_clone);
@@ -9225,6 +9226,10 @@ static void md_end_clone_io(struct bio *bio)
if (bio->bi_status && !orig_bio->bi_status)
orig_bio->bi_status = bio->bi_status;
+ if (bio->bi_status && md_io_clone->rdev &&
+ (id == ID_LINEAR || id == ID_RAID0))
+ md_error(mddev, md_io_clone->rdev);
+
if (md_io_clone->start_time)
bio_end_io_acct(orig_bio, md_io_clone->start_time);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index ac84289664cd..625f8304de3b 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -872,6 +872,7 @@ struct md_thread {
struct md_io_clone {
struct mddev *mddev;
+ struct md_rdev *rdev;
struct bio *orig_bio;
unsigned long start_time;
sector_t offset;
@@ -917,6 +918,11 @@ extern void md_finish_reshape(struct mddev *mddev);
void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
struct bio *bio, sector_t start, sector_t size);
void md_account_bio(struct mddev *mddev, struct bio **bio);
+static inline void md_set_clone_rdev(struct md_io_clone *clone,
+ struct md_rdev *rdev)
+{
+ clone->rdev = rdev;
+}
void md_free_cloned_bio(struct bio *bio);
extern bool __must_check md_flush_request(struct mddev *mddev, struct bio *bio);
@@ -961,11 +967,6 @@ extern void mddev_destroy_serial_pool(struct mddev *mddev,
struct md_rdev *md_find_rdev_nr_rcu(struct mddev *mddev, int nr);
struct md_rdev *md_find_rdev_rcu(struct mddev *mddev, dev_t dev);
-static inline bool is_rdev_broken(struct md_rdev *rdev)
-{
- return !disk_live(rdev->bdev->bd_disk);
-}
-
static inline void rdev_dec_pending(struct md_rdev *rdev, struct mddev *mddev)
{
int faulty = test_bit(Faulty, &rdev->flags);
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index ef0045db409f..e933abeb0d70 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -576,15 +576,10 @@ static void raid0_map_submit_bio(struct mddev *mddev, struct bio *bio)
return;
}
- if (unlikely(is_rdev_broken(tmp_dev))) {
- bio_io_error(bio);
- md_error(mddev, tmp_dev);
- return;
- }
-
bio_set_dev(bio, tmp_dev->bdev);
bio->bi_iter.bi_sector = sector + zone->dev_start +
tmp_dev->data_offset;
+ md_set_clone_rdev((struct md_io_clone *)bio->bi_private, tmp_dev);
mddev_trace_remap(mddev, bio, bio_sector);
mddev_check_write_zeroes(mddev, bio);
submit_bio_noacct(bio);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing
From: Su Yue @ 2026-03-04 3:14 UTC (permalink / raw)
To: Yu Kuai; +Cc: Su Yue, linux-raid, song, xni, linan122, heming.zhao
In-Reply-To: <20fcd6f0-b27d-49aa-8064-79f58f109d5d@fnnas.com>
On Wed 04 Mar 2026 at 10:30, "Yu Kuai" <yukuai@fnnas.com> wrote:
> Hi,
>
> 在 2026/3/3 11:37, Su Yue 写道:
>> Before commit fb8cc3b0d9db ("md/md-bitmap: delay registration
>> of bitmap_ops until creating bitmap")
>> if CONFIG_MD_BITMAP is enabled, both bitmap none, internal and
>> clustered have
>> the sysfs file bitmap/location.
>>
>> After the commit, if bitmap is none, bitmap/location doesn't
>> exist anymore.
>> It breaks 'grow' behavior of a md array of madam with
>> MD_FEATURE_BITMAP_OFFSET.
>> Take level=mirror and metadata=1.2 as an example:
>>
>> $ mdadm --create /dev/md0 -f --bitmap=none --raid-devices=2
>> --level=mirror \
>> --metadata=1.2 /dev/vdd /dev/vde
>> $ mdadm --grow /dev/md0 --bitmap=internal
>> $ cat /sys/block/md0/md/bitmap/location
>> Before:+8
>> After: +2
>>
>> While growing bitmap from none to internal, clustered and
>> llbitmap,
>> mdadm/Grow.c:Grow_addbitmap() tries to detect bitmap/location
>> first.
>> 1)If bitmap/location exists, it sets bitmap/location after
>> getinfo_super().
>> 2)If bitmap/location doesn't exist, mdadm just calls
>> md_set_array_info() then
>> mddev->bitmap_info.default_offset will be used.
>> Situation can be worse if growing none to clustered, bitmap
>> offset of the node
>> calling `madm --grow` will be changed but the other node are
>> reading bitmap sb from
>> the old location.
>
> Now that we have a new sysfs attribute bitmap_type, can we fix
> this by:
> - in the kernel, allow writing to this file in this case;
> - in mdadm and the grow case above, write to this file first,
> and change
> bitmap_type from none to bitmap(For llbitmap, there is still
> more work to do).
>
Yes. It's indeed feasible. But how about old versions mdadm? We
can't require
users' madadm + kernel combinations for old feature. Kernel part
should keep
compatibility with userspace. sysfs changes and broken haviros are
not ideal
especially userspace depends on it unless there's a strong reason.
That's why linux/Documentation/ABI exists.
--
Su
>>
>> Here restore sysfs file bitmap/location for ID_BITMAP_NONE and
>> ID_BITMAP.
>> And it d adds the entry for llbitmap too.
>>
>> New attribute_group md_bitmap_common_group is introduced and
>> created in
>> md_alloc() as before commit fb8cc3b0d9db.
>> Add New operations register_group and unregister_group to
>> struct bitmap_operations.
>>
>> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of
>> bitmap_ops until creating bitmap")
>> Signed-off-by: Su Yue <glass.su@suse.com>
>> ---
>> drivers/md/md-bitmap.c | 32
>> +++++++++++++++++++++++++++++++-
>> drivers/md/md-bitmap.h | 5 +++++
>> drivers/md/md-llbitmap.c | 13 +++++++++++++
>> drivers/md/md.c | 16 ++++++++++++----
>> 4 files changed, 61 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index 83378c033c72..8ff1dc94ed78 100644
>> --- a/drivers/md/md-bitmap.c
>> +++ b/drivers/md/md-bitmap.c
>> @@ -2956,7 +2956,6 @@ __ATTR(max_backlog_used, S_IRUGO |
>> S_IWUSR,
>> behind_writes_used_show, behind_writes_used_reset);
>>
>> static struct attribute *md_bitmap_attrs[] = {
>> - &bitmap_location.attr,
>> &bitmap_space.attr,
>> &bitmap_timeout.attr,
>> &bitmap_backlog.attr,
>> @@ -2967,11 +2966,40 @@ static struct attribute
>> *md_bitmap_attrs[] = {
>> NULL
>> };
>>
>> +static struct attribute *md_bitmap_common_attrs[] = {
>> + &bitmap_location.attr,
>> + NULL
>> +};
>> +
>> static struct attribute_group md_bitmap_group = {
>> .name = "bitmap",
>> .attrs = md_bitmap_attrs,
>> };
>>
>> +static struct attribute_group md_bitmap_common_group = {
>> + .name = "bitmap",
>> + .attrs = md_bitmap_common_attrs,
>> +};
>> +
>> +int md_sysfs_create_common_group(struct mddev *mddev)
>> +{
>> + return sysfs_create_group(&mddev->kobj,
>> &md_bitmap_common_group);
>> +}
>> +
>> +static int bitmap_register_group(struct mddev *mddev)
>> +{
>> + /*
>> + * md_bitmap_group and md_bitmap_common_group are using
>> same name
>> + * 'bitmap'.
>> + */
>> + return sysfs_merge_group(&mddev->kobj, &md_bitmap_group);
>> +}
>> +
>> +static void bitmap_unregister_group(struct mddev *mddev)
>> +{
>> + sysfs_unmerge_group(&mddev->kobj, &md_bitmap_group);
>> +}
>> +
>> static struct bitmap_operations bitmap_ops = {
>> .head = {
>> .type = MD_BITMAP,
>> @@ -3013,6 +3041,8 @@ static struct bitmap_operations
>> bitmap_ops = {
>> .set_pages = bitmap_set_pages,
>> .free = md_bitmap_free,
>>
>> + .register_group = bitmap_register_group,
>> + .unregister_group = bitmap_unregister_group,
>> .group = &md_bitmap_group,
>> };
>>
>> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
>> index b42a28fa83a0..371791e9011d 100644
>> --- a/drivers/md/md-bitmap.h
>> +++ b/drivers/md/md-bitmap.h
>> @@ -125,6 +125,9 @@ struct bitmap_operations {
>> void (*set_pages)(void *data, unsigned long pages);
>> void (*free)(void *data);
>>
>> + int (*register_group)(struct mddev *mddev);
>> + void (*unregister_group)(struct mddev *mddev);
>> +
>> struct attribute_group *group;
>> };
>>
>> @@ -169,6 +172,8 @@ static inline void
>> md_bitmap_end_sync(struct mddev *mddev, sector_t offset,
>> mddev->bitmap_ops->end_sync(mddev, offset, blocks);
>> }
>>
>> +int md_sysfs_create_common_group(struct mddev *mddev);
>> +
>> #ifdef CONFIG_MD_BITMAP
>> int md_bitmap_init(void);
>> void md_bitmap_exit(void);
>> diff --git a/drivers/md/md-llbitmap.c
>> b/drivers/md/md-llbitmap.c
>> index bf398d7476b3..24ff5f7f8751 100644
>> --- a/drivers/md/md-llbitmap.c
>> +++ b/drivers/md/md-llbitmap.c
>> @@ -1561,6 +1561,16 @@ static struct attribute_group
>> md_llbitmap_group = {
>> .attrs = md_llbitmap_attrs,
>> };
>>
>> +static int llbitmap_register_group(struct mddev *mddev)
>> +{
>> + return sysfs_create_group(&mddev->kobj,
>> &md_llbitmap_group);
>> +}
>> +
>> +static void llbitmap_unregister_group(struct mddev *mddev)
>> +{
>> + sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
>> +}
>> +
>> static struct bitmap_operations llbitmap_ops = {
>> .head = {
>> .type = MD_BITMAP,
>> @@ -1597,6 +1607,9 @@ static struct bitmap_operations
>> llbitmap_ops = {
>> .dirty_bits = llbitmap_dirty_bits,
>> .write_all = llbitmap_write_all,
>>
>> + .register_group = llbitmap_register_group,
>> + .unregister_group = llbitmap_unregister_group,
>> +
>> .group = &md_llbitmap_group,
>> };
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 3ce6f9e9d38e..ab969e950ea8 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -703,8 +703,8 @@ static bool mddev_set_bitmap_ops(struct
>> mddev *mddev)
>> mddev->bitmap_ops = (void *)head;
>> xa_unlock(&md_submodule);
>>
>> - if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
>> - if (sysfs_create_group(&mddev->kobj,
>> mddev->bitmap_ops->group))
>> + if (!mddev_is_dm(mddev) &&
>> mddev->bitmap_ops->register_group) {
>> + if (mddev->bitmap_ops->register_group(mddev))
>> pr_warn("md: cannot register extra bitmap
>> attributes for %s\n",
>> mdname(mddev));
>> else
>> @@ -724,8 +724,8 @@ static bool mddev_set_bitmap_ops(struct
>> mddev *mddev)
>> static void mddev_clear_bitmap_ops(struct mddev *mddev)
>> {
>> if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
>> - mddev->bitmap_ops->group)
>> - sysfs_remove_group(&mddev->kobj,
>> mddev->bitmap_ops->group);
>> + mddev->bitmap_ops->unregister_group)
>> + mddev->bitmap_ops->unregister_group(mddev);
>>
>> mddev->bitmap_ops = NULL;
>> }
>> @@ -6369,6 +6369,14 @@ struct mddev *md_alloc(dev_t dev, char
>> *name)
>> return ERR_PTR(error);
>> }
>>
>> + /*
>> + * md_sysfs_remove_common_group is not needed because
>> mddev_delayed_delete
>> + * calls kobject_put(&mddev->kobj) if mddev is to be
>> deleted.
>> + */
>> + if (md_sysfs_create_common_group(mddev))
>> + pr_warn("md: cannot register common bitmap attributes
>> for %s\n",
>> + mdname(mddev));
>> +
>> kobject_uevent(&mddev->kobj, KOBJ_ADD);
>> mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd,
>> "array_state");
>> mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd,
>> "level");
^ permalink raw reply
* [PATCH] md: remove unused mddev argument from export_rdev
From: Chen Cheng @ 2026-03-04 11:14 UTC (permalink / raw)
To: Song Liu, Yu Kuai; +Cc: Li Nan, linux-raid, Chen Cheng
The mddev argument in export_rdev() is never used. Remove it to
simplify callers.
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/md.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index c2cc2302d727..521d9b34cd9e 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -98,7 +98,7 @@ static struct workqueue_struct *md_misc_wq;
static int remove_and_add_spares(struct mddev *mddev,
struct md_rdev *this);
static void mddev_detach(struct mddev *mddev);
-static void export_rdev(struct md_rdev *rdev, struct mddev *mddev);
+static void export_rdev(struct md_rdev *rdev);
static void md_wakeup_thread_directly(struct md_thread __rcu **thread);
/*
@@ -959,7 +959,7 @@ void mddev_unlock(struct mddev *mddev)
list_for_each_entry_safe(rdev, tmp, &delete, same_set) {
list_del_init(&rdev->same_set);
kobject_del(&rdev->kobj);
- export_rdev(rdev, mddev);
+ export_rdev(rdev);
}
if (!legacy_async_del_gendisk) {
@@ -2632,7 +2632,7 @@ void md_autodetect_dev(dev_t dev);
/* just for claiming the bdev */
static struct md_rdev claim_rdev;
-static void export_rdev(struct md_rdev *rdev, struct mddev *mddev)
+static void export_rdev(struct md_rdev *rdev)
{
pr_debug("md: export_rdev(%pg)\n", rdev->bdev);
md_rdev_clear(rdev);
@@ -4850,7 +4850,7 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
err = bind_rdev_to_array(rdev, mddev);
out:
if (err)
- export_rdev(rdev, mddev);
+ export_rdev(rdev);
mddev_unlock_and_resume(mddev);
if (!err)
md_new_event();
@@ -7142,7 +7142,7 @@ static void autorun_devices(int part)
rdev_for_each_list(rdev, tmp, &candidates) {
list_del_init(&rdev->same_set);
if (bind_rdev_to_array(rdev, mddev))
- export_rdev(rdev, mddev);
+ export_rdev(rdev);
}
autorun_array(mddev);
mddev_unlock_and_resume(mddev);
@@ -7152,7 +7152,7 @@ static void autorun_devices(int part)
*/
rdev_for_each_list(rdev, tmp, &candidates) {
list_del_init(&rdev->same_set);
- export_rdev(rdev, mddev);
+ export_rdev(rdev);
}
mddev_put(mddev);
}
@@ -7340,13 +7340,13 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
pr_warn("md: %pg has different UUID to %pg\n",
rdev->bdev,
rdev0->bdev);
- export_rdev(rdev, mddev);
+ export_rdev(rdev);
return -EINVAL;
}
}
err = bind_rdev_to_array(rdev, mddev);
if (err)
- export_rdev(rdev, mddev);
+ export_rdev(rdev);
return err;
}
@@ -7389,7 +7389,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
/* This was a hot-add request, but events doesn't
* match, so reject it.
*/
- export_rdev(rdev, mddev);
+ export_rdev(rdev);
return -EINVAL;
}
@@ -7415,7 +7415,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
}
}
if (has_journal || mddev->bitmap) {
- export_rdev(rdev, mddev);
+ export_rdev(rdev);
return -EBUSY;
}
set_bit(Journal, &rdev->flags);
@@ -7430,7 +7430,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
/* --add initiated by this node */
err = mddev->cluster_ops->add_new_disk(mddev, rdev);
if (err) {
- export_rdev(rdev, mddev);
+ export_rdev(rdev);
return err;
}
}
@@ -7440,7 +7440,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
err = bind_rdev_to_array(rdev, mddev);
if (err)
- export_rdev(rdev, mddev);
+ export_rdev(rdev);
if (mddev_is_clustered(mddev)) {
if (info->state & (1 << MD_DISK_CANDIDATE)) {
@@ -7503,7 +7503,7 @@ int md_add_new_disk(struct mddev *mddev, struct mdu_disk_info_s *info)
err = bind_rdev_to_array(rdev, mddev);
if (err) {
- export_rdev(rdev, mddev);
+ export_rdev(rdev);
return err;
}
}
@@ -7615,7 +7615,7 @@ static int hot_add_disk(struct mddev *mddev, dev_t dev)
return 0;
abort_export:
- export_rdev(rdev, mddev);
+ export_rdev(rdev);
return err;
}
--
2.51.0
^ permalink raw reply related
* [PATCH 2/2] md/raid5: move handle_stripe() comment to correct location
From: Chen Cheng @ 2026-03-04 11:10 UTC (permalink / raw)
To: Song Liu, Yu Kuai; +Cc: Li Nan, linux-raid, Chen Cheng
Move the handle_stripe() documentation comment from above
analyse_stripe() to directly above handle_stripe() where it belongs.
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/raid5.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index a8e8d431071b..48504b0e496b 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -4594,20 +4594,6 @@ static void handle_stripe_expansion(struct r5conf *conf, struct stripe_head *sh)
async_tx_quiesce(&tx);
}
-/*
- * handle_stripe - do things to a stripe.
- *
- * We lock the stripe by setting STRIPE_ACTIVE and then examine the
- * state of various bits to see what needs to be done.
- * Possible results:
- * return some read requests which now have data
- * return some write requests which are safely on storage
- * schedule a read on some buffers
- * schedule a write of some buffers
- * return confirmation of parity correctness
- *
- */
-
static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
{
struct r5conf *conf = sh->raid_conf;
@@ -4901,6 +4887,18 @@ static void break_stripe_batch_list(struct stripe_head *head_sh,
set_bit(STRIPE_HANDLE, &head_sh->state);
}
+/*
+ * handle_stripe - do things to a stripe.
+ *
+ * We lock the stripe by setting STRIPE_ACTIVE and then examine the
+ * state of various bits to see what needs to be done.
+ * Possible results:
+ * return some read requests which now have data
+ * return some write requests which are safely on storage
+ * schedule a read on some buffers
+ * schedule a write of some buffers
+ * return confirmation of parity correctness
+ */
static void handle_stripe(struct stripe_head *sh)
{
struct stripe_head_state s;
--
2.51.0
^ permalink raw reply related
* [PATCH 1/2] md/raid5: remove stale md_raid5_kick_device() declaration
From: Chen Cheng @ 2026-03-04 11:09 UTC (permalink / raw)
To: Song Liu, Yu Kuai; +Cc: Li Nan, linux-raid, Chen Cheng
Remove the unused md_raid5_kick_device() declaration from raid5.h -
no definition exists for this function.
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/raid5.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 110b1c2d0a86..1c7b710fc9c1 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -801,7 +801,6 @@ raid5_get_dev_page(struct stripe_head *sh, int disk_idx)
}
#endif
-void md_raid5_kick_device(struct r5conf *conf);
int raid5_set_cache_size(struct mddev *mddev, int size);
sector_t raid5_compute_blocknr(struct stripe_head *sh, int i, int previous);
void raid5_release_stripe(struct stripe_head *sh);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] md/raid5: set chunk_sectors to enable full stripe I/O splitting
From: Yu Kuai @ 2026-03-04 2:36 UTC (permalink / raw)
To: Paul Menzel, yukuai; +Cc: hch, song, linux-raid, linux-kernel, linan122
In-Reply-To: <a48b64d7-e40f-49e3-a118-b27723b09c39@molgen.mpg.de>
Hi,
在 2026/2/23 15:26, Paul Menzel 写道:
> Dear Kuai,
>
>
> Thank you for your patch.
>
> Am 23.02.26 um 04:58 schrieb Yu Kuai:
>> Set chunk_sectors to the full stripe width (io_opt) so that the block
>> layer splits I/O at full stripe boundaries. This ensures that large
>> writes are aligned to full stripes, avoiding the read-modify-write
>> overhead that occurs with partial stripe writes in RAID-5/6.
>>
>> When chunk_sectors is set, the block layer's bio splitting logic in
>> get_max_io_size() uses blk_boundary_sectors_left() to limit I/O size
>> to the boundary. This naturally aligns split bios to full stripe
>> boundaries, enabling more efficient full stripe writes.
>>
>> Test results with 24-disk RAID5 (chunk_size=64k):
>> dd if=/dev/zero of=/dev/md0 bs=10M oflag=direct
>> Before: 461 MB/s
>> After: 520 MB/s (+12.8%)
>
> Sweet.
>
>> Suggested-by: Christoph Hellwig <hch@infradead.org>
>> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
>
> Once committed, should this be backported to the longterm and stable
> series?
Sorry for the delay, this is just performance improvement, I think this will
not be backported automatically.This patch need to be send to LTS with some
explanation if someone is interested.
>
>> ---
>> drivers/md/raid5.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index 8854e024f311..810d936560d1 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -7783,6 +7783,7 @@ static int raid5_set_limits(struct mddev *mddev)
>> lim.logical_block_size = mddev->logical_block_size;
>> lim.io_min = mddev->chunk_sectors << 9;
>> lim.io_opt = lim.io_min * (conf->raid_disks - conf->max_degraded);
>> + lim.chunk_sectors = lim.io_opt >> 9;
>> lim.features |= BLK_FEAT_RAID_PARTIAL_STRIPES_EXPENSIVE;
>> lim.discard_granularity = stripe;
>> lim.max_write_zeroes_sectors = 0;
>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>
>
> Kind regards,
>
> Paul
>
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: set chunk_sectors to enable full stripe I/O splitting
From: Yu Kuai @ 2026-03-04 2:36 UTC (permalink / raw)
To: hch, song, yukuai; +Cc: linux-raid, linux-kernel, linan122
In-Reply-To: <20260223035834.3132498-1-yukuai@fnnas.com>
在 2026/2/23 11:58, Yu Kuai 写道:
> Set chunk_sectors to the full stripe width (io_opt) so that the block
> layer splits I/O at full stripe boundaries. This ensures that large
> writes are aligned to full stripes, avoiding the read-modify-write
> overhead that occurs with partial stripe writes in RAID-5/6.
>
> When chunk_sectors is set, the block layer's bio splitting logic in
> get_max_io_size() uses blk_boundary_sectors_left() to limit I/O size
> to the boundary. This naturally aligns split bios to full stripe
> boundaries, enabling more efficient full stripe writes.
>
> Test results with 24-disk RAID5 (chunk_size=64k):
> dd if=/dev/zero of=/dev/md0 bs=10M oflag=direct
> Before: 461 MB/s
> After: 520 MB/s (+12.8%)
>
> Suggested-by: Christoph Hellwig<hch@infradead.org>
> Signed-off-by: Yu Kuai<yukuai@fnnas.com>
> ---
> drivers/md/raid5.c | 1 +
> 1 file changed, 1 insertion(+)
Applied to md-7.0
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH 1/3] md: restore bitmap/location to fix wrong bitmap offset while growing
From: Yu Kuai @ 2026-03-04 2:30 UTC (permalink / raw)
To: Su Yue, linux-raid, yukuai; +Cc: song, xni, linan122, heming.zhao, l
In-Reply-To: <20260303033731.83885-2-glass.su@suse.com>
Hi,
在 2026/3/3 11:37, Su Yue 写道:
> Before commit fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
> if CONFIG_MD_BITMAP is enabled, both bitmap none, internal and clustered have
> the sysfs file bitmap/location.
>
> After the commit, if bitmap is none, bitmap/location doesn't exist anymore.
> It breaks 'grow' behavior of a md array of madam with MD_FEATURE_BITMAP_OFFSET.
> Take level=mirror and metadata=1.2 as an example:
>
> $ mdadm --create /dev/md0 -f --bitmap=none --raid-devices=2 --level=mirror \
> --metadata=1.2 /dev/vdd /dev/vde
> $ mdadm --grow /dev/md0 --bitmap=internal
> $ cat /sys/block/md0/md/bitmap/location
> Before:+8
> After: +2
>
> While growing bitmap from none to internal, clustered and llbitmap,
> mdadm/Grow.c:Grow_addbitmap() tries to detect bitmap/location first.
> 1)If bitmap/location exists, it sets bitmap/location after getinfo_super().
> 2)If bitmap/location doesn't exist, mdadm just calls md_set_array_info() then
> mddev->bitmap_info.default_offset will be used.
> Situation can be worse if growing none to clustered, bitmap offset of the node
> calling `madm --grow` will be changed but the other node are reading bitmap sb from
> the old location.
Now that we have a new sysfs attribute bitmap_type, can we fix this by:
- in the kernel, allow writing to this file in this case;
- in mdadm and the grow case above, write to this file first, and change
bitmap_type from none to bitmap(For llbitmap, there is still more work to do).
>
> Here restore sysfs file bitmap/location for ID_BITMAP_NONE and ID_BITMAP.
> And it d adds the entry for llbitmap too.
>
> New attribute_group md_bitmap_common_group is introduced and created in
> md_alloc() as before commit fb8cc3b0d9db.
> Add New operations register_group and unregister_group to struct bitmap_operations.
>
> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
> Signed-off-by: Su Yue <glass.su@suse.com>
> ---
> drivers/md/md-bitmap.c | 32 +++++++++++++++++++++++++++++++-
> drivers/md/md-bitmap.h | 5 +++++
> drivers/md/md-llbitmap.c | 13 +++++++++++++
> drivers/md/md.c | 16 ++++++++++++----
> 4 files changed, 61 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 83378c033c72..8ff1dc94ed78 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -2956,7 +2956,6 @@ __ATTR(max_backlog_used, S_IRUGO | S_IWUSR,
> behind_writes_used_show, behind_writes_used_reset);
>
> static struct attribute *md_bitmap_attrs[] = {
> - &bitmap_location.attr,
> &bitmap_space.attr,
> &bitmap_timeout.attr,
> &bitmap_backlog.attr,
> @@ -2967,11 +2966,40 @@ static struct attribute *md_bitmap_attrs[] = {
> NULL
> };
>
> +static struct attribute *md_bitmap_common_attrs[] = {
> + &bitmap_location.attr,
> + NULL
> +};
> +
> static struct attribute_group md_bitmap_group = {
> .name = "bitmap",
> .attrs = md_bitmap_attrs,
> };
>
> +static struct attribute_group md_bitmap_common_group = {
> + .name = "bitmap",
> + .attrs = md_bitmap_common_attrs,
> +};
> +
> +int md_sysfs_create_common_group(struct mddev *mddev)
> +{
> + return sysfs_create_group(&mddev->kobj, &md_bitmap_common_group);
> +}
> +
> +static int bitmap_register_group(struct mddev *mddev)
> +{
> + /*
> + * md_bitmap_group and md_bitmap_common_group are using same name
> + * 'bitmap'.
> + */
> + return sysfs_merge_group(&mddev->kobj, &md_bitmap_group);
> +}
> +
> +static void bitmap_unregister_group(struct mddev *mddev)
> +{
> + sysfs_unmerge_group(&mddev->kobj, &md_bitmap_group);
> +}
> +
> static struct bitmap_operations bitmap_ops = {
> .head = {
> .type = MD_BITMAP,
> @@ -3013,6 +3041,8 @@ static struct bitmap_operations bitmap_ops = {
> .set_pages = bitmap_set_pages,
> .free = md_bitmap_free,
>
> + .register_group = bitmap_register_group,
> + .unregister_group = bitmap_unregister_group,
> .group = &md_bitmap_group,
> };
>
> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
> index b42a28fa83a0..371791e9011d 100644
> --- a/drivers/md/md-bitmap.h
> +++ b/drivers/md/md-bitmap.h
> @@ -125,6 +125,9 @@ struct bitmap_operations {
> void (*set_pages)(void *data, unsigned long pages);
> void (*free)(void *data);
>
> + int (*register_group)(struct mddev *mddev);
> + void (*unregister_group)(struct mddev *mddev);
> +
> struct attribute_group *group;
> };
>
> @@ -169,6 +172,8 @@ static inline void md_bitmap_end_sync(struct mddev *mddev, sector_t offset,
> mddev->bitmap_ops->end_sync(mddev, offset, blocks);
> }
>
> +int md_sysfs_create_common_group(struct mddev *mddev);
> +
> #ifdef CONFIG_MD_BITMAP
> int md_bitmap_init(void);
> void md_bitmap_exit(void);
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index bf398d7476b3..24ff5f7f8751 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1561,6 +1561,16 @@ static struct attribute_group md_llbitmap_group = {
> .attrs = md_llbitmap_attrs,
> };
>
> +static int llbitmap_register_group(struct mddev *mddev)
> +{
> + return sysfs_create_group(&mddev->kobj, &md_llbitmap_group);
> +}
> +
> +static void llbitmap_unregister_group(struct mddev *mddev)
> +{
> + sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
> +}
> +
> static struct bitmap_operations llbitmap_ops = {
> .head = {
> .type = MD_BITMAP,
> @@ -1597,6 +1607,9 @@ static struct bitmap_operations llbitmap_ops = {
> .dirty_bits = llbitmap_dirty_bits,
> .write_all = llbitmap_write_all,
>
> + .register_group = llbitmap_register_group,
> + .unregister_group = llbitmap_unregister_group,
> +
> .group = &md_llbitmap_group,
> };
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 3ce6f9e9d38e..ab969e950ea8 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -703,8 +703,8 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
> mddev->bitmap_ops = (void *)head;
> xa_unlock(&md_submodule);
>
> - if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
> - if (sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group))
> + if (!mddev_is_dm(mddev) && mddev->bitmap_ops->register_group) {
> + if (mddev->bitmap_ops->register_group(mddev))
> pr_warn("md: cannot register extra bitmap attributes for %s\n",
> mdname(mddev));
> else
> @@ -724,8 +724,8 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
> static void mddev_clear_bitmap_ops(struct mddev *mddev)
> {
> if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
> - mddev->bitmap_ops->group)
> - sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
> + mddev->bitmap_ops->unregister_group)
> + mddev->bitmap_ops->unregister_group(mddev);
>
> mddev->bitmap_ops = NULL;
> }
> @@ -6369,6 +6369,14 @@ struct mddev *md_alloc(dev_t dev, char *name)
> return ERR_PTR(error);
> }
>
> + /*
> + * md_sysfs_remove_common_group is not needed because mddev_delayed_delete
> + * calls kobject_put(&mddev->kobj) if mddev is to be deleted.
> + */
> + if (md_sysfs_create_common_group(mddev))
> + pr_warn("md: cannot register common bitmap attributes for %s\n",
> + mdname(mddev));
> +
> kobject_uevent(&mddev->kobj, KOBJ_ADD);
> mddev->sysfs_state = sysfs_get_dirent_safe(mddev->kobj.sd, "array_state");
> mddev->sysfs_level = sysfs_get_dirent_safe(mddev->kobj.sd, "level");
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH v3] md/raid10: fix deadlock with check operation and nowait requests
From: Yu Kuai @ 2026-03-04 2:18 UTC (permalink / raw)
To: Josh Hunt, song, linan122, linux-raid, yukuai; +Cc: ncroxon, stable
In-Reply-To: <20260303005619.1352958-1-johunt@akamai.com>
在 2026/3/3 8:56, Josh Hunt 写道:
> When an array check is running it will raise the barrier at which point
> normal requests will become blocked and increment the nr_pending value to
> signal there is work pending inside of wait_barrier(). NOWAIT requests
> do not block and so will return immediately with an error, and additionally
> do not increment nr_pending in wait_barrier(). Upstream change
> 43806c3d5b9b ("raid10: cleanup memleak at raid10_make_request") added a
> call to raid_end_bio_io() to fix a memory leak when NOWAIT requests hit
> this condition. raid_end_bio_io() eventually calls allow_barrier() and
> it will unconditionally do an atomic_dec_and_test(&conf->nr_pending) even
> though the corresponding increment on nr_pending didn't happen in the
> NOWAIT case.
>
> This can be easily seen by starting a check operation while an application is
> doing nowait IO on the same array. This results in a deadlocked state due to
> nr_pending value underflowing and so the md resync thread gets stuck waiting
> for nr_pending to == 0.
>
> Output of r10conf state of the array when we hit this condition:
>
> crash> struct r10conf.barrier,nr_pending,nr_waiting,nr_queued <addr of r10conf>
> barrier = 1,
> nr_pending = {
> counter = -41
> },
> nr_waiting = 15,
> nr_queued = 0,
>
> Example of md_sync thread stuck waiting on raise_barrier() and other requests
> stuck in wait_barrier():
>
> md1_resync
> [<0>] raise_barrier+0xce/0x1c0
> [<0>] raid10_sync_request+0x1ca/0x1ed0
> [<0>] md_do_sync+0x779/0x1110
> [<0>] md_thread+0x90/0x160
> [<0>] kthread+0xbe/0xf0
> [<0>] ret_from_fork+0x34/0x50
> [<0>] ret_from_fork_asm+0x1a/0x30
>
> kworker/u1040:2+flush-253:4
> [<0>] wait_barrier+0x1de/0x220
> [<0>] regular_request_wait+0x30/0x180
> [<0>] raid10_make_request+0x261/0x1000
> [<0>] md_handle_request+0x13b/0x230
> [<0>] __submit_bio+0x107/0x1f0
> [<0>] submit_bio_noacct_nocheck+0x16f/0x390
> [<0>] ext4_io_submit+0x24/0x40
> [<0>] ext4_do_writepages+0x254/0xc80
> [<0>] ext4_writepages+0x84/0x120
> [<0>] do_writepages+0x7a/0x260
> [<0>] __writeback_single_inode+0x3d/0x300
> [<0>] writeback_sb_inodes+0x1dd/0x470
> [<0>] __writeback_inodes_wb+0x4c/0xe0
> [<0>] wb_writeback+0x18b/0x2d0
> [<0>] wb_workfn+0x2a1/0x400
> [<0>] process_one_work+0x149/0x330
> [<0>] worker_thread+0x2d2/0x410
> [<0>] kthread+0xbe/0xf0
> [<0>] ret_from_fork+0x34/0x50
> [<0>] ret_from_fork_asm+0x1a/0x30
>
> Fixes: 43806c3d5b9b ("raid10: cleanup memleak at raid10_make_request")
> Cc: stable@vger.kernel.org
> Signed-off-by: Josh Hunt <johunt@akamai.com>
> ---
> v3:
> * Call free_r10bio() as per Yu Kuai's suggestion
> ---
> drivers/md/raid10.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
There are some checkpatch errors and warnings, applied to md-7.0 with some
changes to commit message.
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 9debb20cf129..b4892c5d571c 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1184,7 +1184,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
> }
>
> if (!regular_request_wait(mddev, conf, bio, r10_bio->sectors)) {
> - raid_end_bio_io(r10_bio);
> + free_r10bio(r10_bio);
> return;
> }
>
> @@ -1372,7 +1372,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
>
> sectors = r10_bio->sectors;
> if (!regular_request_wait(mddev, conf, bio, sectors)) {
> - raid_end_bio_io(r10_bio);
> + free_r10bio(r10_bio);
> return;
> }
>
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: clean up stale declaration and misplaced comment
From: Yu Kuai @ 2026-03-04 2:10 UTC (permalink / raw)
To: Chen Cheng, Song Liu, Yu Kuai; +Cc: Li Nan, linux-raid
In-Reply-To: <20260303111608.7259-1-chencheng@fnnas.com>
Hi,
在 2026/3/3 19:16, Chen Cheng 写道:
> Remove the unused md_raid5_kick_device() declaration from raid5.h -
> no definition exists for this function.
>
> Move the handle_stripe() documentation comment from above
> analyse_stripe() to directly above handle_stripe() where it belongs.
Looks fine to me, however, please split and just do one thing in one patch.
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 26 ++++++++++++--------------
> drivers/md/raid5.h | 1 -
> 2 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index a8e8d431071b..48504b0e496b 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -4594,20 +4594,6 @@ static void handle_stripe_expansion(struct r5conf *conf, struct stripe_head *sh)
> async_tx_quiesce(&tx);
> }
>
> -/*
> - * handle_stripe - do things to a stripe.
> - *
> - * We lock the stripe by setting STRIPE_ACTIVE and then examine the
> - * state of various bits to see what needs to be done.
> - * Possible results:
> - * return some read requests which now have data
> - * return some write requests which are safely on storage
> - * schedule a read on some buffers
> - * schedule a write of some buffers
> - * return confirmation of parity correctness
> - *
> - */
> -
> static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
> {
> struct r5conf *conf = sh->raid_conf;
> @@ -4901,6 +4887,18 @@ static void break_stripe_batch_list(struct stripe_head *head_sh,
> set_bit(STRIPE_HANDLE, &head_sh->state);
> }
>
> +/*
> + * handle_stripe - do things to a stripe.
> + *
> + * We lock the stripe by setting STRIPE_ACTIVE and then examine the
> + * state of various bits to see what needs to be done.
> + * Possible results:
> + * return some read requests which now have data
> + * return some write requests which are safely on storage
> + * schedule a read on some buffers
> + * schedule a write of some buffers
> + * return confirmation of parity correctness
> + */
> static void handle_stripe(struct stripe_head *sh)
> {
> struct stripe_head_state s;
> diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
> index 110b1c2d0a86..1c7b710fc9c1 100644
> --- a/drivers/md/raid5.h
> +++ b/drivers/md/raid5.h
> @@ -801,7 +801,6 @@ raid5_get_dev_page(struct stripe_head *sh, int disk_idx)
> }
> #endif
>
> -void md_raid5_kick_device(struct r5conf *conf);
> int raid5_set_cache_size(struct mddev *mddev, int size);
> sector_t raid5_compute_blocknr(struct stripe_head *sh, int i, int previous);
> void raid5_release_stripe(struct stripe_head *sh);
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH 01/25] xor: assert that xor_blocks is not called from interrupt context
From: Eric Biggers @ 2026-03-03 19:55 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Peter Zijlstra, Andrew Morton, Richard Henderson, Matt Turner,
Magnus Lindholm, Russell King, Catalin Marinas, Will Deacon,
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, David S. Miller, Andreas Larsson,
Richard Weinberger, Anton Ivanov, Johannes Berg, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Herbert Xu, Dan Williams, Chris Mason, David Sterba,
Arnd Bergmann, Song Liu, Yu Kuai, Li Nan, linux-alpha,
linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-um, linux-crypto,
linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260303160050.GB7021@lst.de>
On Tue, Mar 03, 2026 at 05:00:50PM +0100, Christoph Hellwig wrote:
> On Fri, Feb 27, 2026 at 03:24:55PM +0100, Peter Zijlstra wrote:
> > > unsigned long *p1, *p2, *p3, *p4;
> > >
> > > + WARN_ON_ONCE(in_interrupt());
> >
> > Your changelog makes it sound like you want:
> >
> > WARN_ON_ONCE(!in_task());
> >
> > But perhaps something like so:
> >
> > lockdep_assert_preempt_enabled();
> >
> > Would do? That ensures we are in preemptible context, which is much the
> > same. That also ensures the cost of this assertion is only paid on debug
> > kernels.
>
> No idea honestly. The kernel FPU/vector helpers generally don't work
> from irq context, and I want to assert that. Happy to do whatever
> version works best for that.
may_use_simd() is the "generic" way to check "can the FPU/vector/SIMD
registers be used". However, what it does varies by architecture, and
it's kind of a questionable abstraction in the first place. It's used
mostly by architecture-specific code.
If you union together the context restrictions from all the
architectures, I think you get: "For may_use_simd() to be guaranteed not
to return false due to the context, the caller needs to be running in
task context without hardirqs or softirqs disabled."
However, some architectures also incorporate a CPU feature check in
may_use_simd() as well, which makes it return false if some
CPU-dependent SIMD feature is not supported.
Because of that CPU feature check, I don't think
"WARN_ON_ONCE(!may_use_simd())" would actually be correct here.
How about "WARN_ON_ONCE(!preemptible())"? I think that covers the union
of the context restrictions correctly. (Compared to in_task(), it
handles the cases where hardirqs or softirqs are disabled.)
Yes, it could be lockdep_assert_preemption_enabled(), but I'm not sure
"ensures the cost of this assertion is only paid on debug kernels" is
worth the cost of hiding this on production kernels. The consequences
of using FPU/vector/SIMD registers when they can't be are very bad: some
random task's registers get corrupted.
- Eric
^ permalink raw reply
* Re: [PATCH 07/25] xor: split xor.h
From: Christoph Hellwig @ 2026-03-03 16:15 UTC (permalink / raw)
To: Eric Biggers
Cc: Christoph Hellwig, Andrew Morton, Richard Henderson, Matt Turner,
Magnus Lindholm, Russell King, Catalin Marinas, Will Deacon,
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, David S. Miller, Andreas Larsson,
Richard Weinberger, Anton Ivanov, Johannes Berg, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Herbert Xu, Dan Williams, Chris Mason, David Sterba,
Arnd Bergmann, Song Liu, Yu Kuai, Li Nan, linux-alpha,
linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-um, linux-crypto,
linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260303160309.GD7021@lst.de>
On Tue, Mar 03, 2026 at 05:03:09PM +0100, Christoph Hellwig wrote:
> On Fri, Feb 27, 2026 at 08:43:55PM -0800, Eric Biggers wrote:
> > On Thu, Feb 26, 2026 at 07:10:19AM -0800, Christoph Hellwig wrote:
> > > Keep xor.h for the public API, and split the struct xor_block_template
> > > definition that is only needed by the xor.ko core and
> > > architecture-specific optimizations into a separate xor_impl.h header.
> > >
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > > arch/arm/lib/xor-neon.c | 1 +
> > > arch/s390/lib/xor.c | 2 +-
> > > include/linux/raid/xor.h | 22 +---------------------
> > > include/linux/raid/xor_impl.h | 25 +++++++++++++++++++++++++
> > > lib/raid/xor/xor-core.c | 1 +
> > > 5 files changed, 29 insertions(+), 22 deletions(-)
> > > create mode 100644 include/linux/raid/xor_impl.h
> >
> > arch/arm64/lib/xor-neon.c needs to be updated to include xor_impl.h.
>
> As of this patch it is not using anything from that header (but
> neither from the public xor.h).
Actually looks like we do need it because it pulls in
arch/arm64/include/asm/xor.h.
Anyway, I think I'll actually move this patch to the end so that the
impl header does not need moving to it's final place.
^ permalink raw reply
* Re: cleanup the RAID5 XOR library
From: Christoph Hellwig @ 2026-03-03 16:11 UTC (permalink / raw)
To: Eric Biggers
Cc: Christoph Hellwig, Andrew Morton, Richard Henderson, Matt Turner,
Magnus Lindholm, Russell King, Catalin Marinas, Will Deacon,
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, David S. Miller, Andreas Larsson,
Richard Weinberger, Anton Ivanov, Johannes Berg, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Herbert Xu, Dan Williams, Chris Mason, David Sterba,
Arnd Bergmann, Song Liu, Yu Kuai, Li Nan, linux-alpha,
linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-um, linux-crypto,
linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260228073553.GL65277@quark>
On Fri, Feb 27, 2026 at 11:35:53PM -0800, Eric Biggers wrote:
> > https://git.infradead.org/?p=users/hch/misc.git;a=shortlog;h=refs/heads/xor-improvements
>
> Overall this looks great. xor_gen() really needs a KUnit test, though.
> Without that, how was this tested?
fio data integrity testing on degraded raid. But yes, a unit test
would be nice.
> Later we should remove some of the obsolete implementations, such as the
> alpha or x86 MMX ones. Those platforms have no optimized code in
> lib/crc/ or lib/crypto/, and I doubt anyone cares.
I'd rather leave that to the architecture maintainers, but overall I
ahree.
> Also, I notice that no one has optimized this for the latest x86_64 CPUs
> by using the vpternlogd instruction to do 3-input XORs. That would be
> another good future project.
Yes, as would rewriting the routines to deal with more than 4 + 1 stripes
as that is a really narrow raid these days.
^ permalink raw reply
* Re: [PATCH 24/25] xor: pass the entire operation to the low-level ops
From: Christoph Hellwig @ 2026-03-03 16:09 UTC (permalink / raw)
To: Eric Biggers
Cc: Christoph Hellwig, Andrew Morton, Richard Henderson, Matt Turner,
Magnus Lindholm, Russell King, Catalin Marinas, Will Deacon,
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, David S. Miller, Andreas Larsson,
Richard Weinberger, Anton Ivanov, Johannes Berg, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Herbert Xu, Dan Williams, Chris Mason, David Sterba,
Arnd Bergmann, Song Liu, Yu Kuai, Li Nan, linux-alpha,
linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-um, linux-crypto,
linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260228065810.GJ65277@quark>
On Fri, Feb 27, 2026 at 10:58:10PM -0800, Eric Biggers wrote:
> On Thu, Feb 26, 2026 at 07:10:36AM -0800, Christoph Hellwig wrote:
> > +#define __DO_XOR_BLOCKS(_name, _handle1, _handle2, _handle3, _handle4) \
> > +void \
> > +xor_gen_##_name(void *dest, void **srcs, unsigned int src_cnt, \
> > + unsigned int bytes) \
> > +{ \
> > + unsigned int src_off = 0; \
> > + \
> > + while (src_cnt > 0) { \
> > + unsigned int this_cnt = min(src_cnt, 4); \
> > + unsigned long *p1 = (unsigned long *)srcs[src_off]; \
> > + unsigned long *p2 = (unsigned long *)srcs[src_off + 1]; \
> > + unsigned long *p3 = (unsigned long *)srcs[src_off + 2]; \
> > + unsigned long *p4 = (unsigned long *)srcs[src_off + 3]; \
>
> This reads out of bounds if src_cnt isn't a multiple of 4.
Assuming the compiler doesn't do the obvious optimization and
drop it, but yes, should be easy enough to avoid this.
^ permalink raw reply
* Re: [PATCH 09/25] xor: move generic implementations out of asm-generic/xor.h
From: Christoph Hellwig @ 2026-03-03 16:09 UTC (permalink / raw)
To: Eric Biggers
Cc: Christoph Hellwig, Andrew Morton, Richard Henderson, Matt Turner,
Magnus Lindholm, Russell King, Catalin Marinas, Will Deacon,
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, David S. Miller, Andreas Larsson,
Richard Weinberger, Anton Ivanov, Johannes Berg, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Herbert Xu, Dan Williams, Chris Mason, David Sterba,
Arnd Bergmann, Song Liu, Yu Kuai, Li Nan, linux-alpha,
linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-um, linux-crypto,
linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260228071521.GK65277@quark>
On Fri, Feb 27, 2026 at 11:15:21PM -0800, Eric Biggers wrote:
> This makes the generic code always be included in xor.ko, even when the
> architecture doesn't need it. For example, x86_64 doesn't need it,
> since it always selects either the AVX or SSE code.
True. OTOH it is tiny.
> Have you considered putting the generic code in xor-core.c (or in
> headers included by it) before xor_arch.h is included, and putting
> __maybe_unused on the xor_block_template structs? Then they'll still be
> available for arch_xor_init() to use, but any of them that aren't used
> in a particular build will be optimized out as dead code by the
> compiler.
And earlier version did this, but it's a bit ugly. What I might
consider is to require architectures that provide optimized version
to opt into any generic one they want to use. This would require
extra kconfig symbols, but be a lot cleaner overall.
^ permalink raw reply
* Re: [PATCH 21/25] xor: add a better public API
From: Christoph Hellwig @ 2026-03-03 16:07 UTC (permalink / raw)
To: Eric Biggers
Cc: Christoph Hellwig, Andrew Morton, Richard Henderson, Matt Turner,
Magnus Lindholm, Russell King, Catalin Marinas, Will Deacon,
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, David S. Miller, Andreas Larsson,
Richard Weinberger, Anton Ivanov, Johannes Berg, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Herbert Xu, Dan Williams, Chris Mason, David Sterba,
Arnd Bergmann, Song Liu, Yu Kuai, Li Nan, linux-alpha,
linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-um, linux-crypto,
linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260228065038.GH65277@quark>
On Fri, Feb 27, 2026 at 10:50:38PM -0800, Eric Biggers wrote:
> The alignment requirements on the vectors should be documented, as
> should which values of bytes are accepted. It looks like, at the very
> least, the vectors have to be 32-byte aligned and the length has to be a
> nonzero multiple of 512 bytes. But I didn't check every implementation.
That would match the original use case in the md raid code and also in
file system uses such as btrfs, yes.
^ permalink raw reply
* Re: [PATCH 20/25] xor: make xor.ko self-contained in lib/raid/
From: Christoph Hellwig @ 2026-03-03 16:06 UTC (permalink / raw)
To: Eric Biggers
Cc: Christoph Hellwig, Andrew Morton, Richard Henderson, Matt Turner,
Magnus Lindholm, Russell King, Catalin Marinas, Will Deacon,
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, David S. Miller, Andreas Larsson,
Richard Weinberger, Anton Ivanov, Johannes Berg, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Herbert Xu, Dan Williams, Chris Mason, David Sterba,
Arnd Bergmann, Song Liu, Yu Kuai, Li Nan, linux-alpha,
linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-um, linux-crypto,
linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260228064249.GG65277@quark>
On Fri, Feb 27, 2026 at 10:42:49PM -0800, Eric Biggers wrote:
> On Thu, Feb 26, 2026 at 07:10:32AM -0800, Christoph Hellwig wrote:
> > diff --git a/arch/um/include/asm/xor.h b/lib/raid/xor/um/xor_arch.h
> > similarity index 61%
> > rename from arch/um/include/asm/xor.h
> > rename to lib/raid/xor/um/xor_arch.h
> > index c9ddedc19301..c75cd9caf792 100644
> > --- a/arch/um/include/asm/xor.h
> > +++ b/lib/raid/xor/um/xor_arch.h
> > @@ -1,7 +1,4 @@
> > /* SPDX-License-Identifier: GPL-2.0 */
> > -#ifndef _ASM_UM_XOR_H
> > -#define _ASM_UM_XOR_H
> > -
> > #ifdef CONFIG_64BIT
> > #undef CONFIG_X86_32
> > #else
> > #define CONFIG_X86_32 1
> > #endif
>
> Due to this change, the above code that sets CONFIG_X86_32 to the
> opposite of CONFIG_64BIT is no longer included in xor-sse.c, which uses
> CONFIG_X86_32. So if the above code actually did anything, this change
> would have broken it for xor-sse.c. However, based on
> arch/x86/um/Kconfig, CONFIG_X86_32 is always the opposite of
> CONFIG_64BIT, so the above code actually has no effect. Does that sound
> right?
This whole thing looked weird to me. I'll try to do a more extensive
cleanup pass on the um mess ahead of the rest of the series.
^ permalink raw reply
* Re: [PATCH 18/25] x86: move the XOR code to lib/raid/
From: Christoph Hellwig @ 2026-03-03 16:05 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Eric Biggers, Christoph Hellwig, Andrew Morton, Richard Henderson,
Matt Turner, Magnus Lindholm, Russell King, Catalin Marinas,
Will Deacon, 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, David S. Miller,
Andreas Larsson, Richard Weinberger, Anton Ivanov, Johannes Berg,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Herbert Xu, Dan Williams, Chris Mason,
David Sterba, Arnd Bergmann, Song Liu, Yu Kuai, Li Nan,
linux-alpha, linux-kernel, linux-arm-kernel, loongarch,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-um,
linux-crypto, linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260228103117.GK1282955@noisy.programming.kicks-ass.net>
On Sat, Feb 28, 2026 at 11:31:17AM +0100, Peter Zijlstra wrote:
> Hmm, I suppose. Its just weird that we now have to look in both
> arch/$foo and lib/*/$foo/ to find all arch code.
We've had instance of that for a long time, e.g. lib/raid6/
> And I don't suppose symlinks would make it better?
Ugg, just purely from the organizaton perspective it would make
things a lot worse. I'm also not sure how well git copes with
symlinks.
^ 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