* [PATCH v5 01/29] md/md-llbitmap: clear flush state after daemon flush
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:28 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 02/29] md/md-llbitmap: use GFP_NOIO for cache allocations Yu Kuai
` (28 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
llbitmap_flush() sets LLPageFlush on each bitmap page before it queues the
daemon worker. The flag tells md_llbitmap_daemon_fn() to ignore the normal
barrier_idle expiry check and clean the page immediately.
The daemon only tested LLPageFlush. Once a page had been flushed explicitly,
the flag stayed set, so later dirty bits on that page also bypassed
barrier_idle and were cleaned the next time the daemon ran. That can make a
new write look clean much earlier than the configured idle window.
Consume LLPageFlush in md_llbitmap_daemon_fn() with test_and_clear_bit() and
use the returned value for the current expiry check. The explicit flush still
forces the current daemon pass, while later writes on the same page wait for
barrier_idle again.
This can be reproduced through normal sysfs operations:
1. Create a small RAID1 with --bitmap=lockless and --assume-clean.
2. Set llbitmap/daemon_sleep=1 and llbitmap/barrier_idle=10.
3. Toggle md/array_state from active to readonly and back to active to call
llbitmap_flush() without destroying the in-memory bitmap.
4. Write one sector and read llbitmap/bits immediately, after 2 seconds,
and after 12 seconds.
On the bad kernel the dirty bit is already clean after 2 seconds. With this
change it remains dirty until the barrier_idle window expires.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 2a2b38c663c3..71e9a21b98b2 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1066,14 +1066,14 @@ static void md_llbitmap_daemon_fn(struct work_struct *work)
for (idx = 0; idx < llbitmap->nr_pages; idx++) {
struct llbitmap_page_ctl *pctl = llbitmap->pctl[idx];
+ bool flush = test_and_clear_bit(LLPageFlush, &pctl->flags);
if (idx > 0) {
start = end + 1;
end = min(end + PAGE_SIZE, llbitmap->chunks - 1);
}
- if (!test_bit(LLPageFlush, &pctl->flags) &&
- time_before(jiffies, pctl->expire)) {
+ if (!flush && time_before(jiffies, pctl->expire)) {
restart = true;
continue;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 01/29] md/md-llbitmap: clear flush state after daemon flush
2026-08-02 19:50 ` [PATCH v5 01/29] md/md-llbitmap: clear flush state after daemon flush Yu Kuai
@ 2026-08-02 20:28 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:28 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The patch unmasks a use-after-free of `llbitmap` during array teardown by allowing the daemon worker to re-arm the timer after it has been deleted.
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but threads waiting for IO barriers will hang indefinitely if page suspension times out.
--
--- Patch [1]: [PATCH v5 01/29] md/md-llbitmap: clear flush state after daemon flush ---
commit 5b6bbbac2eb163d696a69aa1800138e01de10a7e
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: clear flush state after daemon flush
This patch changes the llbitmap daemon to consume the LLPageFlush bit using
test_and_clear_bit() instead of just testing it. This ensures that after an
explicit flush, subsequent dirty bits on the same page will correctly wait
for the barrier_idle window before being cleaned.
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1066,14 +1066,14 @@ static void md_llbitmap_daemon_fn(struct work_struct *work)
>
> for (idx = 0; idx < llbitmap->nr_pages; idx++) {
> struct llbitmap_page_ctl *pctl = llbitmap->pctl[idx];
> + bool flush = test_and_clear_bit(LLPageFlush, &pctl->flags);
>
> if (idx > 0) {
> start = end + 1;
> end = min(end + PAGE_SIZE, llbitmap->chunks - 1);
> }
>
> - if (!test_bit(LLPageFlush, &pctl->flags) &&
> - time_before(jiffies, pctl->expire)) {
> + if (!flush && time_before(jiffies, pctl->expire)) {
^^^^^^
[Severity: High]
Does consuming the LLPageFlush bit here expose a use-after-free during
array teardown?
During normal array teardown, llbitmap_flush() sets LLPageFlush and queues
daemon_work. Because this patch consumes LLPageFlush, a subsequent execution
of the work can see LLPageFlush as clear. It will then evaluate
time_before(jiffies, pctl->expire) as true, set restart = true, and blindly
call mod_timer() at the end of md_llbitmap_daemon_fn().
If this work runs via flush_workqueue(md_llbitmap_io_wq) in
llbitmap_destroy(), it arms the timer after timer_delete_sync() has already
finished:
drivers/md/md-llbitmap.c:llbitmap_destroy() {
...
timer_delete_sync(&llbitmap->pending_timer);
flush_workqueue(md_llbitmap_io_wq);
...
}
The llbitmap is then freed, and the timer can fire on freed memory.
> restart = true;
> continue;
> }
[Severity: High]
This isn't a bug introduced by this patch, but threads waiting for IO
barriers will hang indefinitely if page suspension times out.
If llbitmap_suspend_timeout() times out (e.g., due to slow IO), it resurrects
the percpu_ref but skips calling wake_up(&pctl->wait):
drivers/md/md-llbitmap.c:llbitmap_suspend_timeout() {
...
percpu_ref_resurrect(&pctl->active);
return -ETIMEDOUT;
...
}
The daemon then continues and skips llbitmap_resume() which normally does the
wakeup. Threads waiting in llbitmap_raise_barrier() for the dying state to
clear will never be woken up:
drivers/md/md-llbitmap.c:llbitmap_raise_barrier() {
...
wait_event(pctl->wait, !percpu_ref_is_dying(&pctl->active));
...
}
Can this lead to a permanent hang of IO threads accessing the md array?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=1
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 02/29] md/md-llbitmap: use GFP_NOIO for cache allocations
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
2026-08-02 19:50 ` [PATCH v5 01/29] md/md-llbitmap: clear flush state after daemon flush Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:44 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 03/29] md/md-llbitmap: only end fully synced chunks Yu Kuai
` (27 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
llbitmap allocates its in-memory page cache and page-control structures from
paths that can already be holding MD reconfiguration or bitmap state locks.
For example, component_size_store() takes mddev_lock(), update_size() calls
the personality resize method, and llbitmap_resize() can grow the page cache
through llbitmap_prepare_resize().
Using GFP_KERNEL in those paths allows direct reclaim to enter filesystem or
block I/O while MD resize state is locked. That can recurse back into the
same array and wait on state that cannot make progress until the resize path
finishes.
Use GFP_NOIO for the llbitmap object, cached bitmap pages, page controls,
page-control arrays, and percpu_ref initialization. Leave the explicit
metadata zeroout path unchanged because it is intentional bitmap I/O rather
than reclaim-driven allocation.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 71e9a21b98b2..3cd8373bc9b2 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -521,7 +521,7 @@ static struct page *llbitmap_read_page(struct llbitmap *llbitmap, int idx)
if (page)
return page;
- page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ page = alloc_page(GFP_NOIO | __GFP_ZERO);
if (!page)
return ERR_PTR(-ENOMEM);
@@ -616,12 +616,12 @@ static int llbitmap_cache_pages(struct llbitmap *llbitmap)
int i;
llbitmap->pctl = kmalloc_array(nr_pages, sizeof(void *),
- GFP_KERNEL | __GFP_ZERO);
+ GFP_NOIO | __GFP_ZERO);
if (!llbitmap->pctl)
return -ENOMEM;
size = round_up(size, cache_line_size());
- pctl = kmalloc_array(nr_pages, size, GFP_KERNEL | __GFP_ZERO);
+ pctl = kmalloc_array(nr_pages, size, GFP_NOIO | __GFP_ZERO);
if (!pctl) {
kfree(llbitmap->pctl);
return -ENOMEM;
@@ -640,7 +640,7 @@ static int llbitmap_cache_pages(struct llbitmap *llbitmap)
}
if (percpu_ref_init(&pctl->active, active_release,
- PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
+ PERCPU_REF_ALLOW_REINIT, GFP_NOIO)) {
__free_page(page);
llbitmap_free_pages(llbitmap);
return -ENOMEM;
@@ -1110,7 +1110,7 @@ static int llbitmap_create(struct mddev *mddev)
if (ret)
return ret;
- llbitmap = kzalloc_obj(*llbitmap);
+ llbitmap = kzalloc_obj(*llbitmap, GFP_NOIO);
if (!llbitmap)
return -ENOMEM;
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 02/29] md/md-llbitmap: use GFP_NOIO for cache allocations
2026-08-02 19:50 ` [PATCH v5 02/29] md/md-llbitmap: use GFP_NOIO for cache allocations Yu Kuai
@ 2026-08-02 20:44 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:44 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] `llbitmap_resize()` recalculates capacity limits but fails to reallocate the `pctl` array, causing out-of-bounds pointer dereferences during subsequent operations.
- [High] `blkdev_issue_zeroout()` is called with `GFP_KERNEL` while `mddev_lock()` is held, risking a direct reclaim deadlock.
- [High] The background timer and workqueue can re-arm each other during teardown, leading to a Use-After-Free of the `llbitmap` struct.
--
--- Patch [2]: [PATCH v5 02/29] md/md-llbitmap: use GFP_NOIO for cache allocations ---
commit 0ace115175e6f1ed40865ea8b812e7ca53aadaed
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: use GFP_NOIO for cache allocations
This commit changes the memory allocation flags from GFP_KERNEL to GFP_NOIO for
the llbitmap object and its related structures. This change prevents direct
memory reclaim from triggering block I/O while MD resize state locks are held,
avoiding potential deadlocks.
> llbitmap allocates its in-memory page cache and page-control structures from
> paths that can already be holding MD reconfiguration or bitmap state locks.
> For example, component_size_store() takes mddev_lock(), update_size() calls
> the personality resize method, and llbitmap_resize() can grow the page cache
> through llbitmap_prepare_resize().
[Severity: High]
This is a pre-existing issue, but the commit message claims that
llbitmap_resize() grows the page cache through llbitmap_prepare_resize().
Looking at llbitmap_resize():
llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
{
...
llbitmap->chunks = chunks;
return 0;
}
The function updates the capacity limits but does not update llbitmap->nr_pages
or reallocate the llbitmap->pctl array. The referenced llbitmap_prepare_resize()
function does not seem to exist.
Because the pctl array is not reallocated, won't subsequent I/O operations call
llbitmap_write() with an index that exceeds the original nr_pages length?
llbitmap_write() calculates the index:
idx = pos >> PAGE_SHIFT;
bit = offset_in_page(pos);
llbitmap->pctl[idx]->state[bit] = state;
Could this lead to an out-of-bounds pointer dereference when accessing the new
chunks limit?
> Use GFP_NOIO for the llbitmap object, cached bitmap pages, page controls,
> page-control arrays, and percpu_ref initialization. Leave the explicit
> metadata zeroout path unchanged because it is intentional bitmap I/O rather
> than reclaim-driven allocation.
[Severity: High]
This isn't a bug introduced by this patch, but the commit message explicitly
notes that the metadata zeroout path was left unchanged.
Looking at llbitmap_zero_all_disks(), which is called during array
initialization or reconfiguration with mddev_lock() held:
ret = blkdev_issue_zeroout(rdev->bdev,
rdev->data_offset,
dev_sectors,
GFP_KERNEL, 0);
While the I/O itself is intentional, blkdev_issue_zeroout() uses the provided
GFP flags internally to allocate the bio and payload pages. If memory is tight,
won't GFP_KERNEL enter direct reclaim, potentially submitting a bio back to the
same mddev and causing a deadlock?
Should this also be updated to use GFP_NOIO?
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 71e9a21b98b2e..3cd8373bc9b22 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
[ ... ]
[Severity: High]
This is a pre-existing issue, but there appears to be a resource lifecycle
regression during teardown.
Looking at llbitmap_destroy():
timer_delete_sync(&llbitmap->pending_timer);
flush_workqueue(md_llbitmap_io_wq);
...
kfree(llbitmap);
If the timer had already fired and queued the daemon_work, timer_delete_sync()
returns immediately. flush_workqueue() then blocks waiting for the work item to
execute.
Inside md_llbitmap_daemon_fn():
if (restart)
mod_timer(&llbitmap->pending_timer,
jiffies + llbitmap->mddev->bitmap_info.daemon_sleep * HZ);
If unexpired dirty pages are found, it sets restart = true and unconditionally
calls mod_timer(), re-arming the timer while the subsystem is undergoing
destruction.
After flush_workqueue() finishes, llbitmap_destroy() frees the llbitmap object.
When the newly re-armed timer fires, won't it access the freed object?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=2
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 03/29] md/md-llbitmap: only end fully synced chunks
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
2026-08-02 19:50 ` [PATCH v5 01/29] md/md-llbitmap: clear flush state after daemon flush Yu Kuai
2026-08-02 19:50 ` [PATCH v5 02/29] md/md-llbitmap: use GFP_NOIO for cache allocations Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 19:50 ` [PATCH v5 04/29] md/raid5: reject zero-sector reshape chunks Yu Kuai
` (26 subsequent siblings)
29 siblings, 0 replies; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
llbitmap_cond_end_sync() is called with the sync thread's current sector.
That value is an exclusive progress boundary: sectors below it have
completed, but the llbitmap chunk containing it can still be in progress.
The old code converted that sector directly to the last bit passed to
BitmapActionEndsync. If resync had only advanced part-way into a large
llbitmap chunk, the in-progress chunk was marked synced and flushed before
the rest of the chunk was repaired. A later bitmap-assisted RAID1 resync
could then skip the remainder of that chunk and leave stale mirror data
behind.
This can be reproduced without editing bitmap metadata by creating a large
RAID1 with a lockless bitmap so llbitmap naturally selects a 524288-sector
chunk (with the default 128 KiB bitmap area, an array just over 16 TiB is
enough), making one mirror stale through the normal degraded write/re-add
path, and throttling resync so the daemon checkpoint runs while resync is
still inside the first chunk. On the bad kernel, bit 0 is ended early and a
stale sector later in the same chunk is skipped. With this fix, bit 0
remains Syncing until resync reaches the next chunk boundary.
Round the exclusive progress sector down to the nearest llbitmap chunk
boundary and end only chunks strictly below that boundary. Also honor the
force argument so callers that need an immediate checkpoint are not
suppressed by daemon_sleep.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 3cd8373bc9b2..948bf64c5ad2 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1450,22 +1450,27 @@ static void llbitmap_cond_end_sync(struct mddev *mddev, sector_t sector,
bool force)
{
struct llbitmap *llbitmap = mddev->bitmap;
+ sector_t complete;
if (sector == 0) {
llbitmap->last_end_sync = jiffies;
return;
}
- if (time_before(jiffies, llbitmap->last_end_sync +
- HZ * mddev->bitmap_info.daemon_sleep))
+ if (!force && time_before(jiffies, llbitmap->last_end_sync +
+ HZ * mddev->bitmap_info.daemon_sleep))
return;
wait_event(mddev->recovery_wait, !atomic_read(&mddev->recovery_active));
mddev->curr_resync_completed = sector;
set_bit(MD_SB_CHANGE_CLEAN, &mddev->sb_flags);
- llbitmap_state_machine(llbitmap, 0, sector >> llbitmap->chunkshift,
- BitmapActionEndsync);
+
+ complete = round_down(sector, llbitmap->chunksize);
+ if (complete)
+ llbitmap_state_machine(llbitmap, 0,
+ (complete >> llbitmap->chunkshift) - 1,
+ BitmapActionEndsync);
__llbitmap_flush(mddev);
llbitmap->last_end_sync = jiffies;
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v5 04/29] md/raid5: reject zero-sector reshape chunks
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (2 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 03/29] md/md-llbitmap: only end fully synced chunks Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:31 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 05/29] md/raid5: round bitmap stripes with sector division Yu Kuai
` (25 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Sashiko reported that RAID5 can accept a reshape chunk size that becomes
zero sectors. chunk_size_store() stores the sysfs byte value as n >> 9, so
writing a value below 512 bytes sets mddev->new_chunk_sectors to zero.
RAID5 then accepted that pending reshape geometry and raid5_start_reshape()
installed it into conf->chunk_sectors, letting reshape code divide by zero.
Reject zero-sector chunks both in check_reshape(), where normal sysfs
requests are validated, and in raid5_start_reshape(), so assembly/resume
paths also cannot install zero chunk geometry.
Test script: in QEMU, create a plain three-disk RAID5 array with 64K
chunks, write/read back a small pattern, write 1 to
/sys/block/md0/md/chunk_size, add a fourth disk, and run mdadm --grow
--raid-devices=4 --backup-file=... . The script scans dmesg for divide
error/Oops/KASAN signatures.
Bad kernel, eb29914412c3:
echo 1 > /sys/block/md0/md/chunk_size
mdadm --grow /dev/md0 --raid-devices=4 --backup-file=/root/md0-grow.bak
Oops: divide error: 0000 [#1] SMP KASAN NOPTI
RIP: raid5_get_active_stripe+0x863/0xc10
Call Trace:
raid5_sync_request
md_do_sync
md_thread
Kernel panic - not syncing: Fatal exception
Fixed kernel: echo 1 > /sys/block/md0/md/chunk_size bash: echo: write
error: Invalid argument chunk_write_rc=1 grow_rc=skipped RESULT:
REJECTED_ZERO_CHUNK_NO_OOPS
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/raid5.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index e2c5a7072aca..d128d238e1da 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -8548,6 +8548,8 @@ static int check_reshape(struct mddev *mddev)
return 0; /* nothing to do */
if (has_failed(conf))
return -EINVAL;
+ if (!mddev->new_chunk_sectors)
+ return -EINVAL;
if (mddev->delta_disks < 0 && mddev->reshape_position == MaxSector) {
/* We might be able to shrink, but the devices must
* be made bigger first.
@@ -8591,6 +8593,9 @@ static int raid5_start_reshape(struct mddev *mddev)
if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
return -EBUSY;
+ if (!mddev->new_chunk_sectors)
+ return -EINVAL;
+
if (!check_stripe_cache(mddev))
return -ENOSPC;
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 04/29] md/raid5: reject zero-sector reshape chunks
2026-08-02 19:50 ` [PATCH v5 04/29] md/raid5: reject zero-sector reshape chunks Yu Kuai
@ 2026-08-02 20:31 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:31 UTC (permalink / raw)
To: Yu Kuai; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Incomplete Fix / Integer Overflow and Validation Bypass
- [Critical] State Corruption / Incorrect Error Handling during Reshape Resume
--
--- Patch [4]: [PATCH v5 04/29] md/raid5: reject zero-sector reshape chunks ---
commit 7e4615eab2a0600f99886ba69199c0ee0d8cd320
Author: Yu Kuai <yukuai@fygo.io>
md/raid5: reject zero-sector reshape chunks
Sashiko reported that RAID5 can accept a reshape chunk size that becomes
zero sectors. chunk_size_store() stores the sysfs byte value as n >> 9, so
writing a value below 512 bytes sets mddev->new_chunk_sectors to zero.
RAID5 then accepted that pending reshape geometry and raid5_start_reshape()
installed it into conf->chunk_sectors, letting reshape code divide by zero.
[ ... ]
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index e2c5a7072aca0..d128d238e1daa 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -8548,6 +8548,8 @@ static int check_reshape(struct mddev *mddev)
> return 0; /* nothing to do */
> if (has_failed(conf))
> return -EINVAL;
> + if (!mddev->new_chunk_sectors)
> + return -EINVAL;
[Severity: Critical]
This is a pre-existing issue, but does this check adequately prevent invalid
chunk sizes if mddev->new_chunk_sectors is negative?
If a privileged user writes an excessively large value to the chunk_size sysfs
attribute (like 0x10000000200), chunk_size_store() in drivers/md/md.c shifts
it by 9 and stores it into the 32-bit signed integer mddev->new_chunk_sectors.
This can result in a negative value (e.g., -2147483647).
Since negative values are non-zero, they bypass this check. Will this negative
value then bypass the validation in raid5_check_reshape() which only checks
positive values?
drivers/md/raid5.c:raid5_check_reshape() {
...
if (new_chunk > 0) {
if (!is_power_of_2(new_chunk))
return -EINVAL;
...
}
Could this also cause a 32-bit shift overflow in check_stripe_cache(),
passing the limit checks, and eventually lead to a divide-by-zero panic in
make_discard_request() due to a 32-bit multiplication overflow when
calculating stripe_sectors?
[Severity: Critical]
This isn't a bug introduced by this patch, but could returning an error
code like -EINVAL here cause state corruption during a reshape resume?
Looking at mddev_start_reshape() in drivers/md/md.c, it evaluates the return
value of check_reshape() in a boolean context:
drivers/md/md.c:mddev_start_reshape() {
...
if (mddev->reshape_position == MaxSector ||
mddev->pers->check_reshape == NULL ||
mddev->pers->check_reshape(mddev)) {
clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
ret = mddev->pers->start_reshape(mddev);
...
}
If check_reshape() returns -EINVAL (or -ENOMEM), the condition evaluates
to true in C, causing mddev_start_reshape() to call
mddev->pers->start_reshape(mddev).
If this happens during a resume, won't calling raid5_start_reshape()
incorrectly reset conf->reshape_progress and double-apply the delta_disks?
drivers/md/raid5.c:raid5_start_reshape() {
...
conf->previous_raid_disks = conf->raid_disks;
conf->raid_disks += mddev->delta_disks;
...
}
Since conf->raid_disks is already set to the target size in setup_conf()
during resume, would this result in an out-of-bounds access on conf->disks?
> if (mddev->delta_disks < 0 && mddev->reshape_position == MaxSector) {
> /* We might be able to shrink, but the devices must
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=4
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 05/29] md/raid5: round bitmap stripes with sector division
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (3 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 04/29] md/raid5: reject zero-sector reshape chunks Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:19 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 06/29] md: wait for behind writes before destroying bitmap Yu Kuai
` (24 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
raid5_bitmap_sector_map() aligns the array range to full RAID5 stripe
widths before converting it to component sectors. That width is
chunk_sectors multiplied by the number of data disks, and it is not
always a power of two.
Reproduce with a 4-disk RAID5, 1024-sector chunks, and three data disks.
The full-stripe width is 3072 sectors. For a one-sector write at array
sector 3072, correct rounding gives array range [3072, 6144), which maps
to component range [1024, 2048). The old round_down()/round_up() logic
instead gives [1024, 4096), which maps to [0, 1024).
Use sector_div() based arithmetic so the rounded range is aligned to the
actual RAID5 stripe width.
The deterministic mapper test now reports the fixed component range as
[1024, 2048), while the old mask-based range was [0, 1024).
Fixes: 9c89f604476c ("md/raid5: implement pers->bitmap_sector()")
Reported-by: Mykola Marzhan <mykola@meshstor.io>
Link: https://lore.kernel.org/all/20260726185916.2223460-1-mykola@meshstor.io/
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/raid5.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index d128d238e1da..2cc2546a29ae 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6029,8 +6029,11 @@ static void raid5_bitmap_sector(struct mddev *mddev, sector_t *offset,
sectors_per_chunk = conf->chunk_sectors *
(conf->raid_disks - conf->max_degraded);
- start = round_down(start, sectors_per_chunk);
- end = round_up(end, sectors_per_chunk);
+ sector_div(start, sectors_per_chunk);
+ start *= sectors_per_chunk;
+ if (sector_div(end, sectors_per_chunk))
+ end++;
+ end *= sectors_per_chunk;
start = raid5_compute_sector(conf, start, 0, &dd_idx, NULL);
end = raid5_compute_sector(conf, end, 0, &dd_idx, NULL);
@@ -6048,8 +6051,10 @@ static void raid5_bitmap_sector(struct mddev *mddev, sector_t *offset,
sectors_per_chunk = conf->prev_chunk_sectors *
(conf->previous_raid_disks - conf->max_degraded);
- prev_start = round_down(prev_start, sectors_per_chunk);
- prev_end = round_down(prev_end, sectors_per_chunk);
+ sector_div(prev_start, sectors_per_chunk);
+ prev_start *= sectors_per_chunk;
+ sector_div(prev_end, sectors_per_chunk);
+ prev_end *= sectors_per_chunk;
prev_start = raid5_compute_sector(conf, prev_start, 1, &dd_idx, NULL);
prev_end = raid5_compute_sector(conf, prev_end, 1, &dd_idx, NULL);
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 05/29] md/raid5: round bitmap stripes with sector division
2026-08-02 19:50 ` [PATCH v5 05/29] md/raid5: round bitmap stripes with sector division Yu Kuai
@ 2026-08-02 20:19 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:19 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] `prev_end` is incorrectly rounded down instead of up during a reshape, potentially causing silent data corruption.
--
--- Patch [5]: [PATCH v5 05/29] md/raid5: round bitmap stripes with sector division ---
commit ecc755c8b2e63f314261ab6ac1c1f566e26c875a
Author: Yu Kuai <yukuai@fygo.io>
md/raid5: round bitmap stripes with sector division
This commit changes the rounding logic in raid5_bitmap_sector() and
raid5_bitmap_sector_map() to use sector_div() rather than round_down()
and round_up(). This aligns the array ranges to actual RAID5 stripe widths
that are not always a power of two.
Link: https://lore.kernel.org/all/20260726185916.2223460-1-mykola@meshstor.io/
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -6048,8 +6051,10 @@ static void raid5_bitmap_sector(struct mddev *mddev, sector_t *offset,
>
> sectors_per_chunk = conf->prev_chunk_sectors *
> (conf->previous_raid_disks - conf->max_degraded);
> - prev_start = round_down(prev_start, sectors_per_chunk);
> - prev_end = round_down(prev_end, sectors_per_chunk);
> + sector_div(prev_start, sectors_per_chunk);
> + prev_start *= sectors_per_chunk;
> + sector_div(prev_end, sectors_per_chunk);
> + prev_end *= sectors_per_chunk;
[Severity: Critical]
This is a pre-existing issue, but does rounding down prev_end leave the
tail of the IO unmarked in the bitmap for the previous layout?
If an active reshape reduces the RAID5 stripe width and a write request
spans across the boundary of an old chunk, it appears this code calculates
an incorrectly small component end sector.
If the max(end, prev_end) fallback fails to cover the affected component
sectors for the old layout, could they be left unmarked in the bitmap? A
system crash at this moment might skip rebuilding those dirty sectors upon
recovery, leading to inconsistent parity or outdated data.
Should this translation to sector_div() also include logic to round up
prev_end, similar to how end is handled in raid5_bitmap_sector_map()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=5
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 06/29] md: wait for behind writes before destroying bitmap
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (4 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 05/29] md/raid5: round bitmap stripes with sector division Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:40 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 07/29] md: avoid stale clone I/O accounting timestamps Yu Kuai
` (23 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
__md_stop() destroyed the bitmap before calling mddev_detach(). That made
mddev_detach() skip bitmap_ops->wait_behind_writes(), because the bitmap
was already disconnected from mddev.
This was still safe for the legacy bitmap because bitmap_destroy() waits
for behind writes itself. llbitmap keeps that wait in its
->wait_behind_writes() operation instead, while ->destroy() tears down the
llbitmap storage. With the old ordering, RAID1 behind-write completions
could still run after llbitmap storage had been freed.
Call mddev_detach() before md_bitmap_destroy() so the common detach path
can wait for behind writes while the bitmap is still alive. Only destroy
the bitmap after those users are gone.
Fixes: 5ab829f1971d ("md/md-llbitmap: introduce new lockless bitmap")
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 51b620edbef7..b61040315aef 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7085,8 +7085,8 @@ static void __md_stop(struct mddev *mddev)
{
struct md_personality *pers = mddev->pers;
- md_bitmap_destroy(mddev);
mddev_detach(mddev);
+ md_bitmap_destroy(mddev);
spin_lock(&mddev->lock);
mddev->pers = NULL;
spin_unlock(&mddev->lock);
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 06/29] md: wait for behind writes before destroying bitmap
2026-08-02 19:50 ` [PATCH v5 06/29] md: wait for behind writes before destroying bitmap Yu Kuai
@ 2026-08-02 20:40 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:40 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Dynamic removal of llbitmap via sysfs or ioctl does not wait for behind writes, leading to a NULL pointer dereference.
- [High] Teardown asymmetry in llbitmap_destroy allows the pending_timer to be re-armed after deletion, leading to a Use-After-Free of llbitmap.
- [High] Lockless TOCTOU race on `mddev->bitmap_ops` leading to NULL pointer dereference.
--
--- Patch [6]: [PATCH v5 06/29] md: wait for behind writes before destroying bitmap ---
commit 908390619225b153696b5648f3002688751593a6
Author: Yu Kuai <yukuai@fygo.io>
md: wait for behind writes before destroying bitmap
__md_stop() previously destroyed the bitmap before calling mddev_detach(),
skipping the wait for behind writes. This commit reorders them to call
mddev_detach() first, ensuring behind writes are waited on while the
bitmap is still alive.
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 51b620edbef75..b61040315aef6 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -7085,8 +7085,8 @@ static void __md_stop(struct mddev *mddev)
> {
> struct md_personality *pers = mddev->pers;
>
> - md_bitmap_destroy(mddev);
> mddev_detach(mddev);
> + md_bitmap_destroy(mddev);
[Severity: High]
This is a pre-existing issue, but does dynamic removal of the bitmap via
sysfs fail to wait for behind writes?
When a user writes to bitmap/location to clear the bitmap dynamically,
location_store() calls mddev_suspend_and_lock() to suspend the array.
mddev_suspend() waits for active_io to drop to 0. For RAID1 behind writes,
active_io is dropped by md_end_clone_io() when non-writemostly devices
complete their writes:
drivers/md/md.c:md_handle_request() {
...
percpu_ref_put(&mddev->active_io);
...
}
This means behind writes to writemostly devices can still be in flight.
location_store() then proceeds to free the bitmap:
drivers/md/md-bitmap.c:location_store() {
...
md_bitmap_destroy_nosysfs(mddev);
...
}
When behind writes finally complete, could close_write() dereference the
now NULL bitmap_ops pointer?
drivers/md/raid1.c:close_write() {
...
mddev->bitmap_ops->end_behind_write(mddev);
...
}
[Severity: High]
This is a pre-existing issue, but can the llbitmap pending_timer be re-armed
after it is deleted during destruction?
llbitmap_destroy() deletes the timer and flushes the workqueue:
drivers/md/md-llbitmap.c:llbitmap_destroy() {
...
timer_delete_sync(&llbitmap->pending_timer);
flush_workqueue(md_llbitmap_io_wq);
...
}
If the timer expires right before deletion, the handler queues
md_llbitmap_daemon_fn() to the workqueue. If unexpired dirty pages exist,
the daemon function evaluates time_before(jiffies, pctl->expire). Since
llbitmap_resume() sets pctl->expire to LONG_MAX, this evaluates to true,
setting restart to true. The timer is then unconditionally re-armed:
drivers/md/md-llbitmap.c:md_llbitmap_daemon_fn() {
...
if (restart)
mod_timer(&llbitmap->pending_timer, ...);
...
}
The flush_workqueue() call in llbitmap_destroy() will wait for this work
to finish, but it does not prevent the timer from being re-armed. Could
this leave the timer armed with a freed llbitmap pointer, causing a
use-after-free when it fires?
[Severity: High]
This isn't a bug introduced by this patch, but is there a TOCTOU race on
mddev->bitmap_ops between the teardown path and concurrent readers?
In the background thread, md_check_recovery() checks md_bitmap_enabled(),
which locklessly verifies mddev->bitmap_ops is not NULL:
drivers/md/md.c:md_check_recovery() {
...
if (md_bitmap_enabled(mddev, false) && mddev->bitmap_ops->daemon_work)
mddev->bitmap_ops->daemon_work(mddev);
...
}
Concurrently, a write to bitmap/location via sysfs triggers location_store(),
which acquires reconfig_mutex (but not mddev->lock or bitmap_info.mutex)
and calls md_bitmap_destroy_nosysfs(), unconditionally setting the ops
pointer to NULL:
drivers/md/md.c:md_bitmap_destroy_nosysfs() {
...
mddev->bitmap_ops = NULL;
...
}
Could md_check_recovery() proceed to dereference the now NULL pointer? A
similar race appears to exist with md_seq_show() reading /proc/mdstat,
which holds locks not respected by the sysfs teardown path:
drivers/md/md.c:md_seq_show() {
...
md_bitmap_status(seq, mddev);
...
}
> spin_lock(&mddev->lock);
> mddev->pers = NULL;
> spin_unlock(&mddev->lock);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=6
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 07/29] md: avoid stale clone I/O accounting timestamps
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (5 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 06/29] md: wait for behind writes before destroying bitmap Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:45 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 08/29] md/md-llbitmap: prevent create failure bitmap UAF Yu Kuai
` (22 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
md_clone_bio() always allocates the clone from mddev->io_clone_set, even
when queue I/O stats are disabled. In that case it does not call
bio_start_io_acct(), but it also left md_io_clone->start_time untouched.
The clone private data comes from a mempool and can contain data from a
previous user. md_end_clone_io() checks start_time to decide whether it
needs to call bio_end_io_acct(), so a stale non-zero value can make the
completion path end accounting that was never started for this bio.
Set start_time to 0 in the no-stats branch. This keeps the end path tied
to whether bio_start_io_acct() actually ran.
Fixes: c687297b8845 ("md: also clone new io if io accounting is disabled")
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index b61040315aef..58fb5453a819 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9448,6 +9448,8 @@ static void md_clone_bio(struct mddev *mddev, struct bio **bio)
md_io_clone->mddev = mddev;
if (blk_queue_io_stat(bdev->bd_disk->queue))
md_io_clone->start_time = bio_start_io_acct(*bio);
+ else
+ md_io_clone->start_time = 0;
if (bio_data_dir(*bio) == WRITE && md_bitmap_enabled(mddev, false)) {
md_io_clone->offset = (*bio)->bi_iter.bi_sector;
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 07/29] md: avoid stale clone I/O accounting timestamps
2026-08-02 19:50 ` [PATCH v5 07/29] md: avoid stale clone I/O accounting timestamps Yu Kuai
@ 2026-08-02 20:45 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:45 UTC (permalink / raw)
To: Yu Kuai; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Permanent in-flight I/O counter leak when jiffies wraps to 0.
--
--- Patch [7]: [PATCH v5 07/29] md: avoid stale clone I/O accounting timestamps ---
commit 3831c24d41ef696f06d6f215b39197db739a0831
Author: Yu Kuai <yukuai@fygo.io>
md: avoid stale clone I/O accounting timestamps
This commit avoids stale clone I/O accounting timestamps by explicitly
setting start_time to 0 when queue I/O stats are disabled. This prevents
the completion path in md_end_clone_io() from using a stale non-zero value
from a pooled bio and performing accounting that was never started.
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -9448,6 +9448,8 @@ static void md_clone_bio(struct mddev *mddev, struct bio **bio)
> md_io_clone->mddev = mddev;
> if (blk_queue_io_stat(bdev->bd_disk->queue))
> md_io_clone->start_time = bio_start_io_acct(*bio);
> + else
> + md_io_clone->start_time = 0;
[Severity: High]
This is a pre-existing issue, but does using 0 as a sentinel value here
create a permanent in-flight I/O counter leak when jiffies wraps to 0?
In bio_start_io_acct(), the returned value is the current jiffies. On 32-bit
systems, jiffies is initialized to -300*HZ and naturally wraps to 0 exactly
five minutes after boot. If a bio is cloned exactly at the tick where
jiffies is 0, start_time will be recorded as 0 even though accounting
was started.
When completion occurs in md_end_clone_io(), the check sees 0:
if (md_io_clone->start_time)
bio_end_io_acct(orig_bio, md_io_clone->start_time);
This evaluates to false, bypassing the end accounting and leaving the
in-flight counter permanently incremented. This can cause operations that
wait for a queue freeze, such as suspend or device removal, to hang
indefinitely.
Would it be safer to use a separate boolean flag to track if accounting was
started, rather than overloading start_time?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=7
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 08/29] md/md-llbitmap: prevent create failure bitmap UAF
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (6 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 07/29] md: avoid stale clone I/O accounting timestamps Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:39 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 09/29] md/md-llbitmap: stop daemon timer rearm on destroy Yu Kuai
` (21 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
llbitmap_create() publishes mddev->bitmap before reading the bitmap
superblock. This is needed because llbitmap_read_sb() can initialize a
new bitmap and flush it through helpers that use mddev->bitmap.
If llbitmap_read_sb() fails, the old cleanup dropped bitmap_info.mutex
and freed llbitmap before clearing mddev->bitmap. Readers such as
/proc/mdstat rely on bitmap_info.mutex to keep the bitmap pointer stable
while collecting bitmap stats, so they could observe the stale pointer
after the failed create path released the mutex.
Clear mddev->bitmap while still holding bitmap_info.mutex, then free the
failed llbitmap after dropping the mutex. This makes mutex-protected
readers see either a live bitmap or no bitmap.
Fixes: 5ab829f1971d ("md/md-llbitmap: introduce new lockless bitmap")
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 948bf64c5ad2..af80a630bd21 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1126,10 +1126,11 @@ static int llbitmap_create(struct mddev *mddev)
mutex_lock(&mddev->bitmap_info.mutex);
mddev->bitmap = llbitmap;
ret = llbitmap_read_sb(llbitmap);
+ if (ret)
+ mddev->bitmap = NULL;
mutex_unlock(&mddev->bitmap_info.mutex);
if (ret) {
kfree(llbitmap);
- mddev->bitmap = NULL;
}
return ret;
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 08/29] md/md-llbitmap: prevent create failure bitmap UAF
2026-08-02 19:50 ` [PATCH v5 08/29] md/md-llbitmap: prevent create failure bitmap UAF Yu Kuai
@ 2026-08-02 20:39 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:39 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Early publication of `mddev->bitmap` in `llbitmap_create()` races with lockless deferred IO (unplug callbacks), leading to UAF or NULL pointer dereference.
- [Medium] Improper cleanup sequence in `llbitmap_read_sb()`: `__free_page()` is called before `kunmap_local()`.
--
--- Patch [8]: [PATCH v5 08/29] md/md-llbitmap: prevent create failure bitmap UAF ---
commit f0da60fcfbc2742e91e414951699e4a59d581559
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: prevent create failure bitmap UAF
llbitmap_create() publishes mddev->bitmap before reading the bitmap
superblock. If llbitmap_read_sb() fails, the old cleanup dropped
bitmap_info.mutex and freed llbitmap before clearing mddev->bitmap. This
commit clears mddev->bitmap while still holding the mutex.
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 948bf64c5ad24..af80a630bd21a 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1126,10 +1126,11 @@ static int llbitmap_create(struct mddev *mddev)
> mutex_lock(&mddev->bitmap_info.mutex);
> mddev->bitmap = llbitmap;
> ret = llbitmap_read_sb(llbitmap);
[Severity: Medium]
This is a pre-existing issue, but is the cleanup sequence in
llbitmap_read_sb() correct when returning an error?
When llbitmap_read_sb() encounters an error, it jumps to out_put_page
and calls __free_page() before kunmap_local():
drivers/md/md-llbitmap.c:llbitmap_read_sb() {
...
out_put_page:
__free_page(sb_page);
kunmap_local(sb);
return ret;
}
Does calling __free_page() before kunmap_local() violate the kmap_local API
contract and potentially cause memory debugging warnings?
> + if (ret)
> + mddev->bitmap = NULL;
> mutex_unlock(&mddev->bitmap_info.mutex);
[Severity: High]
This is a pre-existing issue, but does publishing and clearing mddev->bitmap
here still leave a race with lockless deferred IO?
If mddev->bitmap is set to NULL on failure, but mddev->bitmap_ops is still
active, a concurrent unplug callback executing locklessly might trigger a
NULL pointer dereference.
For example, if a user task unplugs and calls raid1_prepare_flush_writes(),
it can execute mddev->bitmap_ops->unplug() locklessly. llbitmap_unplug()
unconditionally reads mddev->bitmap and passes it to llbitmap_dirty():
drivers/md/md-llbitmap.c:llbitmap_unplug() {
struct llbitmap *llbitmap = mddev->bitmap;
if (!llbitmap_dirty(llbitmap))
return;
...
}
Can llbitmap_unplug() crash if it observes the NULL bitmap?
Also, if llbitmap_unplug() runs during llbitmap_cache_pages(), can it
observe llbitmap->nr_pages being set before the llbitmap->pctl[i]
pointers are initialized, leading to a NULL dereference in
llbitmap_dirty() when it accesses llbitmap->pctl[i]->flags?
drivers/md/md-llbitmap.c:llbitmap_cache_pages() {
...
llbitmap->nr_pages = nr_pages;
for (i = 0; i < nr_pages; i++, pctl = (void *)pctl + size) {
struct page *page = llbitmap_read_page(llbitmap, i);
llbitmap->pctl[i] = pctl;
...
}
> if (ret) {
> kfree(llbitmap);
> - mddev->bitmap = NULL;
> }
>
> return ret;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=8
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 09/29] md/md-llbitmap: stop daemon timer rearm on destroy
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (7 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 08/29] md/md-llbitmap: prevent create failure bitmap UAF Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:19 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 10/29] md: skip bitmap accounting for empty write ranges Yu Kuai
` (20 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
llbitmap_destroy() deletes pending_timer before flushing
md_llbitmap_io_wq. However, daemon_work can still be queued or running
after the timer has been deleted, and the daemon path can arm
pending_timer again when it finds dirty chunks that are not ready to
flush yet.
If that happens during teardown, pending_timer can remain armed after
llbitmap is freed and later dereference freed memory.
Add a BITMAP_SHUTDOWN bit to llbitmap->flags, set it before deleting
the timer, and make the timer and daemon paths stop queueing or rearming
work once teardown starts. Cancel daemon_work before flushing the shared
workqueue so no already queued daemon instance can race with the free.
BITMAP_SHUTDOWN is a runtime-only state. Mask it out when reading and
updating the llbitmap superblock so the shutdown state is never loaded
from disk or persisted to disk.
Fixes: 5ab829f1971d ("md/md-llbitmap: introduce new lockless bitmap")
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-bitmap.h | 1 +
drivers/md/md-llbitmap.c | 15 ++++++++++++---
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index 214f623c7e79..890276d9c66e 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -29,6 +29,7 @@ enum bitmap_state {
BITMAP_FIRST_USE = 3, /* llbitmap is just created */
BITMAP_CLEAN = 4, /* llbitmap is created with assume_clean */
BITMAP_DAEMON_BUSY = 5, /* llbitmap daemon is not finished after daemon_sleep */
+ BITMAP_SHUTDOWN = 6, /* llbitmap is being destroyed */
BITMAP_HOSTENDIAN =15,
};
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index af80a630bd21..3ec5b5985d48 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -789,6 +789,7 @@ static enum llbitmap_state llbitmap_state_machine(struct llbitmap *llbitmap,
if (state == BitNeedSync || state == BitNeedSyncUnwritten)
need_resync = !mddev->degraded;
else if (state == BitDirty &&
+ !test_bit(BITMAP_SHUTDOWN, &llbitmap->flags) &&
!timer_pending(&llbitmap->pending_timer))
mod_timer(&llbitmap->pending_timer,
jiffies + mddev->bitmap_info.daemon_sleep * HZ);
@@ -981,7 +982,7 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
else
mddev->bitmap_info.space = mddev->bitmap_info.default_space;
}
- llbitmap->flags = le32_to_cpu(sb->state);
+ llbitmap->flags = le32_to_cpu(sb->state) & ~BIT(BITMAP_SHUTDOWN);
if (test_and_clear_bit(BITMAP_FIRST_USE, &llbitmap->flags)) {
ret = llbitmap_init(llbitmap);
goto out_put_page;
@@ -1037,6 +1038,9 @@ static void llbitmap_pending_timer_fn(struct timer_list *pending_timer)
struct llbitmap *llbitmap =
container_of(pending_timer, struct llbitmap, pending_timer);
+ if (test_bit(BITMAP_SHUTDOWN, &llbitmap->flags))
+ return;
+
if (work_busy(&llbitmap->daemon_work)) {
pr_warn("md/llbitmap: %s daemon_work not finished in %lu seconds\n",
mdname(llbitmap->mddev),
@@ -1057,6 +1061,9 @@ static void md_llbitmap_daemon_fn(struct work_struct *work)
bool restart;
int idx;
+ if (test_bit(BITMAP_SHUTDOWN, &llbitmap->flags))
+ return;
+
if (llbitmap->mddev->degraded)
return;
retry:
@@ -1096,7 +1103,7 @@ static void md_llbitmap_daemon_fn(struct work_struct *work)
goto retry;
/* If some page is dirty but not expired, setup timer again */
- if (restart)
+ if (restart && !test_bit(BITMAP_SHUTDOWN, &llbitmap->flags))
mod_timer(&llbitmap->pending_timer,
jiffies + llbitmap->mddev->bitmap_info.daemon_sleep * HZ);
}
@@ -1179,7 +1186,9 @@ static void llbitmap_destroy(struct mddev *mddev)
mutex_lock(&mddev->bitmap_info.mutex);
+ set_bit(BITMAP_SHUTDOWN, &llbitmap->flags);
timer_delete_sync(&llbitmap->pending_timer);
+ cancel_work_sync(&llbitmap->daemon_work);
flush_workqueue(md_llbitmap_io_wq);
flush_workqueue(md_llbitmap_unplug_wq);
@@ -1523,7 +1532,7 @@ static void llbitmap_update_sb(void *data)
sb = kmap_local_page(sb_page);
sb->events = cpu_to_le64(mddev->events);
- sb->state = cpu_to_le32(llbitmap->flags);
+ sb->state = cpu_to_le32(llbitmap->flags & ~BIT(BITMAP_SHUTDOWN));
sb->chunksize = cpu_to_le32(llbitmap->chunksize);
sb->sync_size = cpu_to_le64(mddev->resync_max_sectors);
sb->events_cleared = cpu_to_le64(llbitmap->events_cleared);
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 09/29] md/md-llbitmap: stop daemon timer rearm on destroy
2026-08-02 19:50 ` [PATCH v5 09/29] md/md-llbitmap: stop daemon timer rearm on destroy Yu Kuai
@ 2026-08-02 20:19 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:19 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Time-of-check to time-of-use (TOCTOU) race in the teardown path allows the timer to be re-armed after deletion, causing a use-after-free.
--
--- Patch [9]: [PATCH v5 09/29] md/md-llbitmap: stop daemon timer rearm on destroy ---
commit ea383c856bd9022c7e57c7a025fcf8fb539acf1e
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: stop daemon timer rearm on destroy
This commit attempts to fix a race where the daemon timer is re-armed
during array teardown. It adds a BITMAP_SHUTDOWN flag, sets it before
deleting the timer and canceling the daemon work, and checks the flag
before queuing or rearming the work.
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
[ ... ]
> @@ -1096,7 +1103,7 @@ static void md_llbitmap_daemon_fn(struct work_struct *work)
> goto retry;
>
> /* If some page is dirty but not expired, setup timer again */
> - if (restart)
> + if (restart && !test_bit(BITMAP_SHUTDOWN, &llbitmap->flags))
> mod_timer(&llbitmap->pending_timer,
> jiffies + llbitmap->mddev->bitmap_info.daemon_sleep * HZ);
> }
[ ... ]
> @@ -1179,7 +1186,9 @@ static void llbitmap_destroy(struct mddev *mddev)
>
> mutex_lock(&mddev->bitmap_info.mutex);
>
> + set_bit(BITMAP_SHUTDOWN, &llbitmap->flags);
> timer_delete_sync(&llbitmap->pending_timer);
> + cancel_work_sync(&llbitmap->daemon_work);
> flush_workqueue(md_llbitmap_io_wq);
> flush_workqueue(md_llbitmap_unplug_wq);
[Severity: High]
This isn't a bug introduced by this patch, but does this teardown sequence
still leave a race condition that can result in a use-after-free?
Since the BITMAP_SHUTDOWN check and mod_timer() in md_llbitmap_daemon_fn()
are not atomic, a worker can evaluate BITMAP_SHUTDOWN as false but get
preempted before arming the timer.
Concurrently, llbitmap_destroy() sets BITMAP_SHUTDOWN, calls
timer_delete_sync() (which returns immediately if the timer isn't pending),
and blocks in cancel_work_sync().
When the preempted worker resumes, it executes mod_timer(), re-arming the
timer. Once the worker finishes, cancel_work_sync() unblocks and
llbitmap_destroy() frees llbitmap, leaving the timer armed and able to
access freed memory when it fires.
Would replacing timer_delete_sync() with timer_shutdown_sync() safely
prevent any subsequent mod_timer() calls from succeeding?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=9
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 10/29] md: skip bitmap accounting for empty write ranges
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (8 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 09/29] md/md-llbitmap: stop daemon timer rearm on destroy Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 19:50 ` [PATCH v5 11/29] md: add helper to split bios at reshape offset Yu Kuai
` (19 subsequent siblings)
29 siblings, 0 replies; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
mkfs.ext4 can submit zero-sector flush/FUA bios. These bios are WRITE
bios for md_write_start() purposes, but they do not cover any data sector
and must not dirty bitmap bits.
md bitmap accounting currently passes such bios to bitmap start_write().
For llbitmap this reaches llbitmap_start_write() with sectors == 0,
which underflows the end chunk calculation.
Personality bitmap mapping can also turn a non-empty bio into an empty
bitmap range when the requested sectors are outside the active bitmap
geometry. Treat both cases as not started, so the completion path will not
call end_write() for an empty range.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 58fb5453a819..f88952371b9b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9399,6 +9399,8 @@ static void md_bitmap_start(struct mddev *mddev,
mddev->pers->bitmap_sector(mddev, &md_io_clone->offset,
&md_io_clone->sectors);
+ if (!md_io_clone->sectors)
+ return;
fn(mddev, md_io_clone->offset, md_io_clone->sectors);
}
@@ -9419,7 +9421,8 @@ static void md_end_clone_io(struct bio *bio)
struct mddev *mddev = md_io_clone->mddev;
struct completion *reshape_completion = bio->bi_private;
- if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
+ if (bio_data_dir(orig_bio) == WRITE && md_io_clone->sectors &&
+ md_bitmap_enabled(mddev, false))
md_bitmap_end(mddev, md_io_clone);
if (bio->bi_status && !orig_bio->bi_status)
@@ -9446,12 +9449,14 @@ static void md_clone_bio(struct mddev *mddev, struct bio **bio)
md_io_clone = container_of(clone, struct md_io_clone, bio_clone);
md_io_clone->orig_bio = *bio;
md_io_clone->mddev = mddev;
+ md_io_clone->sectors = 0;
if (blk_queue_io_stat(bdev->bd_disk->queue))
md_io_clone->start_time = bio_start_io_acct(*bio);
else
md_io_clone->start_time = 0;
- if (bio_data_dir(*bio) == WRITE && md_bitmap_enabled(mddev, false)) {
+ if (bio_data_dir(*bio) == WRITE && bio_sectors(*bio) &&
+ md_bitmap_enabled(mddev, false)) {
md_io_clone->offset = (*bio)->bi_iter.bi_sector;
md_io_clone->sectors = bio_sectors(*bio);
md_io_clone->rw = op_stat_group(bio_op(*bio));
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v5 11/29] md: add helper to split bios at reshape offset
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (9 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 10/29] md: skip bitmap accounting for empty write ranges Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:19 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 12/29] md: add exact bitmap mapping and reshape hooks Yu Kuai
` (18 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Add mddev_bio_split_at_reshape_offset() so personalities can share
reshape-offset bio splitting instead of open-coding the same boundary
handling in multiple places.
The helper first applies the optional max_sectors limit. If reshape is
running and the bio crosses reshape_position, it further limits the front
bio to the current reshape boundary so callers can account and submit one
side of the reshape at a time.
Snapshot reshape_position with READ_ONCE(). RAID5 and RAID10 update this
field as reshape progresses, while the I/O path only needs one consistent
decision point for the current bio. Using an explicit single load avoids a
plain lockless access and prevents the compiler from refetching a different
boundary while deciding whether and where to split.
When a split is needed, bio_submit_split_bioset() submits the remainder and
returns the front bio. Callers must therefore continue processing the
returned bio, not the original pointer.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md.c | 39 +++++++++++++++++++++++++++++++++++++++
drivers/md/md.h | 4 ++++
2 files changed, 43 insertions(+)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index f88952371b9b..f0eecdfff1cc 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9388,6 +9388,45 @@ void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
}
EXPORT_SYMBOL_GPL(md_submit_discard_bio);
+struct bio *mddev_bio_split_at_reshape_offset(struct mddev *mddev,
+ struct bio *bio,
+ unsigned int *max_sectors,
+ struct bio_set *bs)
+{
+ sector_t boundary;
+ sector_t start;
+ sector_t end;
+ unsigned int split_sectors;
+
+ split_sectors = bio_sectors(bio);
+ if (max_sectors && *max_sectors && *max_sectors < split_sectors)
+ split_sectors = *max_sectors;
+
+ if (!test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery))
+ goto split;
+
+ boundary = READ_ONCE(mddev->reshape_position);
+ start = bio->bi_iter.bi_sector;
+ end = bio_end_sector(bio);
+ if (start >= boundary || end <= boundary)
+ goto split;
+
+ if (boundary - start < split_sectors)
+ split_sectors = boundary - start;
+
+split:
+ if (max_sectors)
+ *max_sectors = split_sectors;
+ if (split_sectors < bio_sectors(bio)) {
+ bio = bio_submit_split_bioset(bio, split_sectors, bs);
+ if (bio)
+ bio->bi_opf |= REQ_NOMERGE;
+ }
+
+ return bio;
+}
+EXPORT_SYMBOL_GPL(mddev_bio_split_at_reshape_offset);
+
static void md_bitmap_start(struct mddev *mddev,
struct md_io_clone *md_io_clone)
{
diff --git a/drivers/md/md.h b/drivers/md/md.h
index bb2eb5f39914..8146a6f50a7d 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -920,6 +920,10 @@ extern void md_error(struct mddev *mddev, struct md_rdev *rdev);
extern void md_finish_reshape(struct mddev *mddev);
void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
struct bio *bio, sector_t start, sector_t size);
+struct bio *mddev_bio_split_at_reshape_offset(struct mddev *mddev,
+ struct bio *bio,
+ unsigned int *max_sectors,
+ struct bio_set *bs);
void md_account_bio(struct mddev *mddev, struct bio **bio);
extern bool __must_check md_flush_request(struct mddev *mddev, struct bio *bio);
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 11/29] md: add helper to split bios at reshape offset
2026-08-02 19:50 ` [PATCH v5 11/29] md: add helper to split bios at reshape offset Yu Kuai
@ 2026-08-02 20:19 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:19 UTC (permalink / raw)
To: Yu Kuai; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Torn read of 64-bit reshape_position on 32-bit architectures leads to silent data corruption.
--
--- Patch [11]: [PATCH v5 11/29] md: add helper to split bios at reshape offset ---
commit 7816e7809a77baa291a2c8013717413f881d0cdc
Author: Yu Kuai <yukuai@fygo.io>
md: add helper to split bios at reshape offset
This commit adds mddev_bio_split_at_reshape_offset() to allow md
personalities to share reshape-offset bio splitting. It snapshots
mddev->reshape_position to determine if the bio crosses the reshape
boundary and limits the front bio accordingly.
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
[ ... ]
> @@ -9388,6 +9388,45 @@ void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
> +struct bio *mddev_bio_split_at_reshape_offset(struct mddev *mddev,
> + struct bio *bio,
> + unsigned int *max_sectors,
> + struct bio_set *bs)
> +{
> + sector_t boundary;
> + sector_t start;
> + sector_t end;
> + unsigned int split_sectors;
> +
> + split_sectors = bio_sectors(bio);
> + if (max_sectors && *max_sectors && *max_sectors < split_sectors)
> + split_sectors = *max_sectors;
> +
> + if (!test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery))
> + goto split;
> +
> + boundary = READ_ONCE(mddev->reshape_position);
[Severity: Critical]
Is it safe to use READ_ONCE() on a 64-bit value here?
On 32-bit architectures, sector_t is 64-bit, so READ_ONCE() will generate two
non-atomic 32-bit memory load instructions. If the reshape thread updates
mddev->reshape_position concurrently, this could result in a torn read.
A torn read would make boundary completely incorrect, causing split_sectors
to be miscalculated. Because the reshape boundary separates two different
physical data layouts, could sending IO based on a torn boundary write data
to the wrong physical disk locations and cause data corruption?
> + start = bio->bi_iter.bi_sector;
> + end = bio_end_sector(bio);
> + if (start >= boundary || end <= boundary)
> + goto split;
> +
> + if (boundary - start < split_sectors)
> + split_sectors = boundary - start;
> +
> +split:
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=11
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 12/29] md: add exact bitmap mapping and reshape hooks
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (10 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 11/29] md: add helper to split bios at reshape offset Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 19:50 ` [PATCH v5 13/29] md/md-llbitmap: track bitmap sync_size explicitly Yu Kuai
` (17 subsequent siblings)
29 siblings, 0 replies; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Add bitmap mapping and reshape hooks needed by llbitmap reshape
support without teaching md core to account a single bio against
multiple bitmap ranges.
This also adds the old/new bitmap geometry helpers used by
personalities to describe reshape mapping to llbitmap.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-bitmap.c | 8 ++++++++
drivers/md/md-bitmap.h | 8 ++++++++
drivers/md/md-llbitmap.c | 8 ++++++++
drivers/md/md.c | 11 ++++++++---
drivers/md/md.h | 4 ++++
5 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 7e4fbca93ccb..b7f0d4acce04 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -1730,6 +1730,13 @@ static void bitmap_start_write(struct mddev *mddev, sector_t offset,
}
}
+static void bitmap_prepare_range(struct mddev *mddev, sector_t *offset,
+ unsigned long *sectors)
+{
+ if (mddev->pers->bitmap_sector)
+ mddev->pers->bitmap_sector(mddev, offset, sectors);
+}
+
static void bitmap_end_write(struct mddev *mddev, sector_t offset,
unsigned long sectors)
{
@@ -3081,6 +3088,7 @@ static struct bitmap_operations bitmap_ops = {
.flush = bitmap_flush,
.write_all = bitmap_write_all,
.dirty_bits = bitmap_dirty_bits,
+ .prepare_range = bitmap_prepare_range,
.unplug = bitmap_unplug,
.daemon_work = bitmap_daemon_work,
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index 890276d9c66e..6478cf9d8816 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -94,6 +94,14 @@ struct bitmap_operations {
void (*write_all)(struct mddev *mddev);
void (*dirty_bits)(struct mddev *mddev, unsigned long s,
unsigned long e);
+ /* Prepare a range for this bitmap implementation. */
+ void (*prepare_range)(struct mddev *mddev,
+ sector_t *offset,
+ unsigned long *sectors);
+ void (*reshape_finish)(struct mddev *mddev);
+ int (*reshape_can_start)(struct mddev *mddev);
+ void (*reshape_mark)(struct mddev *mddev, sector_t old_pos,
+ sector_t new_pos);
void (*unplug)(struct mddev *mddev, bool sync);
void (*daemon_work)(struct mddev *mddev);
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 3ec5b5985d48..4583bbc37c2e 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1198,6 +1198,13 @@ static void llbitmap_destroy(struct mddev *mddev)
mutex_unlock(&mddev->bitmap_info.mutex);
}
+static void llbitmap_prepare_range(struct mddev *mddev, sector_t *offset,
+ unsigned long *sectors)
+{
+ if (mddev->pers->bitmap_sector)
+ mddev->pers->bitmap_sector(mddev, offset, sectors);
+}
+
static void llbitmap_start_write(struct mddev *mddev, sector_t offset,
unsigned long sectors)
{
@@ -1789,6 +1796,7 @@ static struct bitmap_operations llbitmap_ops = {
.update_sb = llbitmap_update_sb,
.get_stats = llbitmap_get_stats,
.dirty_bits = llbitmap_dirty_bits,
+ .prepare_range = llbitmap_prepare_range,
.write_all = llbitmap_write_all,
.groups = md_llbitmap_groups,
diff --git a/drivers/md/md.c b/drivers/md/md.c
index f0eecdfff1cc..538ba7bab060 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9427,6 +9427,12 @@ struct bio *mddev_bio_split_at_reshape_offset(struct mddev *mddev,
}
EXPORT_SYMBOL_GPL(mddev_bio_split_at_reshape_offset);
+static void md_bitmap_prepare_range(struct mddev *mddev, sector_t *offset,
+ unsigned long *sectors)
+{
+ mddev->bitmap_ops->prepare_range(mddev, offset, sectors);
+}
+
static void md_bitmap_start(struct mddev *mddev,
struct md_io_clone *md_io_clone)
{
@@ -9434,9 +9440,8 @@ static void md_bitmap_start(struct mddev *mddev,
mddev->bitmap_ops->start_discard :
mddev->bitmap_ops->start_write;
- if (mddev->pers->bitmap_sector)
- mddev->pers->bitmap_sector(mddev, &md_io_clone->offset,
- &md_io_clone->sectors);
+ md_bitmap_prepare_range(mddev, &md_io_clone->offset,
+ &md_io_clone->sectors);
if (!md_io_clone->sectors)
return;
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 8146a6f50a7d..b6d2e8929a0f 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -797,6 +797,10 @@ struct md_personality
/* convert io ranges from array to bitmap */
void (*bitmap_sector)(struct mddev *mddev, sector_t *offset,
unsigned long *sectors);
+ void (*bitmap_sector_map)(struct mddev *mddev, sector_t *offset,
+ unsigned long *sectors, bool previous);
+ sector_t (*bitmap_sync_size)(struct mddev *mddev, bool previous);
+ sector_t (*bitmap_array_sectors)(struct mddev *mddev, bool previous);
};
struct md_sysfs_entry {
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v5 13/29] md/md-llbitmap: track bitmap sync_size explicitly
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (11 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 12/29] md: add exact bitmap mapping and reshape hooks Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:24 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 14/29] md/md-llbitmap: allocate page controls independently Yu Kuai
` (16 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Track llbitmap's own sync_size instead of always using
mddev->resync_max_sectors directly.
This is the minimal bookkeeping needed before llbitmap can track old
and new reshape geometry independently.
Reviewed-by: Su Yue <glass.su@suse.com>
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 4583bbc37c2e..0813cebfbdeb 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -287,6 +287,8 @@ struct llbitmap {
unsigned long chunksize;
/* total number of chunks */
unsigned long chunks;
+ /* total number of sectors tracked by current bitmap geometry */
+ sector_t sync_size;
unsigned long last_end_sync;
/*
* time in seconds that dirty bits will be cleared if the page is not
@@ -919,6 +921,7 @@ static int llbitmap_init(struct llbitmap *llbitmap)
llbitmap->chunkshift = ffz(~chunksize);
llbitmap->chunksize = chunksize;
llbitmap->chunks = chunks;
+ llbitmap->sync_size = blocks;
mddev->bitmap_info.daemon_sleep = DEFAULT_DAEMON_SLEEP;
ret = llbitmap_cache_pages(llbitmap);
@@ -939,6 +942,7 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
unsigned long daemon_sleep;
unsigned long chunksize;
unsigned long events;
+ sector_t sync_size;
struct page *sb_page;
bitmap_super_t *sb;
int ret = -EINVAL;
@@ -988,6 +992,14 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
goto out_put_page;
}
+ sync_size = le64_to_cpu(sb->sync_size);
+ if (!sync_size)
+ sync_size = mddev->resync_max_sectors;
+ if (sync_size > mddev->resync_max_sectors) {
+ pr_err("md/llbitmap: %s: sync_size %llu exceeds array sync size %llu",
+ mdname(mddev), sync_size, mddev->resync_max_sectors);
+ goto out_put_page;
+ }
chunksize = le32_to_cpu(sb->chunksize);
if (!is_power_of_2(chunksize)) {
pr_err("md/llbitmap: %s: chunksize not a power of 2",
@@ -1023,8 +1035,9 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
llbitmap->barrier_idle = DEFAULT_BARRIER_IDLE;
llbitmap->chunksize = chunksize;
- llbitmap->chunks = DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors, chunksize);
+ llbitmap->chunks = DIV_ROUND_UP_SECTOR_T(sync_size, chunksize);
llbitmap->chunkshift = ffz(~chunksize);
+ llbitmap->sync_size = sync_size;
ret = llbitmap_cache_pages(llbitmap);
out_put_page:
@@ -1161,6 +1174,7 @@ static int llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
llbitmap->chunkshift = ffz(~chunksize);
llbitmap->chunksize = chunksize;
llbitmap->chunks = chunks;
+ llbitmap->sync_size = blocks;
return 0;
}
@@ -1541,7 +1555,7 @@ static void llbitmap_update_sb(void *data)
sb->events = cpu_to_le64(mddev->events);
sb->state = cpu_to_le32(llbitmap->flags & ~BIT(BITMAP_SHUTDOWN));
sb->chunksize = cpu_to_le32(llbitmap->chunksize);
- sb->sync_size = cpu_to_le64(mddev->resync_max_sectors);
+ sb->sync_size = cpu_to_le64(llbitmap->sync_size);
sb->events_cleared = cpu_to_le64(llbitmap->events_cleared);
sb->sectors_reserved = cpu_to_le32(mddev->bitmap_info.space);
sb->daemon_sleep = cpu_to_le32(mddev->bitmap_info.daemon_sleep);
@@ -1559,6 +1573,7 @@ static int llbitmap_get_stats(void *data, struct md_bitmap_stats *stats)
stats->missing_pages = 0;
stats->pages = llbitmap->nr_pages;
stats->file_pages = llbitmap->nr_pages;
+ stats->sync_size = llbitmap->sync_size;
stats->behind_writes = atomic_read(&llbitmap->behind_writes);
stats->behind_wait = wq_has_sleeper(&llbitmap->behind_wait);
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 13/29] md/md-llbitmap: track bitmap sync_size explicitly
2026-08-02 19:50 ` [PATCH v5 13/29] md/md-llbitmap: track bitmap sync_size explicitly Yu Kuai
@ 2026-08-02 20:24 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:24 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] Missing bounds check in IO fast paths allows out-of-bounds access to the `llbitmap->pctl` array when writing to regions beyond `llbitmap->sync_size`.
- [High] The `chunksize` validation in `llbitmap_read_sb()` incorrectly checks against `mddev->resync_max_sectors` instead of the newly tracked `sync_size`.
- [High] Strict validation of `sync_size` against `resync_max_sectors` in `llbitmap_read_sb` permanently fails array assembly during shrink operations and crash recovery.
- [Medium] `llbitmap->sync_size` and `chunksize` are modified without consistent locking, causing a data race and potential 64-bit torn reads.
Pre-existing issues:
- [High] `llbitmap_resize` increases tracking limits (`chunks`, `sync_size`) but fails to reallocate the `llbitmap->pctl` array to accommodate the new capacity.
--
--- Patch [13]: [PATCH v5 13/29] md/md-llbitmap: track bitmap sync_size explicitly ---
commit 19aa94c506f3423cf682ebddc1ee49bc66629dd1
Author: Yu Kuai <yukuai@fygo.io>
Subject: md/md-llbitmap: track bitmap sync_size explicitly
This patch modifies the llbitmap layer to track its own sync_size explicitly
rather than relying directly on mddev->resync_max_sectors. It updates the
superblock reader and writer to store and load this tracked sync_size. This
lays the groundwork for tracking old and new reshape geometries independently.
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 4583bbc37c2e7..0813cebfbdeb3 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
[ ... ]
> @@ -939,6 +942,7 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
> unsigned long daemon_sleep;
> unsigned long chunksize;
> unsigned long events;
> + sector_t sync_size;
> struct page *sb_page;
> bitmap_super_t *sb;
> int ret = -EINVAL;
[ ... ]
> @@ -988,6 +992,14 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
> goto out_put_page;
> }
>
> + sync_size = le64_to_cpu(sb->sync_size);
> + if (!sync_size)
> + sync_size = mddev->resync_max_sectors;
[Severity: High]
Does decoupling the bitmap geometry from the array size require updating
the chunksize validation check?
In llbitmap_read_sb(), the chunksize validation still checks if it is large
enough using mddev->resync_max_sectors instead of the newly tracked
sync_size:
if (chunksize < DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors,
mddev->bitmap_info.space << SECTOR_SHIFT)) {
pr_err("md/llbitmap: %s: chunksize too small %lu < %llu / %lu",
If an array was recently expanded, could the old geometry's chunksize
falsely appear too small for the new array size, breaking array assembly?
> + if (sync_size > mddev->resync_max_sectors) {
> + pr_err("md/llbitmap: %s: sync_size %llu exceeds array sync size %llu",
> + mdname(mddev), sync_size, mddev->resync_max_sectors);
> + goto out_put_page;
> + }
[Severity: High]
Will this strict check break array assembly during a crash recovery?
If an array is shrunk and the system crashes after the array superblocks
are updated but before the bitmap superblocks are written, the on-disk
bitmap's sync_size will be larger than mddev->resync_max_sectors.
Does permanently rejecting a larger sync_size here prevent the array
from assembling in this scenario?
> chunksize = le32_to_cpu(sb->chunksize);
> if (!is_power_of_2(chunksize)) {
> pr_err("md/llbitmap: %s: chunksize not a power of 2",
[ ... ]
> @@ -1023,8 +1035,9 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
>
> llbitmap->barrier_idle = DEFAULT_BARRIER_IDLE;
> llbitmap->chunksize = chunksize;
> - llbitmap->chunks = DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors, chunksize);
> + llbitmap->chunks = DIV_ROUND_UP_SECTOR_T(sync_size, chunksize);
[Severity: High]
Could a maliciously crafted superblock with a small sync_size cause an
out-of-bounds dereference?
Because chunks is calculated from the untrusted sync_size, the allocated
pctl array will be small. The md core can still issue I/O up to the full
array size (mddev->resync_max_sectors).
Fast paths like llbitmap_start_write() shift the offset to determine the
page_start index without checking against nr_pages:
int page_start = (start + BITMAP_DATA_OFFSET) >> PAGE_SHIFT;
while (page_start <= page_end) {
llbitmap_raise_barrier(llbitmap, page_start);
page_start++;
}
Will this sequence lead to accessing memory past the end of llbitmap->pctl?
> llbitmap->chunkshift = ffz(~chunksize);
> + llbitmap->sync_size = sync_size;
> ret = llbitmap_cache_pages(llbitmap);
>
> out_put_page:
[ ... ]
> @@ -1161,6 +1174,7 @@ static int llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
> llbitmap->chunkshift = ffz(~chunksize);
> llbitmap->chunksize = chunksize;
> llbitmap->chunks = chunks;
> + llbitmap->sync_size = blocks;
[Severity: High]
This is a pre-existing issue, but does llbitmap_resize() safely expand the
tracking capacity?
When the array is expanded, chunks and sync_size are increased, but there
does not appear to be any code to reallocate the underlying llbitmap->pctl
array.
Subsequent I/O to the newly grown sectors maps to an index that exceeds
the originally allocated nr_pages, which might cause an out-of-bounds
memory access.
>
> return 0;
> }
[ ... ]
> @@ -1541,7 +1555,7 @@ static void llbitmap_update_sb(void *data)
> sb->events = cpu_to_le64(mddev->events);
> sb->state = cpu_to_le32(llbitmap->flags & ~BIT(BITMAP_SHUTDOWN));
> sb->chunksize = cpu_to_le32(llbitmap->chunksize);
> - sb->sync_size = cpu_to_le64(mddev->resync_max_sectors);
> + sb->sync_size = cpu_to_le64(llbitmap->sync_size);
[Severity: Medium]
Is there adequate locking for the 64-bit sync_size field?
In llbitmap_resize(), llbitmap->chunksize and llbitmap->sync_size are
updated while holding only mddev->reconfig_mutex.
Concurrently, md_update_sb() can acquire mddev->lock and call
llbitmap_update_sb() without holding reconfig_mutex (such as during
background metadata updates).
Since these accesses do not share a common lock, can llbitmap_update_sb()
experience a torn read of the 64-bit sync_size on 32-bit systems, or read
an inconsistent state where chunksize and sync_size mismatch?
> sb->events_cleared = cpu_to_le64(llbitmap->events_cleared);
> sb->sectors_reserved = cpu_to_le32(mddev->bitmap_info.space);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=13
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 14/29] md/md-llbitmap: allocate page controls independently
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (12 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 13/29] md/md-llbitmap: track bitmap sync_size explicitly Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:27 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 15/29] md/md-llbitmap: grow the page cache in place for reshape Yu Kuai
` (15 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Allocate one llbitmap page-control object at a time and free each
object through the same model.
Let llbitmap_read_page() return a zeroed page without reading disk when
the page index is beyond the current bitmap size, so page-control
allocation no longer needs a separate read_existing flag.
This keeps the llbitmap page-control lifetime self-consistent and
prepares the page-cache code for later in-place growth.
Reviewed-by: Su Yue <glass.su@suse.com>
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 99 +++++++++++++++++++++++++---------------
1 file changed, 62 insertions(+), 37 deletions(-)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 0813cebfbdeb..300dd8b93b01 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -512,13 +512,19 @@ static void llbitmap_write(struct llbitmap *llbitmap, enum llbitmap_state state,
llbitmap_set_page_dirty(llbitmap, idx, bit, false);
}
+static unsigned int llbitmap_used_pages(struct llbitmap *llbitmap,
+ unsigned long chunks)
+{
+ return DIV_ROUND_UP(chunks + BITMAP_DATA_OFFSET, PAGE_SIZE);
+}
+
static struct page *llbitmap_read_page(struct llbitmap *llbitmap, int idx)
{
struct mddev *mddev = llbitmap->mddev;
struct page *page = NULL;
struct md_rdev *rdev;
- if (llbitmap->pctl && llbitmap->pctl[idx])
+ if (llbitmap->pctl && idx < llbitmap->nr_pages && llbitmap->pctl[idx])
page = llbitmap->pctl[idx]->page;
if (page)
return page;
@@ -526,6 +532,8 @@ static struct page *llbitmap_read_page(struct llbitmap *llbitmap, int idx)
page = alloc_page(GFP_NOIO | __GFP_ZERO);
if (!page)
return ERR_PTR(-ENOMEM);
+ if (idx >= llbitmap_used_pages(llbitmap, llbitmap->chunks))
+ return page;
rdev_for_each(rdev, mddev) {
sector_t sector;
@@ -596,61 +604,78 @@ static void llbitmap_free_pages(struct llbitmap *llbitmap)
for (i = 0; i < llbitmap->nr_pages; i++) {
struct llbitmap_page_ctl *pctl = llbitmap->pctl[i];
- if (!pctl || !pctl->page)
- break;
-
- __free_page(pctl->page);
+ if (!pctl)
+ continue;
+ if (pctl->page)
+ __free_page(pctl->page);
percpu_ref_exit(&pctl->active);
+ kfree(pctl);
}
- kfree(llbitmap->pctl[0]);
kfree(llbitmap->pctl);
llbitmap->pctl = NULL;
}
-static int llbitmap_cache_pages(struct llbitmap *llbitmap)
+static struct llbitmap_page_ctl *
+llbitmap_alloc_page_ctl(struct llbitmap *llbitmap, int idx)
{
struct llbitmap_page_ctl *pctl;
- unsigned int nr_pages = DIV_ROUND_UP(llbitmap->chunks +
- BITMAP_DATA_OFFSET, PAGE_SIZE);
+ struct page *page;
unsigned int size = struct_size(pctl, dirty, BITS_TO_LONGS(
llbitmap->blocks_per_page));
- int i;
-
- llbitmap->pctl = kmalloc_array(nr_pages, sizeof(void *),
- GFP_NOIO | __GFP_ZERO);
- if (!llbitmap->pctl)
- return -ENOMEM;
size = round_up(size, cache_line_size());
- pctl = kmalloc_array(nr_pages, size, GFP_NOIO | __GFP_ZERO);
- if (!pctl) {
- kfree(llbitmap->pctl);
- return -ENOMEM;
+ pctl = kzalloc(size, GFP_NOIO);
+ if (!pctl)
+ return ERR_PTR(-ENOMEM);
+
+ page = llbitmap_read_page(llbitmap, idx);
+
+ if (IS_ERR(page)) {
+ kfree(pctl);
+ return ERR_CAST(page);
}
- llbitmap->nr_pages = nr_pages;
+ if (percpu_ref_init(&pctl->active, active_release,
+ PERCPU_REF_ALLOW_REINIT, GFP_NOIO)) {
+ __free_page(page);
+ kfree(pctl);
+ return ERR_PTR(-ENOMEM);
+ }
- for (i = 0; i < nr_pages; i++, pctl = (void *)pctl + size) {
- struct page *page = llbitmap_read_page(llbitmap, i);
+ pctl->page = page;
+ pctl->state = page_address(page);
+ init_waitqueue_head(&pctl->wait);
+ return pctl;
+}
- llbitmap->pctl[i] = pctl;
+static unsigned int llbitmap_reserved_pages(struct llbitmap *llbitmap)
+{
+ return DIV_ROUND_UP(llbitmap->mddev->bitmap_info.space << SECTOR_SHIFT,
+ PAGE_SIZE);
+}
- if (IS_ERR(page)) {
- llbitmap_free_pages(llbitmap);
- return PTR_ERR(page);
- }
+static int llbitmap_alloc_pages(struct llbitmap *llbitmap)
+{
+ unsigned int used_pages = llbitmap_used_pages(llbitmap, llbitmap->chunks);
+ unsigned int nr_pages = max(used_pages, llbitmap_reserved_pages(llbitmap));
+ int i;
+
+ llbitmap->pctl = kcalloc(nr_pages, sizeof(*llbitmap->pctl), GFP_NOIO);
+ if (!llbitmap->pctl)
+ return -ENOMEM;
- if (percpu_ref_init(&pctl->active, active_release,
- PERCPU_REF_ALLOW_REINIT, GFP_NOIO)) {
- __free_page(page);
+ llbitmap->nr_pages = nr_pages;
+
+ for (i = 0; i < nr_pages; i++) {
+ llbitmap->pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i);
+ if (IS_ERR(llbitmap->pctl[i])) {
+ int ret = PTR_ERR(llbitmap->pctl[i]);
+
+ llbitmap->pctl[i] = NULL;
llbitmap_free_pages(llbitmap);
- return -ENOMEM;
+ return ret;
}
-
- pctl->page = page;
- pctl->state = page_address(page);
- init_waitqueue_head(&pctl->wait);
}
return 0;
@@ -924,7 +949,7 @@ static int llbitmap_init(struct llbitmap *llbitmap)
llbitmap->sync_size = blocks;
mddev->bitmap_info.daemon_sleep = DEFAULT_DAEMON_SLEEP;
- ret = llbitmap_cache_pages(llbitmap);
+ ret = llbitmap_alloc_pages(llbitmap);
if (ret)
return ret;
@@ -1038,7 +1063,7 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
llbitmap->chunks = DIV_ROUND_UP_SECTOR_T(sync_size, chunksize);
llbitmap->chunkshift = ffz(~chunksize);
llbitmap->sync_size = sync_size;
- ret = llbitmap_cache_pages(llbitmap);
+ ret = llbitmap_alloc_pages(llbitmap);
out_put_page:
__free_page(sb_page);
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 14/29] md/md-llbitmap: allocate page controls independently
2026-08-02 19:50 ` [PATCH v5 14/29] md/md-llbitmap: allocate page controls independently Yu Kuai
@ 2026-08-02 20:27 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:27 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Heap buffer overflow in `llbitmap_write()` during array reshape due to missing `BITMAP_DATA_OFFSET` in `reserved_pages` calculation.
- [High] Denial of Service via eager unbounded memory allocation based on untrusted on-disk `sectors_reserved`.
--
--- Patch [14]: [PATCH v5 14/29] md/md-llbitmap: allocate page controls independently ---
commit f1fb318905483b935b4e96f1cbe5fe6c77c4134e
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: allocate page controls independently
This commit allocates llbitmap page-control objects independently rather
than as a single array. It also modifies llbitmap_read_page() to return a
zeroed page when beyond the current size, preparing for in-place growth.
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 0813cebfbdeb3..300dd8b93b01e 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
[ ... ]
> +static unsigned int llbitmap_reserved_pages(struct llbitmap *llbitmap)
> +{
> + return DIV_ROUND_UP(llbitmap->mddev->bitmap_info.space << SECTOR_SHIFT,
> + PAGE_SIZE);
> +}
[Severity: Critical]
Does this reserved pages calculation need to account for BITMAP_DATA_OFFSET?
When the array grows to the max chunks allowed by space during a reshape,
llbitmap_write() will add BITMAP_DATA_OFFSET to the position:
llbitmap_write() {
pos += BITMAP_DATA_OFFSET;
idx = pos >> PAGE_SHIFT;
bit = offset_in_page(pos);
llbitmap->pctl[idx]->state[bit] = state;
...
}
If space happens to align closely to a page boundary, reserved_pages could
evaluate to 1, but the allowed chunks plus BITMAP_DATA_OFFSET could result
in pos crossing into the next page. Will this cause idx to exceed the pctl
array capacity and trigger an out-of-bounds write?
> +static int llbitmap_alloc_pages(struct llbitmap *llbitmap)
> +{
> + unsigned int used_pages = llbitmap_used_pages(llbitmap, llbitmap->chunks);
> + unsigned int nr_pages = max(used_pages, llbitmap_reserved_pages(llbitmap));
> + int i;
> +
> + llbitmap->pctl = kcalloc(nr_pages, sizeof(*llbitmap->pctl), GFP_NOIO);
> + if (!llbitmap->pctl)
> + return -ENOMEM;
> +
> + llbitmap->nr_pages = nr_pages;
> +
> + for (i = 0; i < nr_pages; i++) {
> + llbitmap->pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i);
[Severity: High]
Is it safe to eagerly allocate all possible page controls based on the
on-disk sectors_reserved value?
If a malicious or corrupted superblock provides a massive sectors_reserved
value, this loop will invoke llbitmap_alloc_page_ctl() hundreds of
thousands of times for unused capacity:
llbitmap_alloc_page_ctl() {
pctl = kzalloc(size, GFP_NOIO);
...
page = llbitmap_read_page(llbitmap, idx);
...
if (percpu_ref_init(&pctl->active, active_release,
PERCPU_REF_ALLOW_REINIT, GFP_NOIO)) {
...
}
Could this lead to severe kernel memory exhaustion and trigger the OOM
killer, since GFP_NOIO aggressively reclaims memory during these
allocations?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=14
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 15/29] md/md-llbitmap: grow the page cache in place for reshape
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (13 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 14/29] md/md-llbitmap: allocate page controls independently Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:37 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 16/29] md/md-llbitmap: track target reshape geometry fields Yu Kuai
` (14 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Use the page-control helpers to grow llbitmap's cached pages in place
for resize and later reshape preparation, instead of rebuilding the
whole cache.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 145 +++++++++++++++++++++++++++++++++++----
1 file changed, 133 insertions(+), 12 deletions(-)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 300dd8b93b01..ddeea2098987 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -416,6 +416,19 @@ static char state_machine[BitStateCount][BitmapActionCount] = {
};
static void __llbitmap_flush(struct mddev *mddev);
+static void llbitmap_flush(struct mddev *mddev);
+static void llbitmap_update_sb(void *data);
+
+static void llbitmap_calculate_chunks(struct mddev *mddev, sector_t blocks,
+ unsigned long *chunksize,
+ unsigned long *chunks)
+{
+ *chunks = DIV_ROUND_UP_SECTOR_T(blocks, *chunksize);
+ while (*chunks > mddev->bitmap_info.space << SECTOR_SHIFT) {
+ *chunksize = *chunksize << 1;
+ *chunks = DIV_ROUND_UP_SECTOR_T(blocks, *chunksize);
+ }
+}
static enum llbitmap_state llbitmap_read(struct llbitmap *llbitmap, loff_t pos)
{
@@ -655,6 +668,48 @@ static unsigned int llbitmap_reserved_pages(struct llbitmap *llbitmap)
PAGE_SIZE);
}
+static int llbitmap_expand_pages(struct llbitmap *llbitmap,
+ unsigned long chunks)
+{
+ struct llbitmap_page_ctl **pctl;
+ unsigned int old_nr_pages = llbitmap->nr_pages;
+ unsigned int nr_pages = llbitmap_used_pages(llbitmap, chunks);
+ unsigned int i;
+ int ret;
+
+ if (nr_pages <= old_nr_pages)
+ return 0;
+
+ pctl = kcalloc(nr_pages, sizeof(*pctl), GFP_NOIO);
+ if (!pctl)
+ return -ENOMEM;
+
+ if (llbitmap->pctl)
+ memcpy(pctl, llbitmap->pctl,
+ array_size(old_nr_pages, sizeof(*pctl)));
+
+ for (i = old_nr_pages; i < nr_pages; i++) {
+ pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i);
+ if (IS_ERR(pctl[i]))
+ goto err_alloc_ptr;
+ }
+
+ kfree(llbitmap->pctl);
+ llbitmap->pctl = pctl;
+ llbitmap->nr_pages = nr_pages;
+ return 0;
+
+err_alloc_ptr:
+ ret = PTR_ERR(pctl[i]);
+ while (i-- > old_nr_pages) {
+ __free_page(pctl[i]->page);
+ percpu_ref_exit(&pctl[i]->active);
+ kfree(pctl[i]);
+ }
+ kfree(pctl);
+ return ret;
+}
+
static int llbitmap_alloc_pages(struct llbitmap *llbitmap)
{
unsigned int used_pages = llbitmap_used_pages(llbitmap, llbitmap->chunks);
@@ -730,6 +785,34 @@ static bool llbitmap_zero_all_disks(struct llbitmap *llbitmap)
return true;
}
+static void llbitmap_mark_range(struct llbitmap *llbitmap,
+ unsigned long start,
+ unsigned long end,
+ enum llbitmap_state state)
+{
+ while (start <= end) {
+ llbitmap_write(llbitmap, state, start);
+ start++;
+ }
+}
+
+static int llbitmap_prepare_resize(struct llbitmap *llbitmap,
+ unsigned long old_chunks,
+ unsigned long new_chunks,
+ unsigned long cache_chunks)
+{
+ int ret;
+
+ llbitmap_flush(llbitmap->mddev);
+ ret = llbitmap_expand_pages(llbitmap, cache_chunks);
+ if (ret)
+ return ret;
+ if (new_chunks > old_chunks)
+ llbitmap_mark_range(llbitmap, old_chunks, new_chunks - 1,
+ BitUnwritten);
+ return 0;
+}
+
static void llbitmap_init_state(struct llbitmap *llbitmap)
{
struct mddev *mddev = llbitmap->mddev;
@@ -1032,10 +1115,10 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
goto out_put_page;
}
- if (chunksize < DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors,
+ if (chunksize < DIV_ROUND_UP_SECTOR_T(sync_size,
mddev->bitmap_info.space << SECTOR_SHIFT)) {
pr_err("md/llbitmap: %s: chunksize too small %lu < %llu / %lu",
- mdname(mddev), chunksize, mddev->resync_max_sectors,
+ mdname(mddev), chunksize, sync_size,
mddev->bitmap_info.space);
goto out_put_page;
}
@@ -1184,24 +1267,62 @@ static int llbitmap_create(struct mddev *mddev)
static int llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
{
struct llbitmap *llbitmap = mddev->bitmap;
+ sector_t old_blocks = llbitmap->sync_size;
+ unsigned long old_chunks = llbitmap->chunks;
unsigned long chunks;
+ unsigned long cache_chunks;
+ int ret = 0;
+ unsigned long bitmap_chunksize;
+ bool reshape;
+ bool quiesced = false;
if (chunksize == 0)
chunksize = llbitmap->chunksize;
- /* If there is enough space, leave the chunksize unchanged. */
- chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
- while (chunks > mddev->bitmap_info.space << SECTOR_SHIFT) {
- chunksize = chunksize << 1;
- chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
- }
+ bitmap_chunksize = chunksize;
+ llbitmap_calculate_chunks(mddev, blocks, &bitmap_chunksize, &chunks);
- llbitmap->chunkshift = ffz(~chunksize);
- llbitmap->chunksize = chunksize;
- llbitmap->chunks = chunks;
- llbitmap->sync_size = blocks;
+ reshape = mddev->delta_disks || mddev->new_level != mddev->level ||
+ mddev->new_layout != mddev->layout ||
+ mddev->new_chunk_sectors != mddev->chunk_sectors;
+ if (!reshape && bitmap_chunksize != llbitmap->chunksize)
+ return -EOPNOTSUPP;
+ if (blocks == old_blocks && chunks == llbitmap->chunks)
+ return 0;
+ if (mddev->pers->quiesce) {
+ mddev->pers->quiesce(mddev, 1);
+ quiesced = true;
+ }
+
+ mutex_lock(&mddev->bitmap_info.mutex);
+ cache_chunks = reshape ? max(old_chunks, chunks) : chunks;
+ ret = llbitmap_prepare_resize(llbitmap, old_chunks, chunks, cache_chunks);
+ if (ret)
+ goto out;
+
+ if (reshape) {
+ llbitmap->chunks = max(old_chunks, chunks);
+ } else {
+ if (blocks < old_blocks && chunks < old_chunks)
+ llbitmap_mark_range(llbitmap, chunks, old_chunks - 1,
+ BitUnwritten);
+ mddev->bitmap_info.chunksize = bitmap_chunksize;
+ llbitmap->chunks = chunks;
+ llbitmap->sync_size = blocks;
+ llbitmap_update_sb(llbitmap);
+ }
+ __llbitmap_flush(mddev);
+ mutex_unlock(&mddev->bitmap_info.mutex);
+ if (quiesced)
+ mddev->pers->quiesce(mddev, 0);
return 0;
+
+out:
+ mutex_unlock(&mddev->bitmap_info.mutex);
+ if (quiesced)
+ mddev->pers->quiesce(mddev, 0);
+ return ret;
}
static int llbitmap_load(struct mddev *mddev)
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 15/29] md/md-llbitmap: grow the page cache in place for reshape
2026-08-02 19:50 ` [PATCH v5 15/29] md/md-llbitmap: grow the page cache in place for reshape Yu Kuai
@ 2026-08-02 20:37 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:37 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Critical] Mismatched chunkshift and page allocations during reshape cause out-of-bounds dereferences.
- [High] Concurrent access to `llbitmap->pctl` during array resize leads to Use-After-Free.
- [Medium] Missing `cond_resched()` in `llbitmap_mark_range` loop can cause soft lockups during large array resizing.
Pre-existing issues:
- [Critical] Bitmap metadata writes can overflow the reserved bitmap space on disk because it ignores `BITMAP_DATA_OFFSET`.
--
--- Patch [15]: [PATCH v5 15/29] md/md-llbitmap: grow the page cache in place for reshape ---
commit 73a7214b36a3cd36e140de64fe3e4cd6cfe6795a
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: grow the page cache in place for reshape
Use the page-control helpers to grow llbitmap's cached pages in place
for resize and later reshape preparation, instead of rebuilding the
whole cache.
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 300dd8b93b01e..ddeea20989873 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -416,6 +416,19 @@ static char state_machine[BitStateCount][BitmapActionCount] = {
> };
>
> static void __llbitmap_flush(struct mddev *mddev);
> +static void llbitmap_flush(struct mddev *mddev);
> +static void llbitmap_update_sb(void *data);
> +
> +static void llbitmap_calculate_chunks(struct mddev *mddev, sector_t blocks,
> + unsigned long *chunksize,
> + unsigned long *chunks)
> +{
> + *chunks = DIV_ROUND_UP_SECTOR_T(blocks, *chunksize);
> + while (*chunks > mddev->bitmap_info.space << SECTOR_SHIFT) {
[Severity: Critical]
This is a pre-existing issue, but does this calculation allow the metadata
writes to overflow the reserved disk space? It calculates the required chunks
without reserving space for BITMAP_DATA_OFFSET, which could allow the required
size to silently exceed the reserved disk space. If the array size forces the
chunk calculation to be exactly at or near the reserved space limit, could
used_pages exceed the reserved limit and overwrite sectors past the end of the
reserved space during a flush?
> + *chunksize = *chunksize << 1;
> + *chunks = DIV_ROUND_UP_SECTOR_T(blocks, *chunksize);
> + }
> +}
[ ... ]
> @@ -655,6 +668,48 @@ static unsigned int llbitmap_reserved_pages(struct llbitmap *llbitmap)
> PAGE_SIZE);
> }
>
> +static int llbitmap_expand_pages(struct llbitmap *llbitmap,
> + unsigned long chunks)
> +{
> + struct llbitmap_page_ctl **pctl;
[ ... ]
> + for (i = old_nr_pages; i < nr_pages; i++) {
> + pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i);
> + if (IS_ERR(pctl[i]))
> + goto err_alloc_ptr;
> + }
> +
> + kfree(llbitmap->pctl);
> + llbitmap->pctl = pctl;
[Severity: High]
Can this sequence create a use-after-free window? If the daemon timer had
previously fired while the flush work was running and set BITMAP_DAEMON_BUSY,
the daemon could re-arm pending_timer and exit. Could the timer then fire
concurrently while llbitmap->pctl is being freed here, causing the daemon to
wake up and dereference the freed pctl array?
> + llbitmap->nr_pages = nr_pages;
> + return 0;
[ ... ]
> @@ -730,6 +785,34 @@ static bool llbitmap_zero_all_disks(struct llbitmap *llbitmap)
> return true;
> }
>
> +static void llbitmap_mark_range(struct llbitmap *llbitmap,
> + unsigned long start,
> + unsigned long end,
> + enum llbitmap_state state)
> +{
> + while (start <= end) {
> + llbitmap_write(llbitmap, state, start);
> + start++;
> + }
> +}
[Severity: Medium]
If the array is resized significantly, the difference between start and end
could be hundreds of millions of chunks. Without a cond_resched() in this loop,
could this monopolize the CPU for an extended period and trigger a soft lockup?
[ ... ]
> @@ -1184,24 +1267,62 @@ static int llbitmap_create(struct mddev *mddev)
> static int llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
> {
> struct llbitmap *llbitmap = mddev->bitmap;
[ ... ]
> + mutex_lock(&mddev->bitmap_info.mutex);
> + cache_chunks = reshape ? max(old_chunks, chunks) : chunks;
> + ret = llbitmap_prepare_resize(llbitmap, old_chunks, chunks, cache_chunks);
> + if (ret)
> + goto out;
> +
> + if (reshape) {
> + llbitmap->chunks = max(old_chunks, chunks);
> + } else {
[Severity: Critical]
Does the reshape path here fail to update the chunkshift and chunksize?
It expands the page cache based on the new chunks count, but doesn't seem to
update llbitmap->chunksize and llbitmap->chunkshift. When subsequent I/O
operations are processed, could they calculate their chunk index using the old,
smaller shift, resulting in chunk indices that exceed the newly allocated
pages? Would this lead to an out-of-bounds access on llbitmap->pctl?
> + if (blocks < old_blocks && chunks < old_chunks)
> + llbitmap_mark_range(llbitmap, chunks, old_chunks - 1,
> + BitUnwritten);
> + mddev->bitmap_info.chunksize = bitmap_chunksize;
> + llbitmap->chunks = chunks;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=15
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 16/29] md/md-llbitmap: track target reshape geometry fields
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (14 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 15/29] md/md-llbitmap: grow the page cache in place for reshape Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:25 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 17/29] md/md-llbitmap: finish reshape geometry Yu Kuai
` (13 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Track llbitmap bookkeeping for the target reshape geometry while keeping
a single live bitmap instance.
Add the reshape geometry fields, refresh helper, and update the load and
resize paths to keep the target geometry in sync.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 42 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index ddeea2098987..71a490390142 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -289,6 +289,9 @@ struct llbitmap {
unsigned long chunks;
/* total number of sectors tracked by current bitmap geometry */
sector_t sync_size;
+ unsigned long reshape_chunksize;
+ unsigned long reshape_chunks;
+ sector_t reshape_sync_size;
unsigned long last_end_sync;
/*
* time in seconds that dirty bits will be cleared if the page is not
@@ -430,6 +433,39 @@ static void llbitmap_calculate_chunks(struct mddev *mddev, sector_t blocks,
}
}
+static bool llbitmap_reshaping(struct llbitmap *llbitmap)
+{
+ return llbitmap->mddev->reshape_position != MaxSector;
+}
+
+static sector_t llbitmap_personality_sync_size(struct llbitmap *llbitmap,
+ bool previous)
+{
+ struct mddev *mddev = llbitmap->mddev;
+
+ if (!llbitmap_reshaping(llbitmap) || !mddev->private || !mddev->pers ||
+ !mddev->pers->bitmap_sync_size)
+ return llbitmap->sync_size;
+ return mddev->pers->bitmap_sync_size(mddev, previous);
+}
+
+static void llbitmap_refresh_reshape(struct llbitmap *llbitmap)
+{
+ unsigned long old_chunks = DIV_ROUND_UP_SECTOR_T(llbitmap->sync_size,
+ llbitmap->chunksize);
+ sector_t blocks = llbitmap_personality_sync_size(llbitmap, false);
+ unsigned long chunksize = llbitmap->chunksize;
+ unsigned long chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
+
+ llbitmap->reshape_sync_size = blocks;
+ llbitmap->reshape_chunksize = chunksize;
+ llbitmap->reshape_chunks = chunks;
+ llbitmap_calculate_chunks(llbitmap->mddev, blocks,
+ &llbitmap->reshape_chunksize,
+ &llbitmap->reshape_chunks);
+ llbitmap->chunks = max(old_chunks, llbitmap->reshape_chunks);
+}
+
static enum llbitmap_state llbitmap_read(struct llbitmap *llbitmap, loff_t pos)
{
unsigned int idx;
@@ -1030,6 +1066,7 @@ static int llbitmap_init(struct llbitmap *llbitmap)
llbitmap->chunksize = chunksize;
llbitmap->chunks = chunks;
llbitmap->sync_size = blocks;
+ llbitmap_refresh_reshape(llbitmap);
mddev->bitmap_info.daemon_sleep = DEFAULT_DAEMON_SLEEP;
ret = llbitmap_alloc_pages(llbitmap);
@@ -1146,6 +1183,7 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
llbitmap->chunks = DIV_ROUND_UP_SECTOR_T(sync_size, chunksize);
llbitmap->chunkshift = ffz(~chunksize);
llbitmap->sync_size = sync_size;
+ llbitmap_refresh_reshape(llbitmap);
ret = llbitmap_alloc_pages(llbitmap);
out_put_page:
@@ -1302,6 +1340,9 @@ static int llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
goto out;
if (reshape) {
+ llbitmap->reshape_sync_size = blocks;
+ llbitmap->reshape_chunksize = bitmap_chunksize;
+ llbitmap->reshape_chunks = chunks;
llbitmap->chunks = max(old_chunks, chunks);
} else {
if (blocks < old_blocks && chunks < old_chunks)
@@ -1310,6 +1351,7 @@ static int llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
mddev->bitmap_info.chunksize = bitmap_chunksize;
llbitmap->chunks = chunks;
llbitmap->sync_size = blocks;
+ llbitmap_refresh_reshape(llbitmap);
llbitmap_update_sb(llbitmap);
}
__llbitmap_flush(mddev);
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 16/29] md/md-llbitmap: track target reshape geometry fields
2026-08-02 19:50 ` [PATCH v5 16/29] md/md-llbitmap: track target reshape geometry fields Yu Kuai
@ 2026-08-02 20:25 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:25 UTC (permalink / raw)
To: Yu Kuai; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Out-of-bounds array access and memory corruption during reshaping due to stale chunkshift.
Pre-existing issues:
- [High] Use-After-Free (UAF) due to a TOCTOU race between teardown and timer re-arming in daemon work.
--
--- Patch [16]: [PATCH v5 16/29] md/md-llbitmap: track target reshape geometry fields ---
commit dabf6384b9392c331f7703e778887a4f5f7bb15c
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: track target reshape geometry fields
Track llbitmap bookkeeping for the target reshape geometry while keeping
a single live bitmap instance.
Add the reshape geometry fields, refresh helper, and update the load and
resize paths to keep the target geometry in sync.
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index ddeea20989873..71a4903901425 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
[ ... ]
> @@ -1302,6 +1340,9 @@ static int llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
> goto out;
>
> if (reshape) {
> + llbitmap->reshape_sync_size = blocks;
> + llbitmap->reshape_chunksize = bitmap_chunksize;
> + llbitmap->reshape_chunks = chunks;
> llbitmap->chunks = max(old_chunks, chunks);
[Severity: High]
Does this code need to track or update a chunkshift for the reshape path?
During a reshape that increases the array size, bitmap_chunksize might increase.
While llbitmap->chunks is clamped here, llbitmap->chunkshift is not updated.
Later, if llbitmap_start_write() processes an I/O to the newly extended region,
it uses the stale chunkshift to compute the start chunk:
drivers/md/md-llbitmap.c:llbitmap_start_write() {
...
unsigned long start = offset >> llbitmap->chunkshift;
...
}
Because chunkshift represents the old, smaller chunk size, start can grossly
exceed llbitmap->chunks. Since llbitmap_start_write() lacks bounds checking
against llbitmap->chunks, it calculates an out-of-bounds page_start index
and calls llbitmap_raise_barrier(), which will directly dereference the
llbitmap->pctl array out of bounds. Can this lead to memory corruption?
> } else {
> if (blocks < old_blocks && chunks < old_chunks)
[ ... ]
[Severity: High]
This is a pre-existing issue, but could the teardown sequence in
llbitmap_destroy() race with md_llbitmap_daemon_fn() and cause a
use-after-free?
In llbitmap_destroy(), the teardown sets the shutdown flag, deletes the
timer, and then cancels the daemon work:
drivers/md/md-llbitmap.c:llbitmap_destroy() {
...
set_bit(BITMAP_SHUTDOWN, &llbitmap->flags);
timer_delete_sync(&llbitmap->pending_timer);
cancel_work_sync(&llbitmap->daemon_work);
...
}
However, in md_llbitmap_daemon_fn(), the shutdown check and timer re-arming
are not atomic:
drivers/md/md-llbitmap.c:md_llbitmap_daemon_fn() {
...
if (restart && !test_bit(BITMAP_SHUTDOWN, &llbitmap->flags))
mod_timer(&llbitmap->pending_timer, ...);
...
}
If the work thread evaluates the check to true and is preempted,
llbitmap_destroy() could run, set the shutdown flag, and execute
timer_delete_sync() which returns immediately as the timer isn't pending.
llbitmap_destroy() then blocks in cancel_work_sync().
When the work thread resumes, it calls mod_timer() to re-arm the timer
and exits. cancel_work_sync() then unblocks and llbitmap_destroy() frees
the llbitmap object. When the re-armed timer fires, it will access the
freed memory.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=16
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 17/29] md/md-llbitmap: finish reshape geometry
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (15 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 16/29] md/md-llbitmap: track target reshape geometry fields Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:39 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 18/29] md/md-llbitmap: refuse reshape while llbitmap still needs sync Yu Kuai
` (12 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Commit the staged llbitmap geometry when reshape finishes.
When assembling a stopped reshape, md_run() creates the bitmap before
publishing mddev->pers. llbitmap_read_sb() can therefore only initialize
the reshape fields from the old on-disk sync size. Refresh the staged
reshape geometry again from llbitmap_load(), after mddev->pers is
available, and expand the in-memory page controls before replaying bitmap
state.
Reproduce on the old kernel by creating a RAID10 llbitmap with four
active disks and two spares, growing it to six disks, then stopping and
assembling while reshape is still running. The llbitmap chunk count was
32704 before grow, 49056 during reshape, then rolled back to 32704 after
reassemble.
The fixed kernel kept the target geometry across the same stop/reassemble
flow: 65440 chunks before grow, 98160 during reshape, and 98160 after
reassemble.
Reported-by: Mykola Marzhan <mykola@meshstor.io>
Link: https://lore.kernel.org/all/20260726185916.2223460-1-mykola@meshstor.io/
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 71a490390142..a7c229db3058 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1371,11 +1371,20 @@ static int llbitmap_load(struct mddev *mddev)
{
enum llbitmap_action action = BitmapActionReload;
struct llbitmap *llbitmap = mddev->bitmap;
+ int ret;
if (test_and_clear_bit(BITMAP_STALE, &llbitmap->flags))
action = BitmapActionStale;
+ mutex_lock(&mddev->bitmap_info.mutex);
+ llbitmap_refresh_reshape(llbitmap);
+ ret = llbitmap_expand_pages(llbitmap, llbitmap->chunks);
+ if (ret) {
+ mutex_unlock(&mddev->bitmap_info.mutex);
+ return ret;
+ }
llbitmap_state_machine(llbitmap, 0, llbitmap->chunks - 1, action);
+ mutex_unlock(&mddev->bitmap_info.mutex);
return 0;
}
@@ -1709,6 +1718,30 @@ static void llbitmap_dirty_bits(struct mddev *mddev, unsigned long s,
llbitmap_state_machine(mddev->bitmap, s, e, BitmapActionStartwrite);
}
+static void llbitmap_reshape_finish(struct mddev *mddev)
+{
+ struct llbitmap *llbitmap = mddev->bitmap;
+
+ if (mddev->pers->quiesce)
+ mddev->pers->quiesce(mddev, 1);
+
+ mutex_lock(&mddev->bitmap_info.mutex);
+ llbitmap_flush(mddev);
+
+ llbitmap->chunksize = llbitmap->reshape_chunksize;
+ llbitmap->chunkshift = ffz(~llbitmap->chunksize);
+ llbitmap->chunks = llbitmap->reshape_chunks;
+ llbitmap->sync_size = llbitmap->reshape_sync_size;
+ llbitmap_refresh_reshape(llbitmap);
+ mddev->bitmap_info.chunksize = llbitmap->chunksize;
+ llbitmap_update_sb(llbitmap);
+ __llbitmap_flush(mddev);
+ mutex_unlock(&mddev->bitmap_info.mutex);
+
+ if (mddev->pers->quiesce)
+ mddev->pers->quiesce(mddev, 0);
+}
+
static void llbitmap_write_sb(struct llbitmap *llbitmap)
{
int nr_blocks = DIV_ROUND_UP(BITMAP_DATA_OFFSET, llbitmap->io_size);
@@ -2000,6 +2033,7 @@ static struct bitmap_operations llbitmap_ops = {
.get_stats = llbitmap_get_stats,
.dirty_bits = llbitmap_dirty_bits,
.prepare_range = llbitmap_prepare_range,
+ .reshape_finish = llbitmap_reshape_finish,
.write_all = llbitmap_write_all,
.groups = md_llbitmap_groups,
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 17/29] md/md-llbitmap: finish reshape geometry
2026-08-02 19:50 ` [PATCH v5 17/29] md/md-llbitmap: finish reshape geometry Yu Kuai
@ 2026-08-02 20:39 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:39 UTC (permalink / raw)
To: Yu Kuai; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `reshape_finish` callback is never invoked by the md core, rendering the new geometry commit logic dead code.
- [High] In-flight I/O during an array reshape triggers an out-of-bounds array access in `llbitmap_raise_barrier` due to stale chunkshift values.
--
--- Patch [17]: [PATCH v5 17/29] md/md-llbitmap: finish reshape geometry ---
commit f7a8f4b26cc5a0c01433cbcd7568266fb367e5e1
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: finish reshape geometry
This commit updates the llbitmap reshape geometry when the reshape finishes.
It refreshes the staged reshape geometry in llbitmap_load() after mddev->pers
is available, and expands the in-memory page controls before replaying the
bitmap state.
Link: https://lore.kernel.org/all/20260726185916.2223460-1-mykola@meshstor.io/
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 71a4903901425..a7c229db3058d 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
[ ... ]
> @@ -1709,6 +1718,30 @@ static void llbitmap_dirty_bits(struct mddev *mddev, unsigned long s,
> llbitmap_state_machine(mddev->bitmap, s, e, BitmapActionStartwrite);
> }
>
> +static void llbitmap_reshape_finish(struct mddev *mddev)
> +{
> + struct llbitmap *llbitmap = mddev->bitmap;
> +
> + if (mddev->pers->quiesce)
> + mddev->pers->quiesce(mddev, 1);
> +
> + mutex_lock(&mddev->bitmap_info.mutex);
> + llbitmap_flush(mddev);
> +
> + llbitmap->chunksize = llbitmap->reshape_chunksize;
> + llbitmap->chunkshift = ffz(~llbitmap->chunksize);
[Severity: High]
Does this introduce a regression where in-flight I/O during an array
reshape triggers an out-of-bounds array access due to stale chunkshift
values before they are updated here?
When an array grows, the active chunkshift remains unchanged while the page
controls are allocated for the old chunk limit. If the new chunksize is
forced to double, the new I/O offsets mapping to the expanded capacity
could be processed by llbitmap_start_write():
drivers/md/md-llbitmap.c:llbitmap_start_write() {
unsigned long start = offset >> llbitmap->chunkshift;
unsigned long end = (offset + sectors - 1) >> llbitmap->chunkshift;
int page_start = (start + BITMAP_DATA_OFFSET) >> PAGE_SHIFT;
...
llbitmap_raise_barrier(llbitmap, page_start);
}
Dividing the larger offset by the old, smaller chunkshift would generate a
bit index that exceeds the old chunks.
Would this result in an out-of-bounds read/write without bounds checking
when llbitmap_raise_barrier() is called?
drivers/md/md-llbitmap.c:llbitmap_raise_barrier() {
...
struct llbitmap_page_ctl *pctl = llbitmap->pctl[page_idx];
...
}
> + llbitmap->chunks = llbitmap->reshape_chunks;
> + llbitmap->sync_size = llbitmap->reshape_sync_size;
> + llbitmap_refresh_reshape(llbitmap);
> + mddev->bitmap_info.chunksize = llbitmap->chunksize;
> + llbitmap_update_sb(llbitmap);
> + __llbitmap_flush(mddev);
> + mutex_unlock(&mddev->bitmap_info.mutex);
> +
> + if (mddev->pers->quiesce)
> + mddev->pers->quiesce(mddev, 0);
> +}
> +
[ ... ]
> @@ -2000,6 +2033,7 @@ static struct bitmap_operations llbitmap_ops = {
> .get_stats = llbitmap_get_stats,
> .dirty_bits = llbitmap_dirty_bits,
> .prepare_range = llbitmap_prepare_range,
> + .reshape_finish = llbitmap_reshape_finish,
[Severity: High]
Is there a regression where the md core never calls this reshape_finish
callback?
Looking at the md core code, it does not appear to invoke
mddev->bitmap_ops->reshape_finish anywhere. If this is never called, would
the staged reshape geometry be silently dropped, leaving the bitmap
permanently out of sync with the expanded array capacity after a reshape
completes?
> .write_all = llbitmap_write_all,
>
> .groups = md_llbitmap_groups,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=17
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 18/29] md/md-llbitmap: refuse reshape while llbitmap still needs sync
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (16 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 17/29] md/md-llbitmap: finish reshape geometry Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:44 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 19/29] md/md-llbitmap: add reshape range mapping helpers Yu Kuai
` (11 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Reject reshape when llbitmap still contains NeedSync or Syncing bits.
This keeps reshape from starting until the current llbitmap state has
been reconciled.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index a7c229db3058..f8a1b0f79be6 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1718,6 +1718,29 @@ static void llbitmap_dirty_bits(struct mddev *mddev, unsigned long s,
llbitmap_state_machine(mddev->bitmap, s, e, BitmapActionStartwrite);
}
+static int llbitmap_reshape_can_start(struct mddev *mddev)
+{
+ struct llbitmap *llbitmap = mddev->bitmap;
+ unsigned long chunk;
+ int ret = 0;
+
+ if (!llbitmap)
+ return 0;
+
+ mutex_lock(&mddev->bitmap_info.mutex);
+ for (chunk = 0; chunk < llbitmap->chunks; chunk++) {
+ enum llbitmap_state state = llbitmap_read(llbitmap, chunk);
+
+ if (state == BitNeedSync || state == BitSyncing) {
+ ret = -EBUSY;
+ break;
+ }
+ }
+ mutex_unlock(&mddev->bitmap_info.mutex);
+
+ return ret;
+}
+
static void llbitmap_reshape_finish(struct mddev *mddev)
{
struct llbitmap *llbitmap = mddev->bitmap;
@@ -2034,6 +2057,7 @@ static struct bitmap_operations llbitmap_ops = {
.dirty_bits = llbitmap_dirty_bits,
.prepare_range = llbitmap_prepare_range,
.reshape_finish = llbitmap_reshape_finish,
+ .reshape_can_start = llbitmap_reshape_can_start,
.write_all = llbitmap_write_all,
.groups = md_llbitmap_groups,
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 18/29] md/md-llbitmap: refuse reshape while llbitmap still needs sync
2026-08-02 19:50 ` [PATCH v5 18/29] md/md-llbitmap: refuse reshape while llbitmap still needs sync Yu Kuai
@ 2026-08-02 20:44 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:44 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The newly added reshape callbacks (`reshape_can_start`, `reshape_finish`, and `reshape_mark`) are completely unreferenced by the core MD subsystem, rendering the patch ineffective.
- [Medium] llbitmap_reshape_can_start() ignores proactive sync states for unwritten regions, which would allow reshape to conflict with ongoing or pending syncs if the function were called.
--
--- Patch [18]: [PATCH v5 18/29] md/md-llbitmap: refuse reshape while llbitmap still needs sync ---
commit 4b812e132deb0c081e68cf2925523d86a890d935
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: refuse reshape while llbitmap still needs sync
This commit rejects reshape when llbitmap still contains NeedSync or
Syncing bits. This is intended to keep reshape from starting until the
current llbitmap state has been reconciled.
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index a7c229db3058d..f8a1b0f79be6a 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1718,6 +1718,29 @@ static void llbitmap_dirty_bits(struct mddev *mddev, unsigned long s,
> llbitmap_state_machine(mddev->bitmap, s, e, BitmapActionStartwrite);
> }
>
> +static int llbitmap_reshape_can_start(struct mddev *mddev)
> +{
> + struct llbitmap *llbitmap = mddev->bitmap;
> + unsigned long chunk;
> + int ret = 0;
> +
> + if (!llbitmap)
> + return 0;
> +
> + mutex_lock(&mddev->bitmap_info.mutex);
> + for (chunk = 0; chunk < llbitmap->chunks; chunk++) {
> + enum llbitmap_state state = llbitmap_read(llbitmap, chunk);
> +
> + if (state == BitNeedSync || state == BitSyncing) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: Medium]
Does this check omit the unwritten equivalents of these sync states?
It appears that BitNeedSyncUnwritten and BitSyncingUnwritten are not
checked here.
Since these states also represent active or pending parity syncs, would
omitting them allow a reshape to conflict with an ongoing sync?
> + ret = -EBUSY;
> + break;
> + }
> + }
> + mutex_unlock(&mddev->bitmap_info.mutex);
> +
> + return ret;
> +}
> +
> static void llbitmap_reshape_finish(struct mddev *mddev)
> {
> struct llbitmap *llbitmap = mddev->bitmap;
[ ... ]
> @@ -2034,6 +2057,7 @@ static struct bitmap_operations llbitmap_ops = {
> .dirty_bits = llbitmap_dirty_bits,
> .prepare_range = llbitmap_prepare_range,
> .reshape_finish = llbitmap_reshape_finish,
> + .reshape_can_start = llbitmap_reshape_can_start,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Are there any callers that actually use this new callback?
This patch populates the reshape_can_start callback in llbitmap_ops, but
does not appear to update the core MD subsystem (such as drivers/md/md.c)
to invoke mddev->bitmap_ops->reshape_can_start().
Without a caller, does this validation logic become unreachable dead code
that fails to reject the reshape?
> .write_all = llbitmap_write_all,
>
> .groups = md_llbitmap_groups,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=18
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 19/29] md/md-llbitmap: add reshape range mapping helpers
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (17 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 18/29] md/md-llbitmap: refuse reshape while llbitmap still needs sync Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:31 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 20/29] md/md-llbitmap: don't skip reshape ranges from bitmap state Yu Kuai
` (10 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Teach llbitmap to choose old versus new geometry during reshape and to
encode exact bitmap ranges for the active geometry. This is the mapping
groundwork for checkpoint remapping.
Range preparation now distinguishes writes from discards. Normal writes
must cover every touched bitmap chunk, while discards may only mark fully
covered chunks unwritten. Without this distinction, a discard that starts
or ends inside a chunk can make live data look unwritten after the range
has been mapped and floored.
Reproduce that with a RAID1 llbitmap using 128-sector chunks. A discard
starting halfway into chunk 8 with a 128-sector length changed clean bits
from 16352 to 16350 and unwritten bits from 0 to 2, even though no chunk
was fully discarded. With discard-specific range encoding, both counts
stay unchanged for the same test.
Range preparation also clamps the pre-map range in the same coordinate
space as the incoming IO. RAID5 receives array-sector offsets but tracks
llbitmap sync size in component sectors, so steady-state RAID5 must use
bitmap_array_sectors() before mapping and keep the existing sync-size
clamp after mapping.
Reproduce that with a 4-disk RAID5 llbitmap created --assume-clean. A
write below dev_sectors changed dirty bits from 0 to 512, but a write at
seek=2094080 left the count at 512. With the array-sector pre-map limit,
writing at seek=component_size + 65536 increased dirty bits from 512 to
1024.
Reported-by: Mykola Marzhan <mykola@meshstor.io>
Link: https://lore.kernel.org/all/20260726185916.2223460-1-mykola@meshstor.io/
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-bitmap.c | 2 +-
drivers/md/md-bitmap.h | 3 +-
drivers/md/md-llbitmap.c | 137 +++++++++++++++++++++++++++++++++++----
drivers/md/md.c | 11 ++--
4 files changed, 134 insertions(+), 19 deletions(-)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index b7f0d4acce04..b8325cb09a37 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -1731,7 +1731,7 @@ static void bitmap_start_write(struct mddev *mddev, sector_t offset,
}
static void bitmap_prepare_range(struct mddev *mddev, sector_t *offset,
- unsigned long *sectors)
+ unsigned long *sectors, bool discard)
{
if (mddev->pers->bitmap_sector)
mddev->pers->bitmap_sector(mddev, offset, sectors);
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index 6478cf9d8816..b69c78174f02 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -97,7 +97,8 @@ struct bitmap_operations {
/* Prepare a range for this bitmap implementation. */
void (*prepare_range)(struct mddev *mddev,
sector_t *offset,
- unsigned long *sectors);
+ unsigned long *sectors,
+ bool discard);
void (*reshape_finish)(struct mddev *mddev);
int (*reshape_can_start)(struct mddev *mddev);
void (*reshape_mark)(struct mddev *mddev, sector_t old_pos,
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index f8a1b0f79be6..fa16a4224c45 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -9,6 +9,7 @@
#include <linux/sched.h>
#include <linux/list.h>
#include <linux/file.h>
+#include <linux/math64.h>
#include <linux/seq_file.h>
#include <trace/events/block.h>
@@ -433,22 +434,28 @@ static void llbitmap_calculate_chunks(struct mddev *mddev, sector_t blocks,
}
}
-static bool llbitmap_reshaping(struct llbitmap *llbitmap)
-{
- return llbitmap->mddev->reshape_position != MaxSector;
-}
-
static sector_t llbitmap_personality_sync_size(struct llbitmap *llbitmap,
bool previous)
{
struct mddev *mddev = llbitmap->mddev;
- if (!llbitmap_reshaping(llbitmap) || !mddev->private || !mddev->pers ||
+ if (READ_ONCE(mddev->reshape_position) == MaxSector ||
+ !mddev->private || !mddev->pers ||
!mddev->pers->bitmap_sync_size)
return llbitmap->sync_size;
return mddev->pers->bitmap_sync_size(mddev, previous);
}
+static sector_t llbitmap_logical_size(struct llbitmap *llbitmap, bool previous)
+{
+ struct mddev *mddev = llbitmap->mddev;
+
+ if (!mddev->private || !mddev->pers ||
+ !mddev->pers->bitmap_array_sectors)
+ return llbitmap_personality_sync_size(llbitmap, previous);
+ return mddev->pers->bitmap_array_sectors(mddev, previous);
+}
+
static void llbitmap_refresh_reshape(struct llbitmap *llbitmap)
{
unsigned long old_chunks = DIV_ROUND_UP_SECTOR_T(llbitmap->sync_size,
@@ -466,6 +473,80 @@ static void llbitmap_refresh_reshape(struct llbitmap *llbitmap)
llbitmap->chunks = max(old_chunks, llbitmap->reshape_chunks);
}
+static void llbitmap_map_layout(struct llbitmap *llbitmap, sector_t *offset,
+ unsigned long *sectors, bool previous)
+{
+ sector_t limit = llbitmap_logical_size(llbitmap, previous);
+ sector_t start = *offset;
+ sector_t end = start + *sectors;
+
+ if (start >= limit) {
+ *sectors = 0;
+ return;
+ }
+ if (end > limit)
+ end = limit;
+
+ *offset = start;
+ *sectors = end - start;
+ if (!*sectors)
+ return;
+
+ if (llbitmap->mddev->pers->bitmap_sector_map)
+ llbitmap->mddev->pers->bitmap_sector_map(llbitmap->mddev, offset,
+ sectors, previous);
+ else if (!previous && llbitmap->mddev->pers->bitmap_sector)
+ llbitmap->mddev->pers->bitmap_sector(llbitmap->mddev, offset,
+ sectors);
+}
+
+static void llbitmap_encode_range(struct llbitmap *llbitmap, sector_t *offset,
+ unsigned long *sectors, bool previous)
+{
+ unsigned long chunksize = previous ? llbitmap->chunksize :
+ llbitmap->reshape_chunksize;
+ u64 start;
+ u64 end;
+
+ if (!*sectors) {
+ *offset = 0;
+ return;
+ }
+
+ start = div64_u64(*offset, chunksize);
+ end = div64_u64(*offset + *sectors - 1, chunksize);
+ *offset = (sector_t)start << llbitmap->chunkshift;
+ *sectors = (end - start + 1) << llbitmap->chunkshift;
+}
+
+static void llbitmap_encode_discard_range(struct llbitmap *llbitmap,
+ sector_t *offset,
+ unsigned long *sectors,
+ bool previous)
+{
+ unsigned long chunksize = previous ? llbitmap->chunksize :
+ llbitmap->reshape_chunksize;
+ sector_t end = *offset + *sectors;
+ u64 start;
+ u64 last;
+
+ if (!*sectors) {
+ *offset = 0;
+ return;
+ }
+
+ start = DIV_ROUND_UP_SECTOR_T(*offset, chunksize);
+ last = div64_u64(end, chunksize);
+ if (start >= last) {
+ *offset = 0;
+ *sectors = 0;
+ return;
+ }
+
+ *offset = (sector_t)start << llbitmap->chunkshift;
+ *sectors = (last - start) << llbitmap->chunkshift;
+}
+
static enum llbitmap_state llbitmap_read(struct llbitmap *llbitmap, loff_t pos)
{
unsigned int idx;
@@ -1409,11 +1490,35 @@ static void llbitmap_destroy(struct mddev *mddev)
mutex_unlock(&mddev->bitmap_info.mutex);
}
+static bool llbitmap_map_previous(struct llbitmap *llbitmap, sector_t offset,
+ unsigned long sectors)
+{
+ struct mddev *mddev = llbitmap->mddev;
+ sector_t boundary = READ_ONCE(mddev->reshape_position);
+
+ if (boundary == MaxSector)
+ return false;
+
+ WARN_ON_ONCE(sectors && offset < boundary && offset + sectors > boundary);
+
+ return mddev->reshape_backwards ? offset < boundary : offset >= boundary;
+}
+
static void llbitmap_prepare_range(struct mddev *mddev, sector_t *offset,
- unsigned long *sectors)
+ unsigned long *sectors, bool discard)
{
- if (mddev->pers->bitmap_sector)
- mddev->pers->bitmap_sector(mddev, offset, sectors);
+ struct llbitmap *llbitmap = mddev->bitmap;
+ bool previous;
+
+ if (!llbitmap)
+ return;
+
+ previous = llbitmap_map_previous(llbitmap, *offset, *sectors);
+ llbitmap_map_layout(llbitmap, offset, sectors, previous);
+ if (discard)
+ llbitmap_encode_discard_range(llbitmap, offset, sectors, previous);
+ else
+ llbitmap_encode_range(llbitmap, offset, sectors, previous);
}
static void llbitmap_start_write(struct mddev *mddev, sector_t offset,
@@ -1582,7 +1687,11 @@ static bool llbitmap_blocks_synced(struct mddev *mddev, sector_t offset)
{
struct llbitmap *llbitmap = mddev->bitmap;
unsigned long p = offset >> llbitmap->chunkshift;
- enum llbitmap_state c = llbitmap_read(llbitmap, p);
+ enum llbitmap_state c;
+
+ if (p >= llbitmap->chunks)
+ return false;
+ c = llbitmap_read(llbitmap, p);
return c == BitClean || c == BitDirty || c == BitCleanUnwritten;
}
@@ -1592,7 +1701,11 @@ static sector_t llbitmap_skip_sync_blocks(struct mddev *mddev, sector_t offset)
struct llbitmap *llbitmap = mddev->bitmap;
unsigned long p = offset >> llbitmap->chunkshift;
int blocks = llbitmap->chunksize - (offset & (llbitmap->chunksize - 1));
- enum llbitmap_state c = llbitmap_read(llbitmap, p);
+ enum llbitmap_state c;
+
+ if (p >= llbitmap->chunks)
+ return 0;
+ c = llbitmap_read(llbitmap, p);
/* always skip unwritten blocks */
if (c == BitUnwritten)
@@ -1637,6 +1750,8 @@ static bool llbitmap_start_sync(struct mddev *mddev, sector_t offset,
* if md_do_sync() loop more times.
*/
*blocks = llbitmap->chunksize - (offset & (llbitmap->chunksize - 1));
+ if (p >= llbitmap->chunks)
+ return false;
state = llbitmap_state_machine(llbitmap, p, p, BitmapActionStartsync);
return state == BitSyncing || state == BitSyncingUnwritten;
}
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 538ba7bab060..e88381beb209 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9428,21 +9428,20 @@ struct bio *mddev_bio_split_at_reshape_offset(struct mddev *mddev,
EXPORT_SYMBOL_GPL(mddev_bio_split_at_reshape_offset);
static void md_bitmap_prepare_range(struct mddev *mddev, sector_t *offset,
- unsigned long *sectors)
+ unsigned long *sectors, bool discard)
{
- mddev->bitmap_ops->prepare_range(mddev, offset, sectors);
+ mddev->bitmap_ops->prepare_range(mddev, offset, sectors, discard);
}
static void md_bitmap_start(struct mddev *mddev,
struct md_io_clone *md_io_clone)
{
- md_bitmap_fn *fn = unlikely(md_io_clone->rw == STAT_DISCARD) ?
- mddev->bitmap_ops->start_discard :
+ bool discard = md_io_clone->rw == STAT_DISCARD;
+ md_bitmap_fn *fn = discard ? mddev->bitmap_ops->start_discard :
mddev->bitmap_ops->start_write;
md_bitmap_prepare_range(mddev, &md_io_clone->offset,
- &md_io_clone->sectors);
-
+ &md_io_clone->sectors, discard);
if (!md_io_clone->sectors)
return;
fn(mddev, md_io_clone->offset, md_io_clone->sectors);
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 19/29] md/md-llbitmap: add reshape range mapping helpers
2026-08-02 19:50 ` [PATCH v5 19/29] md/md-llbitmap: add reshape range mapping helpers Yu Kuai
@ 2026-08-02 20:31 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:31 UTC (permalink / raw)
To: Yu Kuai; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Missing implementation of `bitmap_array_sectors` and `bitmap_sector_map` in RAID personalities breaks range mapping and drops valid array writes.
- [Critical] Omitted post-mapping clamp allows out-of-bounds offsets to trigger out-of-bounds array accesses in `llbitmap_start_write`.
- [Critical] Decoupled reshape geometry logic causes `start_sync` to decode modified bits incorrectly during array recovery.
- [High] Skipping reshape boundary splitting when `MD_RECOVERY_RESHAPE` is clear corrupts straddling bios.
--
--- Patch [19]: [PATCH v5 19/29] md/md-llbitmap: add reshape range mapping helpers ---
commit 633da220a726aec9691f97e3c1481f36882f4315
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: add reshape range mapping helpers
Teach llbitmap to choose old versus new geometry during reshape and to
encode exact bitmap ranges for the active geometry. This is the mapping
groundwork for checkpoint remapping.
Link: https://lore.kernel.org/all/20260726185916.2223460-1-mykola@meshstor.io/
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index f8a1b0f79be6a..fa16a4224c45a 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
[ ... ]
> +static sector_t llbitmap_logical_size(struct llbitmap *llbitmap, bool previous)
> +{
> + struct mddev *mddev = llbitmap->mddev;
> +
> + if (!mddev->private || !mddev->pers ||
> + !mddev->pers->bitmap_array_sectors)
> + return llbitmap_personality_sync_size(llbitmap, previous);
> + return mddev->pers->bitmap_array_sectors(mddev, previous);
> +}
[Severity: Critical]
Since bitmap_array_sectors is absent from all RAID personalities in this
patch series, won't this always fall back to
llbitmap_personality_sync_size()?
If a write is issued to a RAID5 array at a logical offset greater than
dev_sectors, it appears this will incorrectly use the physical component
size (dev_sectors) as the limit for the logical array offset, causing
llbitmap_map_layout() to drop valid array writes.
[ ... ]
> +static void llbitmap_map_layout(struct llbitmap *llbitmap, sector_t *offset,
> + unsigned long *sectors, bool previous)
> +{
[ ... ]
> + if (llbitmap->mddev->pers->bitmap_sector_map)
> + llbitmap->mddev->pers->bitmap_sector_map(llbitmap->mddev, offset,
> + sectors, previous);
> + else if (!previous && llbitmap->mddev->pers->bitmap_sector)
> + llbitmap->mddev->pers->bitmap_sector(llbitmap->mddev, offset,
> + sectors);
> +}
[Severity: Critical]
The commit message states we need to keep the existing sync-size clamp after
mapping, but does this code actually enforce that post-mapping clamp?
Without a clamp against llbitmap_personality_sync_size() here at the end
of the function, could an out-of-bounds logical offset bypass filtering?
If that happens, llbitmap_start_write() could calculate an out-of-bounds
page_start index and trigger memory corruption inside
llbitmap_raise_barrier() before the state machine drops it.
[ ... ]
> +static void llbitmap_encode_range(struct llbitmap *llbitmap, sector_t *offset,
> + unsigned long *sectors, bool previous)
> +{
> + unsigned long chunksize = previous ? llbitmap->chunksize :
> + llbitmap->reshape_chunksize;
[ ... ]
> + start = div64_u64(*offset, chunksize);
> + end = div64_u64(*offset + *sectors - 1, chunksize);
> + *offset = (sector_t)start << llbitmap->chunkshift;
> + *sectors = (end - start + 1) << llbitmap->chunkshift;
> +}
[Severity: Critical]
Does this decoupling of reshape geometry logic cause issues for the sync path
during recovery?
Here, writers calculate the chunk index using the new geometry
(reshape_chunksize) but scale it using the old chunkshift. Later, during
array recovery, llbitmap_start_sync() unconditionally shifts the physical
component offset by the old chunkshift:
unsigned long p = offset >> llbitmap->chunkshift;
Will this mismatch cause llbitmap_start_sync() to decode the wrong bit
index and skip recovering modified active data?
[ ... ]
> +static bool llbitmap_map_previous(struct llbitmap *llbitmap, sector_t offset,
> + unsigned long sectors)
> +{
> + struct mddev *mddev = llbitmap->mddev;
> + sector_t boundary = READ_ONCE(mddev->reshape_position);
> +
> + if (boundary == MaxSector)
> + return false;
> +
> + WARN_ON_ONCE(sectors && offset < boundary && offset + sectors > boundary);
> +
> + return mddev->reshape_backwards ? offset < boundary : offset >= boundary;
> +}
[Severity: High]
Can a straddling bio trigger this WARN_ON_ONCE and corrupt the bitmap if a
reshape is paused?
If a user pauses a reshape or the array enters a degraded state,
MD_RECOVERY_RESHAPE might be cleared. If mddev_bio_split_at_reshape_offset()
checks this flag and skips splitting the bio, a fully unsplit straddling
bio could reach here. This would force the entire bio to be evaluated
against a single geometry based purely on its starting sector, destroying
bitmap integrity for the straddling tail.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=19
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 20/29] md/md-llbitmap: don't skip reshape ranges from bitmap state
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (18 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 19/29] md/md-llbitmap: add reshape range mapping helpers Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:31 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 21/29] md/md-llbitmap: remap checkpointed bits as reshape progresses Yu Kuai
` (9 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Reshape progress is tracked by array metadata rather than llbitmap.
Do not let llbitmap skip_sync_blocks() suppress reshape ranges based on
stale bitmap state before the corresponding checkpoint is persisted.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index fa16a4224c45..a20e55fdf82b 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1707,6 +1707,14 @@ static sector_t llbitmap_skip_sync_blocks(struct mddev *mddev, sector_t offset)
return 0;
c = llbitmap_read(llbitmap, p);
+ /*
+ * Reshape progress is tracked by array metadata rather than llbitmap.
+ * Skipping reshape ranges from stale bitmap state can lose data after a
+ * restart before the corresponding bits are checkpointed to disk.
+ */
+ if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery))
+ return 0;
+
/* always skip unwritten blocks */
if (c == BitUnwritten)
return blocks;
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 20/29] md/md-llbitmap: don't skip reshape ranges from bitmap state
2026-08-02 19:50 ` [PATCH v5 20/29] md/md-llbitmap: don't skip reshape ranges from bitmap state Yu Kuai
@ 2026-08-02 20:31 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:31 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but a lockless race condition in `llbitmap_set_page_dirty` causes dirty bitmap blocks to be lost in memory and never flushed to disk.
- [High] This problem wasn't introduced by this patch, but integer truncation of `blocks` can cause an infinite loop in the `md_do_sync` thread.
--
--- Patch [20]: [PATCH v5 20/29] md/md-llbitmap: don't skip reshape ranges from bitmap state ---
commit c6db76f6bc970107f68d37fc602584cc2c0e0a1b
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: don't skip reshape ranges from bitmap state
This commit modifies md-llbitmap to stop using stale bitmap state for
skipping reshape ranges. It ensures that reshape progress is tracked purely
by array metadata. This prevents data loss after a restart before checkpoints
are persisted to disk.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index fa16a4224c45a..a20e55fdf82b0 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1707,6 +1707,14 @@ static sector_t llbitmap_skip_sync_blocks(struct mddev *mddev, sector_t offset)
[Severity: High]
This isn't a bug introduced by this patch, but can integer truncation of
blocks cause an infinite loop in the md_do_sync() thread?
In llbitmap_skip_sync_blocks(), blocks is declared as an int:
int blocks = llbitmap->chunksize - (offset & (llbitmap->chunksize - 1));
If the array size and bitmap space require chunksize to exceed INT_MAX (e.g.,
a >8TB array with a small bitmap space), chunksize will be >= 2^31. Can the
subtraction yield a value >= 2^31, which truncates into a negative 32-bit int
(e.g., -1024)?
When this negative int is returned as a 64-bit sector_t, does it sign-extend
to a massive positive value? If md_do_sync() adds this value to the current
sync sector index, could it cause the index to overflow and decrease, looping
endlessly back and forth?
> return 0;
> c = llbitmap_read(llbitmap, p);
>
> + /*
> + * Reshape progress is tracked by array metadata rather than llbitmap.
> + * Skipping reshape ranges from stale bitmap state can lose data after a
> + * restart before the corresponding bits are checkpointed to disk.
> + */
> + if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery))
> + return 0;
> +
> /* always skip unwritten blocks */
> if (c == BitUnwritten)
> return blocks;
[Severity: Critical]
This is a pre-existing issue, but a lockless race condition in
llbitmap_set_page_dirty() can cause dirty bitmap blocks to be lost in memory
and never flushed to disk.
Can this sequence actually occur? If llbitmap_set_page_dirty() is called
concurrently with the llbitmap_unplug_fn() flusher thread, the writer sets
the LLPageDirty flag before setting the specific block bit:
drivers/md/md-llbitmap.c:llbitmap_set_page_dirty() {
...
if (!test_bit(LLPageDirty, &pctl->flags))
set_bit(LLPageDirty, &pctl->flags);
// race window here
set_bit(block, pctl->dirty);
...
}
If the flusher thread clears LLPageDirty and checks pctl->dirty between these
two operations:
drivers/md/md-llbitmap.c:llbitmap_unplug_fn() {
...
if (!test_bit(LLPageDirty, &llbitmap->pctl[i]->flags) ||
!test_and_clear_bit(LLPageDirty, &llbitmap->pctl[i]->flags))
continue;
...
}
Will it miss the dirty block? The writer then sets the block bit, but
because LLPageDirty is now 0, the flusher will ignore this page indefinitely
until another block happens to be dirtied.
If a dirty block is not flushed to the disk bitmap, will the array fail to
resync that block after a crash or unclean shutdown, leaving stale data on
one of the drives?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=20
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 21/29] md/md-llbitmap: remap checkpointed bits as reshape progresses
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (19 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 20/29] md/md-llbitmap: don't skip reshape ranges from bitmap state Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:43 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 22/29] md/md-llbitmap: clamp state-machine walks to tracked bits Yu Kuai
` (8 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Merge checkpointed old llbitmap state forward as reshape_position advances
and record the checkpoint remap through reshape_mark().
Normal write accounting can run while the reshape thread checkpoints a new
reshape position. llbitmap_reshape_mark() reads old state bytes, merges them
into destination bits, and writes the result back. If llbitmap_start_write()
or llbitmap_start_discard() updates the same state bytes at the same time,
the two read/modify/write paths can overwrite each other and lose the state
from one side.
Serialize only this state-byte race with a rwlock. Normal I/O takes the read
side around llbitmap_state_machine(), after page active references are raised,
so concurrent normal I/O updates still run in parallel. Reshape checkpointing
takes the write side only while merging the checkpointed range, avoiding page
suspension and avoiding a sleeping mutex in the I/O accounting path.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 204 +++++++++++++++++++++++++++++++++++++++
1 file changed, 204 insertions(+)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index a20e55fdf82b..5d95627ff983 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -302,6 +302,11 @@ struct llbitmap {
/* fires on first BitDirty state */
struct timer_list pending_timer;
struct work_struct daemon_work;
+ /*
+ * Serialize reshape checkpoint remapping against normal I/O bitmap
+ * updates without blocking concurrent I/O updates on each other.
+ */
+ rwlock_t reshape_lock;
unsigned long flags;
__u64 events_cleared;
@@ -498,6 +503,14 @@ static void llbitmap_map_layout(struct llbitmap *llbitmap, sector_t *offset,
else if (!previous && llbitmap->mddev->pers->bitmap_sector)
llbitmap->mddev->pers->bitmap_sector(llbitmap->mddev, offset,
sectors);
+
+ limit = llbitmap_personality_sync_size(llbitmap, previous);
+ start = *offset;
+ end = start + *sectors;
+ if (start >= limit)
+ *sectors = 0;
+ else if (end > limit)
+ *sectors = limit - start;
}
static void llbitmap_encode_range(struct llbitmap *llbitmap, sector_t *offset,
@@ -930,6 +943,33 @@ static int llbitmap_prepare_resize(struct llbitmap *llbitmap,
return 0;
}
+static enum llbitmap_state
+llbitmap_rmerge_state(struct llbitmap *llbitmap,
+ enum llbitmap_state dst,
+ enum llbitmap_state src)
+{
+ bool level_456 = raid_is_456(llbitmap->mddev);
+
+ if (dst == BitNeedSync || dst == BitSyncing ||
+ src == BitNeedSync || src == BitSyncing)
+ return BitNeedSync;
+
+ if (dst == BitDirty || src == BitDirty)
+ return BitDirty;
+
+ /*
+ * Reshape generates valid target parity/data for both already-written
+ * and not-yet-written regions in the checkpointed range, so a mix of
+ * clean and unwritten still results in a clean destination bit.
+ */
+ if (level_456 && ((dst == BitClean && src == BitUnwritten) ||
+ (src == BitClean && dst == BitUnwritten)))
+ return BitClean;
+ if (dst == BitClean || src == BitClean)
+ return BitClean;
+ return BitUnwritten;
+}
+
static void llbitmap_init_state(struct llbitmap *llbitmap)
{
struct mddev *mddev = llbitmap->mddev;
@@ -1306,6 +1346,7 @@ static void md_llbitmap_daemon_fn(struct work_struct *work)
if (llbitmap->mddev->degraded)
return;
+
retry:
start = 0;
end = min(llbitmap->chunks, PAGE_SIZE - BITMAP_DATA_OFFSET) - 1;
@@ -1367,6 +1408,7 @@ static int llbitmap_create(struct mddev *mddev)
timer_setup(&llbitmap->pending_timer, llbitmap_pending_timer_fn, 0);
INIT_WORK(&llbitmap->daemon_work, md_llbitmap_daemon_fn);
+ rwlock_init(&llbitmap->reshape_lock);
atomic_set(&llbitmap->behind_writes, 0);
init_waitqueue_head(&llbitmap->behind_wait);
@@ -1535,7 +1577,9 @@ static void llbitmap_start_write(struct mddev *mddev, sector_t offset,
page_start++;
}
+ read_lock(&llbitmap->reshape_lock);
llbitmap_state_machine(llbitmap, start, end, BitmapActionStartwrite);
+ read_unlock(&llbitmap->reshape_lock);
}
static void llbitmap_end_write(struct mddev *mddev, sector_t offset,
@@ -1567,7 +1611,9 @@ static void llbitmap_start_discard(struct mddev *mddev, sector_t offset,
page_start++;
}
+ read_lock(&llbitmap->reshape_lock);
llbitmap_state_machine(llbitmap, start, end, BitmapActionDiscard);
+ read_unlock(&llbitmap->reshape_lock);
}
static void llbitmap_end_discard(struct mddev *mddev, sector_t offset,
@@ -1864,6 +1910,136 @@ static int llbitmap_reshape_can_start(struct mddev *mddev)
return ret;
}
+struct llbitmap_reshape_range {
+ sector_t offset;
+ unsigned long sectors;
+ sector_t start;
+ sector_t end;
+};
+
+static enum llbitmap_state
+llbitmap_reshape_init_dst(struct llbitmap *llbitmap, unsigned long dst,
+ const struct llbitmap_reshape_range *new)
+{
+ u64 bit_start = (u64)dst * llbitmap->reshape_chunksize;
+ u64 bit_end = bit_start + llbitmap->reshape_chunksize;
+
+ if (!llbitmap->mddev->reshape_backwards)
+ return bit_start < new->offset ? llbitmap_read(llbitmap, dst) :
+ BitUnwritten;
+ return bit_end > new->end ? llbitmap_read(llbitmap, dst) : BitUnwritten;
+}
+
+static void llbitmap_reshape_dst_range(struct llbitmap *llbitmap,
+ unsigned long dst,
+ const struct llbitmap_reshape_range *new,
+ struct llbitmap_reshape_range *dst_range)
+{
+ sector_t dst_bit_start = (sector_t)dst * llbitmap->reshape_chunksize;
+
+ dst_range->start = max(dst_bit_start, new->offset);
+ dst_range->end = min(dst_bit_start + llbitmap->reshape_chunksize,
+ new->end);
+ dst_range->offset = dst_range->start;
+ dst_range->sectors = dst_range->end - dst_range->start;
+}
+
+static void llbitmap_reshape_map_range(struct llbitmap *llbitmap,
+ sector_t lo, sector_t hi,
+ bool previous,
+ struct llbitmap_reshape_range *range)
+{
+ range->offset = lo;
+ range->sectors = hi - lo;
+ llbitmap_map_layout(llbitmap, &range->offset, &range->sectors, previous);
+ range->start = range->offset;
+ range->end = range->offset + range->sectors;
+}
+
+static bool llbitmap_reshape_src_range(const struct llbitmap_reshape_range *old,
+ const struct llbitmap_reshape_range *new,
+ const struct llbitmap_reshape_range *dst,
+ struct llbitmap_reshape_range *src)
+{
+ if (!old->sectors)
+ return false;
+
+ src->start = old->offset +
+ mul_u64_u64_div_u64(dst->start - new->offset,
+ old->sectors, new->sectors);
+ src->end = old->offset +
+ mul_u64_u64_div_u64_roundup(dst->end - new->offset,
+ old->sectors, new->sectors);
+ if (src->end > old->end)
+ src->end = old->end;
+ src->offset = src->start;
+ src->sectors = src->end - src->start;
+
+ return src->sectors;
+}
+
+static enum llbitmap_state llbitmap_rmerge_src(struct llbitmap *llbitmap,
+ enum llbitmap_state state,
+ const struct llbitmap_reshape_range *src)
+{
+ unsigned long bit = div64_u64(src->start, llbitmap->chunksize);
+ unsigned long end = div64_u64(src->end - 1, llbitmap->chunksize);
+
+ while (bit <= end) {
+ enum llbitmap_state src_state = llbitmap_read(llbitmap, bit);
+
+ state = llbitmap_rmerge_state(llbitmap, state, src_state);
+ bit++;
+ }
+
+ return state;
+}
+
+static void llbitmap_reshape_merge(struct llbitmap *llbitmap,
+ const struct llbitmap_reshape_range *old,
+ const struct llbitmap_reshape_range *new)
+{
+ unsigned long dst_start;
+ unsigned long dst_end;
+ unsigned long dst;
+ bool backwards = false;
+
+ if (!new->sectors)
+ return;
+
+ dst_start = div64_u64(new->offset, llbitmap->reshape_chunksize);
+ dst_end = div64_u64(new->end - 1, llbitmap->reshape_chunksize);
+ if (old->sectors) {
+ unsigned long src_start = div64_u64(old->offset,
+ llbitmap->chunksize);
+ unsigned long src_end = div64_u64(old->end - 1,
+ llbitmap->chunksize);
+
+ backwards = src_start < dst_start && src_end >= dst_start;
+ }
+
+ dst = backwards ? dst_end : dst_start;
+ while (true) {
+ struct llbitmap_reshape_range dst_range;
+ struct llbitmap_reshape_range src;
+ enum llbitmap_state state;
+
+ llbitmap_reshape_dst_range(llbitmap, dst, new, &dst_range);
+ state = llbitmap_reshape_init_dst(llbitmap, dst, new);
+ if (llbitmap_reshape_src_range(old, new, &dst_range, &src))
+ state = llbitmap_rmerge_src(llbitmap, state, &src);
+ else
+ state = llbitmap_rmerge_state(llbitmap, state, BitUnwritten);
+ llbitmap_write(llbitmap, state, dst);
+ if (dst == (backwards ? dst_start : dst_end))
+ break;
+ if (backwards)
+ dst--;
+ else
+ dst++;
+ }
+}
+
static void llbitmap_reshape_finish(struct mddev *mddev)
{
struct llbitmap *llbitmap = mddev->bitmap;
@@ -1888,6 +2064,33 @@ static void llbitmap_reshape_finish(struct mddev *mddev)
mddev->pers->quiesce(mddev, 0);
}
+static void llbitmap_reshape_mark(struct mddev *mddev, sector_t old_pos,
+ sector_t new_pos)
+{
+ struct llbitmap *llbitmap = mddev->bitmap;
+ sector_t lo;
+ sector_t hi;
+ struct llbitmap_reshape_range old;
+ struct llbitmap_reshape_range new;
+
+ if (!llbitmap || old_pos == new_pos)
+ return;
+
+ lo = min(old_pos, new_pos);
+ hi = max(old_pos, new_pos);
+ if (!hi)
+ return;
+
+ llbitmap_reshape_map_range(llbitmap, lo, hi, true, &old);
+ llbitmap_reshape_map_range(llbitmap, lo, hi, false, &new);
+ if (!new.sectors)
+ return;
+
+ write_lock(&llbitmap->reshape_lock);
+ llbitmap_reshape_merge(llbitmap, &old, &new);
+ write_unlock(&llbitmap->reshape_lock);
+}
+
static void llbitmap_write_sb(struct llbitmap *llbitmap)
{
int nr_blocks = DIV_ROUND_UP(BITMAP_DATA_OFFSET, llbitmap->io_size);
@@ -2181,6 +2384,7 @@ static struct bitmap_operations llbitmap_ops = {
.prepare_range = llbitmap_prepare_range,
.reshape_finish = llbitmap_reshape_finish,
.reshape_can_start = llbitmap_reshape_can_start,
+ .reshape_mark = llbitmap_reshape_mark,
.write_all = llbitmap_write_all,
.groups = md_llbitmap_groups,
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 21/29] md/md-llbitmap: remap checkpointed bits as reshape progresses
2026-08-02 19:50 ` [PATCH v5 21/29] md/md-llbitmap: remap checkpointed bits as reshape progresses Yu Kuai
@ 2026-08-02 20:43 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:43 UTC (permalink / raw)
To: Yu Kuai; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing read_lock(&llbitmap->reshape_lock) in background daemon and sysfs paths causes read-modify-write data races with reshape checkpointing.
- [High] llbitmap_rmerge_state() fails to handle BitCleanUnwritten and other unwritten sync states, incorrectly downgrading them to BitUnwritten.
- [High] The newly introduced reshape_mark feature to record checkpoint remaps is dead code as it is never invoked by the MD subsystem.
--
--- Patch [21]: [PATCH v5 21/29] md/md-llbitmap: remap checkpointed bits as reshape progresses ---
commit f911f407605f9d24ba29497958177e6d0f9e041f
Author: Yu Kuai <yukuai@fygo.io>
Subject: md/md-llbitmap: remap checkpointed bits as reshape progresses
This commit introduces a mechanism to merge checkpointed old llbitmap state
forward as reshape_position advances. It synchronizes reshape checkpoint
remapping against normal I/O bitmap updates using a read-write lock to avoid
data loss during concurrent read/modify/write paths.
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index a20e55fdf82b0..5d95627ff9830 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
[ ... ]
> @@ -930,6 +943,33 @@ static int llbitmap_prepare_resize(struct llbitmap *llbitmap,
> return 0;
> }
>
> +static enum llbitmap_state
> +llbitmap_rmerge_state(struct llbitmap *llbitmap,
> + enum llbitmap_state dst,
> + enum llbitmap_state src)
> +{
> + bool level_456 = raid_is_456(llbitmap->mddev);
> +
> + if (dst == BitNeedSync || dst == BitSyncing ||
> + src == BitNeedSync || src == BitSyncing)
> + return BitNeedSync;
> +
> + if (dst == BitDirty || src == BitDirty)
> + return BitDirty;
> +
> + /*
> + * Reshape generates valid target parity/data for both already-written
> + * and not-yet-written regions in the checkpointed range, so a mix of
> + * clean and unwritten still results in a clean destination bit.
> + */
> + if (level_456 && ((dst == BitClean && src == BitUnwritten) ||
> + (src == BitClean && dst == BitUnwritten)))
> + return BitClean;
> + if (dst == BitClean || src == BitClean)
> + return BitClean;
> + return BitUnwritten;
> +}
[Severity: High]
Does this logic handle unwritten sync states like BitCleanUnwritten?
If a chunk in BitCleanUnwritten, BitNeedSyncUnwritten, or
BitSyncingUnwritten is processed during a reshape on a RAID4/5/6 array,
the state evaluates through these checks and returns BitUnwritten.
Can this cause the array to lose track of valid parity or pending proactive
syncs, potentially triggering a full read-modify-write lazy recovery on the
next write, or leaving unwritten regions permanently unsynced?
[ ... ]
> @@ -1306,6 +1346,7 @@ static void md_llbitmap_daemon_fn(struct work_struct *work)
>
> if (llbitmap->mddev->degraded)
> return;
> +
> retry:
> start = 0;
> end = min(llbitmap->chunks, PAGE_SIZE - BITMAP_DATA_OFFSET) - 1;
[Severity: High]
Are there missing read locks for the reshape_lock in the background daemon
and sysfs paths?
The reshape_mark() function explicitly bypasses page suspension and uses
write_lock(&llbitmap->reshape_lock). However, the background daemon in
md_llbitmap_daemon_fn() calls llbitmap_state_machine() without acquiring
the read lock:
llbitmap_state_machine(llbitmap, start, end, BitmapActionDaemon);
Similarly, sysfs writes via llbitmap_dirty_bits() call it without the lock:
llbitmap_state_machine(mddev->bitmap, s, e, BitmapActionStartwrite);
Could this result in read-modify-write data races? For example, if reshape
copies a BitNeedSync state and writes it, while the background daemon
concurrently evaluates a BitDirty to BitClean transition for the same chunk,
the daemon's write could overwrite the reshape's write and lose the
BitNeedSync state.
[ ... ]
> @@ -2181,6 +2384,7 @@ static struct bitmap_operations llbitmap_ops = {
> .prepare_range = llbitmap_prepare_range,
> .reshape_finish = llbitmap_reshape_finish,
> .reshape_can_start = llbitmap_reshape_can_start,
> + .reshape_mark = llbitmap_reshape_mark,
> .write_all = llbitmap_write_all,
>
> .groups = md_llbitmap_groups,
[Severity: High]
Is the new llbitmap_reshape_mark() function dead code?
Although the reshape_mark operation is implemented and wired up in
struct bitmap_operations, it appears there are no callers in the MD core
subsystem (such as in md.c or raid5.c) that invoke
mddev->bitmap_ops->reshape_mark().
If the system crashes during reshape, could this missing hook prevent the
bitmap state from being correctly recovered because the remaps were never
recorded?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=21
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 22/29] md/md-llbitmap: clamp state-machine walks to tracked bits
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (20 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 21/29] md/md-llbitmap: remap checkpointed bits as reshape progresses Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 19:50 ` [PATCH v5 23/29] md/raid10: reject llbitmap reshape when md chunk shrinks Yu Kuai
` (7 subsequent siblings)
29 siblings, 0 replies; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
llbitmap_state_machine() can be called with an end bit beyond
llbitmap->chunks. In particular, llbitmap_cond_end_sync() passes
sector >> chunkshift, and sector can reach the tracked boundary
exactly.
Clamp the state-machine range to llbitmap->chunks so it cannot walk
past the tracked bitmap.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/md-llbitmap.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 5d95627ff983..2830ce05d941 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1012,7 +1012,10 @@ static enum llbitmap_state llbitmap_state_machine(struct llbitmap *llbitmap,
llbitmap_init_state(llbitmap);
return BitNone;
}
-
+ if (start >= llbitmap->chunks)
+ return BitNone;
+ if (end >= llbitmap->chunks)
+ end = llbitmap->chunks - 1;
while (start <= end) {
enum llbitmap_state c = llbitmap_read(llbitmap, start);
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v5 23/29] md/raid10: reject llbitmap reshape when md chunk shrinks
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (21 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 22/29] md/md-llbitmap: clamp state-machine walks to tracked bits Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:40 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 24/29] md/raid10: wire llbitmap reshape lifecycle Yu Kuai
` (6 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
llbitmap reshape keeps one live bitmap and cannot safely make an
existing bitmap bit cover a smaller data range.
The llbitmap chunksize itself will not shrink when mddev->chunk_sectors
stays the same or grows. However, shrinking mddev->chunk_sectors can
shrink the effective data range covered by each bit for the RAID10
reshape geometry. Reject that reshape while llbitmap is active.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/raid10.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index ed3c6fbe65f7..1c3393467667 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4245,6 +4245,10 @@ static int raid10_check_reshape(struct mddev *mddev)
if (conf->geo.far_copies != 1 && !conf->geo.far_offset)
return -EINVAL;
+ if (mddev->bitmap_id == ID_LLBITMAP &&
+ mddev->new_chunk_sectors &&
+ mddev->new_chunk_sectors < mddev->chunk_sectors)
+ return -EOPNOTSUPP;
if (setup_geo(&geo, mddev, geo_start) != conf->copies)
/* mustn't change number of copies */
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 23/29] md/raid10: reject llbitmap reshape when md chunk shrinks
2026-08-02 19:50 ` [PATCH v5 23/29] md/raid10: reject llbitmap reshape when md chunk shrinks Yu Kuai
@ 2026-08-02 20:40 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:40 UTC (permalink / raw)
To: Yu Kuai; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The patch intends to reject shrinking reshapes when `ID_LLBITMAP` is active by returning `-EOPNOTSUPP` from `raid10_check_reshape()`. However, a severe pre-existing logic flaw in `mddev_start_reshape()` causes it to evaluate any non-zero return (including negative error codes) as a truthy condition. This bypasses the patch's protection entirely and triggers a destructive array corruption if the reshape is interrupted.
--
--- Patch [23]: [PATCH v5 23/29] md/raid10: reject llbitmap reshape when md chunk shrinks ---
commit f65520f6e55cf9e2cea49909a73ee06aeb4f57b1
Author: Yu Kuai <yukuai@fygo.io>
md/raid10: reject llbitmap reshape when md chunk shrinks
llbitmap reshape keeps one live bitmap and cannot safely make an
existing bitmap bit cover a smaller data range. Shrinking mddev->chunk_sectors
can shrink the effective data range covered by each bit for the RAID10
reshape geometry.
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index ed3c6fbe65f78..1c3393467667b 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -4245,6 +4245,10 @@ static int raid10_check_reshape(struct mddev *mddev)
>
> if (conf->geo.far_copies != 1 && !conf->geo.far_offset)
> return -EINVAL;
> + if (mddev->bitmap_id == ID_LLBITMAP &&
> + mddev->new_chunk_sectors &&
> + mddev->new_chunk_sectors < mddev->chunk_sectors)
> + return -EOPNOTSUPP;
[Severity: Critical]
This is a pre-existing issue, but returning an error code here doesn't seem
to prevent the reshape from starting if it was already interrupted.
Looking at mddev_start_reshape() in drivers/md/md.c, it evaluates the negative
error code as a truthy condition:
if (mddev->reshape_position == MaxSector ||
mddev->pers->check_reshape == NULL ||
mddev->pers->check_reshape(mddev)) {
clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
ret = mddev->pers->start_reshape(mddev);
Since -EOPNOTSUPP evaluates to true, will this erroneously call
mddev->pers->start_reshape() instead of aborting the operation?
If so, does this cause raid10_start_reshape() to destructively reset the
reshape progress back to 0 for an interrupted reshape?
} else
conf->reshape_progress = 0;
conf->reshape_safe = conf->reshape_progress;
>
> if (setup_geo(&geo, mddev, geo_start) != conf->copies)
> /* mustn't change number of copies */
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=23
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 24/29] md/raid10: wire llbitmap reshape lifecycle
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (22 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 23/29] md/raid10: reject llbitmap reshape when md chunk shrinks Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:49 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 25/29] md/raid10: split reshape bios before bitmap accounting Yu Kuai
` (5 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Prepare llbitmap before RAID10 starts growing, checkpoint the bitmap
before advancing reshape_position, finish the llbitmap geometry update
when reshape completes, and export the old and new tracked sizes.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/raid10.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 1c3393467667..bac9edd28c97 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4356,6 +4356,12 @@ static int raid10_start_reshape(struct mddev *mddev)
if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
return -EBUSY;
+ if (md_bitmap_enabled(mddev, false) &&
+ mddev->bitmap_ops->reshape_can_start) {
+ ret = mddev->bitmap_ops->reshape_can_start(mddev);
+ if (ret)
+ return ret;
+ }
if (setup_geo(&new, mddev, geo_start) != conf->copies)
return -EINVAL;
@@ -4679,6 +4685,13 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
time_after(jiffies, conf->reshape_checkpoint + 10*HZ)) {
/* Need to update reshape_position in metadata */
wait_barrier(conf);
+ if (md_bitmap_enabled(mddev, false) &&
+ mddev->bitmap_ops->reshape_mark &&
+ conf->reshape_safe != conf->reshape_progress) {
+ mddev->bitmap_ops->reshape_mark(mddev, conf->reshape_safe,
+ conf->reshape_progress);
+ mddev->bitmap_ops->unplug(mddev, true);
+ }
mddev->reshape_position = conf->reshape_progress;
if (mddev->reshape_backwards)
mddev->curr_resync_completed = raid10_size(mddev, 0, 0)
@@ -4877,9 +4890,19 @@ static void reshape_request_write(struct mddev *mddev, struct r10bio *r10_bio)
static void end_reshape(struct r10conf *conf)
{
+ struct mddev *mddev = conf->mddev;
+
if (test_bit(MD_RECOVERY_INTR, &conf->mddev->recovery))
return;
+ if (md_bitmap_enabled(mddev, false) &&
+ mddev->bitmap_ops->reshape_mark &&
+ conf->reshape_safe != conf->reshape_progress) {
+ mddev->bitmap_ops->reshape_mark(mddev, conf->reshape_safe,
+ conf->reshape_progress);
+ mddev->bitmap_ops->unplug(mddev, true);
+ }
+
spin_lock_irq(&conf->device_lock);
conf->prev = conf->geo;
md_finish_reshape(conf->mddev);
@@ -5011,10 +5034,15 @@ static void end_reshape_request(struct r10bio *r10_bio)
static void raid10_finish_reshape(struct mddev *mddev)
{
struct r10conf *conf = mddev->private;
+ bool llbitmap = mddev->bitmap_id == ID_LLBITMAP &&
+ md_bitmap_enabled(mddev, false);
if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
return;
+ if (llbitmap && mddev->bitmap_ops->reshape_finish)
+ mddev->bitmap_ops->reshape_finish(mddev);
+
if (mddev->delta_disks > 0) {
if (mddev->resync_offset > mddev->resync_max_sectors) {
mddev->resync_offset = mddev->resync_max_sectors;
@@ -5041,6 +5069,15 @@ static void raid10_finish_reshape(struct mddev *mddev)
mddev->reshape_backwards = 0;
}
+static sector_t raid10_bitmap_sync_size(struct mddev *mddev, bool previous)
+{
+ struct r10conf *conf = mddev->private;
+
+ if (previous)
+ return raid10_size(mddev, 0, 0);
+ return raid10_size(mddev, 0, conf->geo.raid_disks);
+}
+
static struct md_personality raid10_personality =
{
.head = {
@@ -5067,6 +5104,8 @@ static struct md_personality raid10_personality =
.start_reshape = raid10_start_reshape,
.finish_reshape = raid10_finish_reshape,
.update_reshape_pos = raid10_update_reshape_pos,
+ .bitmap_sync_size = raid10_bitmap_sync_size,
+ .bitmap_array_sectors = raid10_bitmap_sync_size,
};
static int __init raid10_init(void)
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 24/29] md/raid10: wire llbitmap reshape lifecycle
2026-08-02 19:50 ` [PATCH v5 24/29] md/raid10: wire llbitmap reshape lifecycle Yu Kuai
@ 2026-08-02 20:49 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:49 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Hardcoded check for `ID_LLBITMAP` in `raid10_finish_reshape()` violates the `bitmap_ops` vtable abstraction.
--
--- Patch [24]: [PATCH v5 24/29] md/raid10: wire llbitmap reshape lifecycle ---
commit 2a0099dd424ef7b25245673edf0ef62577007cc4
Author: Yu Kuai <yukuai@fygo.io>
md/raid10: wire llbitmap reshape lifecycle
This commit wires up the llbitmap reshape lifecycle in the RAID10 subsystem.
It prepares the bitmap before the array starts growing, checkpoints it
during the reshape process, and finalizes the geometry update upon completion.
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 1c3393467667b..bac9edd28c974 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
[ ... ]
> @@ -5011,10 +5034,15 @@ static void end_reshape_request(struct r10bio *r10_bio)
> static void raid10_finish_reshape(struct mddev *mddev)
> {
> struct r10conf *conf = mddev->private;
> + bool llbitmap = mddev->bitmap_id == ID_LLBITMAP &&
> + md_bitmap_enabled(mddev, false);
>
> if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
> return;
>
> + if (llbitmap && mddev->bitmap_ops->reshape_finish)
> + mddev->bitmap_ops->reshape_finish(mddev);
> +
[Severity: Low]
Does this explicit check for ID_LLBITMAP defeat the purpose of the bitmap_ops
vtable abstraction?
Earlier in the file during raid10_start_reshape() and end_reshape(), the code
relies entirely on checking if the virtual function pointers like
reshape_can_start or reshape_mark are set. By hardcoding a check for
mddev->bitmap_id == ID_LLBITMAP here, it leaks internal knowledge of which
specific bitmap type supports reshape_finish.
Would it be better to just rely on mddev->bitmap_ops->reshape_finish being
non-NULL, so that any future bitmap type implementing this hook won't be
silently bypassed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=24
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 25/29] md/raid10: split reshape bios before bitmap accounting
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (23 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 24/29] md/raid10: wire llbitmap reshape lifecycle Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:46 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 26/29] md/raid5: add exact old and new llbitmap mapping helpers Yu Kuai
` (4 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Use the shared mddev_bio_split_at_reshape_offset() helper so RAID10
submits only one-side bios to llbitmap during reshape.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/raid10.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index bac9edd28c97..562a325a7195 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1848,6 +1848,7 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
{
struct r10conf *conf = mddev->private;
sector_t chunk_mask = (conf->geo.chunk_mask & conf->prev.chunk_mask);
+ const int rw = bio_data_dir(bio);
int chunk_sects = chunk_mask + 1;
int sectors = bio_sectors(bio);
@@ -1873,6 +1874,15 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
sectors = chunk_sects -
(bio->bi_iter.bi_sector &
(chunk_sects - 1));
+
+ bio = mddev_bio_split_at_reshape_offset(mddev, bio, §ors,
+ &conf->bio_split);
+ if (!bio) {
+ if (rw == WRITE)
+ md_write_end(mddev);
+ return true;
+ }
+
if (!__make_request(mddev, bio, sectors))
md_write_end(mddev);
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 25/29] md/raid10: split reshape bios before bitmap accounting
2026-08-02 19:50 ` [PATCH v5 25/29] md/raid10: split reshape bios before bitmap accounting Yu Kuai
@ 2026-08-02 20:46 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:46 UTC (permalink / raw)
To: Yu Kuai; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double-splitting a bio using the same mempool-backed bioset causes a deadlock under concurrent load.
--
--- Patch [25]: [PATCH v5 25/29] md/raid10: split reshape bios before bitmap accounting ---
commit 6f3a3e2c85f7d5d7d630d6c3cb813006807edbdc
Author: Yu Kuai <yukuai@fygo.io>
md/raid10: split reshape bios before bitmap accounting
Use the shared mddev_bio_split_at_reshape_offset() helper so RAID10
submits only one-side bios to llbitmap during reshape.
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1873,6 +1874,15 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
> sectors = chunk_sects -
> (bio->bi_iter.bi_sector &
> (chunk_sects - 1));
> +
> + bio = mddev_bio_split_at_reshape_offset(mddev, bio, §ors,
> + &conf->bio_split);
[Severity: High]
Does this introduce a mempool deadlock if we need to split the bio again later?
If this initial split succeeds, the thread holds a bio from the conf->bio_split
mempool. Later, the thread proceeds to __make_request() and then to
raid10_write_request().
If the target disks have bad blocks, max_sectors is further reduced, causing a
second split attempt from the exact same mempool:
raid10_write_request() {
...
if (max_sectors < r10_bio->sectors) {
...
bio = bio_submit_split_bioset(bio, r10_bio->sectors,
&conf->bio_split);
...
}
Since the conf->bio_split bioset is initialized with BIO_POOL_SIZE (2), if two
concurrent I/O requests cross chunk boundaries and hit bad blocks
simultaneously, could both threads allocate one bio each, and then both
permanently block in mempool_alloc() waiting for a second bio?
> + if (!bio) {
> + if (rw == WRITE)
> + md_write_end(mddev);
> + return true;
> + }
> +
> if (!__make_request(mddev, bio, sectors))
> md_write_end(mddev);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=25
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 26/29] md/raid5: add exact old and new llbitmap mapping helpers
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (24 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 25/29] md/raid10: split reshape bios before bitmap accounting Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 19:50 ` [PATCH v5 27/29] md/raid5: reject llbitmap reshape when md chunk shrinks Yu Kuai
` (3 subsequent siblings)
29 siblings, 0 replies; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Teach RAID5 to export exact old and new llbitmap mappings and the
corresponding sync and array sizes for reshape-aware bitmap users.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/raid5.c | 73 +++++++++++++++++++++++++++++++++-------------
1 file changed, 53 insertions(+), 20 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 2cc2546a29ae..88bf5a9ce573 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6015,28 +6015,46 @@ static enum reshape_loc get_reshape_loc(struct mddev *mddev,
return LOC_BEHIND_RESHAPE;
}
-static void raid5_bitmap_sector(struct mddev *mddev, sector_t *offset,
- unsigned long *sectors)
+static void raid5_bitmap_sector_map(struct mddev *mddev, sector_t *offset,
+ unsigned long *sectors,
+ bool previous)
{
struct r5conf *conf = mddev->private;
sector_t start = *offset;
sector_t end = start + *sectors;
- sector_t prev_start = start;
- sector_t prev_end = end;
int sectors_per_chunk;
- enum reshape_loc loc;
int dd_idx;
- sectors_per_chunk = conf->chunk_sectors *
- (conf->raid_disks - conf->max_degraded);
+ if (previous)
+ sectors_per_chunk = conf->prev_chunk_sectors *
+ (conf->previous_raid_disks - conf->max_degraded);
+ else
+ sectors_per_chunk = conf->chunk_sectors *
+ (conf->raid_disks - conf->max_degraded);
sector_div(start, sectors_per_chunk);
start *= sectors_per_chunk;
if (sector_div(end, sectors_per_chunk))
end++;
end *= sectors_per_chunk;
- start = raid5_compute_sector(conf, start, 0, &dd_idx, NULL);
- end = raid5_compute_sector(conf, end, 0, &dd_idx, NULL);
+ start = raid5_compute_sector(conf, start, previous, &dd_idx, NULL);
+ end = raid5_compute_sector(conf, end, previous, &dd_idx, NULL);
+ *offset = start;
+ *sectors = end - start;
+}
+
+static void raid5_bitmap_sector(struct mddev *mddev, sector_t *offset,
+ unsigned long *sectors)
+{
+ struct r5conf *conf = mddev->private;
+ sector_t start = *offset;
+ sector_t end = start + *sectors;
+ sector_t prev_start = start;
+ unsigned long prev_sectors = end - start;
+ enum reshape_loc loc;
+
+ raid5_bitmap_sector_map(mddev, &start, sectors, false);
+ end = start + *sectors;
/*
* For LOC_INSIDE_RESHAPE, this IO will wait for reshape to make
@@ -6045,19 +6063,10 @@ static void raid5_bitmap_sector(struct mddev *mddev, sector_t *offset,
loc = get_reshape_loc(mddev, conf, prev_start);
if (likely(loc != LOC_AHEAD_OF_RESHAPE)) {
*offset = start;
- *sectors = end - start;
return;
}
- sectors_per_chunk = conf->prev_chunk_sectors *
- (conf->previous_raid_disks - conf->max_degraded);
- sector_div(prev_start, sectors_per_chunk);
- prev_start *= sectors_per_chunk;
- sector_div(prev_end, sectors_per_chunk);
- prev_end *= sectors_per_chunk;
-
- prev_start = raid5_compute_sector(conf, prev_start, 1, &dd_idx, NULL);
- prev_end = raid5_compute_sector(conf, prev_end, 1, &dd_idx, NULL);
+ raid5_bitmap_sector_map(mddev, &prev_start, &prev_sectors, true);
/*
* for LOC_AHEAD_OF_RESHAPE, reshape can make progress before this IO
@@ -6065,7 +6074,7 @@ static void raid5_bitmap_sector(struct mddev *mddev, sector_t *offset,
* we set bits for both.
*/
*offset = min(start, prev_start);
- *sectors = max(end, prev_end) - *offset;
+ *sectors = max(end, prev_start + prev_sectors) - *offset;
}
static enum stripe_result make_stripe_request(struct mddev *mddev,
@@ -9131,6 +9140,21 @@ static void raid5_prepare_suspend(struct mddev *mddev)
wake_up(&conf->wait_for_reshape);
}
+static sector_t raid5_bitmap_sync_size(struct mddev *mddev, bool previous)
+{
+ return mddev->dev_sectors;
+}
+
+static sector_t raid5_bitmap_array_sectors(struct mddev *mddev, bool previous)
+{
+ struct r5conf *conf = mddev->private;
+
+ if (previous)
+ return raid5_size(mddev, mddev->dev_sectors,
+ conf->previous_raid_disks);
+ return raid5_size(mddev, mddev->dev_sectors, conf->raid_disks);
+}
+
static struct md_personality raid6_personality =
{
.head = {
@@ -9160,6 +9184,9 @@ static struct md_personality raid6_personality =
.change_consistency_policy = raid5_change_consistency_policy,
.prepare_suspend = raid5_prepare_suspend,
.bitmap_sector = raid5_bitmap_sector,
+ .bitmap_sector_map = raid5_bitmap_sector_map,
+ .bitmap_sync_size = raid5_bitmap_sync_size,
+ .bitmap_array_sectors = raid5_bitmap_array_sectors,
};
static struct md_personality raid5_personality =
{
@@ -9190,6 +9217,9 @@ static struct md_personality raid5_personality =
.change_consistency_policy = raid5_change_consistency_policy,
.prepare_suspend = raid5_prepare_suspend,
.bitmap_sector = raid5_bitmap_sector,
+ .bitmap_sector_map = raid5_bitmap_sector_map,
+ .bitmap_sync_size = raid5_bitmap_sync_size,
+ .bitmap_array_sectors = raid5_bitmap_array_sectors,
};
static struct md_personality raid4_personality =
@@ -9221,6 +9251,9 @@ static struct md_personality raid4_personality =
.change_consistency_policy = raid5_change_consistency_policy,
.prepare_suspend = raid5_prepare_suspend,
.bitmap_sector = raid5_bitmap_sector,
+ .bitmap_sector_map = raid5_bitmap_sector_map,
+ .bitmap_sync_size = raid5_bitmap_sync_size,
+ .bitmap_array_sectors = raid5_bitmap_array_sectors,
};
static int __init raid5_init(void)
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v5 27/29] md/raid5: reject llbitmap reshape when md chunk shrinks
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (25 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 26/29] md/raid5: add exact old and new llbitmap mapping helpers Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:42 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 28/29] md/raid5: wire llbitmap reshape lifecycle Yu Kuai
` (2 subsequent siblings)
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
llbitmap reshape keeps one live bitmap and cannot safely make an
existing bitmap bit cover a smaller data range.
The llbitmap chunksize itself will not shrink when mddev->chunk_sectors
stays the same or grows. However, shrinking mddev->chunk_sectors shrinks
sectors_per_chunk used by raid5_bitmap_sector_map(). That can shrink the
effective data range covered by each bit across the old and new RAID5
geometry. Reject that reshape while llbitmap is active.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/raid5.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 88bf5a9ce573..67d56c92c8a4 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -8580,6 +8580,9 @@ static int check_reshape(struct mddev *mddev)
if (!check_stripe_cache(mddev))
return -ENOSPC;
+ if (mddev->bitmap_id == ID_LLBITMAP &&
+ mddev->new_chunk_sectors < mddev->chunk_sectors)
+ return -EOPNOTSUPP;
if (mddev->new_chunk_sectors > mddev->chunk_sectors ||
mddev->delta_disks > 0)
if (resize_chunks(conf,
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 27/29] md/raid5: reject llbitmap reshape when md chunk shrinks
2026-08-02 19:50 ` [PATCH v5 27/29] md/raid5: reject llbitmap reshape when md chunk shrinks Yu Kuai
@ 2026-08-02 20:42 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:42 UTC (permalink / raw)
To: Yu Kuai; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete check for shrinking sectors_per_chunk during RAID5 llbitmap reshape allows unsupported geometry changes.
--
--- Patch [27]: [PATCH v5 27/29] md/raid5: reject llbitmap reshape when md chunk shrinks ---
commit 25700e5b0d24da1aa9addea044a934a4d475781a
Author: Yu Kuai <yukuai@fygo.io>
md/raid5: reject llbitmap reshape when md chunk shrinks
This commit prevents llbitmap reshapes from proceeding when the md chunk
size shrinks, as this reduces the effective data range covered by each
bitmap bit.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 88bf5a9ce5737..67d56c92c8a44 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -8580,6 +8580,9 @@ static int check_reshape(struct mddev *mddev)
> if (!check_stripe_cache(mddev))
> return -ENOSPC;
>
> + if (mddev->bitmap_id == ID_LLBITMAP &&
> + mddev->new_chunk_sectors < mddev->chunk_sectors)
> + return -EOPNOTSUPP;
[Severity: High]
Does this check adequately cover reshapes that remove disks?
Looking at raid5_bitmap_sector_map(), sectors_per_chunk is calculated as
chunk_sectors * (raid_disks - max_degraded). If a reshape removes disks
(delta_disks < 0), the number of data disks decreases.
This means the effective sectors_per_chunk can shrink even if
new_chunk_sectors is equal to or slightly larger than chunk_sectors.
Will omitting a check for delta_disks < 0 allow these shrinking reshapes
to proceed, causing a single bitmap bit to incorrectly map to a smaller
logical data range and potentially corrupting the dirty bit mapping?
> if (mddev->new_chunk_sectors > mddev->chunk_sectors ||
> mddev->delta_disks > 0)
> if (resize_chunks(conf,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=27
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 28/29] md/raid5: wire llbitmap reshape lifecycle
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (26 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 27/29] md/raid5: reject llbitmap reshape when md chunk shrinks Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-02 20:46 ` sashiko-bot
2026-08-02 19:50 ` [PATCH v5 29/29] md/raid5: split reshape bios before bitmap accounting Yu Kuai
2026-08-03 12:11 ` [PATCH v5 00/29] md: improve lockless bitmap reshape support Mykola Marzhan
29 siblings, 1 reply; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
Prepare llbitmap before RAID5 reshape starts, checkpoint the bitmap
before advancing reshape_position, and finish the llbitmap geometry
update when reshape completes.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/raid5.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 67d56c92c8a4..5176de5b5956 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6497,6 +6497,13 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
|| test_bit(MD_RECOVERY_INTR, &mddev->recovery));
if (atomic_read(&conf->reshape_stripes) != 0)
return 0;
+ if (md_bitmap_enabled(mddev, false) &&
+ mddev->bitmap_ops->reshape_mark &&
+ conf->reshape_safe != conf->reshape_progress) {
+ mddev->bitmap_ops->reshape_mark(mddev, conf->reshape_safe,
+ conf->reshape_progress);
+ mddev->bitmap_ops->unplug(mddev, true);
+ }
mddev->reshape_position = conf->reshape_progress;
mddev->curr_resync_completed = sector_nr;
if (!mddev->reshape_backwards)
@@ -6606,6 +6613,13 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
|| test_bit(MD_RECOVERY_INTR, &mddev->recovery));
if (atomic_read(&conf->reshape_stripes) != 0)
goto ret;
+ if (md_bitmap_enabled(mddev, false) &&
+ mddev->bitmap_ops->reshape_mark &&
+ conf->reshape_safe != conf->reshape_progress) {
+ mddev->bitmap_ops->reshape_mark(mddev, conf->reshape_safe,
+ conf->reshape_progress);
+ mddev->bitmap_ops->unplug(mddev, true);
+ }
mddev->reshape_position = conf->reshape_progress;
mddev->curr_resync_completed = sector_nr;
if (!mddev->reshape_backwards)
@@ -8648,6 +8662,12 @@ static int raid5_start_reshape(struct mddev *mddev)
mdname(mddev));
return -EINVAL;
}
+ if (md_bitmap_enabled(mddev, false) &&
+ mddev->bitmap_id == ID_LLBITMAP) {
+ i = mddev->bitmap_ops->resize(mddev, mddev->dev_sectors, 0);
+ if (i)
+ return i;
+ }
atomic_set(&conf->reshape_stripes, 0);
spin_lock_irq(&conf->device_lock);
@@ -8732,10 +8752,19 @@ static int raid5_start_reshape(struct mddev *mddev)
*/
static void end_reshape(struct r5conf *conf)
{
+ struct mddev *mddev = conf->mddev;
if (!test_bit(MD_RECOVERY_INTR, &conf->mddev->recovery)) {
struct md_rdev *rdev;
+ if (md_bitmap_enabled(mddev, false) &&
+ mddev->bitmap_ops->reshape_mark &&
+ conf->reshape_safe != conf->reshape_progress) {
+ mddev->bitmap_ops->reshape_mark(mddev, conf->reshape_safe,
+ conf->reshape_progress);
+ mddev->bitmap_ops->unplug(mddev, true);
+ }
+
spin_lock_irq(&conf->device_lock);
conf->previous_raid_disks = conf->raid_disks;
md_finish_reshape(conf->mddev);
@@ -8762,8 +8791,16 @@ static void raid5_finish_reshape(struct mddev *mddev)
{
struct r5conf *conf = mddev->private;
struct md_rdev *rdev;
+ bool llbitmap = mddev->bitmap_id == ID_LLBITMAP &&
+ md_bitmap_enabled(mddev, false);
if (!test_bit(MD_RECOVERY_INTR, &mddev->recovery)) {
+ if (llbitmap && mddev->bitmap_ops->reshape_finish)
+ mddev->bitmap_ops->reshape_finish(mddev);
+ if (llbitmap) {
+ mddev->resync_offset = 0;
+ mddev->resync_max_sectors = mddev->dev_sectors;
+ }
if (mddev->delta_disks <= 0) {
int d;
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 28/29] md/raid5: wire llbitmap reshape lifecycle
2026-08-02 19:50 ` [PATCH v5 28/29] md/raid5: wire llbitmap reshape lifecycle Yu Kuai
@ 2026-08-02 20:46 ` sashiko-bot
0 siblings, 0 replies; 54+ messages in thread
From: sashiko-bot @ 2026-08-02 20:46 UTC (permalink / raw)
To: Yu Kuai; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Torn metadata update between llbitmap data and the array superblock during RAID5 reshape, leading to data corruption upon crash recovery.
--
--- Patch [28]: [PATCH v5 28/29] md/raid5: wire llbitmap reshape lifecycle ---
commit 3a12d425cf5a2752c68bb352670db1a3775e5849
Author: Yu Kuai <yukuai@fygo.io>
md/raid5: wire llbitmap reshape lifecycle
Prepare llbitmap before RAID5 reshape starts, checkpoint the bitmap
before advancing reshape_position, and finish the llbitmap geometry
update when reshape completes.
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 67d56c92c8a44..5176de5b59568 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -6497,6 +6497,13 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
> || test_bit(MD_RECOVERY_INTR, &mddev->recovery));
> if (atomic_read(&conf->reshape_stripes) != 0)
> return 0;
> + if (md_bitmap_enabled(mddev, false) &&
> + mddev->bitmap_ops->reshape_mark &&
> + conf->reshape_safe != conf->reshape_progress) {
> + mddev->bitmap_ops->reshape_mark(mddev, conf->reshape_safe,
> + conf->reshape_progress);
> + mddev->bitmap_ops->unplug(mddev, true);
> + }
[Severity: High]
Can this sequence cause a torn metadata update between the llbitmap data
and the array superblock during a reshape?
If a crash occurs in the window immediately after the bitmap pages are
synchronously flushed to disk here, but before the array superblock write
completes (via wait_event(mddev->sb_wait, mddev->sb_flags == 0) shortly
after), the system might reboot with an inconsistent bitmap state.
Since llbitmap relies entirely on the array's mddev->reshape_position
and doesn't track its own, could dirty bits be misread as clean due to
layout misalignment, potentially leading to degraded arrays skipping necessary
resyncs and causing data corruption upon recovery?
[ ... ]
> @@ -8732,10 +8752,19 @@ static int raid5_start_reshape(struct mddev *mddev)
> */
> static void end_reshape(struct r5conf *conf)
> {
> + struct mddev *mddev = conf->mddev;
>
> if (!test_bit(MD_RECOVERY_INTR, &conf->mddev->recovery)) {
> struct md_rdev *rdev;
>
> + if (md_bitmap_enabled(mddev, false) &&
> + mddev->bitmap_ops->reshape_mark &&
> + conf->reshape_safe != conf->reshape_progress) {
> + mddev->bitmap_ops->reshape_mark(mddev, conf->reshape_safe,
> + conf->reshape_progress);
> + mddev->bitmap_ops->unplug(mddev, true);
> + }
> +
[Severity: High]
Does this have the same torn metadata update risk as above?
This flushes the reshaped bitmap layout to disk synchronously before the
array metadata is finalized by md_finish_reshape() shortly after. A crash
here seems like it would leave the bitmap and superblock out of sync in the
same way.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=28
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v5 29/29] md/raid5: split reshape bios before bitmap accounting
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (27 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 28/29] md/raid5: wire llbitmap reshape lifecycle Yu Kuai
@ 2026-08-02 19:50 ` Yu Kuai
2026-08-03 12:11 ` [PATCH v5 00/29] md: improve lockless bitmap reshape support Mykola Marzhan
29 siblings, 0 replies; 54+ messages in thread
From: Yu Kuai @ 2026-08-02 19:50 UTC (permalink / raw)
To: Song Liu, Li Nan, Xiao Ni
Cc: Yu Kuai, linux-raid, linux-kernel, Mykola Marzhan, Su Yue
From: Yu Kuai <yukuai@fygo.io>
RAID5 maps array sectors through different geometries before and after the
reshape position. During llbitmap reshape, md core cannot account one bio
against both geometries as a single bitmap range, because the old and new
bitmap mappings can cover different chunks.
Split bios that cross reshape_position before md_account_bio(), so the
bitmap only sees ranges that belong to one side of the reshape boundary.
mddev_bio_split_at_reshape_offset() uses bio_submit_split_bioset(), which
submits the remainder immediately and returns the front split bio. If that
front bio later has to wait for reshape, md_handle_request() must not retry
the original bio pointer, because after the split that pointer is the
already-submitted remainder. Track whether the split happened, clear the
temporary BLK_STS_RESOURCE status after the internal clone completion, and
resubmit the front bio directly after the reshape wait. Keep the old
return-false retry path for unsplit bios, where md_handle_request() still
owns the same bio.
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
drivers/md/raid5.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 5176de5b5956..b91545ce090d 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6221,9 +6221,11 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
struct r5conf *conf = mddev->private;
const int rw = bio_data_dir(bi);
struct stripe_request_ctx *ctx;
+ struct bio *front_bio;
sector_t logical_sector;
enum stripe_result res;
int s, stripe_cnt;
+ bool split = false;
bool on_wq;
if (unlikely(bi->bi_opf & REQ_PREFLUSH)) {
@@ -6257,6 +6259,18 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
return true;
}
+ front_bio = bi;
+ bi = mddev_bio_split_at_reshape_offset(mddev, bi, NULL,
+ &conf->bio_split);
+ if (!bi) {
+ if (rw == WRITE)
+ md_write_end(mddev);
+ return true;
+ }
+ if (bi != front_bio)
+ split = true;
+ front_bio = bi;
+
logical_sector = bi->bi_iter.bi_sector & ~((sector_t)RAID5_STRIPE_SECTORS(conf)-1);
bi->bi_next = NULL;
@@ -6348,6 +6362,11 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
bio_endio(bi);
wait_for_completion(&done);
+ front_bio->bi_status = BLK_STS_OK;
+ if (split) {
+ submit_bio_noacct(front_bio);
+ return true;
+ }
return false;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v5 00/29] md: improve lockless bitmap reshape support
2026-08-02 19:50 [PATCH v5 00/29] md: improve lockless bitmap reshape support Yu Kuai
` (28 preceding siblings ...)
2026-08-02 19:50 ` [PATCH v5 29/29] md/raid5: split reshape bios before bitmap accounting Yu Kuai
@ 2026-08-03 12:11 ` Mykola Marzhan
29 siblings, 0 replies; 54+ messages in thread
From: Mykola Marzhan @ 2026-08-03 12:11 UTC (permalink / raw)
To: Yu Kuai; +Cc: Song Liu, Li Nan, Xiao Ni, Su Yue, linux-raid, linux-kernel
Hi Kuai,
I ran v5 through the same rig, on the md-7.3 base.
Since patches 6-9 are new I aimed tests at them: array stops under
sustained write load, including raid1 with write-behind (patch 6),
a forced llbitmap create failure (patch 8), and a check that
BITMAP_SHUTDOWN never reaches the on-disk superblock (patch 9).
All clean, so for patches 6-9 as well:
Tested-by: Mykola Marzhan <mykola@meshstor.io>
Thanks,
Mykola
^ permalink raw reply [flat|nested] 54+ messages in thread