* [PATCH 5/8] raid6: enable lock context analysis
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
The code doesn't have any locking, so this is trivial.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/raid6/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/raid/raid6/Makefile b/lib/raid/raid6/Makefile
index 038d6c74d1ba..5cdb7223de2a 100644
--- a/lib/raid/raid6/Makefile
+++ b/lib/raid/raid6/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
ccflags-y += -I $(src)
ifeq ($(CONFIG_RAID6_PQ_ARCH),y)
--
2.53.0
^ permalink raw reply related
* [PATCH 4/8] xor/kunit: add a benchmark
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
Add a benchmark to test the XOR functions for more representative block
sizes and numbers of disks. Including 64k would be useful here, but
increasing the test buffer size increases the runtime of the functional
kunit test too much unfortunately.
The runtime numbers are reported in GB/s as the numbers of modern
implementations are basically unreadable as MB/s. This means
retro-architectures could report 0, but that is an easy tradeoff.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/Kconfig | 6 ++++
lib/raid/xor/tests/xor_kunit.c | 62 ++++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+)
diff --git a/lib/raid/Kconfig b/lib/raid/Kconfig
index 978cd6ba08ac..c4eeb7c716c8 100644
--- a/lib/raid/Kconfig
+++ b/lib/raid/Kconfig
@@ -29,6 +29,12 @@ config XOR_KUNIT_TEST
This is intended to help people writing architecture-specific
optimized versions. If unsure, say N.
+config XOR_BENCHMARK
+ bool "Benchmark for xor_gen"
+ depends on XOR_KUNIT_TEST
+ help
+ Include benchmarks in the KUnit test suite for xor_gen.
+
config RAID6_PQ
tristate
diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
index 659ae3edbc25..648c6da9464c 100644
--- a/lib/raid/xor/tests/xor_kunit.c
+++ b/lib/raid/xor/tests/xor_kunit.c
@@ -125,8 +125,70 @@ static void xor_test(struct kunit *test)
}
}
+static void xor_benchmark(struct kunit *test)
+{
+ static const unsigned int nr_to_test[] = {
+ 4, 5, 6, 7, 8, 10, 12, 15, 16, 32,
+ };
+ static const unsigned int len_to_test[] = {
+ SZ_4K, SZ_16K,
+ };
+ unsigned int i, j, l;
+ u64 t;
+
+ if (!IS_ENABLED(CONFIG_XOR_BENCHMARK))
+ kunit_skip(test, "not enabled");
+
+ /* warm-up */
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
+ for (l = 0; l < 10; l++) {
+ xor_gen(test_dest, test_buffers, nr_to_test[i],
+ len_to_test[j]);
+ }
+ }
+ }
+
+ /*
+ * Preferably this would be a loop over len_to_test, but the kunit
+ * logging always adds a newline to each logged format string.
+ */
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, " \t%5u bytes\t%5u bytes\n",
+ len_to_test[0], len_to_test[1]);
+
+ for (i = 0; i < ARRAY_SIZE(nr_to_test); i++) {
+ unsigned int nr = nr_to_test[i];
+ u64 speed[ARRAY_SIZE(len_to_test)];
+
+ KUNIT_ASSERT_LE(test, nr, XOR_KUNIT_MAX_BUFFERS);
+
+ for (j = 0; j < ARRAY_SIZE(len_to_test); j++) {
+ unsigned int len = len_to_test[j];
+ const unsigned long num_iters = 1000;
+
+ KUNIT_ASSERT_GT(test, len, 0);
+ KUNIT_ASSERT_LE(test, len, XOR_KUNIT_MAX_BYTES);
+
+ preempt_disable();
+ t = ktime_get_ns();
+ for (l = 0; l < num_iters; l++)
+ xor_gen(test_dest, test_buffers, nr, len);
+ t = ktime_get_ns() - t;
+ preempt_enable();
+
+ speed[j] = div64_u64((u64)len * num_iters * nr, t);
+ }
+
+ static_assert(ARRAY_SIZE(len_to_test) == 2);
+ kunit_info(test, "%3u disks:\t%5llu GB/s\t%5llu GB/s\n",
+ nr, speed[0], speed[1]);
+ }
+}
+
static struct kunit_case xor_test_cases[] = {
KUNIT_CASE(xor_test),
+ KUNIT_CASE(xor_benchmark),
{},
};
--
2.53.0
^ permalink raw reply related
* [PATCH 3/8] xor/kunit: fix a spelling error
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/tests/xor_kunit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/raid/xor/tests/xor_kunit.c b/lib/raid/xor/tests/xor_kunit.c
index 0c2a3a420bf9..659ae3edbc25 100644
--- a/lib/raid/xor/tests/xor_kunit.c
+++ b/lib/raid/xor/tests/xor_kunit.c
@@ -85,7 +85,7 @@ static void xor_test(struct kunit *test)
xor_generate_random_data();
/*
- * If we're not using the entire buffer size, inject randomize
+ * If we're not using the entire buffer size, inject randomized
* alignment into the buffer.
*/
max_alignment = XOR_KUNIT_MAX_BYTES - len;
--
2.53.0
^ permalink raw reply related
* [PATCH 2/8] xor: improve the runtime selection benchmark
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
Use plain ktime_get_ns for the timing, use 4 + 1 disks for a realistic
load, and report the throughput on the data disks instead of the that
on the parity disk, which isn't all that useful.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/xor-core.c | 55 +++++++++++++++++++----------------------
1 file changed, 26 insertions(+), 29 deletions(-)
diff --git a/lib/raid/xor/xor-core.c b/lib/raid/xor/xor-core.c
index 50931fbf0324..a890cec21bbc 100644
--- a/lib/raid/xor/xor-core.c
+++ b/lib/raid/xor/xor-core.c
@@ -10,7 +10,6 @@
#include <linux/gfp.h>
#include <linux/slab.h>
#include <linux/raid/xor.h>
-#include <linux/jiffies.h>
#include <linux/preempt.h>
#include <linux/static_call.h>
#include "xor_impl.h"
@@ -73,59 +72,56 @@ void __init xor_force(struct xor_block_template *tmpl)
forced_template = tmpl;
}
-#define BENCH_SIZE 4096
+#define BENCH_SIZE SZ_4K
+#define NR_SRCS 4
#define REPS 800U
-static void __init
-do_xor_speed(struct xor_block_template *tmpl, void *b1, void *b2)
+static void __init do_xor_speed(struct xor_block_template *tmpl, void *dest,
+ void *srcs[NR_SRCS])
{
- int speed;
- unsigned long reps;
- ktime_t min, start, t0;
- void *srcs[1] = { b2 };
+ u64 t;
+ int i;
preempt_disable();
-
- reps = 0;
- t0 = ktime_get();
- /* delay start until time has advanced */
- while ((start = ktime_get()) == t0)
- cpu_relax();
- do {
+ t = ktime_get_ns();
+ for (i = 0; i < REPS; i++) {
mb(); /* prevent loop optimization */
- tmpl->xor_gen(b1, srcs, 1, BENCH_SIZE);
+ tmpl->xor_gen(dest, srcs, NR_SRCS, BENCH_SIZE);
mb();
- } while (reps++ < REPS || (t0 = ktime_get()) == start);
- min = ktime_sub(t0, start);
-
+ }
+ t = ktime_get_ns() - t;
preempt_enable();
- // bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s]
- speed = (1000 * reps * BENCH_SIZE) / (unsigned int)ktime_to_ns(min);
- tmpl->speed = speed;
+ /* bytes/ns == GB/s, multiply by 1000 to get MB/s [not MiB/s] */
+ tmpl->speed = div64_u64((u64)BENCH_SIZE * REPS * NR_SRCS * 1000, t);
- pr_info(" %-16s: %5d MB/sec\n", tmpl->name, speed);
+ pr_info(" %-16s: %5d MB/sec\n", tmpl->name, tmpl->speed);
}
static int __init calibrate_xor_blocks(void)
{
- void *b1, *b2;
struct xor_block_template *f, *fastest;
+ void *srcs[NR_SRCS];
+ void *buf, *dest;
+ int i;
if (forced_template)
return 0;
- b1 = kmalloc(PAGE_SIZE * 4, GFP_KERNEL);
- if (!b1) {
+ buf = kmalloc(BENCH_SIZE * (NR_SRCS + 1), GFP_KERNEL);
+ if (!buf) {
pr_warn("xor: Yikes! No memory available.\n");
return -ENOMEM;
}
- b2 = b1 + 2*PAGE_SIZE + BENCH_SIZE;
+ get_random_bytes(buf, BENCH_SIZE * (NR_SRCS + 1));
+ dest = buf;
+ for (i = 0; i < NR_SRCS; i++)
+ srcs[i] = buf + (i + 1) * BENCH_SIZE;
pr_info("xor: measuring software checksum speed\n");
fastest = template_list;
for (f = template_list; f; f = f->next) {
- do_xor_speed(f, b1, b2);
+ do_xor_speed(f, dest, srcs);
if (f->speed > fastest->speed)
fastest = f;
}
@@ -133,9 +129,10 @@ static int __init calibrate_xor_blocks(void)
pr_info("xor: using function: %s (%d MB/sec)\n",
fastest->name, fastest->speed);
- kfree(b1);
+ kfree(buf);
return 0;
}
+#undef NR_SRCS
#ifdef CONFIG_XOR_BLOCKS_ARCH
#include "xor_arch.h" /* $SRCARCH/xor_arch.h */
--
2.53.0
^ permalink raw reply related
* [PATCH 1/8] xor: enable lock context analysis
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
In-Reply-To: <20260708090614.1431014-1-hch@lst.de>
The code doesn't have any locking, so this is trivial.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/raid/xor/Makefile | 2 ++
lib/raid/xor/tests/Makefile | 2 ++
2 files changed, 4 insertions(+)
diff --git a/lib/raid/xor/Makefile b/lib/raid/xor/Makefile
index e8ecec3c09f9..9b0fad459cdb 100644
--- a/lib/raid/xor/Makefile
+++ b/lib/raid/xor/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
ccflags-y += -I $(src)
obj-$(CONFIG_XOR_BLOCKS) += xor.o
diff --git a/lib/raid/xor/tests/Makefile b/lib/raid/xor/tests/Makefile
index 661e8f6ffd1f..1cce833cd7fd 100644
--- a/lib/raid/xor/tests/Makefile
+++ b/lib/raid/xor/tests/Makefile
@@ -1,3 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
+CONTEXT_ANALYSIS := y
+
obj-$(CONFIG_XOR_KUNIT_TEST) += xor_kunit.o
--
2.53.0
^ permalink raw reply related
* misc lib/raid/ improvements
From: Christoph Hellwig @ 2026-07-08 9:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Eric Biggers, linux-raid, linux-kernel
Hi all,
this series improves the benchmark-based algorithm selection and
adds kunit benchmarks for both the XOR and raid6 libraries, and tidies
up a few very minor other bits.
diffstat:
lib/raid/Kconfig | 15 ++++++
lib/raid/raid6/Makefile | 2
lib/raid/raid6/algos.c | 112 +++++++++++++++++++++++++---------------------
lib/raid/raid6/tests/raid6_kunit.c | 66 ++++++++++++++++++++++++++-
lib/raid/xor/Makefile | 2
lib/raid/xor/tests/Makefile | 2
lib/raid/xor/tests/xor_kunit.c | 64 +++++++++++++++++++++++++-
lib/raid/xor/xor-core.c | 55 ++++++++++------------
8 files changed, 236 insertions(+), 82 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: Abd-Alrhman Masalkhi @ 2026-07-08 9:05 UTC (permalink / raw)
To: John Garry, song, yukuai, magiclinan, xiao, axboe, vverma,
martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <fbf1a5d6-d362-4eeb-8434-2f4a114a70bd@oracle.com>
Hi John,
On Tue, Jul 07, 2026 at 17:20 +0100, John Garry wrote:
> On 06/07/2026 12:35, Abd-Alrhman Masalkhi wrote:
>>> well it seems to be that everything in the driver is split over
>>> BARRIER_UNIT_SECTOR_SIZE, so is in effect a chunk size.
>>>
>>> Note that atomic_write_hw_unit_max is going to be small always compared
>>> to BARRIER_UNIT_SECTOR_SIZE.
>>>
>>> However, can you check the blk stacking code to make sure that this does
>>> as you want? As I remember, for stacking we take the atomic write limits
>>> of the first bottom device and then stack the other bottom devices and I
>>> don't think that setting atomic_write_hw_unit_max in this way has an
>>> impact - see blk_stack_atomic_writes_limits()
>>>
>> I checked blk_stack_atomic_writes_limits(), and you are right: setting
>> atomic_write_hw_unit_max here alone does not have the intended effect on
>> the final stacked device.
>>
>> Also, since atomic_write_hw_unit_max is expected to always be smaller
>> than BARRIER_UNIT_SECTOR_SIZE,
>
> Yes, always expected to be much smaller.
>
>> it seems that there is no need to set an
>> additional atomic write limit in raid1, or even to set
>> atomic_write_hw_unit_max at all. Is that what you mean?
>
> Even though we expect it to be much smaller, it's good practice to
> ensure this.
>
> One method to do so - which I already mentioned - was to set the chunk
> size to BARRIER_UNIT_SECTOR_SIZE. You were not keen on that.
>
I see, thanks for the clarification. My concern was mainly conceptual,
and I was also worried that setting the chunk size might affect other
operations, such as discard. Since that is not the case, I'll set the
chunk size to BARRIER_UNIT_SECTOR_SIZE and update it accordingly in v2.
> Another method is to set max hw sectors for the RAID1 device to
> BARRIER_UNIT_SECTOR_SIZE.
>
> Thanks,
> John
>
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: Abd-Alrhman Masalkhi @ 2026-07-08 8:59 UTC (permalink / raw)
To: yu kuai, yu kuai, song, magiclinan, xiao
Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <f30acc2d-9153-4a9c-b6e5-79083bd9fedb@fygo.io>
Hi Kuai,
On Wed, Jul 08, 2026 at 16:32 +0800, yu kuai wrote:
> Hi,
>
> 在 2026/7/7 18:35, Abd-Alrhman Masalkhi 写道:
>> Hi Kuai,
>>
>> On Tue, Jul 07, 2026 at 09:12 +0800, yu kuai wrote:
>>> Hi,
>>>
>>> 在 2026/7/6 3:58, Abd-Alrhman Masalkhi 写道:
>>>>> The problem looks real, however, I think this will cause a change that user will be awared,
>>>>> if there are really spares that can be removed from conf, but array is not suspended here,
>>>>> user will still expect rdev will be removed from conf automatically.
>>>>>
>>>>> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is
>>>>> supposed to be true, we can release the lock and retry with suspend = true.
>>>>>
>>>> Yes, I see, and your approach is much better. But what do you think
>>>> about taking the lock first and then checking only once?
>>> I don't get what you mean. If we take the lock and then check that array should
>>> suspend, we still have to release the lock before we suspend the array.
>>>
>> Sorry, I was not clear. I meant, do we need to check twice, once before
>> taking the lock and once after? It seems that the check before taking the
>> lock is redundant. since the result would need to be checked again after
>> taking the lock anyway.
>>
>> Could we drop the check before taking the lock and only check whether
>> suspension is needed once while holding it? If suspension is needed, we
>> would release the lock, suspend the array, and then reacquire the lock.
>
> Thanks for the explanation, I understand now. Howerver, I still prefer to check
> first before holding the lock. Because the checking is much lower overhead than
> acquire reconfig_mutex, and the race window that rdev become spare is small, so
> it's unlikely we'll acquire reconfig_mtuex twice.
>
I see, thanks for the explanation. I'll send a v2 shortly.
>>
>>> --
>>> Thanks,
>>> Kuai
>
> --
> Thanks,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* [PATCH] md/raid10: free r10_bio on write NOWAIT exits
From: Guangshuo Li @ 2026-07-08 8:47 UTC (permalink / raw)
To: Song Liu, Yu Kuai, Li Nan, Xiao Ni, Josh Hunt, linux-raid,
linux-kernel
Cc: Guangshuo Li
The change referenced by the Fixes tag made a NOWAIT early exit in
raid10_write_request() free the r10_bio directly instead of using
raid_end_bio_io(), avoiding an unmatched allow_barrier() while still
returning the r10_bio object to the mempool.
The same function has two other NOWAIT early exits. One is before
regular_request_wait(), where the request has not taken a barrier
reference. The other is in the reshape metadata path, where the barrier
reference is released explicitly with allow_barrier(). Both paths end the
bio and return false, but neither returns the r10_bio object to the
mempool.
Free the r10_bio on both NOWAIT exits. Keep the existing barrier handling
unchanged, since only the reshape path has taken a barrier reference.
Fixes: 7d96f3120a7f ("md/raid10: fix deadlock with check operation and nowait requests")
Signed-off-by: Guangshuo Li <lgs201920130244@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..d480344c5b8f 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1364,6 +1364,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
DEFINE_WAIT(w);
/* Bail out if REQ_NOWAIT is set for the bio */
if (bio->bi_opf & REQ_NOWAIT) {
+ free_r10bio(r10_bio);
bio_wouldblock_error(bio);
return false;
}
@@ -1397,6 +1398,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
md_wakeup_thread(mddev->thread);
if (bio->bi_opf & REQ_NOWAIT) {
allow_barrier(conf);
+ free_r10bio(r10_bio);
bio_wouldblock_error(bio);
return false;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: yu kuai @ 2026-07-08 8:32 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, yu kuai, song, magiclinan, xiao
Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <m2mrw36qap.fsf@gmail.com>
Hi,
在 2026/7/7 18:35, Abd-Alrhman Masalkhi 写道:
> Hi Kuai,
>
> On Tue, Jul 07, 2026 at 09:12 +0800, yu kuai wrote:
>> Hi,
>>
>> 在 2026/7/6 3:58, Abd-Alrhman Masalkhi 写道:
>>>> The problem looks real, however, I think this will cause a change that user will be awared,
>>>> if there are really spares that can be removed from conf, but array is not suspended here,
>>>> user will still expect rdev will be removed from conf automatically.
>>>>
>>>> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is
>>>> supposed to be true, we can release the lock and retry with suspend = true.
>>>>
>>> Yes, I see, and your approach is much better. But what do you think
>>> about taking the lock first and then checking only once?
>> I don't get what you mean. If we take the lock and then check that array should
>> suspend, we still have to release the lock before we suspend the array.
>>
> Sorry, I was not clear. I meant, do we need to check twice, once before
> taking the lock and once after? It seems that the check before taking the
> lock is redundant. since the result would need to be checked again after
> taking the lock anyway.
>
> Could we drop the check before taking the lock and only check whether
> suspension is needed once while holding it? If suspension is needed, we
> would release the lock, suspend the array, and then reacquire the lock.
Thanks for the explanation, I understand now. Howerver, I still prefer to check
first before holding the lock. Because the checking is much lower overhead than
acquire reconfig_mutex, and the race window that rdev become spare is small, so
it's unlikely we'll acquire reconfig_mtuex twice.
>
>> --
>> Thanks,
>> Kuai
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: John Garry @ 2026-07-07 16:20 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, axboe,
vverma, martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <m2pl1073l5.fsf@gmail.com>
On 06/07/2026 12:35, Abd-Alrhman Masalkhi wrote:
>> well it seems to be that everything in the driver is split over
>> BARRIER_UNIT_SECTOR_SIZE, so is in effect a chunk size.
>>
>> Note that atomic_write_hw_unit_max is going to be small always compared
>> to BARRIER_UNIT_SECTOR_SIZE.
>>
>> However, can you check the blk stacking code to make sure that this does
>> as you want? As I remember, for stacking we take the atomic write limits
>> of the first bottom device and then stack the other bottom devices and I
>> don't think that setting atomic_write_hw_unit_max in this way has an
>> impact - see blk_stack_atomic_writes_limits()
>>
> I checked blk_stack_atomic_writes_limits(), and you are right: setting
> atomic_write_hw_unit_max here alone does not have the intended effect on
> the final stacked device.
>
> Also, since atomic_write_hw_unit_max is expected to always be smaller
> than BARRIER_UNIT_SECTOR_SIZE,
Yes, always expected to be much smaller.
> it seems that there is no need to set an
> additional atomic write limit in raid1, or even to set
> atomic_write_hw_unit_max at all. Is that what you mean?
Even though we expect it to be much smaller, it's good practice to
ensure this.
One method to do so - which I already mentioned - was to set the chunk
size to BARRIER_UNIT_SECTOR_SIZE. You were not keen on that.
Another method is to set max hw sectors for the RAID1 device to
BARRIER_UNIT_SECTOR_SIZE.
Thanks,
John
^ permalink raw reply
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: Abd-Alrhman Masalkhi @ 2026-07-07 10:35 UTC (permalink / raw)
To: yu kuai, yu kuai, song, magiclinan, xiao
Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <4f0b2516-65bd-41fa-abc3-3cd76951ec46@fygo.io>
Hi Kuai,
On Tue, Jul 07, 2026 at 09:12 +0800, yu kuai wrote:
> Hi,
>
> 在 2026/7/6 3:58, Abd-Alrhman Masalkhi 写道:
>>> The problem looks real, however, I think this will cause a change that user will be awared,
>>> if there are really spares that can be removed from conf, but array is not suspended here,
>>> user will still expect rdev will be removed from conf automatically.
>>>
>>> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is
>>> supposed to be true, we can release the lock and retry with suspend = true.
>>>
>> Yes, I see, and your approach is much better. But what do you think
>> about taking the lock first and then checking only once?
>
> I don't get what you mean. If we take the lock and then check that array should
> suspend, we still have to release the lock before we suspend the array.
>
Sorry, I was not clear. I meant, do we need to check twice, once before
taking the lock and once after? It seems that the check before taking the
lock is redundant. since the result would need to be checked again after
taking the lock anyway.
Could we drop the check before taking the lock and only check whether
suspension is needed once while holding it? If suspension is needed, we
would release the lock, suspend the array, and then reacquire the lock.
> --
> Thanks,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: yu kuai @ 2026-07-07 1:12 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, yu kuai, song, magiclinan, xiao
Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <m2se5x6wff.fsf@gmail.com>
Hi,
在 2026/7/6 3:58, Abd-Alrhman Masalkhi 写道:
>> The problem looks real, however, I think this will cause a change that user will be awared,
>> if there are really spares that can be removed from conf, but array is not suspended here,
>> user will still expect rdev will be removed from conf automatically.
>>
>> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is
>> supposed to be true, we can release the lock and retry with suspend = true.
>>
> Yes, I see, and your approach is much better. But what do you think
> about taking the lock first and then checking only once?
I don't get what you mean. If we take the lock and then check that array should
suspend, we still have to release the lock before we suspend the array.
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: Abd-Alrhman Masalkhi @ 2026-07-06 11:35 UTC (permalink / raw)
To: John Garry, song, yukuai, magiclinan, xiao, axboe, vverma,
martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <856abedc-d2f3-4248-9583-4d7e1a11abc1@oracle.com>
On Fri, Jul 03, 2026 at 09:28 +0100, John Garry wrote:
> On 30/06/2026 09:39, Abd-Alrhman Masalkhi wrote:
>>
>> Hi John,
>>
>> On Mon, Jun 29, 2026 at 15:48 +0100, John Garry wrote:
>>> On 28/06/2026 15:24, Abd-Alrhman Masalkhi wrote:
>>>> 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://urldefense.com/v3/__https://lore.kernel.org/linux-raid/20260623072456.333437-3-abd.masalkhi@gmail.com/__;!!ACWV5N9M2RV99hQ!LbMSGSClRi0PNBqQti5ZNWGDVjDd34-7saYEAwNyBNjpNTjEA7veqM5RHG8KB1QiscarW4UaIefjm19ywSImtIgh$
>>>> ---
>>>> 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))
>>>
>>> where does BIO_MAX_VECS * (PAGE_SIZE >> 9) even come from?
>>>
>>
>> BIO_MAX_VECS * (PAGE_SIZE >> 9) defines the maximum size supported by
>> write-behind. The write-behind copy (alloc_behind_master_bio) uses a
>> single bio, which can hold at most BIO_MAX_VECS pages, making this the
>> largest payload it can carry. With a 4 KiB PAGE_SIZE, that corresponds
>> to 256 pages, or 1 MiB (2048 sectors).
>> This patch changes the behavior for atomic writes that exceed this
>> limit. Instead of failing the write with -EIO when the number of sectors
>> must be reduced, it disables write-behind and proceeds with the atomic
>> write.
>>
>>>> + /*
>>>> + * Atomic writes cannot be split, so disable
>>>> + * write-behind.
>>>> + */
>>>> + write_behind = false;
>
> It's a bit poor to have write_behind initially = false, then allow it to
> be set = true, and then later be set = false. Can this be improved?
>
Yes, I agree that setting write_behind to true and then resetting it to
false later is not ideal. I will rework this.
>>>> + 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;
>>>
>>> This BARRIER_UNIT_SECTOR_SIZE is a bit like chunk sectors, no? I am just
>>> wondering if we just should set it to chunk sectors =
>>> BARRIER_UNIT_SECTOR_SIZE
>>>
>>> I assume that it affects more than Reads and writes, e.g. discard also.
>>>
>>
>> BARRIER_UNIT_SECTOR_SIZE is the resync barrier-bucket, not the layout
>> chunk size. Unless I'm missing something, using
>> atomic_write_hw_unit_max seems more appropriate than using the chunk
>> size. That way, the limit only applies to atomic writes instead of
>> affecting other operations such.
>
> well it seems to be that everything in the driver is split over
> BARRIER_UNIT_SECTOR_SIZE, so is in effect a chunk size.
>
> Note that atomic_write_hw_unit_max is going to be small always compared
> to BARRIER_UNIT_SECTOR_SIZE.
>
> However, can you check the blk stacking code to make sure that this does
> as you want? As I remember, for stacking we take the atomic write limits
> of the first bottom device and then stack the other bottom devices and I
> don't think that setting atomic_write_hw_unit_max in this way has an
> impact - see blk_stack_atomic_writes_limits()
>
I checked blk_stack_atomic_writes_limits(), and you are right: setting
atomic_write_hw_unit_max here alone does not have the intended effect on
the final stacked device.
Also, since atomic_write_hw_unit_max is expected to always be smaller
than BARRIER_UNIT_SECTOR_SIZE, it seems that there is no need to set an
additional atomic write limit in raid1, or even to set
atomic_write_hw_unit_max at all. Is that what you mean?
>>
>>>> lim.features |= BLK_FEAT_ATOMIC_WRITES;
>>>> lim.features |= BLK_FEAT_PCI_P2PDMA;
>>>> err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
>>>
>>
>
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH 0/8] md/raid5: scalability and rebuild-path improvements
From: yu kuai @ 2026-07-06 2:56 UTC (permalink / raw)
To: Hiroshi Nishida, Song Liu
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, yukuai
In-Reply-To: <20260624155452.211646-1-nishidafmly@gmail.com>
Hi,
在 2026/6/24 23:54, Hiroshi Nishida 写道:
> This series collects small, individually low-risk md/raid5 changes for
> large, many-core, many-disk arrays. Their common theme is reducing
> per-stripe and stripe-cache contention, so the benefit appears mainly
> when the raid5 stripe-handling worker threads are in use
> (group_thread_cnt > 0); at the default group_thread_cnt = 0 (a single
> handling thread) the series is essentially neutral.
>
> - patches 1-3 remove signed arithmetic from a hot-path divisor, lift an
> arbitrary stripe-cache size cap, and widen a badblock length argument
> that currently truncates large ranges;
> - patch 4 raises NR_STRIPE_HASH_LOCKS (8 -> 32) to spread stripe-hash
> contention on high core-count systems;
> - patches 5 and 8 reduce per-stripe overhead in the resync/recovery
> path and bound the share of the stripe cache a rebuild may hold while
> user I/O is competing;
> - patch 6 allocates each worker group's array on its own NUMA node;
> - patch 7 raises MAX_STRIPE_BATCH (8 -> 32).
>
> Measured effect, treatment vs baseline, % change in mean IOPS (N=3),
> swept over group_thread_cnt (RAID6 4+2, 22-core host, ramdisk members):
Testing with ramdisk does serve as a useful reference, but it does not reflect
real world usage.
>
> workload gtc=0 gtc=2 gtc=4 gtc=8
> random 4K write (RMW) +4.2% +8.1% +17.4% +6.5%
> DB mixed 75/25 8K +0.4% +4.2% +10.3% +4.7%
> high-concurrency 70/30 4K +3.9% +1.2% +10.0% +0.2%
> OLTP 70/30 16K -0.3% +4.7% +10.1% +9.3%
> partial-stripe write 8K +1.1% +4.8% +11.2% +14.2%
With a quick review I saw many static configurations is changed, I agree
these changes can improve arrays with ssd/nvme and a system with large
memory available. However, we already tested with hdd and about 8G memory
available, these changes will not improve performance at all, with the
extra memory overhead.
I can accept make those values configurable, but not direct modifications.
As validation is required for numerous scenarios. Memory resources are precious
especially for most consumer NAS devices.
>
> At the default single handling thread (group_thread_cnt = 0) the series is
> neutral (no regression). As worker threads are added the gain grows,
> peaking broadly around group_thread_cnt = 4 at roughly +10-17% across the
> whole mix; at gtc = 8 the write-heavy workloads keep gaining while the
> read-heavy high-concurrency case has saturated. (Per-run cv was <1%
> except the random-write test, ~5-9%, from a cold first run.)
>
> These numbers are on a ramdisk, which removes device latency and so
> overstates the CPU-side contention effect relative to a real device;
> they show the direction and the group_thread_cnt dependence, not an
> absolute speedup. The stripe-hash/batch patches (4, 7) and the cache cap
> (2) drive this; patch 6 only matters on multi-socket systems (not
> exercised above) and patches 5/8 act on the resync/recovery path rather
> than this steady-state workload.
>
> Reproduction (stock mdadm + fio):
> mdadm --create /dev/md0 --level=6 --raid-devices=6 --chunk=512 \
> --assume-clean <6 members>
> echo 16384 > /sys/block/md0/md/stripe_cache_size
> echo N > /sys/block/md0/md/group_thread_cnt # N = 0,2,4,8
> fio --filename=/dev/md0 --direct=1 --ioengine=libaio --group_reporting \
> --time_based --runtime=15 --name=w <per-workload opts>:
> random write : --rw=randwrite --bs=4k --numjobs=4 --iodepth=32
> DB mixed : --rw=randrw --rwmixread=75 --bs=8k --numjobs=8 --iodepth=16
> high-concur. : --rw=randrw --rwmixread=70 --bs=4k --numjobs=16 --iodepth=8
> OLTP : --rw=randrw --rwmixread=70 --bs=16k --numjobs=6 --iodepth=16
> partial-stripe : --rw=randwrite --bs=8k --numjobs=4 --iodepth=32
>
> Each patch stands on its own; I am happy to drop or defer any that is not
> justified on its own merit.
>
> Functional testing on RAID5 and RAID6: create, fail a member, rebuild
> onto a spare / re-add, full data read-back verified, and scrub
> ("check") reporting mismatch_cnt == 0. The series was also exercised
> with KASAN and lockdep enabled -- including heavy group_thread_cnt
> churn on a multi-node setup to stress the per-NUMA-node worker
> allocation and the raid5_quiesce hash-lock-all path -- with no reports.
>
> Hiroshi Nishida (8):
> md: change chunk_sectors and stripe cache counts to unsigned int
> md/raid5: raise stripe cache limit from 32768 to 262144
> md: widen badblock sectors param from int to sector_t
> md/raid5: raise NR_STRIPE_HASH_LOCKS from 8 to 32
> md/raid5: submit a window of stripes during resync/recovery
> md/raid5: allocate worker groups per NUMA node
> md/raid5: raise MAX_STRIPE_BATCH from 8 to 32
> md/raid5: reserve stripe cache for user I/O during rebuild
>
> drivers/md/md.c | 4 +-
> drivers/md/md.h | 10 ++--
> drivers/md/raid5.c | 129 ++++++++++++++++++++++++++++++++-------------
> drivers/md/raid5.h | 33 ++++++++----
> 4 files changed, 121 insertions(+), 55 deletions(-)
>
> base-commit: 55b77337bdd088c77461588e5ec094421b89911b
>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: Abd-Alrhman Masalkhi @ 2026-07-05 19:58 UTC (permalink / raw)
To: yu kuai, song, magiclinan, xiao, yukuai
Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <0a7dc7e2-30af-44de-8c29-b6e27a62fa55@fygo.io>
Hi Kuai,
On Mon, Jul 06, 2026 at 00:16 +0800, yu kuai wrote:
> Hi,
>
> 在 2026/6/30 15:56, Abd-Alrhman Masalkhi 写道:
>> remove_spares() and remove_and_add_spares() modify the array's rdev
>> configuration. These operations are only safe after the array has been
>> suspended.
>>
>> Today, md_start_sync() can call md_choose_sync_action() even when the
>> array has not been suspended. As a result, md_choose_sync_action() may
>> remove or replace rdevs while normal I/O is still accessing them.
>>
>> The race can occur as follows:
>>
>> raid10d Worker Normal IO
>> ____________ _______________________ ______________________
>>
>> raid10_write_request()
>> wait_blocked_dev()
>> set Blocked
>> set Faulty
>> Skip Faulty rdev
>> rrdev->nr_pending++
>> .repl_bio = bio
>> removeable_rdev = false .
>> array not suspended .
>> lock mddev goto err_handle
>> lock mddev (wait)
>> .
>> update sb .
>> clear Blocked .
>> .
>> unlock mddev .
>> lock mddev (acquires)
>> remove_spares()
>> removeable_rdev = true
>>
>> raid10_remove_disk()
>> rdev = replacement
>> replacement = NULL
>> rdev_dec_pending(NULL)
>> unlock mddev (NULL)->nr_pending--
>>
>> In this case, rdev_dec_pending() is called with a NULL pointer,
>> resulting in a NULL pointer dereference when attempting to decrement
>> nr_pending.
>>
>> Fix this by ensuring that remove_spares() and remove_and_add_spares()
>> are only called after the array has been suspended, preventing
>> concurrent rdev configuration changes while normal I/O is in progress.
>>
>> Fixes: bc08041b32ab ("md: suspend array in md_start_sync() if array need reconfiguration")
>> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
>> Closes: https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3
>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>> ---
>> drivers/md/md.c | 18 +++++++++++++-----
>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 66a41d482e59..c85ebb59535b 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -10116,7 +10116,8 @@ static int remove_and_add_spares(struct mddev *mddev,
>> return spares;
>> }
>>
>> -static bool md_choose_sync_action(struct mddev *mddev, int *spares)
>> +static bool md_choose_sync_action(struct mddev *mddev, int *spares,
>> + bool array_suspended)
>> {
>> /* Check if reshape is in progress first. */
>> if (mddev->reshape_position != MaxSector) {
>> @@ -10132,7 +10133,9 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
>>
>> /* Check if resync is in progress. */
>> if (mddev->resync_offset < MaxSector) {
>> - remove_spares(mddev, NULL);
>> + if (array_suspended)
>> + remove_spares(mddev, NULL);
>
> The problem looks real, however, I think this will cause a change that user will be awared,
> if there are really spares that can be removed from conf, but array is not suspended here,
> user will still expect rdev will be removed from conf automatically.
>
> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is
> supposed to be true, we can release the lock and retry with suspend = true.
>
Yes, I see, and your approach is much better. But what do you think
about taking the lock first and then checking only once?
>> +
>> set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
>> clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
>> clear_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery);
>> @@ -10144,7 +10147,11 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
>> * also removed and re-added, to allow the personality to fail the
>> * re-add.
>> */
>> - *spares = remove_and_add_spares(mddev, NULL);
>> + if (array_suspended)
>> + *spares = remove_and_add_spares(mddev, NULL);
>> + else
>> + *spares = 0;
>> +
>> if (*spares || test_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery)) {
>> clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
>> clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
>> @@ -10189,11 +10196,12 @@ static void md_start_sync(struct work_struct *ws)
>> * As we only add devices that are already in-sync, we can
>> * activate the spares immediately.
>> */
>> - remove_and_add_spares(mddev, NULL);
>> + if (suspend)
>> + remove_and_add_spares(mddev, NULL);
>> goto not_running;
>> }
>>
>> - if (!md_choose_sync_action(mddev, &spares))
>> + if (!md_choose_sync_action(mddev, &spares, suspend))
>> goto not_running;
>>
>> if (!mddev->pers->sync_request)
>
> --
> Thanks,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH] md: avoid modifying spares while the array is not suspended
From: yu kuai @ 2026-07-05 16:16 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, magiclinan, xiao, yukuai
Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <20260630075640.1081634-1-abd.masalkhi@gmail.com>
Hi,
在 2026/6/30 15:56, Abd-Alrhman Masalkhi 写道:
> remove_spares() and remove_and_add_spares() modify the array's rdev
> configuration. These operations are only safe after the array has been
> suspended.
>
> Today, md_start_sync() can call md_choose_sync_action() even when the
> array has not been suspended. As a result, md_choose_sync_action() may
> remove or replace rdevs while normal I/O is still accessing them.
>
> The race can occur as follows:
>
> raid10d Worker Normal IO
> ____________ _______________________ ______________________
>
> raid10_write_request()
> wait_blocked_dev()
> set Blocked
> set Faulty
> Skip Faulty rdev
> rrdev->nr_pending++
> .repl_bio = bio
> removeable_rdev = false .
> array not suspended .
> lock mddev goto err_handle
> lock mddev (wait)
> .
> update sb .
> clear Blocked .
> .
> unlock mddev .
> lock mddev (acquires)
> remove_spares()
> removeable_rdev = true
>
> raid10_remove_disk()
> rdev = replacement
> replacement = NULL
> rdev_dec_pending(NULL)
> unlock mddev (NULL)->nr_pending--
>
> In this case, rdev_dec_pending() is called with a NULL pointer,
> resulting in a NULL pointer dereference when attempting to decrement
> nr_pending.
>
> Fix this by ensuring that remove_spares() and remove_and_add_spares()
> are only called after the array has been suspended, preventing
> concurrent rdev configuration changes while normal I/O is in progress.
>
> Fixes: bc08041b32ab ("md: suspend array in md_start_sync() if array need reconfiguration")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
> drivers/md/md.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 66a41d482e59..c85ebb59535b 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -10116,7 +10116,8 @@ static int remove_and_add_spares(struct mddev *mddev,
> return spares;
> }
>
> -static bool md_choose_sync_action(struct mddev *mddev, int *spares)
> +static bool md_choose_sync_action(struct mddev *mddev, int *spares,
> + bool array_suspended)
> {
> /* Check if reshape is in progress first. */
> if (mddev->reshape_position != MaxSector) {
> @@ -10132,7 +10133,9 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
>
> /* Check if resync is in progress. */
> if (mddev->resync_offset < MaxSector) {
> - remove_spares(mddev, NULL);
> + if (array_suspended)
> + remove_spares(mddev, NULL);
The problem looks real, however, I think this will cause a change that user will be awared,
if there are really spares that can be removed from conf, but array is not suspended here,
user will still expect rdev will be removed from conf automatically.
In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is
supposed to be true, we can release the lock and retry with suspend = true.
> +
> set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
> clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
> clear_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery);
> @@ -10144,7 +10147,11 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
> * also removed and re-added, to allow the personality to fail the
> * re-add.
> */
> - *spares = remove_and_add_spares(mddev, NULL);
> + if (array_suspended)
> + *spares = remove_and_add_spares(mddev, NULL);
> + else
> + *spares = 0;
> +
> if (*spares || test_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery)) {
> clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
> clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
> @@ -10189,11 +10196,12 @@ static void md_start_sync(struct work_struct *ws)
> * As we only add devices that are already in-sync, we can
> * activate the spares immediately.
> */
> - remove_and_add_spares(mddev, NULL);
> + if (suspend)
> + remove_and_add_spares(mddev, NULL);
> goto not_running;
> }
>
> - if (!md_choose_sync_action(mddev, &spares))
> + if (!md_choose_sync_action(mddev, &spares, suspend))
> goto not_running;
>
> if (!mddev->pers->sync_request)
--
Thanks,
Kuai
^ permalink raw reply
* Re: [RFC PATCH] md,dm: caller-owned memalloc_noio token in mddev_suspend()/resume()
From: yu kuai @ 2026-07-05 13:10 UTC (permalink / raw)
To: Chen Cheng, linux-raid, dm-devel, yukuai; +Cc: linux-kernel
In-Reply-To: <20260624101353.2678126-1-chencheng@fnnas.com>
Hi,
在 2026/6/24 18:13, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> Save token as mddev-scoped in mddev->noio_flags cause PF_MEMALLOC_NOIO
> leak into task A, while task B restores a token that it never saved.
>
> scenario:
>
> task A mddev task B
> ====== ======= ============
> write suspend_lo
> mddev_suspend()
> suspended == 0
> drain active_io
> suspended = 1
> A: noio_A = memalloc_noio_save()
> A returns with PF_MEMALLOC_NOIO set
>
> write suspend_hi
> mddev_suspend()
> suspended == 1
> suspended = 2
> B returns
> (no save)
>
> mddev_resume()
> suspended = 1
> not last resume
> A returns
> A still has PF_MEMALLOC_NOIO <-- leaked
>
> mddev_resume()
> suspended = 0
> memalloc_noio_restore(noio_A)
> (restores A's token in B)
>
> Fixed by:
> - return each caller's noio_flags from mddev_suspend()
> - pass that token back into mddev_resume()
> - update the suspend-and-lock helpers to carry the token
> - store the token in struct raid_set for dm-raid paths where suspend
> and resume are paired across callbacks
>
> Validation:
> repeatedly updates the array's suspend_lo and suspend_hi sysfs from many
> concurrent userspace workers. That makes multiple tasks to call
> mddev_suspend()/mddev_resume() concurrently.
>
> Each worker:
> - reads its initial /proc/self/stat flags and verifies that PF_MEMALLOC_NOIO is not already
> set
> - writes 0 to either suspend_lo or suspend_hi
> - immediately reads its own task flags again
> - reports success if flags & PF_MEMALLOC_NOIO is true after the write returns
>
> Link: https://github.com/chencheng-fnnas/reproducer/blob/main/repro-md-noio-token-leak.sh
>
> Fixes: 78f57ef9d50a ("md: use memalloc scope APIs in mddev_suspend()/mddev_resume()")
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/dm-raid.c | 7 ++--
> drivers/md/md-autodetect.c | 5 ++-
> drivers/md/md-bitmap.c | 12 +++---
> drivers/md/md.c | 85 ++++++++++++++++++++++----------------
> drivers/md/md.h | 23 ++++++-----
> drivers/md/raid5-cache.c | 11 +++--
> drivers/md/raid5.c | 25 ++++++-----
> 7 files changed, 97 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
> index 8f5a5e1342a9..d89207e3722a 100644
> --- a/drivers/md/dm-raid.c
> +++ b/drivers/md/dm-raid.c
> @@ -239,10 +239,11 @@ struct raid_set {
> int raid_disks;
> int delta_disks;
> int data_offset;
> int raid10_copies;
> int requested_bitmap_chunk_sectors;
> + unsigned int suspend_noio_flags;
>
> struct mddev md;
> struct raid_type *raid_type;
>
> sector_t array_sectors;
> @@ -3251,11 +3252,11 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)
> /* Start raid set read-only and assumed clean to change in raid_resume() */
> rs->md.ro = MD_RDONLY;
> rs->md.in_sync = 1;
>
> /* Has to be held on running the array */
> - mddev_suspend_and_lock_nointr(&rs->md);
> + mddev_suspend_and_lock_nointr(&rs->md, &rs->suspend_noio_flags);
>
> /* Keep array frozen until resume. */
> md_frozen_sync_thread(&rs->md);
>
> r = md_run(&rs->md);
> @@ -3863,11 +3864,11 @@ static void raid_postsuspend(struct dm_target *ti)
> /*
> * sync_thread must be stopped during suspend, and writes have
> * to be stopped before suspending to avoid deadlocks.
> */
> md_stop_writes(&rs->md);
> - mddev_suspend(&rs->md, false);
> + mddev_suspend(&rs->md, false, &rs->suspend_noio_flags);
> rs->md.ro = MD_RDONLY;
> }
> clear_bit(MD_DM_SUSPENDING, &mddev->flags);
>
> }
> @@ -4141,11 +4142,11 @@ static void raid_resume(struct dm_target *ti)
> lockdep_is_held(&mddev->reconfig_mutex)));
> clear_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags);
> mddev->ro = MD_RDWR;
> mddev->in_sync = 0;
> md_unfrozen_sync_thread(mddev);
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, rs->suspend_noio_flags);
> }
> }
For mdraid, changes looks fine. However, for dm-raid, for example:
dmsetup suspend ...
In this case, array will be suspended while task returned to user. Take a look at
the commit to introduce memalloc_noio_save(), it's supposed to be called for mdraid
arrays to allocate memory while array is suspended, I don't see why dm-raid need it.
So I'll suggest just bypass the memalloc_noio_save() for dm-raid where mddev_suspend()
is not paired with mddev_resume() in the same task context.
>
> static struct target_type raid_target = {
> .name = "raid",
> diff --git a/drivers/md/md-autodetect.c b/drivers/md/md-autodetect.c
> index 4b80165afd23..58e062cd0580 100644
> --- a/drivers/md/md-autodetect.c
> +++ b/drivers/md/md-autodetect.c
> @@ -126,10 +126,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
> dev_t devices[MD_SB_DISKS + 1], mdev;
> struct mdu_array_info_s ainfo = { };
> struct mddev *mddev;
> int err = 0, i;
> char name[16];
> + unsigned int noio_flags;
>
> if (args->partitioned) {
> mdev = MKDEV(mdp_major, args->minor << MdpMinorShift);
> sprintf(name, "md_d%d", args->minor);
> } else {
> @@ -173,11 +174,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
> if (IS_ERR(mddev)) {
> pr_err("md: md_alloc failed - cannot start array %s\n", name);
> return;
> }
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err) {
> pr_err("md: failed to lock array %s\n", name);
> goto out_mddev_put;
> }
>
> @@ -219,11 +220,11 @@ static void __init md_setup_drive(struct md_setup_args *args)
> if (!err)
> err = do_md_run(mddev);
> if (err)
> pr_warn("md: starting %s failed\n", name);
> out_unlock:
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> out_mddev_put:
> mddev_put(mddev);
> }
>
> static int __init raid_setup(char *str)
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 028b9ca8ce52..74b7f569a3f4 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -2620,13 +2620,14 @@ location_show(struct mddev *mddev, char *page)
> }
>
> static ssize_t
> location_store(struct mddev *mddev, const char *buf, size_t len)
> {
> + unsigned int noio_flags;
> int rv;
>
> - rv = mddev_suspend_and_lock(mddev);
> + rv = mddev_suspend_and_lock(mddev, &noio_flags);
> if (rv)
> return rv;
>
> if (mddev->pers) {
> if (mddev->recovery || mddev->sync_thread) {
> @@ -2711,11 +2712,11 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
> set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
> md_wakeup_thread(mddev->thread);
> }
> rv = 0;
> out:
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> if (rv)
> return rv;
> return len;
>
> merge_err:
> @@ -2831,17 +2832,18 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
> {
> unsigned long backlog;
> unsigned long old_mwb = mddev->bitmap_info.max_write_behind;
> struct md_rdev *rdev;
> bool has_write_mostly = false;
> + unsigned int noio_flags;
> int rv = kstrtoul(buf, 10, &backlog);
> if (rv)
> return rv;
> if (backlog > COUNTER_MAX)
> return -EINVAL;
>
> - rv = mddev_suspend_and_lock(mddev);
> + rv = mddev_suspend_and_lock(mddev, &noio_flags);
> if (rv)
> return rv;
>
> /*
> * Without write mostly device, it doesn't make sense to set
> @@ -2854,11 +2856,11 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
> }
> }
> if (!has_write_mostly) {
> pr_warn_ratelimited("%s: can't set backlog, no write mostly device available\n",
> mdname(mddev));
> - mddev_unlock(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return -EINVAL;
> }
>
> mddev->bitmap_info.max_write_behind = backlog;
> if (!backlog && mddev->serial_info_pool) {
> @@ -2871,11 +2873,11 @@ backlog_store(struct mddev *mddev, const char *buf, size_t len)
> mddev_create_serial_pool(mddev, rdev);
> }
> if (old_mwb != backlog)
> bitmap_update_sb(mddev->bitmap);
>
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return len;
> }
>
> static struct md_sysfs_entry bitmap_backlog =
> __ATTR(backlog, S_IRUGO|S_IWUSR, backlog_show, backlog_store);
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 1377c407614c..86d938dee50a 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -459,11 +459,12 @@ static void md_submit_bio(struct bio *bio)
>
> /*
> * Make sure no new requests are submitted to the device, and any requests that
> * have been submitted are completely handled.
> */
> -int mddev_suspend(struct mddev *mddev, bool interruptible)
> +int mddev_suspend(struct mddev *mddev, bool interruptible,
> + unsigned int *noio_flags)
> {
> int err = 0;
>
> /*
> * hold reconfig_mutex to wait for normal io will deadlock, because
> @@ -478,10 +479,11 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
> mutex_lock(&mddev->suspend_mutex);
> if (err)
> return err;
>
> if (mddev->suspended) {
> + *noio_flags = memalloc_noio_save();
> WRITE_ONCE(mddev->suspended, mddev->suspended + 1);
> mutex_unlock(&mddev->suspend_mutex);
> return 0;
> }
>
> @@ -515,31 +517,30 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
> * prevent deadlock.
> */
> WRITE_ONCE(mddev->suspended, mddev->suspended + 1);
>
> /* restrict memory reclaim I/O during raid array is suspend */
> - mddev->noio_flag = memalloc_noio_save();
> + *noio_flags = memalloc_noio_save();
>
> mutex_unlock(&mddev->suspend_mutex);
> return 0;
> }
> EXPORT_SYMBOL_GPL(mddev_suspend);
>
> -static void __mddev_resume(struct mddev *mddev, bool recovery_needed)
> +static void __mddev_resume(struct mddev *mddev, bool recovery_needed,
> + unsigned int noio_flags)
> {
> lockdep_assert_not_held(&mddev->reconfig_mutex);
>
> mutex_lock(&mddev->suspend_mutex);
> + memalloc_noio_restore(noio_flags);
> WRITE_ONCE(mddev->suspended, mddev->suspended - 1);
> if (mddev->suspended) {
> mutex_unlock(&mddev->suspend_mutex);
> return;
> }
>
> - /* entred the memalloc scope from mddev_suspend() */
> - memalloc_noio_restore(mddev->noio_flag);
> -
> percpu_ref_resurrect(&mddev->active_io);
> wake_up(&mddev->sb_wait);
>
> if (recovery_needed)
> set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
> @@ -547,13 +548,13 @@ static void __mddev_resume(struct mddev *mddev, bool recovery_needed)
> md_wakeup_thread(mddev->sync_thread); /* possibly kick off a reshape */
>
> mutex_unlock(&mddev->suspend_mutex);
> }
>
> -void mddev_resume(struct mddev *mddev)
> +void mddev_resume(struct mddev *mddev, unsigned int noio_flags)
> {
> - return __mddev_resume(mddev, true);
> + return __mddev_resume(mddev, true, noio_flags);
> }
> EXPORT_SYMBOL_GPL(mddev_resume);
>
> /* sync bdev before setting device to readonly or stopping raid*/
> static int mddev_set_closing_and_sync_blockdev(struct mddev *mddev, int opener_num)
> @@ -3737,10 +3738,11 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
> {
> struct rdev_sysfs_entry *entry = container_of(attr, struct rdev_sysfs_entry, attr);
> struct md_rdev *rdev = container_of(kobj, struct md_rdev, kobj);
> struct kernfs_node *kn = NULL;
> bool suspend = false;
> + unsigned int noio_flags = 0;
> ssize_t rv;
> struct mddev *mddev = READ_ONCE(rdev->mddev);
>
> if (!entry->store)
> return -EIO;
> @@ -3756,17 +3758,17 @@ rdev_attr_store(struct kobject *kobj, struct attribute *attr,
> cmd_match(page, "writemostly") ||
> cmd_match(page, "-writemostly"))
> suspend = true;
> }
>
> - rv = suspend ? mddev_suspend_and_lock(mddev) : mddev_lock(mddev);
> + rv = suspend ? mddev_suspend_and_lock(mddev, &noio_flags) : mddev_lock(mddev);
> if (!rv) {
> if (rdev->mddev == NULL)
> rv = -ENODEV;
> else
> rv = entry->store(rdev, page, length);
> - suspend ? mddev_unlock_and_resume(mddev) : mddev_unlock(mddev);
> + suspend ? mddev_unlock_and_resume(mddev, noio_flags) : mddev_unlock(mddev);
> }
>
> if (kn)
> sysfs_unbreak_active_protection(kn);
>
> @@ -4049,15 +4051,16 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> size_t slen = len;
> struct md_personality *pers, *oldpers;
> long level;
> void *priv, *oldpriv;
> struct md_rdev *rdev;
> + unsigned int noio_flags;
>
> if (slen == 0 || slen >= sizeof(clevel))
> return -EINVAL;
>
> - rv = mddev_suspend_and_lock(mddev);
> + rv = mddev_suspend_and_lock(mddev, &noio_flags);
> if (rv)
> return rv;
>
> if (mddev->pers == NULL) {
> memcpy(mddev->clevel, buf, slen);
> @@ -4231,11 +4234,11 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> md_update_sb(mddev, 1);
> sysfs_notify_dirent_safe(mddev->sysfs_level);
> md_new_event();
> rv = len;
> out_unlock:
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return rv;
> }
>
> static struct md_sysfs_entry md_level =
> __ATTR(level, S_IRUGO|S_IWUSR, level_show, level_store);
> @@ -4410,17 +4413,18 @@ static int update_raid_disks(struct mddev *mddev, int raid_disks);
>
> static ssize_t
> raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
> {
> unsigned int n;
> + unsigned int noio_flags;
> int err;
>
> err = kstrtouint(buf, 10, &n);
> if (err < 0)
> return err;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
> if (mddev->pers) {
> if (n != mddev->raid_disks)
> err = update_raid_disks(mddev, n);
> @@ -4442,11 +4446,11 @@ raid_disks_store(struct mddev *mddev, const char *buf, size_t len)
> mddev->raid_disks = n;
> mddev->reshape_backwards = (mddev->delta_disks < 0);
> } else
> mddev->raid_disks = n;
> out_unlock:
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return err ? err : len;
> }
> static struct md_sysfs_entry md_raid_disks =
> __ATTR(raid_disks, S_IRUGO|S_IWUSR, raid_disks_show, raid_disks_store);
>
> @@ -4822,10 +4826,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
> char *e;
> int major = simple_strtoul(buf, &e, 10);
> int minor;
> dev_t dev;
> struct md_rdev *rdev;
> + unsigned int noio_flags;
> int err;
>
> if (!*buf || *e != ':' || !e[1] || e[1] == '\n')
> return -EINVAL;
> minor = simple_strtoul(e+1, &e, 10);
> @@ -4834,11 +4839,11 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
> dev = MKDEV(major, minor);
> if (major != MAJOR(dev) ||
> minor != MINOR(dev))
> return -EOVERFLOW;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
> if (mddev->persistent) {
> rdev = md_import_device(dev, mddev->major_version,
> mddev->minor_version);
> @@ -4855,18 +4860,18 @@ new_dev_store(struct mddev *mddev, const char *buf, size_t len)
> rdev = md_import_device(dev, -2, -1);
> else
> rdev = md_import_device(dev, -1, -1);
>
> if (IS_ERR(rdev)) {
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return PTR_ERR(rdev);
> }
> err = bind_rdev_to_array(rdev, mddev);
> out:
> if (err)
> export_rdev(rdev);
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> if (!err)
> md_new_event();
> return err ? err : len;
> }
>
> @@ -5257,28 +5262,29 @@ static int mddev_start_reshape(struct mddev *mddev)
> static ssize_t
> action_store(struct mddev *mddev, const char *page, size_t len)
> {
> int ret;
> enum sync_action action;
> + unsigned int noio_flags = 0;
>
> if (!mddev->pers || !mddev->pers->sync_request)
> return -EINVAL;
>
> action = md_sync_action_by_name(page);
> retry:
> if (work_busy(&mddev->sync_work))
> flush_work(&mddev->sync_work);
>
> ret = (action == ACTION_RESHAPE) ?
> - mddev_suspend_and_lock(mddev) :
> + mddev_suspend_and_lock(mddev, &noio_flags) :
> mddev_lock(mddev);
> if (ret)
> return ret;
>
> if (work_busy(&mddev->sync_work)) {
> if (action == ACTION_RESHAPE)
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> else
> mddev_unlock(mddev);
> goto retry;
> }
>
> @@ -5349,11 +5355,11 @@ action_store(struct mddev *mddev, const char *page, size_t len)
> sysfs_notify_dirent_safe(mddev->sysfs_action);
> ret = len;
>
> out:
> if (action == ACTION_RESHAPE)
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> else
> mddev_unlock(mddev);
> return ret;
> }
>
> @@ -5640,24 +5646,25 @@ suspend_lo_show(struct mddev *mddev, char *page)
>
> static ssize_t
> suspend_lo_store(struct mddev *mddev, const char *buf, size_t len)
> {
> unsigned long long new;
> + unsigned int noio_flags;
> int err;
>
> err = kstrtoull(buf, 10, &new);
> if (err < 0)
> return err;
> if (new != (sector_t)new)
> return -EINVAL;
>
> - err = mddev_suspend(mddev, true);
> + err = mddev_suspend(mddev, true, &noio_flags);
> if (err)
> return err;
>
> WRITE_ONCE(mddev->suspend_lo, new);
> - mddev_resume(mddev);
> + mddev_resume(mddev, noio_flags);
>
> return len;
> }
> static struct md_sysfs_entry md_suspend_lo =
> __ATTR(suspend_lo, S_IRUGO|S_IWUSR, suspend_lo_show, suspend_lo_store);
> @@ -5671,24 +5678,25 @@ suspend_hi_show(struct mddev *mddev, char *page)
>
> static ssize_t
> suspend_hi_store(struct mddev *mddev, const char *buf, size_t len)
> {
> unsigned long long new;
> + unsigned int noio_flags;
> int err;
>
> err = kstrtoull(buf, 10, &new);
> if (err < 0)
> return err;
> if (new != (sector_t)new)
> return -EINVAL;
>
> - err = mddev_suspend(mddev, true);
> + err = mddev_suspend(mddev, true, &noio_flags);
> if (err)
> return err;
>
> WRITE_ONCE(mddev->suspend_hi, new);
> - mddev_resume(mddev);
> + mddev_resume(mddev, noio_flags);
>
> return len;
> }
> static struct md_sysfs_entry md_suspend_hi =
> __ATTR(suspend_hi, S_IRUGO|S_IWUSR, suspend_hi_show, suspend_hi_store);
> @@ -5928,19 +5936,20 @@ static ssize_t serialize_policy_show(struct mddev *mddev, char *page)
> static ssize_t
> serialize_policy_store(struct mddev *mddev, const char *buf, size_t len)
> {
> int err;
> bool value;
> + unsigned int noio_flags;
>
> err = kstrtobool(buf, &value);
> if (err)
> return err;
>
> if (value == test_bit(MD_SERIALIZE_POLICY, &mddev->flags))
> return len;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
> if (mddev->pers == NULL || (mddev->pers->head.id != ID_RAID1)) {
> pr_err("md: serialize_policy is only effective for raid1\n");
> err = -EINVAL;
> @@ -5953,11 +5962,11 @@ serialize_policy_store(struct mddev *mddev, const char *buf, size_t len)
> } else {
> mddev_destroy_serial_pool(mddev, NULL);
> clear_bit(MD_SERIALIZE_POLICY, &mddev->flags);
> }
> unlock:
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return err ?: len;
> }
>
> static struct md_sysfs_entry md_serialize_policy =
> __ATTR(serialize_policy, S_IRUGO | S_IWUSR, serialize_policy_show,
> @@ -6263,21 +6272,22 @@ EXPORT_SYMBOL_GPL(mddev_stack_new_rdev);
>
> /* update the optimal I/O size after a reshape */
> void mddev_update_io_opt(struct mddev *mddev, unsigned int nr_stripes)
> {
> struct queue_limits lim;
> + unsigned int noio_flags;
>
> if (mddev_is_dm(mddev))
> return;
>
> /* don't bother updating io_opt if we can't suspend the array */
> - if (mddev_suspend(mddev, false) < 0)
> + if (mddev_suspend(mddev, false, &noio_flags) < 0)
> return;
> lim = queue_limits_start_update(mddev->gendisk->queue);
> lim.io_opt = lim.io_min * nr_stripes;
> queue_limits_commit_update(mddev->gendisk->queue, &lim);
> - mddev_resume(mddev);
> + mddev_resume(mddev, noio_flags);
> }
> EXPORT_SYMBOL_GPL(mddev_update_io_opt);
>
> static void mddev_delayed_delete(struct work_struct *ws)
> {
> @@ -7255,10 +7265,11 @@ static void autorun_array(struct mddev *mddev)
> */
> static void autorun_devices(int part)
> {
> struct md_rdev *rdev0, *rdev, *tmp;
> struct mddev *mddev;
> + unsigned int noio_flags;
>
> pr_info("md: autorun ...\n");
> while (!list_empty(&pending_raid_disks)) {
> int unit;
> dev_t dev;
> @@ -7295,27 +7306,27 @@ static void autorun_devices(int part)
>
> mddev = md_alloc(dev, NULL);
> if (IS_ERR(mddev))
> break;
>
> - if (mddev_suspend_and_lock(mddev))
> + if (mddev_suspend_and_lock(mddev, &noio_flags))
> pr_warn("md: %s locked, cannot run\n", mdname(mddev));
> else if (mddev->raid_disks || mddev->major_version
> || !list_empty(&mddev->disks)) {
> pr_warn("md: %s already running, cannot run %pg\n",
> mdname(mddev), rdev0->bdev);
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> } else {
> pr_debug("md: created %s\n", mdname(mddev));
> mddev->persistent = 1;
> rdev_for_each_list(rdev, tmp, &candidates) {
> list_del_init(&rdev->same_set);
> if (bind_rdev_to_array(rdev, mddev))
> export_rdev(rdev);
> }
> autorun_array(mddev);
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> }
> /* on success, candidates will be empty, on error
> * it won't...
> */
> rdev_for_each_list(rdev, tmp, &candidates) {
> @@ -8329,10 +8340,11 @@ static int __md_set_array_info(struct mddev *mddev, void __user *argp)
>
> static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
> unsigned int cmd, unsigned long arg)
> {
> int err = 0;
> + unsigned int noio_flags = 0;
> void __user *argp = (void __user *)arg;
> struct mddev *mddev = NULL;
>
> err = md_ioctl_valid(cmd);
> if (err)
> @@ -8380,11 +8392,11 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
> }
>
> if (!md_is_rdwr(mddev))
> flush_work(&mddev->sync_work);
>
> - err = md_ioctl_need_suspend(cmd) ? mddev_suspend_and_lock(mddev) :
> + err = md_ioctl_need_suspend(cmd) ? mddev_suspend_and_lock(mddev, &noio_flags) :
> mddev_lock(mddev);
> if (err) {
> pr_debug("md: ioctl lock interrupted, reason %d, cmd %d\n",
> err, cmd);
> goto out;
> @@ -8511,11 +8523,11 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
> unlock:
> if (mddev->hold_active == UNTIL_IOCTL &&
> err != -EINVAL)
> mddev->hold_active = 0;
>
> - md_ioctl_need_suspend(cmd) ? mddev_unlock_and_resume(mddev) :
> + md_ioctl_need_suspend(cmd) ? mddev_unlock_and_resume(mddev, noio_flags) :
> mddev_unlock(mddev);
>
> out:
> if (cmd == STOP_ARRAY_RO || (err && cmd == STOP_ARRAY))
> clear_bit(MD_CLOSING, &mddev->flags);
> @@ -10180,20 +10192,21 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
> static void md_start_sync(struct work_struct *ws)
> {
> struct mddev *mddev = container_of(ws, struct mddev, sync_work);
> int spares = 0;
> bool suspend = false;
> + unsigned int noio_flags = 0;
> char *name;
>
> /*
> * If reshape is still in progress, spares won't be added or removed
> * from conf until reshape is done.
> */
> if (mddev->reshape_position == MaxSector &&
> md_spares_need_change(mddev)) {
> suspend = true;
> - mddev_suspend(mddev, false);
> + mddev_suspend(mddev, false, &noio_flags);
> }
>
> mddev_lock_nointr(mddev);
> if (!md_is_rdwr(mddev)) {
> /*
> @@ -10237,11 +10250,11 @@ static void md_start_sync(struct work_struct *ws)
> * not set it again. Otherwise, we may cause issue like this one:
> * https://bugzilla.kernel.org/show_bug.cgi?id=218200
> * Therefore, use __mddev_resume(mddev, false).
> */
> if (suspend)
> - __mddev_resume(mddev, false);
> + __mddev_resume(mddev, false, noio_flags);
> md_wakeup_thread(mddev->sync_thread);
> sysfs_notify_dirent_safe(mddev->sysfs_action);
> md_new_event();
> return;
>
> @@ -10257,11 +10270,11 @@ static void md_start_sync(struct work_struct *ws)
> * not set it again. Otherwise, we may cause issue like this one:
> * https://bugzilla.kernel.org/show_bug.cgi?id=218200
> * Therefore, use __mddev_resume(mddev, false).
> */
> if (suspend)
> - __mddev_resume(mddev, false);
> + __mddev_resume(mddev, false, noio_flags);
>
> wake_up(&resync_wait);
> if (test_and_clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery) &&
> mddev->sysfs_action)
> sysfs_notify_dirent_safe(mddev->sysfs_action);
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index d8daf0f75cbb..3337cd21eb30 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -619,11 +619,10 @@ struct mddev {
> mempool_t *serial_info_pool;
> void (*sync_super)(struct mddev *mddev, struct md_rdev *rdev);
> struct md_cluster_info *cluster_info;
> struct md_cluster_operations *cluster_ops;
> unsigned int good_device_nr; /* good device num within cluster raid */
> - unsigned int noio_flag; /* for memalloc scope API */
>
> /*
> * Temporarily store rdev that will be finally removed when
> * reconfig_mutex is unlocked, protected by reconfig_mutex.
> */
> @@ -953,12 +952,13 @@ extern void md_stop(struct mddev *mddev);
> extern void md_stop_writes(struct mddev *mddev);
> extern int md_rdev_init(struct md_rdev *rdev);
> extern void md_rdev_clear(struct md_rdev *rdev);
>
> extern bool md_handle_request(struct mddev *mddev, struct bio *bio);
> -extern int mddev_suspend(struct mddev *mddev, bool interruptible);
> -extern void mddev_resume(struct mddev *mddev);
> +extern int mddev_suspend(struct mddev *mddev, bool interruptible,
> + unsigned int *noio_flags);
> +extern void mddev_resume(struct mddev *mddev, unsigned int noio_flags);
> extern void md_idle_sync_thread(struct mddev *mddev);
> extern void md_frozen_sync_thread(struct mddev *mddev);
> extern void md_unfrozen_sync_thread(struct mddev *mddev);
>
> extern void md_update_sb(struct mddev *mddev, int force);
> @@ -999,35 +999,38 @@ static inline void mddev_check_write_zeroes(struct mddev *mddev, struct bio *bio
> if (bio_op(bio) == REQ_OP_WRITE_ZEROES &&
> !bio->bi_bdev->bd_disk->queue->limits.max_write_zeroes_sectors)
> mddev->gendisk->queue->limits.max_write_zeroes_sectors = 0;
> }
>
> -static inline int mddev_suspend_and_lock(struct mddev *mddev)
> +static inline int mddev_suspend_and_lock(struct mddev *mddev,
> + unsigned int *noio_flags)
> {
> int ret;
>
> - ret = mddev_suspend(mddev, true);
> + ret = mddev_suspend(mddev, true, noio_flags);
> if (ret)
> return ret;
>
> ret = mddev_lock(mddev);
> if (ret)
> - mddev_resume(mddev);
> + mddev_resume(mddev, *noio_flags);
>
> return ret;
> }
>
> -static inline void mddev_suspend_and_lock_nointr(struct mddev *mddev)
> +static inline void mddev_suspend_and_lock_nointr(struct mddev *mddev,
> + unsigned int *noio_flags)
> {
> - mddev_suspend(mddev, false);
> + mddev_suspend(mddev, false, noio_flags);
> mddev_lock_nointr(mddev);
> }
>
> -static inline void mddev_unlock_and_resume(struct mddev *mddev)
> +static inline void mddev_unlock_and_resume(struct mddev *mddev,
> + unsigned int noio_flags)
> {
> mddev_unlock(mddev);
> - mddev_resume(mddev);
> + mddev_resume(mddev, noio_flags);
> }
>
> struct mdu_array_info_s;
> struct mdu_disk_info_s;
>
> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
> index 7b7546bfa21f..6f8e3a624456 100644
> --- a/drivers/md/raid5-cache.c
> +++ b/drivers/md/raid5-cache.c
> @@ -693,13 +693,15 @@ static void r5c_disable_writeback_async(struct work_struct *work)
> !READ_ONCE(conf->log) ||
> !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));
>
> log = READ_ONCE(conf->log);
> if (log) {
> - mddev_suspend(mddev, false);
> + unsigned int noio_flags;
> +
> + mddev_suspend(mddev, false, &noio_flags);
> log->r5c_journal_mode = R5C_JOURNAL_MODE_WRITE_THROUGH;
> - mddev_resume(mddev);
> + mddev_resume(mddev, noio_flags);
> }
> }
>
> static void r5l_submit_current_io(struct r5l_log *log)
> {
> @@ -2603,10 +2605,11 @@ EXPORT_SYMBOL(r5c_journal_mode_set);
> static ssize_t r5c_journal_mode_store(struct mddev *mddev,
> const char *page, size_t length)
> {
> int mode = ARRAY_SIZE(r5c_journal_mode_str);
> size_t len = length;
> + unsigned int noio_flags;
> int ret;
>
> if (len < 2)
> return -EINVAL;
>
> @@ -2615,15 +2618,15 @@ static ssize_t r5c_journal_mode_store(struct mddev *mddev,
>
> while (mode--)
> if (strlen(r5c_journal_mode_str[mode]) == len &&
> !strncmp(page, r5c_journal_mode_str[mode], len))
> break;
> - ret = mddev_suspend_and_lock(mddev);
> + ret = mddev_suspend_and_lock(mddev, &noio_flags);
> if (ret)
> return ret;
> ret = r5c_journal_mode_set(mddev, mode);
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return ret ?: length;
> }
>
> struct md_sysfs_entry
> r5c_journal_mode = __ATTR(journal_mode, 0644,
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 65ae7d8930fc..6062c4b62cc8 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -6992,11 +6992,11 @@ raid5_show_stripe_size(struct mddev *mddev, char *page)
> #if PAGE_SIZE != DEFAULT_STRIPE_SIZE
> static ssize_t
> raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
> {
> struct r5conf *conf;
> - unsigned long new;
> + unsigned long new, noio_flags;
> int err;
> int size;
>
> if (len >= PAGE_SIZE)
> return -EINVAL;
> @@ -7011,11 +7011,11 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
> if (new % DEFAULT_STRIPE_SIZE != 0 ||
> new > PAGE_SIZE || new == 0 ||
> new != roundup_pow_of_two(new))
> return -EINVAL;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
>
> conf = mddev->private;
> if (!conf) {
> @@ -7049,11 +7049,11 @@ raid5_store_stripe_size(struct mddev *mddev, const char *page, size_t len)
> err = -ENOMEM;
> }
> mutex_unlock(&conf->cache_size_mutex);
>
> out_unlock:
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return err ?: len;
> }
>
> static struct md_sysfs_entry
> raid5_stripe_size = __ATTR(stripe_size, 0644,
> @@ -7127,19 +7127,20 @@ raid5_show_skip_copy(struct mddev *mddev, char *page)
> static ssize_t
> raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
> {
> struct r5conf *conf;
> unsigned long new;
> + unsigned int noio_flags;
> int err;
>
> if (len >= PAGE_SIZE)
> return -EINVAL;
> if (kstrtoul(page, 10, &new))
> return -EINVAL;
> new = !!new;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
> conf = mddev->private;
> if (!conf)
> err = -ENODEV;
> @@ -7152,11 +7153,11 @@ raid5_store_skip_copy(struct mddev *mddev, const char *page, size_t len)
> lim.features |= BLK_FEAT_STABLE_WRITES;
> else
> lim.features &= ~BLK_FEAT_STABLE_WRITES;
> err = queue_limits_commit_update(q, &lim);
> }
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return err ?: len;
> }
>
> static struct md_sysfs_entry
> raid5_skip_copy = __ATTR(skip_copy, S_IRUGO | S_IWUSR,
> @@ -7195,10 +7196,11 @@ static int alloc_thread_groups(struct r5conf *conf, int cnt,
> static ssize_t
> raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
> {
> struct r5conf *conf;
> unsigned int new;
> + unsigned int noio_flags;
> int err;
> struct r5worker_group *new_groups, *old_groups;
> int group_cnt;
>
> if (len >= PAGE_SIZE)
> @@ -7207,16 +7209,16 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
> return -EINVAL;
> /* 8192 should be big enough */
> if (new > 8192)
> return -EINVAL;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
> conf = mddev->private;
> if (!conf) {
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return -ENODEV;
> }
> raid5_quiesce(mddev, true);
>
> if (new != conf->worker_cnt_per_group) {
> @@ -7237,11 +7239,11 @@ raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
> kfree(old_groups);
> }
> }
>
> raid5_quiesce(mddev, false);
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
>
> return err ?: len;
> }
>
> static struct md_sysfs_entry
> @@ -8940,18 +8942,19 @@ static void *raid6_takeover(struct mddev *mddev)
> }
>
> static int raid5_change_consistency_policy(struct mddev *mddev, const char *buf)
> {
> struct r5conf *conf;
> + unsigned int noio_flags;
> int err;
>
> - err = mddev_suspend_and_lock(mddev);
> + err = mddev_suspend_and_lock(mddev, &noio_flags);
> if (err)
> return err;
> conf = mddev->private;
> if (!conf) {
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
> return -ENODEV;
> }
>
> if (strncmp(buf, "ppl", 3) == 0) {
> /* ppl only works with RAID 5 */
> @@ -8990,11 +8993,11 @@ static int raid5_change_consistency_policy(struct mddev *mddev, const char *buf)
> }
>
> if (!err)
> md_update_sb(mddev, 1);
>
> - mddev_unlock_and_resume(mddev);
> + mddev_unlock_and_resume(mddev, noio_flags);
>
> return err;
> }
>
> static int raid5_start(struct mddev *mddev)
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2] md: use READ_ONCE() for lockless reads of sb_flags
From: yu kuai @ 2026-07-05 10:55 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260623111617.2500313-1-chencheng@fnnas.com>
Hi,
在 2026/6/23 19:16, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> sb_flags is checked without a lock in md, raid1, raid5, and raid10.
> KCSAN reports these reads as data races.
>
> The write side uses atomic bit ops.
> The read side still has plain loads in a few places.
>
> Use READ_ONCE() for the lockless reads of sb_flags.
>
>
> v1 -> v2:
> - Add lock-free read paths for other array levels.
>
> KCSAN reports #1:
> ======================================
>
> BUG: KCSAN: data-race in md_check_recovery / md_write_start
>
> write (marked) to 0xffff8e39f897f030 of 8 bytes by task 248146 on cpu 8:
> md_write_start+0x5dd/0x910
> raid10_make_request+0x9b/0x1080
> md_handle_request+0x4a2/0xa40
> [........]
>
> read to 0xffff8e39f897f030 of 8 bytes by task 250445 on cpu 11:
> md_check_recovery+0x574/0x900
> raid10d+0xb7/0x2950
> [........]
>
> KCSAN reports #2:
> ======================================
> BUG: KCSAN: data-race in md_check_recovery / md_write_start
>
> write (marked) to 0xffff8e39e953f030 of 8 bytes by task 540091 on cpu 11:
> md_write_start+0x5dd/0x910
> raid1_make_request+0x141/0x1990
> [........]
>
> read to 0xffff8e39e953f030 of 8 bytes by task 580822 on cpu 0:
> md_check_recovery+0x574/0x900
> raid1d+0xcc/0x3840
> [........]
>
> value changed: 0x0000000000000002 -> 0x0000000000000006
>
> KCSAN reports #3:
> ======================================
> BUG: KCSAN: data-race in md_check_recovery / md_do_sync.cold
>
> write (marked) to 0xffff8e39e9404030 of 8 bytes by task 492473 on cpu 6:
> md_do_sync.cold+0x3f6/0x1686
> [........]
>
> read to 0xffff8e39e9404030 of 8 bytes by task 492402 on cpu 3:
> md_check_recovery+0x16d/0x900
> raid1d+0xcc/0x3840
> [........]
>
> value changed: 0x0000000000000000 -> 0x0000000000000002
>
> KCSAN reports #4:
> ======================================
> BUG: KCSAN: data-race in md_do_sync.cold / raid5d
>
> write (marked) to 0xffff8e39c35cb030 of 8 bytes by task 192196 on cpu 10:
> md_do_sync.cold+0x3f6/0x1686
> md_thread+0x15a/0x2d0
> [........]
>
> read to 0xffff8e39c35cb030 of 8 bytes by task 190759 on cpu 5:
> raid5d+0x7f9/0xba0
> md_thread+0x15a/0x2d0
> [........]
>
> value changed: 0x0000000000000000 -> 0x0000000000000002
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/md.c | 8 ++++----
> drivers/md/raid1.c | 2 +-
> drivers/md/raid10.c | 4 ++--
> drivers/md/raid5.c | 7 ++++---
> 4 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 096bb64e87bd..c5c50640b684 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6830,11 +6830,11 @@ int md_run(struct mddev *mddev)
> * via sysfs - until a lack of spares is confirmed.
> */
> set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
> set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
>
> - if (mddev->sb_flags)
> + if (READ_ONCE(mddev->sb_flags))
> md_update_sb(mddev, 0);
>
> if (IS_ENABLED(CONFIG_MD_BITMAP) && !mddev->bitmap_info.file &&
> !mddev->bitmap_info.offset)
> md_bitmap_set_none(mddev);
> @@ -7024,11 +7024,11 @@ static void __md_stop_writes(struct mddev *mddev)
> mddev->bitmap_ops->flush(mddev);
> }
>
> if (md_is_rdwr(mddev) &&
> ((!mddev->in_sync && !mddev_is_clustered(mddev)) ||
> - mddev->sb_flags)) {
> + READ_ONCE(mddev->sb_flags))) {
> /* mark array as shutdown cleanly */
> if (!mddev_is_clustered(mddev))
> mddev->in_sync = 1;
> md_update_sb(mddev, 1);
> }
> @@ -10294,11 +10294,11 @@ static bool md_should_do_recovery(struct mddev *mddev)
> /*
> * MD_SB_CHANGE_PENDING indicates that the array is switching from clean to
> * active, and no action is needed for now.
> * All other MD_SB_* flags require to update the superblock.
> */
> - if (mddev->sb_flags & ~ (1<<MD_SB_CHANGE_PENDING))
> + if (READ_ONCE(mddev->sb_flags) & ~ (1<<MD_SB_CHANGE_PENDING))
Since this line is involved, please also fix checkpatch warning as well, like what
you did below.
> return true;
>
> /*
> * If the array is not using external metadata and there has been no data
> * written for some time, then the array's status needs to be set to
> @@ -10423,11 +10423,11 @@ void md_check_recovery(struct mddev *mddev)
> spin_lock(&mddev->lock);
> set_in_sync(mddev);
> spin_unlock(&mddev->lock);
> }
>
> - if (mddev->sb_flags)
> + if (READ_ONCE(mddev->sb_flags))
> md_update_sb(mddev, 0);
>
> /*
> * Never start a new sync thread if MD_RECOVERY_RUNNING is
> * still set.
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 29b58583e381..bd6808656edb 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -2738,11 +2738,11 @@ static void raid1d(struct md_thread *thread)
> handle_read_error(conf, r1_bio);
> else
> WARN_ON_ONCE(1);
>
> cond_resched();
> - if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
> + if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
> md_check_recovery(mddev);
> }
> blk_finish_plug(&plug);
> }
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index adaf9e432e25..3ffa5a19964d 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -3030,11 +3030,11 @@ static void raid10d(struct md_thread *thread)
> handle_read_error(mddev, r10_bio);
> else
> WARN_ON_ONCE(1);
>
> cond_resched();
> - if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
> + if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
> md_check_recovery(mddev);
> }
> blk_finish_plug(&plug);
> }
>
> @@ -4698,11 +4698,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
> else
> mddev->curr_resync_completed = conf->reshape_progress;
> conf->reshape_checkpoint = jiffies;
> set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
> md_wakeup_thread(mddev->thread);
> - wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
> + wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
> test_bit(MD_RECOVERY_INTR, &mddev->recovery));
> if (test_bit(MD_RECOVERY_INTR, &mddev->recovery)) {
> allow_barrier(conf);
> return sectors_done;
> }
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index ded6a69f7795..cb58b4353995 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -1219,11 +1219,11 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
> break;
>
> if (bad < 0) {
> set_bit(BlockedBadBlocks, &rdev->flags);
> if (!conf->mddev->external &&
> - conf->mddev->sb_flags) {
> + READ_ONCE(conf->mddev->sb_flags)) {
> /* It is very unlikely, but we might
> * still need to write out the
> * bad block log - better give it
> * a chance*/
> md_check_recovery(conf->mddev);
> @@ -6469,11 +6469,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
> 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, mddev->sb_flags == 0 ||
> + wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
> test_bit(MD_RECOVERY_INTR, &mddev->recovery));
> if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
> return 0;
> spin_lock_irq(&conf->device_lock);
> conf->reshape_safe = mddev->reshape_position;
> @@ -6913,11 +6913,12 @@ static void raid5d(struct md_thread *thread)
> conf->temp_inactive_list);
> if (!batch_size && !released)
> break;
> handled += batch_size;
>
> - if (mddev->sb_flags & ~(1 << MD_SB_CHANGE_PENDING)) {
> + if (READ_ONCE(mddev->sb_flags) &
> + ~(1 << MD_SB_CHANGE_PENDING)) {
> spin_unlock_irq(&conf->device_lock);
> md_check_recovery(mddev);
> spin_lock_irq(&conf->device_lock);
> }
> }
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid1: protect sequential read hints for read balance
From: yu kuai @ 2026-07-05 9:31 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260623075940.2476255-1-chencheng@fnnas.com>
在 2026/6/23 15:59, Chen Cheng 写道:
> The patch just suppress KCSAN noise. No functional change.
>
> KCSAN reports a race, point to update_read_sectors() update next_seq_sect vs.
> read next_seq_sect.
>
> Protect next_seq_sect and seq_start with READ_ONCE/WRITE_ONCE, otherwise,
> read balance see stale sequential-read hints.
>
> KCSAN report:
> ==============
> BUG: KCSAN: data-race in raid1_read_request / raid1_read_request
>
> write to 0xffff8e3a2d6736d0 of 8 bytes by task 593784 on cpu 10:
> raid1_read_request+0xe5a/0x19f0
> raid1_make_request+0xdf/0x1990
> md_handle_request+0x4a2/0xa40
> [...]
>
> read to 0xffff8e3a2d6736d0 of 8 bytes by task 593776 on cpu 11:
> raid1_read_request+0xe3f/0x19f0
> raid1_make_request+0xdf/0x1990
> md_handle_request+0x4a2/0xa40
> [...]
>
> value changed: 0x0000000000356368 -> 0x0000000000356370
>
> Signed-off-by: Chen Cheng<chencheng@fnnas.com>
> ---
> drivers/md/raid1.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2] md/raid5-ppl: fix use-after-free in ppl_do_flush()
From: yu kuai @ 2026-07-05 9:02 UTC (permalink / raw)
To: Sajal Gupta, linux-raid, song
Cc: tomasz.majchrzak, linux-kernel, error27, skhan, me,
linux-kernel-mentees, yukuai
In-Reply-To: <20260622142146.56637-1-sajal2005gupta@gmail.com>
在 2026/6/22 22:06, Sajal Gupta 写道:
> The loop in ppl_do_flush() continues iterating after calling
> ppl_io_unit_finished(), touching io->pending_flushes and leading to a
> use-after-free.
>
> Add a break statement to stop the loop once io is freed.
>
> Fixes: 1532d9e87e8b ("raid5-ppl: PPL support for disks with write-back cache enabled")
> Reported-by: Dan Carpenter<error27@gmail.com>
> Closes:https://lore.kernel.org/all/ajJF2wKYWRk4GGCK@stanley.mountain/
> Signed-off-by: Sajal Gupta<sajal2005gupta@gmail.com>
> ---
> Compile tested only.
>
> Changes in v2:
> - drop the refcount_t conversion
>
> v1:https://lore.kernel.org/all/20260622080656.22786-1-sajal2005gupta@gmail.com/
>
> drivers/md/raid5-ppl.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Looks correct.
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2] md/raid5: protect bitmap batch counters aka seq_flush/seq_write consistency
From: yu kuai @ 2026-07-05 8:50 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: chenchneg33, linux-kernel
In-Reply-To: <20260617142839.882378-1-chencheng@fnnas.com>
在 2026/6/17 22:28, Chen Cheng 写道:
> From: Chen Cheng<chencheng@fnnas.com>
>
> kcsan detect race :
> - raid5d() closes the current bitmap batch by updating
> conf->seq_flush under conf->device_lock.
> - __add_stripe_bio() read conf->seq_flush without that
> lock when assigning sh->bm_seq.
>
> so, protect seq_flush/seq_write consistency for multiple CPUs by
> READ_ONCE()/WRITE_ONCE() under the path without held device_lock.
>
> re-explain the stripe batch sequence number update flow:
> 1. sh->bm_seq declare which batch number the stripe belongs to
> when perform bitmap-related write.
> ==> bm_seq = seq_flush+1
>
> 2. stripe be handled,
> * if sh->bm_seq - conf->seq_write > 0, means the
> batch stripes **newer than** the last written
> batch, it cannot proceed yet, queued on bitmap_list.
> * otherwise , has already proceed.
>
> 3. raid5d() `++seq_flush` to closes the current batch, means
> * no more stripes join that old batch
> * just-closed batch ready to write-out to disk
>
> 4. raid5d() calls bitmap hooks unplug() or writeout, then,
> `++seq_write` to the same as bm_seq.
>
> - seq_flush - for producer, to close batches.
> - seq_write - for consumer, the checkpoint number.
>
> the report:
> ====================================
> BUG: KCSAN: data-race in __add_stripe_bio / raid5d
>
> write to 0xffff88ba5625d470 of 4 bytes by task 82401 on cpu 0:
> raid5d+0x1d9/0xba0
> [.....]
>
> read to 0xffff88ba5625d470 of 4 bytes by task 82421 on cpu 8:
> __add_stripe_bio+0x332/0x400
> raid5_make_request+0x6ac/0x2930
> md_handle_request+0x4a2/0xa40
> md_submit_bio+0x109/0x1a0
> __submit_bio+0x2ec/0x390
> [.....]
>
> v1 -> v2:
> - remove WRITE_ONCE(conf->seq_write) in held device_lock path.
> - remove READ_ONCE(conf->seq_flush) in held device_lock path.
>
> Signed-off-by: Chen Cheng<chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v6 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio()
From: yu kuai @ 2026-07-05 7:48 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260623123840.2521340-4-chencheng@fnnas.com>
Hi,
在 2026/6/23 20:38, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> origin flow:
>
> bio_endio(master_bio); /* may drop active_io to zero */
> allow_barrier(conf);
> free_r10bio(r10_bio); /* reads conf->geo, returns to pool */
>
> one scenario is:
>
> CPU A (softirq, raid_end_bio_io) CPU B (action_store) --> reshape
> ================================ ===============================
> bio_endio(master_bio)
> md_end_clone_io
> percpu_ref_put -> 0
> wait_event wakeup, and,
> mddev_suspend return
> raid10_start_reshape:
> setup_geo(&conf->geo, new)
> ...
> mempool_destroy(old_pool)
> conf->r10bio_pool = new_pool
> allow_barrier(conf)
> free_r10bio(r10_bio)
> put_all_bios:
> for (i=0; i<conf->geo.raid_disks; i++)
> ==> old obj, new geo, OOB
> mempool_free(r10_bio, conf->r10bio_pool)
> ==> old-geometry obj freed into new pool
>
> so .. fix by reorder the flow:
>
> free_r10bio(r10_bio)
> allow_barrier(conf)
> bio_endio(master_io)
>
> raid_end_discard_bio() is exactly the same.
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid10.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index d740744a9746..e44a9b6e95c7 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -330,24 +330,25 @@ static void reschedule_retry(struct r10bio *r10_bio)
> */
> static void raid_end_bio_io(struct r10bio *r10_bio)
> {
> struct bio *bio = r10_bio->master_bio;
> struct r10conf *conf = r10_bio->mddev->private;
> + unsigned long state = r10_bio->state;
>
> - if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
> - if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
> - bio->bi_status = BLK_STS_IOERR;
> - bio_endio(bio);
> - }
> + free_r10bio(r10_bio);
>
> /*
> * Wake up any possible resync thread that waits for the device
> * to go idle.
> */
> allow_barrier(conf);
>
> - free_r10bio(r10_bio);
> + if (!test_and_set_bit(R10BIO_Returned, &state)) {
> + if (!test_bit(R10BIO_Uptodate, &state))
> + bio->bi_status = BLK_STS_IOERR;
> + bio_endio(bio);
> + }
Why is this moved to the end? I feel it's not necessary.
> }
>
> /*
> * Update disk head position estimator based on IRQ completion info.
> */
> @@ -1580,13 +1581,15 @@ static void raid_end_discard_bio(struct r10bio *r10bio)
> if (!test_bit(R10BIO_Discard, &r10bio->state)) {
> first_r10bio = (struct r10bio *)r10bio->master_bio;
> free_r10bio(r10bio);
> r10bio = first_r10bio;
> } else {
> + struct bio *master_bio = r10bio->master_bio;
> +
> md_write_end(r10bio->mddev);
> - bio_endio(r10bio->master_bio);
> free_r10bio(r10bio);
> + bio_endio(master_bio);
> break;
> }
> }
> }
>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v6 1/3] md: suspend array when sync_action=reshape
From: yu kuai @ 2026-07-05 7:43 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260623123840.2521340-2-chencheng@fnnas.com>
Hi，
在 2026/6/23 20:38, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> raid10 needs to resize/swap r10bio_pool when reshape changes
> raid_disks, and, don't let new requests keep allocating r10bio
> objects from the old pool while that transition is in progress.
>
> suspend and lock array before mddev_start_reshape(), and resume
> it on exit.
>
> Other sync_action ops are unchanged.
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/md.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 096bb64e87bd..1377c407614c 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -5261,25 +5261,29 @@ action_store(struct mddev *mddev, const char *page, size_t len)
> enum sync_action action;
>
> if (!mddev->pers || !mddev->pers->sync_request)
> return -EINVAL;
>
> + action = md_sync_action_by_name(page);
> retry:
> if (work_busy(&mddev->sync_work))
> flush_work(&mddev->sync_work);
>
> - ret = mddev_lock(mddev);
> + ret = (action == ACTION_RESHAPE) ?
> + mddev_suspend_and_lock(mddev) :
> + mddev_lock(mddev);
I already suggestted in previous verions that suspend for retry is too
heavy, please suspend before retry.
> if (ret)
> return ret;
>
> if (work_busy(&mddev->sync_work)) {
> - mddev_unlock(mddev);
> + if (action == ACTION_RESHAPE)
> + mddev_unlock_and_resume(mddev);
> + else
> + mddev_unlock(mddev);
> goto retry;
> }
>
> - action = md_sync_action_by_name(page);
> -
> /* TODO: mdadm rely on "idle" to start sync_thread. */
> if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) {
> switch (action) {
> case ACTION_FROZEN:
> md_frozen_sync_thread(mddev);
> @@ -5344,11 +5348,14 @@ action_store(struct mddev *mddev, const char *page, size_t len)
> md_wakeup_thread(mddev->thread);
> sysfs_notify_dirent_safe(mddev->sysfs_action);
> ret = len;
>
> out:
> - mddev_unlock(mddev);
> + if (action == ACTION_RESHAPE)
> + mddev_unlock_and_resume(mddev);
> + else
> + mddev_unlock(mddev);
> return ret;
> }
>
> static struct md_sysfs_entry md_scan_mode =
> __ATTR_PREALLOC(sync_action, S_IRUGO|S_IWUSR, action_show, action_store);
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: John Garry @ 2026-07-03 8:28 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, axboe,
vverma, martin.petersen, linux-kernel
Cc: linux-raid
In-Reply-To: <m2se64zak9.fsf@gmail.com>
On 30/06/2026 09:39, Abd-Alrhman Masalkhi wrote:
>
> Hi John,
>
> On Mon, Jun 29, 2026 at 15:48 +0100, John Garry wrote:
>> On 28/06/2026 15:24, Abd-Alrhman Masalkhi wrote:
>>> 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://urldefense.com/v3/__https://lore.kernel.org/linux-raid/20260623072456.333437-3-abd.masalkhi@gmail.com/__;!!ACWV5N9M2RV99hQ!LbMSGSClRi0PNBqQti5ZNWGDVjDd34-7saYEAwNyBNjpNTjEA7veqM5RHG8KB1QiscarW4UaIefjm19ywSImtIgh$
>>> ---
>>> 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))
>>
>> where does BIO_MAX_VECS * (PAGE_SIZE >> 9) even come from?
>>
>
> BIO_MAX_VECS * (PAGE_SIZE >> 9) defines the maximum size supported by
> write-behind. The write-behind copy (alloc_behind_master_bio) uses a
> single bio, which can hold at most BIO_MAX_VECS pages, making this the
> largest payload it can carry. With a 4 KiB PAGE_SIZE, that corresponds
> to 256 pages, or 1 MiB (2048 sectors).
> This patch changes the behavior for atomic writes that exceed this
> limit. Instead of failing the write with -EIO when the number of sectors
> must be reduced, it disables write-behind and proceeds with the atomic
> write.
>
>>> + /*
>>> + * Atomic writes cannot be split, so disable
>>> + * write-behind.
>>> + */
>>> + write_behind = false;
It's a bit poor to have write_behind initially = false, then allow it to
be set = true, and then later be set = false. Can this be improved?
>>> + 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;
>>
>> This BARRIER_UNIT_SECTOR_SIZE is a bit like chunk sectors, no? I am just
>> wondering if we just should set it to chunk sectors =
>> BARRIER_UNIT_SECTOR_SIZE
>>
>> I assume that it affects more than Reads and writes, e.g. discard also.
>>
>
> BARRIER_UNIT_SECTOR_SIZE is the resync barrier-bucket, not the layout
> chunk size. Unless I'm missing something, using
> atomic_write_hw_unit_max seems more appropriate than using the chunk
> size. That way, the limit only applies to atomic writes instead of
> affecting other operations such.
well it seems to be that everything in the driver is split over
BARRIER_UNIT_SECTOR_SIZE, so is in effect a chunk size.
Note that atomic_write_hw_unit_max is going to be small always compared
to BARRIER_UNIT_SECTOR_SIZE.
However, can you check the blk stacking code to make sure that this does
as you want? As I remember, for stacking we take the atomic write limits
of the first bottom device and then stack the other bottom devices and I
don't think that setting atomic_write_hw_unit_max in this way has an
impact - see blk_stack_atomic_writes_limits()
>
>>> lim.features |= BLK_FEAT_ATOMIC_WRITES;
>>> lim.features |= BLK_FEAT_PCI_P2PDMA;
>>> err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
>>
>
^ 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