[PATCH v3 00/21] md/md-llbitmap: support reshape for RAID10 and RAID5

[PATCH v3 00/21] md/md-llbitmap: support reshape for RAID10 and RAID5

[Yu Kuai]

From: Yu Kuai <yukuai@fygo.io>

Hi,

This series adds llbitmap support for online reshape in RAID10 and RAID5.

llbitmap has a different set of constraints from the existing bitmap code:
there is one live bitmap instance, each bit state has richer semantics, and
reshape can change the mapping from logical array ranges to bitmap ranges.
The series therefore adds exact bitmap range mapping hooks, tracks old and
new llbitmap geometry during reshape, remaps checkpointed bits as reshape
progresses, and wires the reshape lifecycle into RAID10 and RAID5.

The main rules are:

1. split bios at the reshape position before bitmap accounting, so one bio
   is never accounted with mixed old/new geometry;
2. do not skip reshape ranges from stale llbitmap state, because reshape
   progress is checkpointed by array metadata;
3. remap llbitmap bits when reshape progress is checkpointed;
4. reject llbitmap reshape if mddev->chunk_sectors shrinks, because the
   effective data range represented by existing bitmap bits can shrink.

The first patch fixes RAID5 bitmap stripe rounding before the exact mapping
hooks are added, so the later RAID5 helper split starts from correct
sector_t division semantics. The rest of the series prepares generic bitmap
and llbitmap infrastructure, wires RAID10, and then wires RAID5 with exact
old/new stripe mapping.

Changes since v2:
- Fix RAID5 bitmap stripe rounding for non-power-of-two stripe widths in
  patch 1.
- Fix partial discard chunks being marked unwritten in patch 11.
- Fix RAID5 writes past component size being skipped by llbitmap accounting
  in patch 11.
- Fix stopped reshape reassembly losing target llbitmap geometry in patch 9.
- Fix the page-cache-grow patch using target-geometry fields before they are
  introduced in patch 7.

Changes since v1:
- Add Reviewed-by tags from Su Yue.
- Rename llbitmap_resize_chunks() to llbitmap_calculate_chunks().
- Use an unsigned index in llbitmap_expand_pages() error cleanup.
- Rebase on mdraid/md-7.2, including the mddev_bio_split_at_reshape_offset()
  declaration needed by the RAID5 build reported by kernel test robot.

Yu Kuai (21):
  md/raid5: round bitmap stripes with sector division
  md: add exact bitmap mapping and reshape hooks
  md: skip bitmap accounting for empty write ranges
  md: add helper to split bios at reshape offset
  md/md-llbitmap: track bitmap sync_size explicitly
  md/md-llbitmap: allocate page controls independently
  md/md-llbitmap: grow the page cache in place for reshape
  md/md-llbitmap: track target reshape geometry fields
  md/md-llbitmap: finish reshape geometry
  md/md-llbitmap: refuse reshape while llbitmap still needs sync
  md/md-llbitmap: add reshape range mapping helpers
  md/md-llbitmap: don't skip reshape ranges from bitmap state
  md/md-llbitmap: remap checkpointed bits as reshape progresses
  md/md-llbitmap: clamp state-machine walks to tracked bits
  md/raid10: reject llbitmap reshape when md chunk shrinks
  md/raid10: wire llbitmap reshape lifecycle
  md/raid10: split reshape bios before bitmap accounting
  md/raid5: add exact old and new llbitmap mapping helpers
  md/raid5: reject llbitmap reshape when md chunk shrinks
  md/raid5: wire llbitmap reshape lifecycle
  md/raid5: split reshape bios before bitmap accounting

 drivers/md/md-bitmap.c   |   8 +
 drivers/md/md-bitmap.h   |   9 +
 drivers/md/md-llbitmap.c | 659 ++++++++++++++++++++++++++++++++++++---
 drivers/md/md.c          |  64 +++-
 drivers/md/md.h          |   8 +
 drivers/md/raid10.c      |  49 +++
 drivers/md/raid5.c       | 123 ++++++--
 7 files changed, 842 insertions(+), 78 deletions(-)


base-commit: 55b77337bdd088c77461588e5ec094421b89911b
-- 
2.51.0

[PATCH v3 01/21] md/raid5: round bitmap stripes with sector division

[Yu Kuai]

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/
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 0c5c9fb0606e..35910bc6683b 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5956,8 +5956,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);
@@ -5975,8 +5978,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

Re: [PATCH v3 01/21] md/raid5: round bitmap stripes with sector division

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The commit message references a non-existent function `raid5_bitmap_sector_map()`.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the logic for calculating the previous array layout's upper bound `prev_end` incorrectly rounds down instead of rounding up, truncating the bitmap update range during an array reshape.
--

--- Patch [1]: [PATCH v3 01/21] md/raid5: round bitmap stripes with sector division ---
commit a735680352819f937150899bfef4b4a4111690e0
Author: Yu Kuai <yukuai@fygo.io>

md/raid5: round bitmap stripes with sector division

This commit fixes rounding logic when aligning the array range to full RAID5
stripe widths. It replaces the mask-based round_down() and round_up() logic
with sector_div() based arithmetic to correctly handle stripe widths that are
not powers of two.

Link: https://lore.kernel.org/all/20260726185916.2223460-1-mykola@meshstor.io/

> md/raid5: round bitmap stripes with sector division
> 
> raid5_bitmap_sector_map() aligns the array range to full RAID5 stripe
> widths before converting it to component sectors. That width is

[Severity: Medium]
Does the commit message reference a function that isn't included in this
patch? The modifications appear to be applied directly inline within
raid5_bitmap_sector(), rather than a new raid5_bitmap_sector_map() helper.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 0c5c9fb0606ee..35910bc6683bf 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -5956,8 +5956,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);

[ ... ]

> @@ -5975,8 +5978,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: High]
This isn't a bug introduced by this patch, but does the division logic for
prev_end truncate the tracked bitmap range during an array reshape?

Because prev_end serves as an exclusive upper bound, mathematically rounding
it down (which the previous round_down() logic also did) means that if an I/O
write crosses a chunk boundary, it gets a remainder when divided by
sectors_per_chunk. 

Since the remainder doesn't trigger an increment (like it now does for end),
dirty sectors at the end of the I/O are not marked in the bitmap for the
previous layout. If a crash occurs, these unmarked sectors wouldn't be
resynced, which could lead to stale parity and silent data corruption.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=1

[PATCH v3 02/21] md: add exact bitmap mapping and reshape hooks

[Yu Kuai]

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.

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          | 12 ++++++++----
 drivers/md/md.h          |  4 ++++
 5 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 0f02e2956398..4c31807e6bcc 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)
 {
@@ -3083,6 +3090,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 f46674bdfeb9..26cd07737aab 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -93,6 +93,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 5a4e2abaa757..f0c20afa354e 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1188,6 +1188,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)
 {
@@ -1780,6 +1787,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 d1465bcd86c8..3b20a57b8c7e 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9361,6 +9361,12 @@ void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
 }
 EXPORT_SYMBOL_GPL(md_submit_discard_bio);
 
+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)
 {
@@ -9368,10 +9374,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);
 	fn(mddev, md_io_clone->offset, md_io_clone->sectors);
 }
 
diff --git a/drivers/md/md.h b/drivers/md/md.h
index d8daf0f75cbb..110cf0f8b107 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -798,6 +798,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

[PATCH v3 03/21] md: skip bitmap accounting for empty write ranges

[Yu Kuai]

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.

The new bitmap prepare_range() hook 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.

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 3b20a57b8c7e..0b59c676f7c0 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9376,6 +9376,8 @@ static void md_bitmap_start(struct mddev *mddev,
 
 	md_bitmap_prepare_range(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);
 }
 
@@ -9396,7 +9398,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)
@@ -9423,10 +9426,12 @@ 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);
 
-	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

[PATCH v3 04/21] md: add helper to split bios at reshape offset

[Yu Kuai]

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 helper in
multiple places.

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 0b59c676f7c0..c18a7f3c27e6 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9361,6 +9361,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 = 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_prepare_range(struct mddev *mddev, sector_t *offset,
 				    unsigned long *sectors)
 {
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 110cf0f8b107..ebfc6da83161 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -925,6 +925,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

Re: [PATCH v3 04/21] md: add helper to split bios at reshape offset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lockless read of the 64-bit `mddev->reshape_position` and missing memory barriers after `MD_RECOVERY_RESHAPE` cause torn reads and inconsistent state observation.
--

--- Patch [4]: [PATCH v3 04/21] md: add helper to split bios at reshape offset ---
commit df6c72a7c4a6271ac6b7e8dc1420ce49f5ef19d3
Author: Yu Kuai <yukuai@fygo.io>

md: add helper to split bios at reshape offset

This commit adds mddev_bio_split_at_reshape_offset() so personalities can
share reshape-offset bio splitting instead of open-coding the same helper
in multiple places.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 0b59c676f7c03..c18a7f3c27e65 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -9361,6 +9361,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 = mddev->reshape_position;

[Severity: High]
Can this lockless read of mddev->reshape_position lead to torn reads?

Since reshape_position is a 64-bit value, reading it concurrently with
background reshape updates from sync_thread might result in a torn read on
32-bit architectures. A torn read could produce an arbitrary split boundary,
potentially causing misaligned bio splits and IO errors.

Additionally, are memory barriers needed after testing MD_RECOVERY_RESHAPE
to ensure a consistent value is observed on weakly ordered architectures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=4

[PATCH v3 05/21] md/md-llbitmap: track bitmap sync_size explicitly

[Yu Kuai]

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>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
 drivers/md/md-llbitmap.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index f0c20afa354e..e3500a1a52c0 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
@@ -918,6 +920,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);
@@ -938,6 +941,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;
@@ -987,6 +991,9 @@ 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;
 	chunksize = le32_to_cpu(sb->chunksize);
 	if (!is_power_of_2(chunksize)) {
 		pr_err("md/llbitmap: %s: chunksize not a power of 2",
@@ -1022,8 +1029,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:
@@ -1153,6 +1161,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;
 }
@@ -1526,7 +1535,7 @@ static void llbitmap_update_sb(void *data)
 	sb->events = cpu_to_le64(mddev->events);
 	sb->state = cpu_to_le32(llbitmap->flags);
 	sb->chunksize = cpu_to_le32(llbitmap->chunksize);
-	sb->sync_size = cpu_to_le64(mddev->resync_max_sectors);
+	sb->sync_size = cpu_to_le64(llbitmap->sync_size);
 	sb->events_cleared = cpu_to_le64(llbitmap->events_cleared);
 	sb->sectors_reserved = cpu_to_le32(mddev->bitmap_info.space);
 	sb->daemon_sleep = cpu_to_le32(mddev->bitmap_info.daemon_sleep);
@@ -1544,6 +1553,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

Re: [PATCH v3 05/21] md/md-llbitmap: track bitmap sync_size explicitly

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Critical] Missing upper and lower bounds validation on the newly tracked `sync_size` read from the superblock leads to arbitrary out-of-bounds memory accesses.
- [High] Validation of `chunksize` in `llbitmap_read_sb` incorrectly uses `mddev->resync_max_sectors` instead of `sync_size`.
- [High] Torn read of the 64-bit `sync_size` field leading to superblock corruption and memory allocation bugs on reboot, along with unprotected bitmap geometry updates.

Pre-existing issues:
- [High] A teardown race condition in `llbitmap_destroy()` fails to prevent a workqueue task from re-arming a timer after it has been deleted, causing a UAF.
--

--- Patch [5]: [PATCH v3 05/21] md/md-llbitmap: track bitmap sync_size explicitly ---
commit e1b87a51dfe616ddb9c0f12bcc69bdac5d5044d5
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: track bitmap sync_size explicitly
    
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.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index f0c20afa354eb..e3500a1a52c07 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c

[ ... ]

> @@ -987,6 +991,9 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
>  		goto out_put_page;
>  	}
>  
> +	sync_size = le64_to_cpu(sb->sync_size);

[Severity: Critical]
Is it necessary to validate the upper and lower bounds of the sync_size
read from the superblock here?

If sync_size is manipulated to be near U64_MAX, calculating nr_pages
might truncate to a small 32-bit integer, while chunks remains huge.
Could this cause llbitmap_init_state() to perform massive out-of-bounds
writes into the undersized pctl array?

Alternatively, if sync_size is too small, nr_pages is small, but incoming
IO up to mddev->resync_max_sectors calculates a page_start that exceeds
the bounds of pctl:

drivers/md/md-llbitmap.c:llbitmap_start_write() {
    ...
    int page_start = (start + BITMAP_DATA_OFFSET) >> PAGE_SHIFT;
    ...
    while (page_start <= page_end) {
        llbitmap_raise_barrier(llbitmap, page_start);
    ...
}

Could this lead to out-of-bounds memory accesses if a maliciously crafted
bitmap superblock is mounted?


[Severity: High]
Should the chunksize validation further down in llbitmap_read_sb() be
updated to use sync_size as well?

    if (chunksize < DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors,
                                          mddev->bitmap_info.space << SECTOR_SHIFT)) {

If an array is grown and stopped before the bitmap is resized,
mddev->resync_max_sectors reflects the new larger size, but the bitmap
superblock retains the old sync_size and chunksize.

Will checking if the old chunksize can cover the new resync_max_sectors
cause a false failure and abort bitmap initialization in this scenario?

> +	if (!sync_size)
> +		sync_size = mddev->resync_max_sectors;
>  	chunksize = le32_to_cpu(sb->chunksize);
>  	if (!is_power_of_2(chunksize)) {

[ ... ]

> @@ -1153,6 +1161,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;
>  }

[Severity: High]
Are these updates to the 64-bit sync_size and geometry variables
adequately protected from concurrent access?

Unlike the legacy bitmap which uses mddev->pers->quiesce() to pause IO,
llbitmap_resize() does not pause IO. This allows active IO paths to read
a mix of old and new geometry values.

Additionally, llbitmap_update_sb() concurrently reads llbitmap->sync_size
locklessly:

drivers/md/md-llbitmap.c:llbitmap_update_sb() {
    ...
    sb->sync_size = cpu_to_le64(llbitmap->sync_size);
    ...
}

On 32-bit platforms, might this race result in a torn read of the 64-bit
value, causing a corrupted value to be written to the disk superblock?

[ ... ]

This is a pre-existing issue, but does the teardown sequence in
llbitmap_destroy() allow the pending_timer to be re-armed after deletion?

In llbitmap_destroy():

    timer_delete_sync(&llbitmap->pending_timer);
    flush_workqueue(md_llbitmap_io_wq);

If the daemon work is concurrently running in the workqueue,
timer_delete_sync() returns immediately because the timer is no longer
pending.

The workqueue is then flushed, but at the end of the worker function
md_llbitmap_daemon_fn(), it unconditionally re-arms the timer:

    if (restart)
        mod_timer(&llbitmap->pending_timer,
                  jiffies + mddev->bitmap_info.daemon_sleep * HZ);

When llbitmap is subsequently freed, will the re-armed timer dereference
the freed memory when it fires?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=5

[PATCH v3 06/21] md/md-llbitmap: allocate page controls independently

[Yu Kuai]

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>
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 e3500a1a52c0..4c3fc4b35aff 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_KERNEL | __GFP_ZERO);
 	if (!page)
 		return ERR_PTR(-ENOMEM);
+	if (idx >= llbitmap_used_pages(llbitmap, llbitmap->chunks))
+		return page;
 
 	rdev_for_each(rdev, mddev) {
 		sector_t sector;
@@ -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_KERNEL | __GFP_ZERO);
-	if (!llbitmap->pctl)
-		return -ENOMEM;
 
 	size = round_up(size, cache_line_size());
-	pctl = kmalloc_array(nr_pages, size, GFP_KERNEL | __GFP_ZERO);
-	if (!pctl) {
-		kfree(llbitmap->pctl);
-		return -ENOMEM;
+	pctl = kzalloc(size, GFP_KERNEL);
+	if (!pctl)
+		return ERR_PTR(-ENOMEM);
+
+	page = llbitmap_read_page(llbitmap, idx);
+
+	if (IS_ERR(page)) {
+		kfree(pctl);
+		return ERR_CAST(page);
 	}
 
-	llbitmap->nr_pages = nr_pages;
+	if (percpu_ref_init(&pctl->active, active_release,
+			    PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
+		__free_page(page);
+		kfree(pctl);
+		return ERR_PTR(-ENOMEM);
+	}
 
-	for (i = 0; i < nr_pages; i++, pctl = (void *)pctl + size) {
-		struct page *page = llbitmap_read_page(llbitmap, i);
+	pctl->page = page;
+	pctl->state = page_address(page);
+	init_waitqueue_head(&pctl->wait);
+	return pctl;
+}
 
-		llbitmap->pctl[i] = pctl;
+static unsigned int llbitmap_reserved_pages(struct llbitmap *llbitmap)
+{
+	return DIV_ROUND_UP(llbitmap->mddev->bitmap_info.space << SECTOR_SHIFT,
+			    PAGE_SIZE);
+}
 
-		if (IS_ERR(page)) {
-			llbitmap_free_pages(llbitmap);
-			return PTR_ERR(page);
-		}
+static int llbitmap_alloc_pages(struct llbitmap *llbitmap)
+{
+	unsigned int used_pages = llbitmap_used_pages(llbitmap, llbitmap->chunks);
+	unsigned int nr_pages = max(used_pages, llbitmap_reserved_pages(llbitmap));
+	int i;
+
+	llbitmap->pctl = kcalloc(nr_pages, sizeof(*llbitmap->pctl), GFP_KERNEL);
+	if (!llbitmap->pctl)
+		return -ENOMEM;
 
-		if (percpu_ref_init(&pctl->active, active_release,
-				    PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) {
-			__free_page(page);
+	llbitmap->nr_pages = nr_pages;
+
+	for (i = 0; i < nr_pages; i++) {
+		llbitmap->pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i);
+		if (IS_ERR(llbitmap->pctl[i])) {
+			int ret = PTR_ERR(llbitmap->pctl[i]);
+
+			llbitmap->pctl[i] = NULL;
 			llbitmap_free_pages(llbitmap);
-			return -ENOMEM;
+			return ret;
 		}
-
-		pctl->page = page;
-		pctl->state = page_address(page);
-		init_waitqueue_head(&pctl->wait);
 	}
 
 	return 0;
@@ -923,7 +948,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;
 
@@ -1032,7 +1057,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

Re: [PATCH v3 06/21] md/md-llbitmap: allocate page controls independently

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Out-of-bounds array access in `llbitmap_write()` during bitmap resize due to omitted `BITMAP_DATA_OFFSET` in `llbitmap_reserved_pages()`.
--

--- Patch [6]: [PATCH v3 06/21] md/md-llbitmap: allocate page controls independently ---
commit f9c192fc9c795037298b0412c1ab2a50e3c88637
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: allocate page controls independently

This commit modifies the allocation of llbitmap page controls to
allocate and free them individually. It also updates llbitmap_read_page()
to return a zeroed page when the index is beyond the current size,
preparing the code for in-place growth.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index e3500a1a52c07..4c3fc4b35aff0 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: High]
Does this calculation underestimate the maximum number of pages needed
for in-place growth by omitting BITMAP_DATA_OFFSET?

If an array is configured with a small initial capacity and a bitmap space
that is a multiple of PAGE_SIZE, llbitmap_alloc_pages() might allocate
the pctl array with too few pages because this function omits the offset.

If the array is later resized up to the maximum capacity allowed by the space,
would this cause an out-of-bounds access in llbitmap_write()?

llbitmap_write() calculates the page index as:

    pos += BITMAP_DATA_OFFSET;
    idx = pos >> PAGE_SHIFT;

    llbitmap->pctl[idx]->state[bit] = state;

If pos pushes idx beyond the under-allocated nr_pages, it looks like this
will result in an out-of-bounds array access.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=6

[PATCH v3 07/21] md/md-llbitmap: grow the page cache in place for reshape

[Yu Kuai]

From: Yu Kuai <yukuai@fygo.io>

Use the page-control helpers to grow llbitmap's cached pages in place
for resize and later reshape preparation, instead of rebuilding the
whole cache.

Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
 drivers/md/md-llbitmap.c | 136 +++++++++++++++++++++++++++++++++++----
 1 file changed, 124 insertions(+), 12 deletions(-)

diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 4c3fc4b35aff..1ab9b1689fd8 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_KERNEL);
+	if (!pctl)
+		return -ENOMEM;
+
+	if (llbitmap->pctl)
+		memcpy(pctl, llbitmap->pctl,
+		       array_size(old_nr_pages, sizeof(*pctl)));
+
+	for (i = old_nr_pages; i < nr_pages; i++) {
+		pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i);
+		if (IS_ERR(pctl[i]))
+			goto err_alloc_ptr;
+	}
+
+	kfree(llbitmap->pctl);
+	llbitmap->pctl = pctl;
+	llbitmap->nr_pages = nr_pages;
+	return 0;
+
+err_alloc_ptr:
+	ret = PTR_ERR(pctl[i]);
+	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;
@@ -1026,10 +1109,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;
 	}
@@ -1171,24 +1254,53 @@ static int llbitmap_create(struct mddev *mddev)
 static int llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
 {
 	struct llbitmap *llbitmap = mddev->bitmap;
+	sector_t old_blocks = llbitmap->sync_size;
+	unsigned long old_chunks = llbitmap->chunks;
 	unsigned long chunks;
+	unsigned long cache_chunks;
+	int ret = 0;
+	unsigned long bitmap_chunksize;
+	bool reshape;
 
 	if (chunksize == 0)
 		chunksize = llbitmap->chunksize;
 
-	/* If there is enough space, leave the chunksize unchanged. */
-	chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
-	while (chunks > mddev->bitmap_info.space << SECTOR_SHIFT) {
-		chunksize = chunksize << 1;
-		chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
-	}
+	bitmap_chunksize = chunksize;
+	llbitmap_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;
+
+	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);
 	return 0;
+
+out:
+	mutex_unlock(&mddev->bitmap_info.mutex);
+	return ret;
 }
 
 static int llbitmap_load(struct mddev *mddev)
-- 
2.51.0

Re: [PATCH v3 07/21] md/md-llbitmap: grow the page cache in place for reshape

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

New issues:
- [High] Use-After-Free on `llbitmap->pctl` array during bitmap resize.
- [High] Block IO recursion deadlock due to `GFP_KERNEL` allocation under `reconfig_mutex`.
- [High] Out-Of-Bounds array access during resize due to stale `chunkshift`.

Pre-existing issues:
- [High] Divide-by-zero crash in `llbitmap_calculate_chunks` on 32-bit systems due to `chunksize` overflow.
- [High] Use-After-Free timer race during `llbitmap_destroy`.
- [High] Out-of-Bounds disk write due to missing `BITMAP_DATA_OFFSET` in calculation.
--

--- Patch [7]: [PATCH v3 07/21] md/md-llbitmap: grow the page cache in place for reshape ---
commit 7e002f8cabd473784285f63600d75c1d7d7d30c4
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: grow the page cache in place for reshape

This patch uses the page-control helpers to grow the 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 4c3fc4b35aff0..1ab9b1689fd80 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: High]
This is a pre-existing issue, but does this calculation allow the on-disk
footprint to exceed the reserved space limit?

The check ensures *chunks does not exceed mddev->bitmap_info.space, but
the on-disk layout includes an additional 1024 bytes (BITMAP_DATA_OFFSET)
for the superblock before the raw bitmap state data.

When llbitmap_write_page() writes the tail chunks to disk, could it write
1024 bytes past the reserved bitmap region and corrupt adjacent data?

[Severity: High]
This is a pre-existing issue, but does this loop risk a divide by zero on
32-bit systems?

If *chunksize is 32 bits, and the array is extremely large with very small
reserved space, *chunksize could shift until it overflows to 0.

Would the subsequent call to DIV_ROUND_UP_SECTOR_T(blocks, 0) cause a crash
when the sysfs size is written?

> +		*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;
> +	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_KERNEL);

[Severity: High]
Can this allocation trigger a block IO recursion deadlock?

This is reached via array_size_store() which acquires mddev_lock()
(taking reconfig_mutex) without setting memalloc_noio_save() context.

Under memory pressure, GFP_KERNEL can enter direct reclaim and issue I/O
writeback to the MD array. If the array superblock needs updating,
md_write_start() waits on mddev->sb_wait for MD_SB_CHANGE_PENDING to clear.

Since the management thread must clear this flag but blocks trying to acquire
reconfig_mutex held by this resizing thread, does this result in a deadlock?

> +	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;

[Severity: High]
Could this cause a use-after-free if concurrent IO is happening?

The MD array does not suspend IO for sysfs-triggered resize operations.
The concurrent IO path accesses this array in llbitmap_start_write():

llbitmap_start_write()
    llbitmap_raise_barrier(llbitmap, page_start);

This happens without holding mddev->bitmap_info.mutex and without RCU
protection. If the IO path dereferences the array just after this kfree()
completes, will it trigger a panic?

> +	llbitmap->nr_pages = nr_pages;
> +	return 0;
> +

[ ... ]

> @@ -1171,24 +1254,53 @@ static int llbitmap_create(struct mddev *mddev)
>  static int llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
>  {

[ ... ]

> +	if (reshape) {
> +		llbitmap->chunks = max(old_chunks, chunks);
> +	} else {

[Severity: High]
Does this cause an out-of-bounds array access during a reshape?

During reshape, this path updates llbitmap->chunks to a potentially larger
capacity but does not update llbitmap->chunkshift.

The active IO path continues to map array offsets to bitmap chunk indices
using the smaller old chunkshift:

llbitmap_start_write()
    start = offset >> llbitmap->chunkshift;
    ...
    page_start = (start + BITMAP_DATA_OFFSET) >> PAGE_SHIFT;

Because the old chunkshift maps to a larger number of total chunks than the
new chunks capacity (which was reduced by doubling bitmap_chunksize), could
an IO towards the end of the new array compute a page_start index that
exceeds the capacity of the newly reallocated llbitmap->pctl array?

> +		if (blocks < old_blocks && chunks < old_chunks)
> +			llbitmap_mark_range(llbitmap, chunks, old_chunks - 1,
> +					    BitUnwritten);
> +		mddev->bitmap_info.chunksize = bitmap_chunksize;
> +		llbitmap->chunks = chunks;
> +		llbitmap->sync_size = blocks;
> +		llbitmap_update_sb(llbitmap);
> +	}
> +	__llbitmap_flush(mddev);
> +	mutex_unlock(&mddev->bitmap_info.mutex);
>  	return 0;

[Severity: High]
This is a pre-existing issue, but is there a use-after-free race between
llbitmap_destroy() and the daemon work queue?

llbitmap_destroy() deletes the timer and then flushes the workqueue:

llbitmap_destroy()
    timer_delete_sync(&llbitmap->pending_timer);
    flush_workqueue(md_llbitmap_io_wq);

If md_llbitmap_daemon_fn() is actively executing in the workqueue while
teardown happens, the timer is not pending, so timer_delete_sync() returns
immediately.

The running daemon work then evaluates restart = true and re-arms the timer:

md_llbitmap_daemon_fn()
    if (restart)
        mod_timer(&llbitmap->pending_timer, ...);

Could flush_workqueue() complete, allowing llbitmap_destroy() to free the
llbitmap structure, while the re-armed timer later fires and accesses the
freed memory?

> +
> +out:
> +	mutex_unlock(&mddev->bitmap_info.mutex);
> +	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=7

[PATCH v3 08/21] md/md-llbitmap: track target reshape geometry fields

[Yu Kuai]

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.

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 1ab9b1689fd8..cdced2126e97 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;
@@ -1029,6 +1065,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);
@@ -1140,6 +1177,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:
@@ -1284,6 +1322,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)
@@ -1292,6 +1333,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

Re: [PATCH v3 08/21] md/md-llbitmap: track target reshape geometry fields

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Mismatch between bitmap memory allocation (scaled by the new doubled chunksize) and IO indexing (which continues to use the old smaller chunkshift), leading to an out-of-bounds read/write on the `llbitmap->pctl` array.
- [High] The newly added reshape geometry tracking fields (reshape_sync_size, reshape_chunksize, reshape_chunks) are written to but never applied, causing the bitmap to permanently use the old geometry after an array reshape completes.
--

--- Patch [8]: [PATCH v3 08/21] md/md-llbitmap: track target reshape geometry fields ---
commit 63e12bf7c87e70924898a94a3939282826b19c8e
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: track target reshape geometry fields

This commit adds tracking for target reshape geometry in the llbitmap module.
It adds the reshape geometry fields to the llbitmap struct and introduces a
refresh helper. It also updates 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 1ab9b1689fd80..cdced2126e975 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c

[ ... ]

> @@ -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);

[Severity: Critical]
Can this mismatch between allocation and IO indexing cause an out-of-bounds
access on the pctl array?

When an array is grown significantly, chunksize doubles, and the memory
allocation here scales based on the new larger chunksize. However, IO
indexing in llbitmap_start_write() computes the start using the older,
smaller chunkshift:

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;
    int page_end = (end + BITMAP_DATA_OFFSET) >> PAGE_SHIFT;

    while (page_start <= page_end) {
        llbitmap_raise_barrier(llbitmap, page_start);
        page_start++;
    }
}

If writes hit the newly expanded region, won't page_start vastly exceed
llbitmap->nr_pages and crash before any bounds-checking can occur?

> +}

[ ... ]

> @@ -1284,6 +1322,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;

[Severity: High]
Does this code ever apply these new geometry tracking fields to the active
state after a reshape completes?

The resize path populates reshape_sync_size, reshape_chunksize, and
reshape_chunks, but there doesn't appear to be any transition logic or
a reshape finish hook to apply them to the active geometry. 

Will array writes to newly grown regions fall outside the bitmap's tracked
chunks and be silently ignored, resulting in a loss of redundancy?

>  		llbitmap->chunks = max(old_chunks, chunks);
>  	} else {
>  		if (blocks < old_blocks && chunks < old_chunks)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=8

[PATCH v3 09/21] md/md-llbitmap: finish reshape geometry

[Yu Kuai]

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/
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 cdced2126e97..55ab2a8a11e5 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1349,11 +1349,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;
 }
 
@@ -1680,6 +1689,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);
@@ -1977,6 +2010,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

Re: [PATCH v3 09/21] md/md-llbitmap: finish reshape geometry

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] The newly implemented `.reshape_finish` callback is never called by the md core, leaving the staged geometry uncommitted at runtime.
- [High] Concurrent array resize causes Use-After-Free of `llbitmap->pctl`.

Pre-existing issues:
- [High] Teardown symmetry violation in `llbitmap_destroy` leading to UAF on `llbitmap`.
- [High] Permanent crippling of the bitmap daemon expiration logic after any flush operation.
--

--- Patch [9]: [PATCH v3 09/21] md/md-llbitmap: finish reshape geometry ---
commit 83899f955ca5cb817105e296dabf1a5632ece1df
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: finish reshape geometry
    
When assembling a stopped reshape, md_run() creates the bitmap before
publishing mddev->pers. This commit refreshes the staged reshape geometry
again from llbitmap_load() after mddev->pers is available, and expands
the in-memory page controls before replaying 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 cdced2126e975..55ab2a8a11e5f 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1349,11 +1349,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);

[Severity: High]
Can this concurrent array resize cause a use-after-free of the page control
array?

If llbitmap_expand_pages() frees the active pctl array while replacing it
with a new one:

drivers/md/md-llbitmap.c:llbitmap_expand_pages() {
    ...
    kfree(llbitmap->pctl);
    llbitmap->pctl = pctl;
    ...
}

Normal read/write I/O paths like llbitmap_start_write() call
llbitmap_raise_barrier(), which locklessly dereferences the array:

drivers/md/md-llbitmap.c:llbitmap_raise_barrier() {
    ...
    struct llbitmap_page_ctl *pctl = llbitmap->pctl[page_idx];
    ...
}

Does this require synchronization, such as quiesce or RCU, to safely update
the array without causing a use-after-free during concurrent I/O?

> +	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;
>  }
>  

[ ... ]

> @@ -1680,6 +1689,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);

[Severity: High]
This is a pre-existing issue, but does calling llbitmap_flush() permanently
cripple the bitmap daemon's expiration logic?

llbitmap_flush() sets the LLPageFlush flag on all page control structures to
force immediate flushing:

drivers/md/md-llbitmap.c:llbitmap_flush() {
    ...
    for (i = 0; i < llbitmap->nr_pages; i++)
        set_bit(LLPageFlush, &llbitmap->pctl[i]->flags);
    ...
}

The daemon skips the expiration check if this flag is set:

drivers/md/md-llbitmap.c:md_llbitmap_daemon_fn() {
    ...
    if (!test_bit(LLPageFlush, &pctl->flags) &&
        time_before(jiffies, pctl->expire)) {
        restart = true;
        continue;
    ...
}

Since there doesn't seem to be a corresponding mechanism to ever clear the
LLPageFlush flag, won't all subsequent I/O bypass write batching and flush
immediately?

> +
> +	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);
> +}
> +

[ ... ]

> @@ -1977,6 +2010,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 the newly added .reshape_finish callback ever invoked by the md core?

Without a caller in the md core, it appears the staged reshape geometry
remains uncommitted at runtime, which would leave the active geometry stale.
Can this result in incorrect bitmap behavior or data corruption during or
after a reshape?

>  	.write_all		= llbitmap_write_all,
>  
>  	.groups			= md_llbitmap_groups,

As a side note, I noticed another pre-existing issue in the teardown path:

[Severity: High]
This is a pre-existing issue, but does llbitmap_destroy() have a teardown
symmetry violation that could lead to a use-after-free on the llbitmap
structure?

When stopping an array, llbitmap_destroy() deletes the timer and flushes the
workqueue without setting a termination flag:

drivers/md/md-llbitmap.c:llbitmap_destroy() {
    ...
    timer_delete_sync(&llbitmap->pending_timer);
    flush_workqueue(md_llbitmap_io_wq);
    flush_workqueue(md_llbitmap_unplug_wq);

    mddev->bitmap = NULL;
    llbitmap_free_pages(llbitmap);
    kfree(llbitmap);
}

If the daemon work item executes on the workqueue, it can re-arm the timer if
there are unexpired dirty pages:

drivers/md/md-llbitmap.c:md_llbitmap_daemon_fn() {
    ...
    if (restart)
        mod_timer(&llbitmap->pending_timer,
                  jiffies + llbitmap->mddev->bitmap_info.daemon_sleep * HZ);
    ...
}

Because md_llbitmap_daemon_fn() doesn't check for a teardown flag, couldn't
the timer be re-armed, and then fire later on the freed llbitmap memory after
llbitmap_destroy() completes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=9

[PATCH v3 10/21] md/md-llbitmap: refuse reshape while llbitmap still needs sync

[Yu Kuai]

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.

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 55ab2a8a11e5..f5df82adef64 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1689,6 +1689,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;
@@ -2011,6 +2034,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

Re: [PATCH v3 10/21] md/md-llbitmap: refuse reshape while llbitmap still needs sync

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The newly introduced reshape rejection callback is never invoked by the core MD subsystem, rendering the fix ineffective.
- [Medium] The `llbitmap_reshape_can_start` function caches the bitmap pointer outside of the mutex lock, creating a Time-Of-Check to Time-Of-Use vulnerability that can lead to a Use-After-Free.
- [Medium] The reshape check ignores `BitNeedSyncUnwritten` and `BitSyncingUnwritten`, allowing reshape to start during proactive sync.
--

--- Patch [10]: [PATCH v3 10/21] md/md-llbitmap: refuse reshape while llbitmap still needs sync ---
commit b7405e7962d5eb002f38e66bc1e8cec9737a66ea
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: refuse reshape while llbitmap still needs sync

This commit rejects reshape operations when the llbitmap still contains
NeedSync or Syncing bits. It aims to prevent 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 55ab2a8a11e5f..f5df82adef640 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c

[ ... ]

> @@ -1689,6 +1689,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);

[Severity: Medium]
Is there a race condition between reading the bitmap pointer and taking the
mutex?

If llbitmap_destroy() executes concurrently, acquires the mutex, sets
mddev->bitmap to NULL, and frees the bitmap while we are checking the pointer
outside the lock, could we end up dereferencing a freed pointer inside the loop
once the lock is acquired here?

> +	for (chunk = 0; chunk < llbitmap->chunks; chunk++) {
> +		enum llbitmap_state state = llbitmap_read(llbitmap, chunk);
> +
> +		if (state == BitNeedSync || state == BitSyncing) {

[Severity: Medium]
Does this condition miss the unwritten sync states?

If BitNeedSyncUnwritten or BitSyncingUnwritten are omitted here, could reshape
be allowed to start during proactive parity synchronization on unwritten
regions, potentially leading to data corruption?

> +			ret = -EBUSY;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&mddev->bitmap_info.mutex);
> +
> +	return ret;
> +}
> +

[ ... ]

> @@ -2011,6 +2034,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]
Is the reshape_can_start callback invoked anywhere in the core MD subsystem?

It doesn't appear that any callers for reshape_can_start are added in this
patch or exist in the current subsystem code. Without a caller, is this
reshape rejection logic effectively dead code?

>  	.write_all		= llbitmap_write_all,
>  
>  	.groups			= md_llbitmap_groups,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=10

[PATCH v3 11/21] md/md-llbitmap: add reshape range mapping helpers

[Yu Kuai]

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/
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 | 129 +++++++++++++++++++++++++++++++++++++--
 drivers/md/md.c          |  10 +--
 4 files changed, 132 insertions(+), 12 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 4c31807e6bcc..52dfbca28b64 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 26cd07737aab..97cdd7cea5dd 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -96,7 +96,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 f5df82adef64..929c1f40ddec 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>
 
@@ -449,6 +450,16 @@ static sector_t llbitmap_personality_sync_size(struct llbitmap *llbitmap,
 	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 +477,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;
@@ -1385,11 +1470,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 = mddev->reshape_position;
+
+	if (!llbitmap_reshaping(llbitmap))
+		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,
@@ -1558,7 +1667,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;
 }
@@ -1568,7 +1681,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)
@@ -1613,6 +1730,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 c18a7f3c27e6..a00df52ab40b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9401,20 +9401,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

Re: [PATCH v3 11/21] md/md-llbitmap: add reshape range mapping helpers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Missing post-map sync-size clamp in llbitmap_map_layout() allows mapped physical sectors to exceed the bitmap capacity, causing OOB array accesses.
- [Critical] The `bitmap_array_sectors` callback is not implemented for any personality (e.g. RAID5), causing the pre-map clamp to incorrectly use component coordinates against array-sector IOs.
- [Critical] Coordinate mapping is completely skipped for the old geometry during reshape, leading to array coordinates being treated as component coordinates.
- [Critical] Bios crossing the reshape boundary are not split before bitmap accounting, causing `llbitmap` to encode the entire range with a single geometry.
--

--- Patch [11]: [PATCH v3 11/21] md/md-llbitmap: add reshape range mapping helpers ---
commit b480f867a40652c2963e56247233a10fff0a56b8
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: add reshape range mapping helpers

This commit adds reshape range mapping helpers to llbitmap to differentiate
between old and new geometry during a reshape. It also introduces
discard-specific range encoding and clamps pre-map ranges in the same
coordinate space as incoming IO.

Link: https://lore.kernel.org/all/20260726185916.2223460-1-mykola@meshstor.io/

> steady-state RAID5 must use bitmap_array_sectors() before mapping and keep
> the existing sync-size clamp after mapping.

Does the implementation match this commit message description? Looking at
llbitmap_map_layout(), it appears the post-map sync-size clamp was omitted.

> 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
[ ... ]
> @@ -449,6 +450,16 @@ static sector_t llbitmap_personality_sync_size(struct llbitmap *llbitmap,
>  	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);

[Severity: Critical]
Is bitmap_array_sectors implemented for the personalities that need it?

If personalities like RAID5 do not wire up bitmap_array_sectors, this falls
back to returning llbitmap_personality_sync_size(), which uses the much
smaller component coordinates.

Because RAID5 receives IOs in array coordinates, and array coordinates are
significantly larger than component coordinates, wouldn't any write targeting
the upper portion of a RAID5 array evaluate to start >= limit inside
llbitmap_map_layout() and be silently dropped? This could lead to silent data
corruption on an unclean shutdown.

> +	return mddev->pers->bitmap_array_sectors(mddev, previous);
> +}
> +
>  static void llbitmap_refresh_reshape(struct llbitmap *llbitmap)
[ ... ]
> +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);

[Severity: Critical]
Does this logic incorrectly skip coordinate mapping for the old geometry?

During an active reshape, IOs targeting the old geometry evaluate previous to
true. Since the !previous condition prevents bitmap_sector() from being called,
array-sector offsets are passed directly to llbitmap_encode_range().

Because array offsets are much larger than component sectors, this produces
chunk indices that wildly exceed llbitmap->chunks. This can lead to
out-of-bounds memory accesses on the pctl array or corrupt the bitmap by
dirtying the wrong chunks.

[Severity: Critical]
Is a post-map sync-size clamp missing at the end of this function?

When a RAID array's size is not perfectly aligned to a multiple of chunk sizes,
bitmap_sector() rounds the end offset up to the next chunk boundary. Without
clamping the result against llbitmap_personality_sync_size(), the mapped
component sector range can exceed the physical capacity.

This un-clamped offset can yield an out-of-bounds page_end when passed into
llbitmap_start_write(), causing an out-of-bounds dereference on the
dynamically allocated llbitmap->pctl array via llbitmap_raise_barrier().

> +}
> +
> +static void llbitmap_encode_range(struct llbitmap *llbitmap, sector_t *offset,
[ ... ]
> @@ -1385,11 +1470,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 = mddev->reshape_position;
> +
> +	if (!llbitmap_reshaping(llbitmap))
> +		return false;
> +
> +	WARN_ON_ONCE(sectors && offset < boundary && offset + sectors > boundary);

[Severity: Critical]
Will this WARN_ON_ONCE trigger if bios crossing the reshape boundary are not
split beforehand?

The block layer currently accounts the entire contiguous bio as a single range
in md_account_bio(). While the helper mddev_bio_split_at_reshape_offset is
exported in this patch series, it appears it is never actually invoked by the
personalities (such as in raid5_make_request) prior to accounting.

Because this function returns a single geometry decision for the entire IO,
llbitmap_encode_range() will map the entire crossing bio using a single
chunksize. This miscalculates chunk indices for the portion of the bio sitting
in the other geometry, dirtying the wrong chunks and risking data corruption if
resync skips the correctly-associated chunks after a crash.

> +
> +	return mddev->reshape_backwards ? offset < boundary : offset >= boundary;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=11

[PATCH v3 12/21] md/md-llbitmap: don't skip reshape ranges from bitmap state

[Yu Kuai]

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.

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 929c1f40ddec..bdc6ec8972f0 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1687,6 +1687,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

[PATCH v3 13/21] md/md-llbitmap: remap checkpointed bits as reshape progresses

[Yu Kuai]

From: Yu Kuai <yukuai@fygo.io>

Merge checkpointed old llbitmap state forward as reshape_position
advances and record the checkpoint remap through reshape_mark().

Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
 drivers/md/md-llbitmap.c | 172 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 172 insertions(+)

diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index bdc6ec8972f0..794ac0eaf437 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -502,6 +502,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,
@@ -934,6 +942,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;
@@ -1839,6 +1874,120 @@ 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;
+
+	if (!new->sectors)
+		return;
+
+	dst_start = div64_u64(new->offset, llbitmap->reshape_chunksize);
+	dst_end = div64_u64(new->end - 1, llbitmap->reshape_chunksize);
+
+	for (dst = dst_start; dst <= dst_end; dst++) {
+		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);
+	}
+}
+
 static void llbitmap_reshape_finish(struct mddev *mddev)
 {
 	struct llbitmap *llbitmap = mddev->bitmap;
@@ -1863,6 +2012,28 @@ 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);
+	llbitmap_reshape_merge(llbitmap, &old, &new);
+}
+
 static void llbitmap_write_sb(struct llbitmap *llbitmap)
 {
 	int nr_blocks = DIV_ROUND_UP(BITMAP_DATA_OFFSET, llbitmap->io_size);
@@ -2162,6 +2333,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

Re: [PATCH v3 13/21] md/md-llbitmap: remap checkpointed bits as reshape progresses

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] In-place bitmap array merge corrupts states due to forward iteration overlapping with unread source chunks (equivalent to `memcpy` vs `memmove` bug).
- [High] The reshape state merge function drops proactive sync unwritten states (`BitNeedSyncUnwritten`, `BitCleanUnwritten`), reverting them to basic states and losing sync status.
- [High] `llbitmap_reshape_map_range` fails to map `old` device sectors to array sectors when `bitmap_sector_map` is unavailable, corrupting source chunk calculations.
- [High] Lockless read-modify-write in `llbitmap_reshape_merge` races with concurrent I/O (`llbitmap_start_write`), causing lost BitDirty flags and silent data corruption.
- [Medium] Redundant condition in `llbitmap_rmerge_state` forces all RAID levels to treat mixed BitClean/BitUnwritten as BitClean, bypassing the `level_456` restriction.
- [Medium] Truncation of 64-bit sector counts to 32-bit unsigned long causes incomplete reshape mapping on 32-bit kernels.
--

--- Patch [13]: [PATCH v3 13/21] md/md-llbitmap: remap checkpointed bits as reshape progresses ---
commit 745e2cefd189066340bdf6deca751ae219d34b84
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: remap checkpointed bits as reshape progresses
    
Merge checkpointed old llbitmap state forward as reshape_position
advances and record the checkpoint remap through reshape_mark().

> 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

[ ... ]

> @@ -934,6 +942,33 @@ static int llbitmap_prepare_resize(struct llbitmap *llbitmap,

[ ... ]

> +	if (level_456 && ((dst == BitClean && src == BitUnwritten) ||
> +			  (src == BitClean && dst == BitUnwritten)))
> +		return BitClean;
> +	if (dst == BitClean || src == BitClean)
> +		return BitClean;

[Severity: Medium]
Does this unconditional check override the preceding level_456 restriction?

It looks like mixed clean and unwritten states will evaluate to clean on all
array types, including non-parity arrays like RAID 1 or RAID 10. This
effectively renders the level_456 restriction above it dead code.

> +	return BitUnwritten;
> +}

[Severity: High]
Are we unintentionally dropping proactive sync unwritten states here?

When reshaping an array with states like BitNeedSyncUnwritten,
BitSyncingUnwritten, or BitCleanUnwritten, they appear to fall through the
basic checks and downgrade to BitUnwritten. This silently loses sync status
for proactive parity builds on RAID 4/5/6.

[ ... ]

> @@ -1839,6 +1874,120 @@ static int llbitmap_reshape_can_start(struct mddev *mddev)

[ ... ]

> +struct llbitmap_reshape_range {
> +	sector_t offset;
> +	unsigned long sectors;

[Severity: Medium]
Will this truncate the 64-bit sector count to 32 bits on 32-bit architectures?

If the reshape process skips a large fully clean region of more than 2TB, it
seems this sector count might silently overflow since sectors is declared as
unsigned long.

> +	sector_t start;
> +	sector_t end;
> +};

[ ... ]

> +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);

[Severity: High]
Does this fail to convert logical array sectors to bitmap device sectors for
the old layout when bitmap_sector_map is NULL?

For example, during a RAID5 reshape where bitmap_sector is implemented but
not bitmap_sector_map, llbitmap_map_layout skips mapping the old layout but
correctly maps the new one.

> +	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);

[Severity: High]
Could this math incorrectly mix old array sectors with new device sectors?

If the mapping failed in the step above, combining these mismatched units
could lead to corrupted source chunk calculations.

> +	src->end = old->offset +

[ ... ]

> +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;
> +
> +	if (!new->sectors)
> +		return;
> +
> +	dst_start = div64_u64(new->offset, llbitmap->reshape_chunksize);
> +	dst_end = div64_u64(new->end - 1, llbitmap->reshape_chunksize);
> +
> +	for (dst = dst_start; dst <= dst_end; dst++) {

[Severity: High]
Could this forward iteration cause in-place state corruption during the merge?

If the old to new layout mapping causes source chunks to be located at higher
indices than destination chunks, writing to dst could overwrite the state that
a subsequent iteration needs to read as src. This propagates the corrupted
state forward.

> +		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);

[Severity: High]
Does this lockless read-modify-write race with concurrent I/O updates to the
same chunk?

Because reshape checkpoint boundaries might not be chunk-aligned, these
boundary chunks can cover sectors outside the suspended reshape window. A
concurrent llbitmap_start_write to those unsuspended sectors performs a
lockless read-modify-write on the chunk state. 

If llbitmap_reshape_merge performs a lockless write on the exact same chunk
index concurrently, one could overwrite the other, losing BitDirty flags and
risking silent data corruption.

> +	}
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=13

[PATCH v3 14/21] md/md-llbitmap: clamp state-machine walks to tracked bits

[Yu Kuai]

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.

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 794ac0eaf437..bb83d15247d2 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1011,7 +1011,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

Re: [PATCH v3 14/21] md/md-llbitmap: clamp state-machine walks to tracked bits

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] `llbitmap_cond_end_sync` prematurely clears the sync state for partially synced chunks, leading to skipped sectors and data corruption.
--

--- Patch [14]: [PATCH v3 14/21] md/md-llbitmap: clamp state-machine walks to tracked bits ---
commit d30fecfeaabb2957753288e01ac48b29a04d716a
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: clamp state-machine walks to tracked bits

This commit clamps the state-machine range to llbitmap->chunks to
prevent it from walking past the tracked bitmap when handling boundary
conditions.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 794ac0eaf4379..bb83d15247d23 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1011,7 +1011,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;

[Severity: Critical]
This is a pre-existing issue, but does llbitmap_cond_end_sync() prematurely
clear the sync state for partially synced chunks?

During array resync or initial build, the current resync position (sector)
typically advances by stripe size or resync sectors, which is smaller than
the bitmap chunk size. This means sector almost always falls in the middle of
a chunk.

When llbitmap_cond_end_sync() calls:

drivers/md/md-llbitmap.c:llbitmap_cond_end_sync() {
    ...
	llbitmap_state_machine(llbitmap, 0, sector >> llbitmap->chunkshift,
			       BitmapActionEndsync);
    ...
}

it calculates the inclusive end bound for Endsync but fails to ensure only
fully completed chunks are processed, applying BitmapActionEndsync to the
partially completed chunk that sector resides in.

Because the state machine transitions partially-synced chunks to BitDirty or
BitCleanUnwritten, the next call to md_bitmap_start_sync() in the same chunk
will evaluate to BitNone and return false.

Will this cause the sync thread to skip the remaining sectors of that chunk
without resyncing them, leaving stale parity or unsynced mirrors on the disk?

>  	while (start <= end) {
>  		enum llbitmap_state c = llbitmap_read(llbitmap, start);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=14

[PATCH v3 15/21] md/raid10: reject llbitmap reshape when md chunk shrinks

[Yu Kuai]

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.

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 0a3cfdd3f5df..1242b8d3bb6b 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4284,6 +4284,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

Re: [PATCH v3 15/21] md/raid10: reject llbitmap reshape when md chunk shrinks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Missing Validation in Array Assembly Path

Pre-existing issues:
- [High] Logic Error in Resume Path forces reshape start despite check failure
--

--- Patch [15]: [PATCH v3 15/21] md/raid10: reject llbitmap reshape when md chunk shrinks ---
commit 0f54995b397bafdc9d0ce8dd025c97e68e5b1284
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.

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.

> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0a3cfdd3f5df8..1242b8d3bb6bd 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -4284,6 +4284,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: High]
This is a pre-existing issue, but does returning an error code here actually
prevent the reshape from starting?

Looking at mddev_start_reshape() in drivers/md/md.c, it seems to treat a
non-zero return value from check_reshape() as a trigger to execute
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);

Since raid10_check_reshape() returns -EOPNOTSUPP,
mddev->pers->check_reshape(mddev) will evaluate to true. Will this bypass
the rejection and force raid10_start_reshape() to proceed with the invalid
geometry?

[Severity: High]
Does this same llbitmap constraint need to be validated during array
assembly?

If an array with an active llbitmap and a suspended reshape with a shrunk
chunk size is assembled, md_run() invokes raid10_run(), which assigns the
new geometry via setup_conf() without validating the llbitmap constraint:

drivers/md/raid10.c:setup_conf() {
    ...
    if (mddev->reshape_position == MaxSector) {
        conf->prev = conf->geo;
        conf->reshape_progress = MaxSector;
    } else {
        if (setup_geo(&conf->prev, mddev, geo_old) != conf->copies) {
            err = -EINVAL;
            goto out;
        }
    ...
}

Could this allow the array to mount and serve I/O using a geometry where
the bitmap bits cover a smaller data range than originally intended, causing
data corruption because the bitmap won't properly track the full range
written?

>  
>  	if (setup_geo(&geo, mddev, geo_start) != conf->copies)
>  		/* mustn't change number of copies */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=15

[PATCH v3 16/21] md/raid10: wire llbitmap reshape lifecycle

[Yu Kuai]

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.

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 1242b8d3bb6b..e50e4adee389 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4394,6 +4394,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;
@@ -4707,6 +4713,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, false);
+		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)
@@ -4905,9 +4918,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);
@@ -5039,10 +5062,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;
@@ -5069,6 +5097,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 = {
@@ -5095,6 +5132,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

Re: [PATCH v3 16/21] md/raid10: wire llbitmap reshape lifecycle

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `raid10_finish_reshape()` function hardcodes a check for `ID_LLBITMAP` before calling the polymorphic `reshape_finish` callback, bypassing the abstraction provided by `bitmap_ops`.
--

--- Patch [16]: [PATCH v3 16/21] md/raid10: wire llbitmap reshape lifecycle ---
commit 2d6aab72980238a6e16cc2a451989159951188f3
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 growing, checkpoints it before advancing the
reshape position, and finishes the update when reshape completes.

> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 1242b8d3bb6bd..e50e4adee389f 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
[ ... ]
> @@ -5039,10 +5062,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]
This isn't a bug, but does this code unnecessarily hardcode a check for
ID_LLBITMAP? It looks like this restricts the reshape_finish callback to
llbitmap, bypassing the abstraction provided by the generic bitmap_ops
interface.

>  	if (mddev->delta_disks > 0) {
>  		if (mddev->resync_offset > mddev->resync_max_sectors) {
>  			mddev->resync_offset = mddev->resync_max_sectors;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=16

[PATCH v3 17/21] md/raid10: split reshape bios before bitmap accounting

[Yu Kuai]

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.

Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
 drivers/md/raid10.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index e50e4adee389..fd364a2e2fd9 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1911,6 +1911,12 @@ 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, &sectors,
+						&conf->bio_split);
+	if (!bio)
+		return true;
+
 	if (!__make_request(mddev, bio, sectors))
 		md_write_end(mddev);
 
-- 
2.51.0

Re: [PATCH v3 17/21] md/raid10: split reshape bios before bitmap accounting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing `md_write_end()` when `mddev_bio_split_at_reshape_offset()` fails on a write bio, leaking `mddev->writes_pending` reference count.
- [High] Mempool exhaustion deadlock due to double bio allocation from `conf->bio_split` without a rescuer thread.
--

--- Patch [17]: [PATCH v3 17/21] md/raid10: split reshape bios before bitmap accounting ---
commit f435d38ffb898e662e1e9b6225ad90d266d26363
Author: Yu Kuai <yukuai@fygo.io>

md/raid10: split reshape bios before bitmap accounting
    
This patch updates raid10 to use the shared mddev_bio_split_at_reshape_offset()
helper. This ensures that RAID10 submits only one-side bios to llbitmap during
reshape operations.

> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index e50e4adee389f..fd364a2e2fd98 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1911,6 +1911,12 @@ 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, &sectors,
> +						&conf->bio_split);

[Severity: High]
Could this lead to a mempool exhaustion deadlock if bad blocks are present?

This splits the bio using conf->bio_split, and passes the split bio down
to __make_request(). If there are bad blocks that restrict the sector count
further, the lower level routines perform a second split using the exact
same bio_set:

drivers/md/raid10.c:raid10_write_request() {
    ...
    bio = bio_submit_split_bioset(bio, r10_bio->sectors,
                                  &conf->bio_split);
    ...
}

Since conf->bio_split lacks a rescuer thread, would two concurrent I/Os
that both cross a chunk boundary and hit a bad block exhaust the pool and
stall indefinitely?

> +	if (!bio)
> +		return true;

[Severity: High]
Does this early return leak the mddev->writes_pending reference?

Earlier in raid10_make_request(), we call md_write_start() which increments
the writes_pending count for write bios:

drivers/md/raid10.c:raid10_make_request() {
    ...
    md_write_start(mddev, bio);
    ...
}

If the bio is an atomic write that cannot be split,
mddev_bio_split_at_reshape_offset() returns NULL. When we return true here,
we bypass the md_write_end() cleanup at the end of the function, which
might hang the array when it is later suspended or stopped.

> +
>  	if (!__make_request(mddev, bio, sectors))
>  		md_write_end(mddev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=17

[PATCH v3 18/21] md/raid5: add exact old and new llbitmap mapping helpers

[Yu Kuai]

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.

Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
 drivers/md/raid5.c | 72 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 52 insertions(+), 20 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 35910bc6683b..8b28afa538e2 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5942,28 +5942,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
@@ -5972,19 +5990,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
@@ -5992,7 +6001,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,
@@ -9054,6 +9063,20 @@ 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, 0, 0);
+	return raid5_size(mddev, mddev->dev_sectors, conf->raid_disks);
+}
+
 static struct md_personality raid6_personality =
 {
 	.head = {
@@ -9083,6 +9106,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 =
 {
@@ -9113,6 +9139,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 =
@@ -9144,6 +9173,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

Re: [PATCH v3 18/21] md/raid5: add exact old and new llbitmap mapping helpers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Integer overflow in `raid5_bitmap_sector` leading to silent data corruption on 32-bit architectures during reshape.
--

--- Patch [18]: [PATCH v3 18/21] md/raid5: add exact old and new llbitmap mapping helpers ---
commit c431d4e1772c99b9b41c44d5e8369daa1885b7eb
Author: Yu Kuai <yukuai@fygo.io>

md/raid5: add exact old and new llbitmap mapping helpers

This commit refactors the bitmap sector calculations in RAID5 to export
exact old and new mappings, along with sync and array sizes, to support
reshape-aware bitmap users.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 35910bc6683bf..8b28afa538e2b 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c

[ ... ]

> @@ -5992,7 +6001,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;

[Severity: Critical]
This is a pre-existing issue, but does this calculation overflow on 32-bit
architectures during a large array reshape?

When an IO is issued ahead of the reshape progress, raid5_bitmap_sector()
merges the old and new physical mappings into a single range. For very large
arrays, the distance between the old and new physical device offsets can
easily exceed 2TB.

Because the sectors parameter is an unsigned long pointer, which is 32-bit
on 32-bit systems, subtracting these 64-bit sector_t endpoints produces a
64-bit difference that is silently truncated when assigned to *sectors.

If a crash occurs during this reshape, wouldn't the bitmap layer record only
a truncated fraction of the required range, causing the array to fail to
resync the full region and potentially resulting in silent data corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=18

[PATCH v3 19/21] md/raid5: reject llbitmap reshape when md chunk shrinks

[Yu Kuai]

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.

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 8b28afa538e2..f0817599dd27 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -8509,6 +8509,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

Re: [PATCH v3 19/21] md/raid5: reject llbitmap reshape when md chunk shrinks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Writing a value less than 512 to the `chunk_size` sysfs attribute bypasses chunk validation and causes a divide-by-zero kernel panic during reshape.
--

--- Patch [19]: [PATCH v3 19/21] md/raid5: reject llbitmap reshape when md chunk shrinks ---
commit fd741104c871960b0c928c6a03adfc7d3df39d70
Author: Yu Kuai <yukuai@fygo.io>

md/raid5: reject llbitmap reshape when md chunk shrinks
    
This commit prevents llbitmap reshapes from shrinking the chunk size,
as doing so cannot safely make an existing bitmap bit cover a smaller
data range.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 8b28afa538e2b..f0817599dd271 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -8509,6 +8509,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]
This is a pre-existing issue, but since we are looking at new_chunk_sectors
validation, what prevents a user from triggering a divide-by-zero panic by
writing a value smaller than 512 bytes to the chunk_size sysfs attribute?

In chunk_size_store(), user input less than 512 bytes is converted to
0 sectors before calling into the reshape checks:

drivers/md/md.c:chunk_size_store() {
    ...
    mddev->new_chunk_sectors = n >> 9;
    err = mddev->pers->check_reshape(mddev);
    ...
}

The downstream chunk validation logic skips the chunk check if the value
is zero:

    if (new_chunk > 0) {
        if (!is_power_of_2(new_chunk))
            return -EINVAL;
    }

If a user has no llbitmap configured, this 0 sector value appears to bypass
validation. If conf->chunk_sectors is updated to 0, will subsequent I/O or
reshape operations that divide by conf->chunk_sectors (such as
raid5_bitmap_sector_map() or raid5_bio_lowest_chunk_sector()) trigger a
divide-by-zero panic?

>  	if (mddev->new_chunk_sectors > mddev->chunk_sectors ||
>  	    mddev->delta_disks > 0)
>  		if (resize_chunks(conf,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=19

[PATCH v3 20/21] md/raid5: wire llbitmap reshape lifecycle

[Yu Kuai]

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.

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 f0817599dd27..f70b2f611980 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6433,6 +6433,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)
@@ -6542,6 +6549,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)
@@ -8574,6 +8588,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);
@@ -8658,10 +8678,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);
@@ -8688,8 +8717,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

Re: [PATCH v3 20/21] md/raid5: wire llbitmap reshape lifecycle

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Setting mddev->resync_offset = 0 unconditionally upon reshape finish forces an unnecessary full parity resync for RAID5.
--

--- Patch [20]: [PATCH v3 20/21] md/raid5: wire llbitmap reshape lifecycle ---
commit 692471147c5f67f1af148855c6b48cd2fffeec7a
Author: Yu Kuai <yukuai@fygo.io>

md/raid5: wire llbitmap reshape lifecycle

This commit prepares the llbitmap before a RAID5 reshape starts, checkpoints
the bitmap before advancing the reshape position, and completes the llbitmap
geometry update when the reshape finishes.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
[ ... ]
> @@ -8688,8 +8717,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;

[Severity: High]
Does setting mddev->resync_offset to 0 here force an unnecessary full parity
resync for the array?

Since a RAID reshape already migrates data and parity, unconditionally resetting
the resync_offset to 0 informs the MD core that the parity needs to be fully
re-synchronized from sector 0.

> +			mddev->resync_max_sectors = mddev->dev_sectors;
> +		}
>  
>  		if (mddev->delta_disks <= 0) {
>  			int d;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=20

[PATCH v3 21/21] md/raid5: split reshape bios before bitmap accounting

[Yu Kuai]

From: Yu Kuai <yukuai@fygo.io>

Use the shared mddev_bio_split_at_reshape_offset() helper so RAID5
submits only one-side bios to llbitmap during reshape.

Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
 drivers/md/raid5.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index f70b2f611980..30e18ce7c0e3 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6184,6 +6184,14 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
 		return true;
 	}
 
+	bi = mddev_bio_split_at_reshape_offset(mddev, bi, NULL,
+					       &conf->bio_split);
+	if (!bi) {
+		if (rw == WRITE)
+			md_write_end(mddev);
+		return true;
+	}
+
 	logical_sector = bi->bi_iter.bi_sector & ~((sector_t)RAID5_STRIPE_SECTORS(conf)-1);
 	bi->bi_next = NULL;
 
-- 
2.51.0

Re: [PATCH v3 21/21] md/raid5: split reshape bios before bitmap accounting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Returning `false` from `raid5_make_request()` after a split bio encounters `STRIPE_WAIT_RESHAPE` causes double-submission of the remainder bio, leaks the split bio, and hangs IO indefinitely.
--

--- Patch [21]: [PATCH v3 21/21] md/raid5: split reshape bios before bitmap accounting ---
commit a1d64516f61f7855ab4b16be870b2ea6fb0d09a0
Author: Yu Kuai <yukuai@fygo.io>

md/raid5: split reshape bios before bitmap accounting

This patch updates raid5_make_request() to use the shared
mddev_bio_split_at_reshape_offset() helper. This ensures RAID5 submits
only one-side bios to llbitmap during reshape.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index f70b2f6119801..30e18ce7c0e3d 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -6184,6 +6184,14 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
>  		return true;
>  	}
>  
> +	bi = mddev_bio_split_at_reshape_offset(mddev, bi, NULL,
> +					       &conf->bio_split);
> +	if (!bi) {
> +		if (rw == WRITE)
> +			md_write_end(mddev);
> +		return true;
> +	}
> +

[Severity: Critical]
If this bio is split, `bi` is updated to the first half, while the original
bio (the remainder) is submitted asynchronously.

Later in raid5_make_request(), if make_stripe_request() returns
STRIPE_WAIT_RESHAPE, it sleeps and returns false:

	if (res == STRIPE_WAIT_RESHAPE) {
		DECLARE_COMPLETION_ONSTACK(done);
		WRITE_ONCE(bi->bi_private, &done);

		bio_endio(bi);

		wait_for_completion(&done);
		return false;
	}

When raid5_make_request() returns false, md_handle_request() retries the
request using its original `bio` pointer:

drivers/md/md.c:md_handle_request() {
	...
	if (!mddev->pers->make_request(mddev, bio)) {
		percpu_ref_put(&mddev->active_io);
		if (mddev_is_dm(mddev) && mddev->pers->prepare_suspend)
			return false;
		goto check_suspended;
	}
	...
}

Because the original `bio` is now the remainder that was already submitted
asynchronously, won't returning false here cause a double-submission
of the remainder bio?

Additionally, if the completion handler intercepts the completion of the first
half to wake the sleeper but skips calling bio_endio() on the split bio, won't
this leak the split bio? Because the bio was split and chained, does skipping
the completion prevent the parent bio's reference count from dropping to zero,
causing IO to hang indefinitely?

>  	logical_sector = bi->bi_iter.bi_sector & ~((sector_t)RAID5_STRIPE_SECTORS(conf)-1);
>  	bi->bi_next = NULL;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=21