* Re: [PATCH V3 2/6] md: ignore discard return value
From: Chaitanya Kulkarni @ 2026-01-24 21:29 UTC (permalink / raw)
To: song@kernel.org, yukuai@fnnas.com
Cc: linux-raid@vger.kernel.org, hch@lst.de, Chaitanya Kulkarni,
Chaitanya Kulkarni, Johannes Thumshirn, Martin K. Petersen
In-Reply-To: <938be45c-dcd9-45ba-b6ba-0fd29b0815db@nvidia.com>
On 11/30/25 22:22, Chaitanya Kulkarni wrote:
> Hi Song Liu and Yu Kuai,
>
> On 11/24/25 15:48, Chaitanya Kulkarni wrote:
>> __blkdev_issue_discard() always returns 0, making all error checking at
>> call sites dead code.
>>
>> Simplify md to only check !discard_bio by ignoring the
>> __blkdev_issue_discard() value.
>>
>> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
>> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> Signed-off-by: Chaitanya Kulkarni <ckulkarnilinux@gmail.com>
>> ---
>> drivers/md/md.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 7b5c5967568f..aeb62df39828 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -9132,8 +9132,8 @@ void md_submit_discard_bio(struct mddev *mddev,
>> struct md_rdev *rdev,
>> {
>> struct bio *discard_bio = NULL;
>> - if (__blkdev_issue_discard(rdev->bdev, start, size, GFP_NOIO,
>> - &discard_bio) || !discard_bio)
>> + __blkdev_issue_discard(rdev->bdev, start, size, GFP_NOIO,
>> &discard_bio);
>> + if (!discard_bio)
>> return;
>> bio_chain(discard_bio, bio);
>
>
> Gentle ping on this.
>
> -ck
>
>
Hi Song Liu and Yu Kuai,
Just pulled latest changes I don't see this patch been applied [1].
Unless my git repo is messed up.
Can you please provide review comments and let me know the blocker ?
-ck
9169 /* This is used by raid0 and raid10 */
9170 void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
9171 struct bio *bio, sector_t start, sector_t size)
9172 {
9173 struct bio *discard_bio = NULL;
9174
9175 if (__blkdev_issue_discard(rdev->bdev, start, size, GFP_NOIO,
9176 &discard_bio) || !discard_bio)
9177 return;
9178
9179 bio_chain(discard_bio, bio);
9180 bio_clone_blkg_association(discard_bio, bio);
9181 mddev_trace_remap(mddev, discard_bio, bio->bi_iter.bi_sector);
9182 submit_bio_noacct(discard_bio);
9183 }
9184 EXPORT_SYMBOL_GPL(md_submit_discard_bio);
^ permalink raw reply
* Re: [PATCH 0/2] md/md-llbimtap: fix two corner case problems
From: Yu Kuai @ 2026-01-24 15:04 UTC (permalink / raw)
To: Jens Axboe, linux-block, linux-raid; +Cc: linux-kernel, linan122, xni
In-Reply-To: <c3d421ba-0151-4a1b-b4f0-6d3ffe4b7d1f@kernel.dk>
Hi，
在 2026/1/24 11:38, Jens Axboe 写道:
> On 1/23/26 11:26 AM, Yu Kuai wrote:
>> Yu Kuai (2):
>> md/raid5: fix IO hang with degraded array with llbitmap
>> md/md-llbitmap: fix percpu_ref not resurrected on suspend timeout
>>
>> drivers/md/md-llbitmap.c | 4 +++-
>> drivers/md/raid5.c | 7 ++++++-
>> 2 files changed, 9 insertions(+), 2 deletions(-)
> Is this just for review, or do you want them picked up off the list
> too?
>
> Both look fine to me, fwiw.
I'll send a pull request soon.
Sorry for the noise, I used the wrong script that is used to send pull
request for this set.
>
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH 1/2] md/raid5: fix IO hang with degraded array with llbitmap
From: Li Nan @ 2026-01-24 6:42 UTC (permalink / raw)
To: Yu Kuai, axboe, linux-block, linux-raid; +Cc: linux-kernel, xni
In-Reply-To: <20260123182623.3718551-2-yukuai@fnnas.com>
在 2026/1/24 2:26, Yu Kuai 写道:
> When llbitmap bit state is still unwritten, any new write should force
> rcw, as bitmap_ops->blocks_synced() is checked in handle_stripe_dirting().
s/handle_stripe_dirting/handle_stripe_dirtying/
Besides this, LGTM
Reviewed-by: Li Nan <linan122@huawei.com>
> However, later the same check is missing in need_this_block(), causing
> stripe to deadloop during handling because handle_stripe() will decide
> to go to handle_stripe_fill(), meanwhile need_this_block() always return
> 0 and nothing is handled.
>
> Fixes: 5ab829f1971d ("md/md-llbitmap: introduce new lockless bitmap")
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
> ---
> drivers/md/raid5.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 8dc98f545969..93e672b3432b 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3751,9 +3751,14 @@ static int need_this_block(struct stripe_head *sh, struct stripe_head_state *s,
> struct r5dev *dev = &sh->dev[disk_idx];
> struct r5dev *fdev[2] = { &sh->dev[s->failed_num[0]],
> &sh->dev[s->failed_num[1]] };
> + struct mddev *mddev = sh->raid_conf->mddev;
> + bool force_rcw = false;
> int i;
> - bool force_rcw = (sh->raid_conf->rmw_level == PARITY_DISABLE_RMW);
>
> + if (sh->raid_conf->rmw_level == PARITY_DISABLE_RMW ||
> + (mddev->bitmap_ops && mddev->bitmap_ops->blocks_synced &&
> + !mddev->bitmap_ops->blocks_synced(mddev, sh->sector)))
> + force_rcw = true;
>
> if (test_bit(R5_LOCKED, &dev->flags) ||
> test_bit(R5_UPTODATE, &dev->flags))
--
Thanks,
Nan
^ permalink raw reply
* Re: [PATCH 0/2] md/md-llbimtap: fix two corner case problems
From: Jens Axboe @ 2026-01-24 3:38 UTC (permalink / raw)
To: Yu Kuai, linux-block, linux-raid; +Cc: linux-kernel, linan122, xni
In-Reply-To: <20260123182623.3718551-1-yukuai@fnnas.com>
On 1/23/26 11:26 AM, Yu Kuai wrote:
> Yu Kuai (2):
> md/raid5: fix IO hang with degraded array with llbitmap
> md/md-llbitmap: fix percpu_ref not resurrected on suspend timeout
>
> drivers/md/md-llbitmap.c | 4 +++-
> drivers/md/raid5.c | 7 ++++++-
> 2 files changed, 9 insertions(+), 2 deletions(-)
Is this just for review, or do you want them picked up off the list
too?
Both look fine to me, fwiw.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 2/2] md/md-llbitmap: fix percpu_ref not resurrected on suspend timeout
From: Li Nan @ 2026-01-24 1:58 UTC (permalink / raw)
To: Yu Kuai, axboe, linux-block, linux-raid; +Cc: linux-kernel, xni
In-Reply-To: <20260123182623.3718551-3-yukuai@fnnas.com>
在 2026/1/24 2:26, Yu Kuai 写道:
> When llbitmap_suspend_timeout() times out waiting for percpu_ref to
> become zero, it returns -ETIMEDOUT without resurrecting the percpu_ref.
> The caller (md_llbitmap_daemon_fn) then continues to the next page
> without calling llbitmap_resume(), leaving the percpu_ref in a killed
> state permanently.
>
> Fix this by resurrecting the percpu_ref before returning the error,
> ensuring the page control structure remains usable for subsequent
> operations.
>
> Fixes: 5ab829f1971d ("md/md-llbitmap: introduce new lockless bitmap")
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
> ---
> drivers/md/md-llbitmap.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index b482a1db0861..7df15756142d 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -779,8 +779,10 @@ static int llbitmap_suspend_timeout(struct llbitmap *llbitmap, int page_idx)
> percpu_ref_kill(&pctl->active);
>
> if (!wait_event_timeout(pctl->wait, percpu_ref_is_zero(&pctl->active),
> - llbitmap->mddev->bitmap_info.daemon_sleep * HZ))
> + llbitmap->mddev->bitmap_info.daemon_sleep * HZ)) {
> + percpu_ref_resurrect(&pctl->active);
> return -ETIMEDOUT;
> + }
>
> return 0;
> }
LGTM
Reviewed-by: Li Nan <linan122@huawei.com>
--
Thanks,
Nan
^ permalink raw reply
* [PATCH 2/2] md/md-llbitmap: fix percpu_ref not resurrected on suspend timeout
From: Yu Kuai @ 2026-01-23 18:26 UTC (permalink / raw)
To: axboe, linux-block, linux-raid; +Cc: linux-kernel, yukuai, linan122, xni
In-Reply-To: <20260123182623.3718551-1-yukuai@fnnas.com>
When llbitmap_suspend_timeout() times out waiting for percpu_ref to
become zero, it returns -ETIMEDOUT without resurrecting the percpu_ref.
The caller (md_llbitmap_daemon_fn) then continues to the next page
without calling llbitmap_resume(), leaving the percpu_ref in a killed
state permanently.
Fix this by resurrecting the percpu_ref before returning the error,
ensuring the page control structure remains usable for subsequent
operations.
Fixes: 5ab829f1971d ("md/md-llbitmap: introduce new lockless bitmap")
Signed-off-by: Yu Kuai <yukuai@fnnas.com>
---
drivers/md/md-llbitmap.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index b482a1db0861..7df15756142d 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -779,8 +779,10 @@ static int llbitmap_suspend_timeout(struct llbitmap *llbitmap, int page_idx)
percpu_ref_kill(&pctl->active);
if (!wait_event_timeout(pctl->wait, percpu_ref_is_zero(&pctl->active),
- llbitmap->mddev->bitmap_info.daemon_sleep * HZ))
+ llbitmap->mddev->bitmap_info.daemon_sleep * HZ)) {
+ percpu_ref_resurrect(&pctl->active);
return -ETIMEDOUT;
+ }
return 0;
}
--
2.51.0
^ permalink raw reply related
* [PATCH 1/2] md/raid5: fix IO hang with degraded array with llbitmap
From: Yu Kuai @ 2026-01-23 18:26 UTC (permalink / raw)
To: axboe, linux-block, linux-raid; +Cc: linux-kernel, yukuai, linan122, xni
In-Reply-To: <20260123182623.3718551-1-yukuai@fnnas.com>
When llbitmap bit state is still unwritten, any new write should force
rcw, as bitmap_ops->blocks_synced() is checked in handle_stripe_dirting().
However, later the same check is missing in need_this_block(), causing
stripe to deadloop during handling because handle_stripe() will decide
to go to handle_stripe_fill(), meanwhile need_this_block() always return
0 and nothing is handled.
Fixes: 5ab829f1971d ("md/md-llbitmap: introduce new lockless bitmap")
Signed-off-by: Yu Kuai <yukuai@fnnas.com>
---
drivers/md/raid5.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 8dc98f545969..93e672b3432b 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3751,9 +3751,14 @@ static int need_this_block(struct stripe_head *sh, struct stripe_head_state *s,
struct r5dev *dev = &sh->dev[disk_idx];
struct r5dev *fdev[2] = { &sh->dev[s->failed_num[0]],
&sh->dev[s->failed_num[1]] };
+ struct mddev *mddev = sh->raid_conf->mddev;
+ bool force_rcw = false;
int i;
- bool force_rcw = (sh->raid_conf->rmw_level == PARITY_DISABLE_RMW);
+ if (sh->raid_conf->rmw_level == PARITY_DISABLE_RMW ||
+ (mddev->bitmap_ops && mddev->bitmap_ops->blocks_synced &&
+ !mddev->bitmap_ops->blocks_synced(mddev, sh->sector)))
+ force_rcw = true;
if (test_bit(R5_LOCKED, &dev->flags) ||
test_bit(R5_UPTODATE, &dev->flags))
--
2.51.0
^ permalink raw reply related
* [PATCH 0/2] md/md-llbimtap: fix two corner case problems
From: Yu Kuai @ 2026-01-23 18:26 UTC (permalink / raw)
To: axboe, linux-block, linux-raid; +Cc: linux-kernel, yukuai, linan122, xni
Yu Kuai (2):
md/raid5: fix IO hang with degraded array with llbitmap
md/md-llbitmap: fix percpu_ref not resurrected on suspend timeout
drivers/md/md-llbitmap.c | 4 +++-
drivers/md/raid5.c | 7 ++++++-
2 files changed, 9 insertions(+), 2 deletions(-)
--
2.51.0
^ permalink raw reply
* Re: [PATCH 06/15] md/raid1,raid10: use folio for sync path IO
From: Xiao Ni @ 2026-01-22 7:01 UTC (permalink / raw)
To: Li Nan; +Cc: song, yukuai, linux-raid, linux-kernel, yangerkun, yi.zhang
In-Reply-To: <a0423bf6-c924-0012-172b-1217069ff3ec@huaweicloud.com>
On Thu, Jan 22, 2026 at 10:12 AM Li Nan <linan666@huaweicloud.com> wrote:
>
>
>
> 在 2026/1/20 23:53, Xiao Ni 写道:
> > On Wed, Dec 17, 2025 at 8:11 PM <linan666@huaweicloud.com> wrote:
> >>
> >> From: Li Nan <linan122@huawei.com>
> >>
> >> Convert all IO on the sync path to use folios. Rename page-related
> >> identifiers to match folio.
> >>
> >> Retain some now-unnecessary while and for loops to minimize code
> >> changes, clean them up in a subsequent patch.
> >>
> >> Signed-off-by: Li Nan <linan122@huawei.com >> -static inline int resync_alloc_pages(struct resync_pages *rp,
> >> +static inline int resync_alloc_folio(struct resync_folio *rf,
> >> gfp_t gfp_flags)
> >> {
> >> - int i;
> >> -
> >> - for (i = 0; i < RESYNC_PAGES; i++) {
> >> - rp->pages[i] = alloc_page(gfp_flags);
> >> - if (!rp->pages[i])
> >> - goto out_free;
> >> - }
> >> + rf->folio = folio_alloc(gfp_flags, get_order(RESYNC_BLOCK_SIZE));
> >> + if (!rf->folio)
> >> + return -ENOMEM;
> >
> > Is it ok to add an error log here? Compare with the multipage
> > situation, the possibility of failure will be somewhat higher because
> > it needs to alloc a contiguous block of physical memory.
> >
>
> Hi, Xiao
>
> Thanks for your review.
>
> In patch 15 we fall back to a smaller order if sync folio alloc fails.
> After that the alloc usually succeeds, so an error log seems noisy. Should
> I add a log before the fallback and keep it in patch 15?
Thanks for the explanation. No change is needed here :)
>
> >> -static void md_bio_reset_resync_pages(struct bio *bio, struct resync_pages *rp,
> >> +static void md_bio_reset_resync_folio(struct bio *bio, struct resync_folio *rf,
> >> int size)
> >> {
> >> - int idx = 0;
> >> -
> >> /* initialize bvec table again */
> >> do {
> >> - struct page *page = resync_fetch_page(rp, idx);
> >> - int len = min_t(int, size, PAGE_SIZE);
> >> + struct folio *folio = resync_fetch_folio(rf);
> >> + int len = min_t(int, size, RESYNC_BLOCK_SIZE);
> >>
> >> - if (WARN_ON(!bio_add_page(bio, page, len, 0))) {
> >> + if (WARN_ON(!bio_add_folio(bio, folio, len, 0))) {
> >
> > Is it ok to use bio_add_folio(bio, folio, RESYNC_BLOCK_SIZE, 0)
> > directly here? It removes `size -= len` below, so it's not useless to
> > compare size and RESYNC_BLOCK_SIZE above?
> >
>
> Same as the previous one, the size is no longer a fixed value after
> patch 15. I think keeping it here gives better compatibility.
Thanks for the explanation.
>
> >> /*
> >> - * Allocate RESYNC_PAGES data pages and attach them to
> >> - * the first bio.
> >> + * Allocate data folio and attach them to the first bio.
> >
> > typo error
> > s/attach them/attach it/g
> >
>
> I will fix it later. Thanks.
>
> >> @@ -2284,44 +2278,30 @@ static void process_checks(struct r1bio *r1_bio)
> >> }
> >> r1_bio->read_disk = primary;
> >> for (i = 0; i < conf->raid_disks * 2; i++) {
> >> - int j = 0;
> >> struct bio *pbio = r1_bio->bios[primary];
> >> struct bio *sbio = r1_bio->bios[i];
> >> blk_status_t status = sbio->bi_status;
> >> - struct page **ppages = get_resync_pages(pbio)->pages;
> >> - struct page **spages = get_resync_pages(sbio)->pages;
> >> - struct bio_vec *bi;
> >> - int page_len[RESYNC_PAGES] = { 0 };
> >> - struct bvec_iter_all iter_all;
> >> + struct folio *pfolio = get_resync_folio(pbio)->folio;
> >> + struct folio *sfolio = get_resync_folio(sbio)->folio;
> >>
> >> if (sbio->bi_end_io != end_sync_read)
> >> continue;
> >> /* Now we can 'fixup' the error value */
> >> sbio->bi_status = 0;
> >>
> >> - bio_for_each_segment_all(bi, sbio, iter_all)
> >> - page_len[j++] = bi->bv_len;
> >> -
> >> - if (!status) {
> >> - for (j = vcnt; j-- ; ) {
> >> - if (memcmp(page_address(ppages[j]),
> >> - page_address(spages[j]),
> >> - page_len[j]))
> >> - break;
> >> - }
> >> - } else
> >> - j = 0;
> >> - if (j >= 0)
> >> + if (status || memcmp(folio_address(pfolio),
> >> + folio_address(sfolio),
> >> + r1_bio->sectors << 9)) {
> >> atomic64_add(r1_bio->sectors, &mddev->resync_mismatches);
> >> - if (j < 0 || (test_bit(MD_RECOVERY_CHECK, &mddev->recovery)
> >> - && !status)) {
> >> - /* No need to write to this device. */
> >> - sbio->bi_end_io = NULL;
> >> - rdev_dec_pending(conf->mirrors[i].rdev, mddev);
> >> - continue;
> >> + if (!test_bit(MD_RECOVERY_CHECK, &mddev->recovery)) {
> >> + bio_copy_data(sbio, pbio);
> >> + continue;
> >> + }
> >
> > The logic is changed here. The original logic:
> > 1. read ok, no mismatch: no bio_copy_data
> > 2. read ok, mismatch, check: no bio_copy_data
> > 3. read ok, mismatch, no check: need bio_copy_data
> > 4. read fail: need bio_copy_data
> >
> > The 4 is broken.
> >
> > How about adding a temporary need_write to make logic more clear?
> >
> > something like:
> > if (!status) {
> > int ret = 0;
> > ret = memcpy(folio_address(pfolio),
> > folio_address(sfolio),
> > r1_bio->sectors << 9);
> > if (ret) {
> > atomic64_add(r1_bio->sectors, &mddev->resync_mismatches);
> > if (!test_bit(MD_RECOVERY_CHECK, &mddev->recovery))
> > need_write = true;
> > }
> > } else
> > need_write = true;
> >
> > if (need_write)
> > bio_copy_data(sbio, pbio);
> > else {
> > /* No need to write to this device. */
> > sbio->bi_end_io = NULL;
> > rdev_dec_pending(conf->mirrors[i].rdev, mddev);
> > }
> >
>
> Nice catch, read failis is indeed broken. I’ll fix it in v2.
>
> >> }
> >>
> >> - bio_copy_data(sbio, pbio);
> >> + /* No need to write to this device. */
> >> + sbio->bi_end_io = NULL;
> >> + rdev_dec_pending(conf->mirrors[i].rdev, mddev);
> >> }
> >> }
> >> >> @@ -3020,24 +2997,24 @@ static sector_t (struct mddev *mddev, sector_t
> sector_nr,
> >> }
> >>
> >> for (i = 0 ; i < conf->raid_disks * 2; i++) {
> >> - struct resync_pages *rp;
> >> + struct resync_folio *rf;
> >>
> >> bio = r1_bio->bios[i];
> >> - rp = get_resync_pages(bio);
> >> + rf = get_resync_folio(bio);
> >> if (bio->bi_end_io) {
> >> - page = resync_fetch_page(rp, page_idx);
> >> + folio = resync_fetch_folio(rf);
> >>
> >> /*
> >> * won't fail because the vec table is big
> >> * enough to hold all these pages
> >> */
> >
> > The comments above may not be needed anymore. Because there is only
> > one vec in the bio.
> >
>
> I will clean it up.
>
> >> - __bio_add_page(bio, page, len, 0);
> >> + bio_add_folio_nofail(bio, folio, len, 0);
> >> }
> >> }
> >> nr_sectors += len>>9;
> >> sector_nr += len>>9;
> >> sync_blocks -= (len>>9);
> >
> > These three lines are not needed anymore.
> >
>
> It is cleaned up in later patches. In this patch I only want minimal
> changes, just folio API and naming replacements. Do you think I should move
> those cleanups into this patch?
I think it's a better choice. But it really depends on you. I'm ok if
you prefer your patch sequence.
>
>
> >> /*
> >> - * Allocate RESYNC_PAGES data pages and attach them
> >> - * where needed.
> >> + * Allocate data folio and attach them where needed.
> >
> > typo error
> > s/attach them/attach it/g
> >
>
> I will fix it in v2.
>
> >> */
> >> for (j = 0; j < nalloc; j++) {
> >> struct bio *rbio = r10_bio->devs[j].repl_bio;
> >> - struct resync_pages *rp, *rp_repl;
> >> + struct resync_folio *rf, *rf_repl;
> >>
> >> - rp = &rps[j];
> >> + rf = &rfs[j];
> >> if (rbio)
> >> - rp_repl = &rps[nalloc + j];
> >> + rf_repl = &rfs[nalloc + j];
> >>
> >> bio = r10_bio->devs[j].bio;
> >>
> >> if (!j || test_bit(MD_RECOVERY_SYNC,
> >> &conf->mddev->recovery)) {
> >> - if (resync_alloc_pages(rp, gfp_flags))
> >> + if (resync_alloc_folio(rf, gfp_flags))
> >> goto out_free_pages;
> >
> > s/out_free_pages/out_free_folio/g
> >
>
> I will fix it in v2.
>
> >> } else {
> >> - memcpy(rp, &rps[0], sizeof(*rp));
> >> - resync_get_all_pages(rp);
> >> + memcpy(rf, &rfs[0], sizeof(*rf));
> >> + resync_get_all_folio(rf);
> >
> > Maybe the name resync_get_folio is better?
>
> Agree, I will rename it in v2.
>
> >> @@ -2492,19 +2480,21 @@ static void fix_recovery_read_error(struct r10bio *r10_bio)
> >>
> >> rdev = conf->mirrors[dr].rdev;
> >> addr = r10_bio->devs[0].addr + sect;
> >> - ok = sync_page_io(rdev,
> >> - addr,
> >> - s << 9,
> >> - pages[idx],
> >> - REQ_OP_READ, false);
> >> + ok = sync_folio_io(rdev,
> >> + addr,
> >> + s << 9,
> >> + sect << 9,
> >> + folio,
> >> + REQ_OP_READ, false);
> >
> > By the comments at the beginning of fix_recovery_read_error, it needs
> > to submit io with a page size unit, right? If so, it still needs to
> > use sync_page_io here.
> >
>
> Here 's' is PAGE_SIZE. We just use a 'page' from folio. In patch 10,
> I will change it to use logical block size instead, which should be more
> reasonable.
Ok.
>
> >> /*
> >> * Allow skipping a full rebuild for incremental assembly
> >> @@ -3277,7 +3265,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> >> * with 2 bios in each, that correspond to the bios in the main one.
> >> * In this case, the subordinate r10bios link back through a
> >> * borrowed master_bio pointer, and the counter in the master
> >> - * includes a ref from each subordinate.
> >> + * bio_add_folio includes a ref from each subordinate.
> >
> > What's the reason change this? And I don't understand the new version.
> >
>
> It looks like a typo. I will remove it.
>
> > Best Regards
> > Xiao
>
> Thanks again for your careful review.
You're welcome.
Best Regards
Xiao
>
> --
> Thanks,
> Nan
>
^ permalink raw reply
* Re: [PATCH 06/15] md/raid1,raid10: use folio for sync path IO
From: Li Nan @ 2026-01-22 2:12 UTC (permalink / raw)
To: Xiao Ni, linan666
Cc: song, yukuai, linux-raid, linux-kernel, yangerkun, yi.zhang
In-Reply-To: <CALTww28w8EJL2TVs06f_h98SjkiGAiYD9+BUvqEG2HKsrssSsg@mail.gmail.com>
在 2026/1/20 23:53, Xiao Ni 写道:
> On Wed, Dec 17, 2025 at 8:11 PM <linan666@huaweicloud.com> wrote:
>>
>> From: Li Nan <linan122@huawei.com>
>>
>> Convert all IO on the sync path to use folios. Rename page-related
>> identifiers to match folio.
>>
>> Retain some now-unnecessary while and for loops to minimize code
>> changes, clean them up in a subsequent patch.
>>
>> Signed-off-by: Li Nan <linan122@huawei.com >> -static inline int resync_alloc_pages(struct resync_pages *rp,
>> +static inline int resync_alloc_folio(struct resync_folio *rf,
>> gfp_t gfp_flags)
>> {
>> - int i;
>> -
>> - for (i = 0; i < RESYNC_PAGES; i++) {
>> - rp->pages[i] = alloc_page(gfp_flags);
>> - if (!rp->pages[i])
>> - goto out_free;
>> - }
>> + rf->folio = folio_alloc(gfp_flags, get_order(RESYNC_BLOCK_SIZE));
>> + if (!rf->folio)
>> + return -ENOMEM;
>
> Is it ok to add an error log here? Compare with the multipage
> situation, the possibility of failure will be somewhat higher because
> it needs to alloc a contiguous block of physical memory.
>
Hi, Xiao
Thanks for your review.
In patch 15 we fall back to a smaller order if sync folio alloc fails.
After that the alloc usually succeeds, so an error log seems noisy. Should
I add a log before the fallback and keep it in patch 15?
>> -static void md_bio_reset_resync_pages(struct bio *bio, struct resync_pages *rp,
>> +static void md_bio_reset_resync_folio(struct bio *bio, struct resync_folio *rf,
>> int size)
>> {
>> - int idx = 0;
>> -
>> /* initialize bvec table again */
>> do {
>> - struct page *page = resync_fetch_page(rp, idx);
>> - int len = min_t(int, size, PAGE_SIZE);
>> + struct folio *folio = resync_fetch_folio(rf);
>> + int len = min_t(int, size, RESYNC_BLOCK_SIZE);
>>
>> - if (WARN_ON(!bio_add_page(bio, page, len, 0))) {
>> + if (WARN_ON(!bio_add_folio(bio, folio, len, 0))) {
>
> Is it ok to use bio_add_folio(bio, folio, RESYNC_BLOCK_SIZE, 0)
> directly here? It removes `size -= len` below, so it's not useless to
> compare size and RESYNC_BLOCK_SIZE above?
>
Same as the previous one, the size is no longer a fixed value after
patch 15. I think keeping it here gives better compatibility.
>> /*
>> - * Allocate RESYNC_PAGES data pages and attach them to
>> - * the first bio.
>> + * Allocate data folio and attach them to the first bio.
>
> typo error
> s/attach them/attach it/g
>
I will fix it later. Thanks.
>> @@ -2284,44 +2278,30 @@ static void process_checks(struct r1bio *r1_bio)
>> }
>> r1_bio->read_disk = primary;
>> for (i = 0; i < conf->raid_disks * 2; i++) {
>> - int j = 0;
>> struct bio *pbio = r1_bio->bios[primary];
>> struct bio *sbio = r1_bio->bios[i];
>> blk_status_t status = sbio->bi_status;
>> - struct page **ppages = get_resync_pages(pbio)->pages;
>> - struct page **spages = get_resync_pages(sbio)->pages;
>> - struct bio_vec *bi;
>> - int page_len[RESYNC_PAGES] = { 0 };
>> - struct bvec_iter_all iter_all;
>> + struct folio *pfolio = get_resync_folio(pbio)->folio;
>> + struct folio *sfolio = get_resync_folio(sbio)->folio;
>>
>> if (sbio->bi_end_io != end_sync_read)
>> continue;
>> /* Now we can 'fixup' the error value */
>> sbio->bi_status = 0;
>>
>> - bio_for_each_segment_all(bi, sbio, iter_all)
>> - page_len[j++] = bi->bv_len;
>> -
>> - if (!status) {
>> - for (j = vcnt; j-- ; ) {
>> - if (memcmp(page_address(ppages[j]),
>> - page_address(spages[j]),
>> - page_len[j]))
>> - break;
>> - }
>> - } else
>> - j = 0;
>> - if (j >= 0)
>> + if (status || memcmp(folio_address(pfolio),
>> + folio_address(sfolio),
>> + r1_bio->sectors << 9)) {
>> atomic64_add(r1_bio->sectors, &mddev->resync_mismatches);
>> - if (j < 0 || (test_bit(MD_RECOVERY_CHECK, &mddev->recovery)
>> - && !status)) {
>> - /* No need to write to this device. */
>> - sbio->bi_end_io = NULL;
>> - rdev_dec_pending(conf->mirrors[i].rdev, mddev);
>> - continue;
>> + if (!test_bit(MD_RECOVERY_CHECK, &mddev->recovery)) {
>> + bio_copy_data(sbio, pbio);
>> + continue;
>> + }
>
> The logic is changed here. The original logic:
> 1. read ok, no mismatch: no bio_copy_data
> 2. read ok, mismatch, check: no bio_copy_data
> 3. read ok, mismatch, no check: need bio_copy_data
> 4. read fail: need bio_copy_data
>
> The 4 is broken.
>
> How about adding a temporary need_write to make logic more clear?
>
> something like:
> if (!status) {
> int ret = 0;
> ret = memcpy(folio_address(pfolio),
> folio_address(sfolio),
> r1_bio->sectors << 9);
> if (ret) {
> atomic64_add(r1_bio->sectors, &mddev->resync_mismatches);
> if (!test_bit(MD_RECOVERY_CHECK, &mddev->recovery))
> need_write = true;
> }
> } else
> need_write = true;
>
> if (need_write)
> bio_copy_data(sbio, pbio);
> else {
> /* No need to write to this device. */
> sbio->bi_end_io = NULL;
> rdev_dec_pending(conf->mirrors[i].rdev, mddev);
> }
>
Nice catch, read failis is indeed broken. I’ll fix it in v2.
>> }
>>
>> - bio_copy_data(sbio, pbio);
>> + /* No need to write to this device. */
>> + sbio->bi_end_io = NULL;
>> + rdev_dec_pending(conf->mirrors[i].rdev, mddev);
>> }
>> }
>> >> @@ -3020,24 +2997,24 @@ static sector_t (struct mddev *mddev, sector_t
sector_nr,
>> }
>>
>> for (i = 0 ; i < conf->raid_disks * 2; i++) {
>> - struct resync_pages *rp;
>> + struct resync_folio *rf;
>>
>> bio = r1_bio->bios[i];
>> - rp = get_resync_pages(bio);
>> + rf = get_resync_folio(bio);
>> if (bio->bi_end_io) {
>> - page = resync_fetch_page(rp, page_idx);
>> + folio = resync_fetch_folio(rf);
>>
>> /*
>> * won't fail because the vec table is big
>> * enough to hold all these pages
>> */
>
> The comments above may not be needed anymore. Because there is only
> one vec in the bio.
>
I will clean it up.
>> - __bio_add_page(bio, page, len, 0);
>> + bio_add_folio_nofail(bio, folio, len, 0);
>> }
>> }
>> nr_sectors += len>>9;
>> sector_nr += len>>9;
>> sync_blocks -= (len>>9);
>
> These three lines are not needed anymore.
>
It is cleaned up in later patches. In this patch I only want minimal
changes, just folio API and naming replacements. Do you think I should move
those cleanups into this patch?
>> /*
>> - * Allocate RESYNC_PAGES data pages and attach them
>> - * where needed.
>> + * Allocate data folio and attach them where needed.
>
> typo error
> s/attach them/attach it/g
>
I will fix it in v2.
>> */
>> for (j = 0; j < nalloc; j++) {
>> struct bio *rbio = r10_bio->devs[j].repl_bio;
>> - struct resync_pages *rp, *rp_repl;
>> + struct resync_folio *rf, *rf_repl;
>>
>> - rp = &rps[j];
>> + rf = &rfs[j];
>> if (rbio)
>> - rp_repl = &rps[nalloc + j];
>> + rf_repl = &rfs[nalloc + j];
>>
>> bio = r10_bio->devs[j].bio;
>>
>> if (!j || test_bit(MD_RECOVERY_SYNC,
>> &conf->mddev->recovery)) {
>> - if (resync_alloc_pages(rp, gfp_flags))
>> + if (resync_alloc_folio(rf, gfp_flags))
>> goto out_free_pages;
>
> s/out_free_pages/out_free_folio/g
>
I will fix it in v2.
>> } else {
>> - memcpy(rp, &rps[0], sizeof(*rp));
>> - resync_get_all_pages(rp);
>> + memcpy(rf, &rfs[0], sizeof(*rf));
>> + resync_get_all_folio(rf);
>
> Maybe the name resync_get_folio is better?
Agree, I will rename it in v2.
>> @@ -2492,19 +2480,21 @@ static void fix_recovery_read_error(struct r10bio *r10_bio)
>>
>> rdev = conf->mirrors[dr].rdev;
>> addr = r10_bio->devs[0].addr + sect;
>> - ok = sync_page_io(rdev,
>> - addr,
>> - s << 9,
>> - pages[idx],
>> - REQ_OP_READ, false);
>> + ok = sync_folio_io(rdev,
>> + addr,
>> + s << 9,
>> + sect << 9,
>> + folio,
>> + REQ_OP_READ, false);
>
> By the comments at the beginning of fix_recovery_read_error, it needs
> to submit io with a page size unit, right? If so, it still needs to
> use sync_page_io here.
>
Here 's' is PAGE_SIZE. We just use a 'page' from folio. In patch 10,
I will change it to use logical block size instead, which should be more
reasonable.
>> /*
>> * Allow skipping a full rebuild for incremental assembly
>> @@ -3277,7 +3265,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
>> * with 2 bios in each, that correspond to the bios in the main one.
>> * In this case, the subordinate r10bios link back through a
>> * borrowed master_bio pointer, and the counter in the master
>> - * includes a ref from each subordinate.
>> + * bio_add_folio includes a ref from each subordinate.
>
> What's the reason change this? And I don't understand the new version.
>
It looks like a typo. I will remove it.
> Best Regards
> Xiao
Thanks again for your careful review.
--
Thanks,
Nan
^ permalink raw reply
* Make test failed raid19check
From: David Niklas @ 2026-01-20 18:08 UTC (permalink / raw)
To: Linux RAID
[-- Attachment #1: Type: text/plain, Size: 2304 bytes --]
Hello,
Recently, mdadm-4.5 was released. I upgraded my system to the latest
version, uninstalling the packaged mdadm for the new one.
I was particularly interested in using raid19check as you may recall, my
raid6 array developed some errors.
I ran the tests for raid6check and two of them failed.
I've attached the logs.
I'm running Devuan (Debian) Linux with a custom configured kernel.
Nothing special, I just needed the newer features for my GPU.
Here's the output of the "test" command.
# ./test
Warning! Tests are performed on system level mdadm!
If you want to test local build, you need to install it first!
test: skipping tests for multipath, which is removed in upstream 6.8+ kernels
Warning! Test suite will set up systemd environment!
Use "systemctl show-environment" to show systemd environment variables
/root/working/mdadm/tests/func.sh: line 228: systemctl: command not found
Added IMSM_DEVNAME_AS_SERIAL=1 to systemd environment, use "systemctl unset-environment IMSM_DEVNAME_AS_SERIAL=1" to remove it.
/root/working/mdadm/tests/func.sh: line 228: systemctl: command not found
Added IMSM_NO_PLATFORM=1 to systemd environment, use "systemctl unset-environment IMSM_NO_PLATFORM=1" to remove it.
setenforce: SELinux is disabled
Testing on linux-6.14.11-nopreempt-AMDGPU-dav15-noxz kernel
/root/working/mdadm/tests/19raid6auto-repair... Execution time (seconds): 8 FAILED - see /var/tmp/19raid6auto-repair.log and /var/tmp/fail19raid6auto-repair.log for details
(KNOWN BROKEN TEST: always fails)
/root/working/mdadm/tests/19raid6check... Execution time (seconds): 340 succeeded
/root/working/mdadm/tests/19raid6repair... Execution time (seconds): 8 FAILED - see /var/tmp/19raid6repair.log and /var/tmp/fail19raid6repair.log for details
(KNOWN BROKEN TEST: always fails)
/root/working/mdadm/tests/19repair-does-not-destroy... Execution time (seconds): 10 succeeded
setenforce: SELinux is disabled
/root/working/mdadm/tests/func.sh: line 237: systemctl: command not found
Removed IMSM_DEVNAME_AS_SERIAL=1 from systemd environment.
/root/working/mdadm/tests/func.sh: line 237: systemctl: command not found
Removed IMSM_NO_PLATFORM=1 from systemd environment.
I think I'll start running the other tests and see what happens with them.
Any ideas on what's going wrong?
Thanks,
David
[-- Attachment #2: fail19raid6repair.log --]
[-- Type: text/x-log, Size: 2053 bytes --]
## Core-Ultra-2-x20: saving dmesg.
[4501074.465567] md/raid:md0: not clean -- starting background reconstruction
[4501074.465590] md/raid:md0: device loop4 operational as raid disk 3
[4501074.465592] md/raid:md0: device loop3 operational as raid disk 2
[4501074.465593] md/raid:md0: device loop2 operational as raid disk 1
[4501074.465594] md/raid:md0: device loop1 operational as raid disk 0
[4501074.465820] md/raid:md0: raid level 6 active with 4 out of 4 devices, algorithm 2
[4501074.465838] md0: detected capacity change from 0 to 71680
[4501074.466242] md: resync of RAID array md0
[4501075.294307] md: md0: resync done.
[4501081.959606] test (31434): drop_caches: 3
[4501082.158217] test (31434): drop_caches: 3
## Core-Ultra-2-x20: saving proc mdstat.
Personalities : [raid0] [linear] [raid1] [raid6] [raid5] [raid4] [raid10]
md0 : active raid6 loop4[3] loop3[2] loop2[1] loop1[0]
35840 blocks super 1.2 level 6, 512k chunk, algorithm 2 [4/4] [UUUU]
unused devices: <none>
## Core-Ultra-2-x20: mdadm -D /dev/md0
/dev/md0:
Version : 1.2
Creation Time : Tue Jan 20 12:51:04 2026
Raid Level : raid6
Array Size : 35840 (35.00 MiB 36.70 MB)
Used Dev Size : 17920 (17.50 MiB 18.35 MB)
Raid Devices : 4
Total Devices : 4
Persistence : Superblock is persistent
Update Time : Tue Jan 20 12:51:05 2026
State : clean
Active Devices : 4
Working Devices : 4
Failed Devices : 0
Spare Devices : 0
Layout : left-symmetric
Chunk Size : 512K
Consistency Policy : resync
Name : Core-Ultra-2-x20:0 (local to host Core-Ultra-2-x20)
UUID : 472d803a:08838b9e:c943ddb8:9f55f6f0
Events : 19
Number Major Minor RaidDevice State
0 7 1 0 active sync /dev/loop1
1 7 2 1 active sync /dev/loop2
2 7 3 2 active sync /dev/loop3
3 7 4 3 active sync /dev/loop4
[-- Attachment #3: 19raid6repair.log --]
[-- Type: text/x-log, Size: 3989 bytes --]
+ . /root/working/mdadm/tests/19raid6repair
++ number_of_disks=4
++ chunksize_in_kib=512
++ chunksize_in_b=524288
++ array_data_size_in_kib=4096
++ array_data_size_in_b=4194304
++ devs='/dev/loop1 /dev/loop2 /dev/loop3 /dev/loop4'
++ data_offset_in_kib=1024
++ layouts='ls rs la ra parity-first ddf-zero-restart ddf-N-restart ddf-N-continue left-asymmetric-6 right-asymmetric-6 left-symmetric-6 right-symmetric-6 parity-first-6'
++ for layout in $layouts
++ for failure in "$dev3 3 3 2" "$dev3 3 2 3" "$dev3 3 2 1" "$dev3 3 2 0" "$dev4 3 3 0" "$dev4 3 3 1" "$dev4 3 3 2" "$dev1 3 0 1" "$dev1 3 0 2" "$dev1 3 0 3" "$dev2 3 1 0" "$dev2 3 1 2" "$dev2 3 1 3"
++ failure_split=($failure)
++ device_with_error=/dev/loop3
++ stripe_with_error=3
++ repair_params='3 3 2'
++ start_of_errors_in_kib=2560
++ dd if=/dev/urandom of=/tmp/RandFile bs=1024 count=4096
4096+0 records in
4096+0 records out
4194304 bytes (4.2 MB, 4.0 MiB) copied, 0.0142637 s, 294 MB/s
++ mdadm -CR /dev/md0 -l6 --layout=ls -n4 -c 512 /dev/loop1 /dev/loop2 /dev/loop3 /dev/loop4
++ rm -f /var/tmp/stderr
++ case $* in
++ case $* in
++ for args in $*
++ [[ -CR =~ /dev/ ]]
++ for args in $*
++ [[ /dev/md0 =~ /dev/ ]]
++ [[ /dev/md0 =~ md ]]
++ for args in $*
++ [[ -l6 =~ /dev/ ]]
++ for args in $*
++ [[ --layout=ls =~ /dev/ ]]
++ for args in $*
++ [[ -n4 =~ /dev/ ]]
++ for args in $*
++ [[ -c =~ /dev/ ]]
++ for args in $*
++ [[ 512 =~ /dev/ ]]
++ for args in $*
++ [[ /dev/loop1 =~ /dev/ ]]
++ [[ /dev/loop1 =~ md ]]
++ /sbin/mdadm --zero /dev/loop1
mdadm: Unrecognised md component device - /dev/loop1
++ for args in $*
++ [[ /dev/loop2 =~ /dev/ ]]
++ [[ /dev/loop2 =~ md ]]
++ /sbin/mdadm --zero /dev/loop2
mdadm: Unrecognised md component device - /dev/loop2
++ for args in $*
++ [[ /dev/loop3 =~ /dev/ ]]
++ [[ /dev/loop3 =~ md ]]
++ /sbin/mdadm --zero /dev/loop3
mdadm: Unrecognised md component device - /dev/loop3
++ for args in $*
++ [[ /dev/loop4 =~ /dev/ ]]
++ [[ /dev/loop4 =~ md ]]
++ /sbin/mdadm --zero /dev/loop4
mdadm: Unrecognised md component device - /dev/loop4
++ /sbin/mdadm -CR /dev/md0 -l6 --layout=ls -n4 -c 512 /dev/loop1 /dev/loop2 /dev/loop3 /dev/loop4
mdadm: Defaulting to version 1.2 metadata
mdadm: array /dev/md0 started.
++ rv=0
++ case $* in
++ cat /var/tmp/stderr
++ return 0
++ dd if=/tmp/RandFile of=/dev/md0 bs=1024 count=4096
4096+0 records in
4096+0 records out
4194304 bytes (4.2 MB, 4.0 MiB) copied, 0.0321578 s, 130 MB/s
++ blockdev --flushbufs /dev/md0
++ sync
++ check wait
++ case $1 in
+++ cat /proc/sys/dev/raid/speed_limit_min
++ min=100
+++ cat /proc/sys/dev/raid/speed_limit_max
++ max=500
++ echo 200000
++ sleep 0.1
++ iterations=0
++ '[' 0 -le 10 ']'
+++ grep -Ec '(resync|recovery|reshape|check|repair) *=' /proc/mdstat
++ sync_action=1
++ (( 1 == 0 ))
++ break
++ echo 'Reshape has not started after 10 seconds'
Reshape has not started after 10 seconds
++ echo 'Waiting for grow-continue to finish'
Waiting for grow-continue to finish
++ wait_for_reshape_end
++ true
+++ grep -Ec '(resync|recovery|reshape|check|repair) *=' /proc/mdstat
++ sync_action=1
++ (( 1 != 0 ))
++ sleep 2
++ continue
++ true
+++ grep -Ec '(resync|recovery|reshape|check|repair) *=' /proc/mdstat
++ sync_action=0
++ (( 0 != 0 ))
+++ pgrep -f 'mdadm --grow --continue'
++ [[ '' != '' ]]
++ break
++ sleep 5
++ wait_for_reshape_end
++ true
+++ grep -Ec '(resync|recovery|reshape|check|repair) *=' /proc/mdstat
++ sync_action=0
++ (( 0 != 0 ))
+++ pgrep -f 'mdadm --grow --continue'
++ [[ '' != '' ]]
++ break
++ echo 100
++ echo 500
++ blockdev --flushbufs /dev/loop1 /dev/loop2 /dev/loop3 /dev/loop4
++ sync
++ echo 3
++ cmp -s -n 4194304 /dev/md0 /tmp/RandFile
++ dd if=/dev/urandom of=/dev/loop3 bs=1024 count=512 seek=2560
512+0 records in
512+0 records out
524288 bytes (524 kB, 512 KiB) copied, 0.00223738 s, 234 MB/s
++ blockdev --flushbufs /dev/loop3
++ sync
++ echo 3
++ /raid6check /dev/md0 0 0
++ grep -qs Error
++ echo should detect errors
should detect errors
++ exit 2
[-- Attachment #4: fail19raid6auto-repair.log --]
[-- Type: text/x-log, Size: 2199 bytes --]
## Core-Ultra-2-x20: saving dmesg.
[4500726.128927] md/raid:md0: not clean -- starting background reconstruction
[4500726.128949] md/raid:md0: device loop4 operational as raid disk 4
[4500726.128951] md/raid:md0: device loop3 operational as raid disk 3
[4500726.128952] md/raid:md0: device loop2 operational as raid disk 2
[4500726.128953] md/raid:md0: device loop1 operational as raid disk 1
[4500726.128954] md/raid:md0: device loop0 operational as raid disk 0
[4500726.129347] md/raid:md0: raid level 6 active with 5 out of 5 devices, algorithm 2
[4500726.129362] md0: detected capacity change from 0 to 107520
[4500726.129513] md: resync of RAID array md0
[4500726.903794] md: md0: resync done.
[4500733.827662] test (26245): drop_caches: 3
[4500734.121635] test (26245): drop_caches: 3
## Core-Ultra-2-x20: saving proc mdstat.
Personalities : [raid0] [linear] [raid1] [raid6] [raid5] [raid4] [raid10]
md0 : active raid6 loop4[4] loop3[3] loop2[2] loop1[1] loop0[0]
53760 blocks super 1.2 level 6, 512k chunk, algorithm 2 [5/5] [UUUUU]
unused devices: <none>
## Core-Ultra-2-x20: mdadm -D /dev/md0
/dev/md0:
Version : 1.2
Creation Time : Tue Jan 20 12:45:15 2026
Raid Level : raid6
Array Size : 53760 (52.50 MiB 55.05 MB)
Used Dev Size : 17920 (17.50 MiB 18.35 MB)
Raid Devices : 5
Total Devices : 5
Persistence : Superblock is persistent
Update Time : Tue Jan 20 12:45:16 2026
State : clean
Active Devices : 5
Working Devices : 5
Failed Devices : 0
Spare Devices : 0
Layout : left-symmetric
Chunk Size : 512K
Consistency Policy : resync
Name : Core-Ultra-2-x20:0 (local to host Core-Ultra-2-x20)
UUID : 43e4b762:7978a8e7:59af7d77:2747b753
Events : 19
Number Major Minor RaidDevice State
0 7 0 0 active sync /dev/loop0
1 7 1 1 active sync /dev/loop1
2 7 2 2 active sync /dev/loop2
3 7 3 3 active sync /dev/loop3
4 7 4 4 active sync /dev/loop4
[-- Attachment #5: 19raid6auto-repair.log --]
[-- Type: text/x-log, Size: 4563 bytes --]
+ . /root/working/mdadm/tests/19raid6auto-repair
++ number_of_disks=5
++ chunksize_in_kib=512
++ chunksize_in_b=524288
++ array_data_size_in_kib=7680
++ array_data_size_in_b=7864320
++ devs='/dev/loop0 /dev/loop1 /dev/loop2 /dev/loop3 /dev/loop4'
++ data_offset_in_kib=1024
++ dd if=/dev/urandom of=/tmp/RandFile bs=1024 count=7680
7680+0 records in
7680+0 records out
7864320 bytes (7.9 MB, 7.5 MiB) copied, 0.0342983 s, 229 MB/s
++ layouts='ls rs la ra parity-first ddf-zero-restart ddf-N-restart ddf-N-continue left-asymmetric-6 right-asymmetric-6 left-symmetric-6 right-symmetric-6 parity-first-6'
++ for layout in $layouts
++ mdadm -CR /dev/md0 -l6 --layout=ls -n5 -c 512 /dev/loop0 /dev/loop1 /dev/loop2 /dev/loop3 /dev/loop4
++ rm -f /var/tmp/stderr
++ case $* in
++ case $* in
++ for args in $*
++ [[ -CR =~ /dev/ ]]
++ for args in $*
++ [[ /dev/md0 =~ /dev/ ]]
++ [[ /dev/md0 =~ md ]]
++ for args in $*
++ [[ -l6 =~ /dev/ ]]
++ for args in $*
++ [[ --layout=ls =~ /dev/ ]]
++ for args in $*
++ [[ -n5 =~ /dev/ ]]
++ for args in $*
++ [[ -c =~ /dev/ ]]
++ for args in $*
++ [[ 512 =~ /dev/ ]]
++ for args in $*
++ [[ /dev/loop0 =~ /dev/ ]]
++ [[ /dev/loop0 =~ md ]]
++ /sbin/mdadm --zero /dev/loop0
mdadm: Unrecognised md component device - /dev/loop0
++ for args in $*
++ [[ /dev/loop1 =~ /dev/ ]]
++ [[ /dev/loop1 =~ md ]]
++ /sbin/mdadm --zero /dev/loop1
mdadm: Unrecognised md component device - /dev/loop1
++ for args in $*
++ [[ /dev/loop2 =~ /dev/ ]]
++ [[ /dev/loop2 =~ md ]]
++ /sbin/mdadm --zero /dev/loop2
mdadm: Unrecognised md component device - /dev/loop2
++ for args in $*
++ [[ /dev/loop3 =~ /dev/ ]]
++ [[ /dev/loop3 =~ md ]]
++ /sbin/mdadm --zero /dev/loop3
mdadm: Unrecognised md component device - /dev/loop3
++ for args in $*
++ [[ /dev/loop4 =~ /dev/ ]]
++ [[ /dev/loop4 =~ md ]]
++ /sbin/mdadm --zero /dev/loop4
mdadm: Unrecognised md component device - /dev/loop4
++ /sbin/mdadm -CR /dev/md0 -l6 --layout=ls -n5 -c 512 /dev/loop0 /dev/loop1 /dev/loop2 /dev/loop3 /dev/loop4
mdadm: Defaulting to version 1.2 metadata
mdadm: array /dev/md0 started.
++ rv=0
++ case $* in
++ cat /var/tmp/stderr
++ return 0
++ dd if=/tmp/RandFile of=/dev/md0 bs=1024 count=7680
7680+0 records in
7680+0 records out
7864320 bytes (7.9 MB, 7.5 MiB) copied, 0.0407638 s, 193 MB/s
++ blockdev --flushbufs /dev/md0
++ sync
++ check wait
++ case $1 in
+++ cat /proc/sys/dev/raid/speed_limit_min
++ min=100
+++ cat /proc/sys/dev/raid/speed_limit_max
++ max=500
++ echo 200000
++ sleep 0.1
++ iterations=0
++ '[' 0 -le 10 ']'
+++ grep -Ec '(resync|recovery|reshape|check|repair) *=' /proc/mdstat
++ sync_action=1
++ (( 1 == 0 ))
++ break
++ echo 'Reshape has not started after 10 seconds'
Reshape has not started after 10 seconds
++ echo 'Waiting for grow-continue to finish'
Waiting for grow-continue to finish
++ wait_for_reshape_end
++ true
+++ grep -Ec '(resync|recovery|reshape|check|repair) *=' /proc/mdstat
++ sync_action=1
++ (( 1 != 0 ))
++ sleep 2
++ continue
++ true
+++ grep -Ec '(resync|recovery|reshape|check|repair) *=' /proc/mdstat
++ sync_action=0
++ (( 0 != 0 ))
+++ pgrep -f 'mdadm --grow --continue'
++ [[ '' != '' ]]
++ break
++ sleep 5
++ wait_for_reshape_end
++ true
+++ grep -Ec '(resync|recovery|reshape|check|repair) *=' /proc/mdstat
++ sync_action=0
++ (( 0 != 0 ))
+++ pgrep -f 'mdadm --grow --continue'
++ [[ '' != '' ]]
++ break
++ echo 100
++ echo 500
++ blockdev --flushbufs /dev/loop0 /dev/loop1 /dev/loop2 /dev/loop3 /dev/loop4
++ sync
++ echo 3
++ cmp -s -n 7864320 /dev/md0 /tmp/RandFile
++ dd if=/dev/urandom of=/dev/loop0 bs=1024 count=2560 seek=1024
2560+0 records in
2560+0 records out
2621440 bytes (2.6 MB, 2.5 MiB) copied, 0.0104878 s, 250 MB/s
++ dd if=/dev/urandom of=/dev/loop1 bs=1024 count=2560 seek=3584
2560+0 records in
2560+0 records out
2621440 bytes (2.6 MB, 2.5 MiB) copied, 0.00968777 s, 271 MB/s
++ dd if=/dev/urandom of=/dev/loop2 bs=1024 count=2560 seek=6144
2560+0 records in
2560+0 records out
2621440 bytes (2.6 MB, 2.5 MiB) copied, 0.0147139 s, 178 MB/s
++ dd if=/dev/urandom of=/dev/loop3 bs=1024 count=2560 seek=8704
2560+0 records in
2560+0 records out
2621440 bytes (2.6 MB, 2.5 MiB) copied, 0.0108583 s, 241 MB/s
++ dd if=/dev/urandom of=/dev/loop4 bs=1024 count=2560 seek=11264
2560+0 records in
2560+0 records out
2621440 bytes (2.6 MB, 2.5 MiB) copied, 0.0121655 s, 215 MB/s
++ blockdev --flushbufs /dev/loop0 /dev/loop1 /dev/loop2 /dev/loop3 /dev/loop4
++ sync
++ echo 3
++ /raid6check /dev/md0 0 0
++ grep -qs Error
++ echo should detect errors
should detect errors
++ exit 2
^ permalink raw reply
* Re: [PATCH 07/15] md: Clean up folio sync support related code
From: Xiao Ni @ 2026-01-20 16:01 UTC (permalink / raw)
To: linan666; +Cc: song, yukuai, linux-raid, linux-kernel, yangerkun, yi.zhang
In-Reply-To: <20251217120013.2616531-8-linan666@huaweicloud.com>
On Wed, Dec 17, 2025 at 8:11 PM <linan666@huaweicloud.com> wrote:
>
> From: Li Nan <linan122@huawei.com>
>
> 1. Remove resync_get_all_folio() and invoke folio_get() directly instead.
> 2. Clean up redundant while(0) loop in md_bio_reset_resync_folio().
> 3. Clean up bio variable by directly referencing r10_bio->devs[j].bio
> instead in r1buf_pool_alloc() and r10buf_pool_alloc().
> 4. Clean up RESYNC_PAGES.
>
> Signed-off-by: Li Nan <linan122@huawei.com>
> ---
> drivers/md/raid1-10.c | 22 ++++++----------------
> drivers/md/raid1.c | 6 ++----
> drivers/md/raid10.c | 6 ++----
> 3 files changed, 10 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
> index b8f2cc32606f..568ab002691f 100644
> --- a/drivers/md/raid1-10.c
> +++ b/drivers/md/raid1-10.c
> @@ -1,7 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> /* Maximum size of each resync request */
> #define RESYNC_BLOCK_SIZE (64*1024)
> -#define RESYNC_PAGES ((RESYNC_BLOCK_SIZE + PAGE_SIZE-1) / PAGE_SIZE)
> #define RESYNC_SECTORS (RESYNC_BLOCK_SIZE >> 9)
>
> /*
> @@ -56,11 +55,6 @@ static inline void resync_free_folio(struct resync_folio *rf)
> folio_put(rf->folio);
> }
>
> -static inline void resync_get_all_folio(struct resync_folio *rf)
> -{
> - folio_get(rf->folio);
> -}
> -
> static inline struct folio *resync_fetch_folio(struct resync_folio *rf)
> {
> return rf->folio;
> @@ -80,16 +74,12 @@ static void md_bio_reset_resync_folio(struct bio *bio, struct resync_folio *rf,
> int size)
> {
> /* initialize bvec table again */
> - do {
> - struct folio *folio = resync_fetch_folio(rf);
> - int len = min_t(int, size, RESYNC_BLOCK_SIZE);
> -
> - if (WARN_ON(!bio_add_folio(bio, folio, len, 0))) {
> - bio->bi_status = BLK_STS_RESOURCE;
> - bio_endio(bio);
> - return;
> - }
> - } while (0);
> + if (WARN_ON(!bio_add_folio(bio, resync_fetch_folio(rf),
> + min_t(int, size, RESYNC_BLOCK_SIZE),
> + 0))) {
> + bio->bi_status = BLK_STS_RESOURCE;
> + bio_endio(bio);
> + }
> }
>
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 370bdecf5487..f01bab41da95 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -181,18 +181,16 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data)
> for (j = 0; j < conf->raid_disks * 2; j++) {
> struct resync_folio *rf = &rfs[j];
>
> - bio = r1_bio->bios[j];
> -
> if (j < need_folio) {
> if (resync_alloc_folio(rf, gfp_flags))
> goto out_free_folio;
> } else {
> memcpy(rf, &rfs[0], sizeof(*rf));
> - resync_get_all_folio(rf);
> + folio_get(rf->folio);
> }
>
> rf->raid_bio = r1_bio;
> - bio->bi_private = rf;
> + r1_bio->bios[j]->bi_private = rf;
> }
>
> r1_bio->master_bio = NULL;
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index c93706806358..a03afa9a6a5b 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -183,19 +183,17 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
> if (rbio)
> rf_repl = &rfs[nalloc + j];
>
> - bio = r10_bio->devs[j].bio;
> -
> if (!j || test_bit(MD_RECOVERY_SYNC,
> &conf->mddev->recovery)) {
> if (resync_alloc_folio(rf, gfp_flags))
> goto out_free_pages;
> } else {
> memcpy(rf, &rfs[0], sizeof(*rf));
> - resync_get_all_folio(rf);
> + folio_get(rf->folio);
> }
>
> rf->raid_bio = r10_bio;
> - bio->bi_private = rf;
> + r10_bio->devs[j].bio->bi_private = rf;
> if (rbio) {
> memcpy(rf_repl, rf, sizeof(*rf));
> rbio->bi_private = rf_repl;
> --
> 2.39.2
>
Reviewed-by: Xiao Ni <xni@redhat.com>
^ permalink raw reply
* Re: [PATCH 06/15] md/raid1,raid10: use folio for sync path IO
From: Xiao Ni @ 2026-01-20 15:53 UTC (permalink / raw)
To: linan666; +Cc: song, yukuai, linux-raid, linux-kernel, yangerkun, yi.zhang
In-Reply-To: <20251217120013.2616531-7-linan666@huaweicloud.com>
On Wed, Dec 17, 2025 at 8:11 PM <linan666@huaweicloud.com> wrote:
>
> From: Li Nan <linan122@huawei.com>
>
> Convert all IO on the sync path to use folios. Rename page-related
> identifiers to match folio.
>
> Retain some now-unnecessary while and for loops to minimize code
> changes, clean them up in a subsequent patch.
>
> Signed-off-by: Li Nan <linan122@huawei.com>
> ---
> drivers/md/md.c | 2 +-
> drivers/md/raid1-10.c | 60 ++++--------
> drivers/md/raid1.c | 155 ++++++++++++++-----------------
> drivers/md/raid10.c | 207 +++++++++++++++++++-----------------------
> 4 files changed, 179 insertions(+), 245 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 0732bbcdb95d..dac03b831efa 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -9409,7 +9409,7 @@ static bool sync_io_within_limit(struct mddev *mddev)
> {
> /*
> * For raid456, sync IO is stripe(4k) per IO, for other levels, it's
> - * RESYNC_PAGES(64k) per IO.
> + * RESYNC_BLOCK_SIZE(64k) per IO.
> */
> return atomic_read(&mddev->recovery_active) <
> (raid_is_456(mddev) ? 8 : 128) * sync_io_depth(mddev);
> diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
> index 260d7fd7ccbe..b8f2cc32606f 100644
> --- a/drivers/md/raid1-10.c
> +++ b/drivers/md/raid1-10.c
> @@ -25,9 +25,9 @@
> #define MAX_PLUG_BIO 32
>
> /* for managing resync I/O pages */
> -struct resync_pages {
> +struct resync_folio {
> void *raid_bio;
> - struct page *pages[RESYNC_PAGES];
> + struct folio *folio;
> };
>
> struct raid1_plug_cb {
> @@ -41,77 +41,55 @@ static void rbio_pool_free(void *rbio, void *data)
> kfree(rbio);
> }
>
> -static inline int resync_alloc_pages(struct resync_pages *rp,
> +static inline int resync_alloc_folio(struct resync_folio *rf,
> gfp_t gfp_flags)
> {
> - int i;
> -
> - for (i = 0; i < RESYNC_PAGES; i++) {
> - rp->pages[i] = alloc_page(gfp_flags);
> - if (!rp->pages[i])
> - goto out_free;
> - }
> + rf->folio = folio_alloc(gfp_flags, get_order(RESYNC_BLOCK_SIZE));
> + if (!rf->folio)
> + return -ENOMEM;
Is it ok to add an error log here? Compare with the multipage
situation, the possibility of failure will be somewhat higher because
it needs to alloc a contiguous block of physical memory.
>
> return 0;
> -
> -out_free:
> - while (--i >= 0)
> - put_page(rp->pages[i]);
> - return -ENOMEM;
> }
>
> -static inline void resync_free_pages(struct resync_pages *rp)
> +static inline void resync_free_folio(struct resync_folio *rf)
> {
> - int i;
> -
> - for (i = 0; i < RESYNC_PAGES; i++)
> - put_page(rp->pages[i]);
> + folio_put(rf->folio);
> }
>
> -static inline void resync_get_all_pages(struct resync_pages *rp)
> +static inline void resync_get_all_folio(struct resync_folio *rf)
> {
> - int i;
> -
> - for (i = 0; i < RESYNC_PAGES; i++)
> - get_page(rp->pages[i]);
> + folio_get(rf->folio);
> }
>
> -static inline struct page *resync_fetch_page(struct resync_pages *rp,
> - unsigned idx)
> +static inline struct folio *resync_fetch_folio(struct resync_folio *rf)
> {
> - if (WARN_ON_ONCE(idx >= RESYNC_PAGES))
> - return NULL;
> - return rp->pages[idx];
> + return rf->folio;
> }
>
> /*
> - * 'strct resync_pages' stores actual pages used for doing the resync
> + * 'strct resync_folio' stores actual pages used for doing the resync
> * IO, and it is per-bio, so make .bi_private points to it.
> */
> -static inline struct resync_pages *get_resync_pages(struct bio *bio)
> +static inline struct resync_folio *get_resync_folio(struct bio *bio)
> {
> return bio->bi_private;
> }
>
> /* generally called after bio_reset() for reseting bvec */
> -static void md_bio_reset_resync_pages(struct bio *bio, struct resync_pages *rp,
> +static void md_bio_reset_resync_folio(struct bio *bio, struct resync_folio *rf,
> int size)
> {
> - int idx = 0;
> -
> /* initialize bvec table again */
> do {
> - struct page *page = resync_fetch_page(rp, idx);
> - int len = min_t(int, size, PAGE_SIZE);
> + struct folio *folio = resync_fetch_folio(rf);
> + int len = min_t(int, size, RESYNC_BLOCK_SIZE);
>
> - if (WARN_ON(!bio_add_page(bio, page, len, 0))) {
> + if (WARN_ON(!bio_add_folio(bio, folio, len, 0))) {
Is it ok to use bio_add_folio(bio, folio, RESYNC_BLOCK_SIZE, 0)
directly here? It removes `size -= len` below, so it's not useless to
compare size and RESYNC_BLOCK_SIZE above?
> bio->bi_status = BLK_STS_RESOURCE;
> bio_endio(bio);
> return;
> }
> -
> - size -= len;
> - } while (idx++ < RESYNC_PAGES && size > 0);
> + } while (0);
> }
>
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 43453f1a04f4..370bdecf5487 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -120,11 +120,11 @@ static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
>
> /*
> * for resync bio, r1bio pointer can be retrieved from the per-bio
> - * 'struct resync_pages'.
> + * 'struct resync_folio'.
> */
> static inline struct r1bio *get_resync_r1bio(struct bio *bio)
> {
> - return get_resync_pages(bio)->raid_bio;
> + return get_resync_folio(bio)->raid_bio;
> }
>
> static void *r1bio_pool_alloc(gfp_t gfp_flags, struct r1conf *conf)
> @@ -146,70 +146,69 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data)
> struct r1conf *conf = data;
> struct r1bio *r1_bio;
> struct bio *bio;
> - int need_pages;
> + int need_folio;
> int j;
> - struct resync_pages *rps;
> + struct resync_folio *rfs;
>
> r1_bio = r1bio_pool_alloc(gfp_flags, conf);
> if (!r1_bio)
> return NULL;
>
> - rps = kmalloc_array(conf->raid_disks * 2, sizeof(struct resync_pages),
> + rfs = kmalloc_array(conf->raid_disks * 2, sizeof(struct resync_folio),
> gfp_flags);
> - if (!rps)
> + if (!rfs)
> goto out_free_r1bio;
>
> /*
> * Allocate bios : 1 for reading, n-1 for writing
> */
> for (j = conf->raid_disks * 2; j-- ; ) {
> - bio = bio_kmalloc(RESYNC_PAGES, gfp_flags);
> + bio = bio_kmalloc(1, gfp_flags);
> if (!bio)
> goto out_free_bio;
> - bio_init_inline(bio, NULL, RESYNC_PAGES, 0);
> + bio_init_inline(bio, NULL, 1, 0);
> r1_bio->bios[j] = bio;
> }
> /*
> - * Allocate RESYNC_PAGES data pages and attach them to
> - * the first bio.
> + * Allocate data folio and attach them to the first bio.
typo error
s/attach them/attach it/g
> * If this is a user-requested check/repair, allocate
> - * RESYNC_PAGES for each bio.
> + * folio for each bio.
> */
> if (test_bit(MD_RECOVERY_REQUESTED, &conf->mddev->recovery))
> - need_pages = conf->raid_disks * 2;
> + need_folio = conf->raid_disks * 2;
> else
> - need_pages = 1;
> + need_folio = 1;
> for (j = 0; j < conf->raid_disks * 2; j++) {
> - struct resync_pages *rp = &rps[j];
> + struct resync_folio *rf = &rfs[j];
>
> bio = r1_bio->bios[j];
>
> - if (j < need_pages) {
> - if (resync_alloc_pages(rp, gfp_flags))
> - goto out_free_pages;
> + if (j < need_folio) {
> + if (resync_alloc_folio(rf, gfp_flags))
> + goto out_free_folio;
> } else {
> - memcpy(rp, &rps[0], sizeof(*rp));
> - resync_get_all_pages(rp);
> + memcpy(rf, &rfs[0], sizeof(*rf));
> + resync_get_all_folio(rf);
> }
>
> - rp->raid_bio = r1_bio;
> - bio->bi_private = rp;
> + rf->raid_bio = r1_bio;
> + bio->bi_private = rf;
> }
>
> r1_bio->master_bio = NULL;
>
> return r1_bio;
>
> -out_free_pages:
> +out_free_folio:
> while (--j >= 0)
> - resync_free_pages(&rps[j]);
> + resync_free_folio(&rfs[j]);
>
> out_free_bio:
> while (++j < conf->raid_disks * 2) {
> bio_uninit(r1_bio->bios[j]);
> kfree(r1_bio->bios[j]);
> }
> - kfree(rps);
> + kfree(rfs);
>
> out_free_r1bio:
> rbio_pool_free(r1_bio, data);
> @@ -221,17 +220,17 @@ static void r1buf_pool_free(void *__r1_bio, void *data)
> struct r1conf *conf = data;
> int i;
> struct r1bio *r1bio = __r1_bio;
> - struct resync_pages *rp = NULL;
> + struct resync_folio *rf = NULL;
>
> for (i = conf->raid_disks * 2; i--; ) {
> - rp = get_resync_pages(r1bio->bios[i]);
> - resync_free_pages(rp);
> + rf = get_resync_folio(r1bio->bios[i]);
> + resync_free_folio(rf);
> bio_uninit(r1bio->bios[i]);
> kfree(r1bio->bios[i]);
> }
>
> - /* resync pages array stored in the 1st bio's .bi_private */
> - kfree(rp);
> + /* resync folio stored in the 1st bio's .bi_private */
> + kfree(rf);
>
> rbio_pool_free(r1bio, data);
> }
> @@ -2095,10 +2094,10 @@ static void end_sync_write(struct bio *bio)
> put_sync_write_buf(r1_bio);
> }
>
> -static int r1_sync_page_io(struct md_rdev *rdev, sector_t sector,
> - int sectors, struct page *page, blk_opf_t rw)
> +static int r1_sync_folio_io(struct md_rdev *rdev, sector_t sector, int sectors,
> + int off, struct folio *folio, blk_opf_t rw)
> {
> - if (sync_page_io(rdev, sector, sectors << 9, page, rw, false))
> + if (sync_folio_io(rdev, sector, sectors << 9, off, folio, rw, false))
> /* success */
> return 1;
> if (rw == REQ_OP_WRITE) {
> @@ -2129,10 +2128,10 @@ static int fix_sync_read_error(struct r1bio *r1_bio)
> struct mddev *mddev = r1_bio->mddev;
> struct r1conf *conf = mddev->private;
> struct bio *bio = r1_bio->bios[r1_bio->read_disk];
> - struct page **pages = get_resync_pages(bio)->pages;
> + struct folio *folio = get_resync_folio(bio)->folio;
> sector_t sect = r1_bio->sector;
> int sectors = r1_bio->sectors;
> - int idx = 0;
> + int off = 0;
> struct md_rdev *rdev;
>
> rdev = conf->mirrors[r1_bio->read_disk].rdev;
> @@ -2162,9 +2161,8 @@ static int fix_sync_read_error(struct r1bio *r1_bio)
> * active, and resync is currently active
> */
> rdev = conf->mirrors[d].rdev;
> - if (sync_page_io(rdev, sect, s<<9,
> - pages[idx],
> - REQ_OP_READ, false)) {
> + if (sync_folio_io(rdev, sect, s<<9, off, folio,
> + REQ_OP_READ, false)) {
> success = 1;
> break;
> }
> @@ -2197,7 +2195,7 @@ static int fix_sync_read_error(struct r1bio *r1_bio)
> /* Try next page */
> sectors -= s;
> sect += s;
> - idx++;
> + off += s << 9;
> continue;
> }
>
> @@ -2210,8 +2208,7 @@ static int fix_sync_read_error(struct r1bio *r1_bio)
> if (r1_bio->bios[d]->bi_end_io != end_sync_read)
> continue;
> rdev = conf->mirrors[d].rdev;
> - if (r1_sync_page_io(rdev, sect, s,
> - pages[idx],
> + if (r1_sync_folio_io(rdev, sect, s, off, folio,
> REQ_OP_WRITE) == 0) {
> r1_bio->bios[d]->bi_end_io = NULL;
> rdev_dec_pending(rdev, mddev);
> @@ -2225,14 +2222,13 @@ static int fix_sync_read_error(struct r1bio *r1_bio)
> if (r1_bio->bios[d]->bi_end_io != end_sync_read)
> continue;
> rdev = conf->mirrors[d].rdev;
> - if (r1_sync_page_io(rdev, sect, s,
> - pages[idx],
> + if (r1_sync_folio_io(rdev, sect, s, off, folio,
> REQ_OP_READ) != 0)
> atomic_add(s, &rdev->corrected_errors);
> }
> sectors -= s;
> sect += s;
> - idx ++;
> + off += s << 9;
> }
> set_bit(R1BIO_Uptodate, &r1_bio->state);
> bio->bi_status = 0;
> @@ -2252,14 +2248,12 @@ static void process_checks(struct r1bio *r1_bio)
> struct r1conf *conf = mddev->private;
> int primary;
> int i;
> - int vcnt;
>
> /* Fix variable parts of all bios */
> - vcnt = (r1_bio->sectors + PAGE_SIZE / 512 - 1) >> (PAGE_SHIFT - 9);
> for (i = 0; i < conf->raid_disks * 2; i++) {
> blk_status_t status;
> struct bio *b = r1_bio->bios[i];
> - struct resync_pages *rp = get_resync_pages(b);
> + struct resync_folio *rf = get_resync_folio(b);
> if (b->bi_end_io != end_sync_read)
> continue;
> /* fixup the bio for reuse, but preserve errno */
> @@ -2269,11 +2263,11 @@ static void process_checks(struct r1bio *r1_bio)
> b->bi_iter.bi_sector = r1_bio->sector +
> conf->mirrors[i].rdev->data_offset;
> b->bi_end_io = end_sync_read;
> - rp->raid_bio = r1_bio;
> - b->bi_private = rp;
> + rf->raid_bio = r1_bio;
> + b->bi_private = rf;
>
> /* initialize bvec table again */
> - md_bio_reset_resync_pages(b, rp, r1_bio->sectors << 9);
> + md_bio_reset_resync_folio(b, rf, r1_bio->sectors << 9);
> }
> for (primary = 0; primary < conf->raid_disks * 2; primary++)
> if (r1_bio->bios[primary]->bi_end_io == end_sync_read &&
> @@ -2284,44 +2278,30 @@ static void process_checks(struct r1bio *r1_bio)
> }
> r1_bio->read_disk = primary;
> for (i = 0; i < conf->raid_disks * 2; i++) {
> - int j = 0;
> struct bio *pbio = r1_bio->bios[primary];
> struct bio *sbio = r1_bio->bios[i];
> blk_status_t status = sbio->bi_status;
> - struct page **ppages = get_resync_pages(pbio)->pages;
> - struct page **spages = get_resync_pages(sbio)->pages;
> - struct bio_vec *bi;
> - int page_len[RESYNC_PAGES] = { 0 };
> - struct bvec_iter_all iter_all;
> + struct folio *pfolio = get_resync_folio(pbio)->folio;
> + struct folio *sfolio = get_resync_folio(sbio)->folio;
>
> if (sbio->bi_end_io != end_sync_read)
> continue;
> /* Now we can 'fixup' the error value */
> sbio->bi_status = 0;
>
> - bio_for_each_segment_all(bi, sbio, iter_all)
> - page_len[j++] = bi->bv_len;
> -
> - if (!status) {
> - for (j = vcnt; j-- ; ) {
> - if (memcmp(page_address(ppages[j]),
> - page_address(spages[j]),
> - page_len[j]))
> - break;
> - }
> - } else
> - j = 0;
> - if (j >= 0)
> + if (status || memcmp(folio_address(pfolio),
> + folio_address(sfolio),
> + r1_bio->sectors << 9)) {
> atomic64_add(r1_bio->sectors, &mddev->resync_mismatches);
> - if (j < 0 || (test_bit(MD_RECOVERY_CHECK, &mddev->recovery)
> - && !status)) {
> - /* No need to write to this device. */
> - sbio->bi_end_io = NULL;
> - rdev_dec_pending(conf->mirrors[i].rdev, mddev);
> - continue;
> + if (!test_bit(MD_RECOVERY_CHECK, &mddev->recovery)) {
> + bio_copy_data(sbio, pbio);
> + continue;
> + }
The logic is changed here. The original logic:
1. read ok, no mismatch: no bio_copy_data
2. read ok, mismatch, check: no bio_copy_data
3. read ok, mismatch, no check: need bio_copy_data
4. read fail: need bio_copy_data
The 4 is broken.
How about adding a temporary need_write to make logic more clear?
something like:
if (!status) {
int ret = 0;
ret = memcpy(folio_address(pfolio),
folio_address(sfolio),
r1_bio->sectors << 9);
if (ret) {
atomic64_add(r1_bio->sectors, &mddev->resync_mismatches);
if (!test_bit(MD_RECOVERY_CHECK, &mddev->recovery))
need_write = true;
}
} else
need_write = true;
if (need_write)
bio_copy_data(sbio, pbio);
else {
/* No need to write to this device. */
sbio->bi_end_io = NULL;
rdev_dec_pending(conf->mirrors[i].rdev, mddev);
}
> }
>
> - bio_copy_data(sbio, pbio);
> + /* No need to write to this device. */
> + sbio->bi_end_io = NULL;
> + rdev_dec_pending(conf->mirrors[i].rdev, mddev);
> }
> }
>
> @@ -2446,9 +2426,8 @@ static void fix_read_error(struct r1conf *conf, struct r1bio *r1_bio)
> if (rdev &&
> !test_bit(Faulty, &rdev->flags)) {
> atomic_inc(&rdev->nr_pending);
> - r1_sync_page_io(rdev, sect, s,
> - folio_page(conf->tmpfolio, 0),
> - REQ_OP_WRITE);
> + r1_sync_folio_io(rdev, sect, s, 0,
> + conf->tmpfolio, REQ_OP_WRITE);
> rdev_dec_pending(rdev, mddev);
> }
> }
> @@ -2461,9 +2440,8 @@ static void fix_read_error(struct r1conf *conf, struct r1bio *r1_bio)
> if (rdev &&
> !test_bit(Faulty, &rdev->flags)) {
> atomic_inc(&rdev->nr_pending);
> - if (r1_sync_page_io(rdev, sect, s,
> - folio_page(conf->tmpfolio, 0),
> - REQ_OP_READ)) {
> + if (r1_sync_folio_io(rdev, sect, s, 0,
> + conf->tmpfolio, REQ_OP_READ)) {
> atomic_add(s, &rdev->corrected_errors);
> pr_info("md/raid1:%s: read error corrected (%d sectors at %llu on %pg)\n",
> mdname(mddev), s,
> @@ -2799,7 +2777,6 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
> int good_sectors = RESYNC_SECTORS;
> int min_bad = 0; /* number of sectors that are bad in all devices */
> int idx = sector_to_idx(sector_nr);
> - int page_idx = 0;
>
> if (!mempool_initialized(&conf->r1buf_pool))
> if (init_resync(conf))
> @@ -3003,8 +2980,8 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
> nr_sectors = 0;
> sync_blocks = 0;
> do {
> - struct page *page;
> - int len = PAGE_SIZE;
> + struct folio *folio;
> + int len = RESYNC_BLOCK_SIZE;
> if (sector_nr + (len>>9) > max_sector)
> len = (max_sector - sector_nr) << 9;
> if (len == 0)
> @@ -3020,24 +2997,24 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
> }
>
> for (i = 0 ; i < conf->raid_disks * 2; i++) {
> - struct resync_pages *rp;
> + struct resync_folio *rf;
>
> bio = r1_bio->bios[i];
> - rp = get_resync_pages(bio);
> + rf = get_resync_folio(bio);
> if (bio->bi_end_io) {
> - page = resync_fetch_page(rp, page_idx);
> + folio = resync_fetch_folio(rf);
>
> /*
> * won't fail because the vec table is big
> * enough to hold all these pages
> */
The comments above may not be needed anymore. Because there is only
one vec in the bio.
> - __bio_add_page(bio, page, len, 0);
> + bio_add_folio_nofail(bio, folio, len, 0);
> }
> }
> nr_sectors += len>>9;
> sector_nr += len>>9;
> sync_blocks -= (len>>9);
These three lines are not needed anymore.
> - } while (++page_idx < RESYNC_PAGES);
> + } while (0);
>
> r1_bio->sectors = nr_sectors;
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 09238dc9cde6..c93706806358 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -96,11 +96,11 @@ static void end_reshape(struct r10conf *conf);
>
> /*
> * for resync bio, r10bio pointer can be retrieved from the per-bio
> - * 'struct resync_pages'.
> + * 'struct resync_folio'.
> */
> static inline struct r10bio *get_resync_r10bio(struct bio *bio)
> {
> - return get_resync_pages(bio)->raid_bio;
> + return get_resync_folio(bio)->raid_bio;
> }
>
> static void * r10bio_pool_alloc(gfp_t gfp_flags, void *data)
> @@ -133,8 +133,8 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
> struct r10bio *r10_bio;
> struct bio *bio;
> int j;
> - int nalloc, nalloc_rp;
> - struct resync_pages *rps;
> + int nalloc, nalloc_rf;
> + struct resync_folio *rfs;
>
> r10_bio = r10bio_pool_alloc(gfp_flags, conf);
> if (!r10_bio)
> @@ -148,58 +148,57 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
>
> /* allocate once for all bios */
> if (!conf->have_replacement)
> - nalloc_rp = nalloc;
> + nalloc_rf = nalloc;
> else
> - nalloc_rp = nalloc * 2;
> - rps = kmalloc_array(nalloc_rp, sizeof(struct resync_pages), gfp_flags);
> - if (!rps)
> + nalloc_rf = nalloc * 2;
> + rfs = kmalloc_array(nalloc_rf, sizeof(struct resync_folio), gfp_flags);
> + if (!rfs)
> goto out_free_r10bio;
>
> /*
> * Allocate bios.
> */
> for (j = nalloc ; j-- ; ) {
> - bio = bio_kmalloc(RESYNC_PAGES, gfp_flags);
> + bio = bio_kmalloc(1, gfp_flags);
> if (!bio)
> goto out_free_bio;
> - bio_init_inline(bio, NULL, RESYNC_PAGES, 0);
> + bio_init_inline(bio, NULL, 1, 0);
> r10_bio->devs[j].bio = bio;
> if (!conf->have_replacement)
> continue;
> - bio = bio_kmalloc(RESYNC_PAGES, gfp_flags);
> + bio = bio_kmalloc(1, gfp_flags);
> if (!bio)
> goto out_free_bio;
> - bio_init_inline(bio, NULL, RESYNC_PAGES, 0);
> + bio_init_inline(bio, NULL, 1, 0);
> r10_bio->devs[j].repl_bio = bio;
> }
> /*
> - * Allocate RESYNC_PAGES data pages and attach them
> - * where needed.
> + * Allocate data folio and attach them where needed.
typo error
s/attach them/attach it/g
> */
> for (j = 0; j < nalloc; j++) {
> struct bio *rbio = r10_bio->devs[j].repl_bio;
> - struct resync_pages *rp, *rp_repl;
> + struct resync_folio *rf, *rf_repl;
>
> - rp = &rps[j];
> + rf = &rfs[j];
> if (rbio)
> - rp_repl = &rps[nalloc + j];
> + rf_repl = &rfs[nalloc + j];
>
> bio = r10_bio->devs[j].bio;
>
> if (!j || test_bit(MD_RECOVERY_SYNC,
> &conf->mddev->recovery)) {
> - if (resync_alloc_pages(rp, gfp_flags))
> + if (resync_alloc_folio(rf, gfp_flags))
> goto out_free_pages;
s/out_free_pages/out_free_folio/g
> } else {
> - memcpy(rp, &rps[0], sizeof(*rp));
> - resync_get_all_pages(rp);
> + memcpy(rf, &rfs[0], sizeof(*rf));
> + resync_get_all_folio(rf);
Maybe the name resync_get_folio is better?
> }
>
> - rp->raid_bio = r10_bio;
> - bio->bi_private = rp;
> + rf->raid_bio = r10_bio;
> + bio->bi_private = rf;
> if (rbio) {
> - memcpy(rp_repl, rp, sizeof(*rp));
> - rbio->bi_private = rp_repl;
> + memcpy(rf_repl, rf, sizeof(*rf));
> + rbio->bi_private = rf_repl;
> }
> }
>
> @@ -207,7 +206,7 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
>
> out_free_pages:
> while (--j >= 0)
> - resync_free_pages(&rps[j]);
> + resync_free_folio(&rfs[j]);
>
> j = 0;
> out_free_bio:
> @@ -219,7 +218,7 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
> bio_uninit(r10_bio->devs[j].repl_bio);
> kfree(r10_bio->devs[j].repl_bio);
> }
> - kfree(rps);
> + kfree(rfs);
> out_free_r10bio:
> rbio_pool_free(r10_bio, conf);
> return NULL;
> @@ -230,14 +229,14 @@ static void r10buf_pool_free(void *__r10_bio, void *data)
> struct r10conf *conf = data;
> struct r10bio *r10bio = __r10_bio;
> int j;
> - struct resync_pages *rp = NULL;
> + struct resync_folio *rf = NULL;
>
> for (j = conf->copies; j--; ) {
> struct bio *bio = r10bio->devs[j].bio;
>
> if (bio) {
> - rp = get_resync_pages(bio);
> - resync_free_pages(rp);
> + rf = get_resync_folio(bio);
> + resync_free_folio(rf);
> bio_uninit(bio);
> kfree(bio);
> }
> @@ -250,7 +249,7 @@ static void r10buf_pool_free(void *__r10_bio, void *data)
> }
>
> /* resync pages array stored in the 1st bio's .bi_private */
> - kfree(rp);
> + kfree(rf);
>
> rbio_pool_free(r10bio, conf);
> }
> @@ -2342,8 +2341,7 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
> struct r10conf *conf = mddev->private;
> int i, first;
> struct bio *tbio, *fbio;
> - int vcnt;
> - struct page **tpages, **fpages;
> + struct folio *tfolio, *ffolio;
>
> atomic_set(&r10_bio->remaining, 1);
>
> @@ -2359,14 +2357,13 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
> fbio = r10_bio->devs[i].bio;
> fbio->bi_iter.bi_size = r10_bio->sectors << 9;
> fbio->bi_iter.bi_idx = 0;
> - fpages = get_resync_pages(fbio)->pages;
> + ffolio = get_resync_folio(fbio)->folio;
>
> - vcnt = (r10_bio->sectors + (PAGE_SIZE >> 9) - 1) >> (PAGE_SHIFT - 9);
> /* now find blocks with errors */
> for (i=0 ; i < conf->copies ; i++) {
> - int j, d;
> + int d;
> struct md_rdev *rdev;
> - struct resync_pages *rp;
> + struct resync_folio *rf;
>
> tbio = r10_bio->devs[i].bio;
>
> @@ -2375,31 +2372,23 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
> if (i == first)
> continue;
>
> - tpages = get_resync_pages(tbio)->pages;
> + tfolio = get_resync_folio(tbio)->folio;
> d = r10_bio->devs[i].devnum;
> rdev = conf->mirrors[d].rdev;
> if (!r10_bio->devs[i].bio->bi_status) {
> /* We know that the bi_io_vec layout is the same for
> * both 'first' and 'i', so we just compare them.
> - * All vec entries are PAGE_SIZE;
> */
> - int sectors = r10_bio->sectors;
> - for (j = 0; j < vcnt; j++) {
> - int len = PAGE_SIZE;
> - if (sectors < (len / 512))
> - len = sectors * 512;
> - if (memcmp(page_address(fpages[j]),
> - page_address(tpages[j]),
> - len))
> - break;
> - sectors -= len/512;
> + if (memcmp(folio_address(ffolio),
> + folio_address(tfolio),
> + r10_bio->sectors << 9)) {
> + atomic64_add(r10_bio->sectors,
> + &mddev->resync_mismatches);
> + if (test_bit(MD_RECOVERY_CHECK,
> + &mddev->recovery))
> + /* Don't fix anything. */
> + continue;
> }
> - if (j == vcnt)
> - continue;
> - atomic64_add(r10_bio->sectors, &mddev->resync_mismatches);
> - if (test_bit(MD_RECOVERY_CHECK, &mddev->recovery))
> - /* Don't fix anything. */
> - continue;
> } else if (test_bit(FailFast, &rdev->flags)) {
> /* Just give up on this device */
> md_error(rdev->mddev, rdev);
> @@ -2410,13 +2399,13 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
> * First we need to fixup bv_offset, bv_len and
> * bi_vecs, as the read request might have corrupted these
> */
> - rp = get_resync_pages(tbio);
> + rf = get_resync_folio(tbio);
> bio_reset(tbio, conf->mirrors[d].rdev->bdev, REQ_OP_WRITE);
>
> - md_bio_reset_resync_pages(tbio, rp, fbio->bi_iter.bi_size);
> + md_bio_reset_resync_folio(tbio, rf, fbio->bi_iter.bi_size);
>
> - rp->raid_bio = r10_bio;
> - tbio->bi_private = rp;
> + rf->raid_bio = r10_bio;
> + tbio->bi_private = rf;
> tbio->bi_iter.bi_sector = r10_bio->devs[i].addr;
> tbio->bi_end_io = end_sync_write;
>
> @@ -2476,10 +2465,9 @@ static void fix_recovery_read_error(struct r10bio *r10_bio)
> struct bio *bio = r10_bio->devs[0].bio;
> sector_t sect = 0;
> int sectors = r10_bio->sectors;
> - int idx = 0;
> int dr = r10_bio->devs[0].devnum;
> int dw = r10_bio->devs[1].devnum;
> - struct page **pages = get_resync_pages(bio)->pages;
> + struct folio *folio = get_resync_folio(bio)->folio;
>
> while (sectors) {
> int s = sectors;
> @@ -2492,19 +2480,21 @@ static void fix_recovery_read_error(struct r10bio *r10_bio)
>
> rdev = conf->mirrors[dr].rdev;
> addr = r10_bio->devs[0].addr + sect;
> - ok = sync_page_io(rdev,
> - addr,
> - s << 9,
> - pages[idx],
> - REQ_OP_READ, false);
> + ok = sync_folio_io(rdev,
> + addr,
> + s << 9,
> + sect << 9,
> + folio,
> + REQ_OP_READ, false);
By the comments at the beginning of fix_recovery_read_error, it needs
to submit io with a page size unit, right? If so, it still needs to
use sync_page_io here.
> if (ok) {
> rdev = conf->mirrors[dw].rdev;
> addr = r10_bio->devs[1].addr + sect;
> - ok = sync_page_io(rdev,
> - addr,
> - s << 9,
> - pages[idx],
> - REQ_OP_WRITE, false);
> + ok = sync_folio_io(rdev,
> + addr,
> + s << 9,
> + sect << 9,
> + folio,
> + REQ_OP_WRITE, false);
> if (!ok) {
> set_bit(WriteErrorSeen, &rdev->flags);
> if (!test_and_set_bit(WantReplacement,
> @@ -2539,7 +2529,6 @@ static void fix_recovery_read_error(struct r10bio *r10_bio)
>
> sectors -= s;
> sect += s;
> - idx++;
> }
> }
>
> @@ -3174,7 +3163,6 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> int max_sync = RESYNC_SECTORS;
> sector_t sync_blocks;
> sector_t chunk_mask = conf->geo.chunk_mask;
> - int page_idx = 0;
>
> /*
> * Allow skipping a full rebuild for incremental assembly
> @@ -3277,7 +3265,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> * with 2 bios in each, that correspond to the bios in the main one.
> * In this case, the subordinate r10bios link back through a
> * borrowed master_bio pointer, and the counter in the master
> - * includes a ref from each subordinate.
> + * bio_add_folio includes a ref from each subordinate.
What's the reason change this? And I don't understand the new version.
Best Regards
Xiao
> */
> /* First, we decide what to do and set ->bi_end_io
> * To end_sync_read if we want to read, and
> @@ -3642,25 +3630,26 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> if (sector_nr + max_sync < max_sector)
> max_sector = sector_nr + max_sync;
> do {
> - struct page *page;
> - int len = PAGE_SIZE;
> + int len = RESYNC_BLOCK_SIZE;
> +
> if (sector_nr + (len>>9) > max_sector)
> len = (max_sector - sector_nr) << 9;
> if (len == 0)
> break;
> for (bio= biolist ; bio ; bio=bio->bi_next) {
> - struct resync_pages *rp = get_resync_pages(bio);
> - page = resync_fetch_page(rp, page_idx);
> - if (WARN_ON(!bio_add_page(bio, page, len, 0))) {
> + struct resync_folio *rf = get_resync_folio(bio);
> + struct folio *folio = resync_fetch_folio(rf);
> +
> + if (WARN_ON(!bio_add_folio(bio, folio, len, 0))) {
> bio->bi_status = BLK_STS_RESOURCE;
> bio_endio(bio);
> *skipped = 1;
> - return max_sync;
> + return len;
> }
> }
> nr_sectors += len>>9;
> sector_nr += len>>9;
> - } while (++page_idx < RESYNC_PAGES);
> + } while (0);
> r10_bio->sectors = nr_sectors;
>
> if (mddev_is_clustered(mddev) &&
> @@ -4578,7 +4567,7 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
> int *skipped)
> {
> /* We simply copy at most one chunk (smallest of old and new)
> - * at a time, possibly less if that exceeds RESYNC_PAGES,
> + * at a time, possibly less if that exceeds RESYNC_BLOCK_SIZE,
> * or we hit a bad block or something.
> * This might mean we pause for normal IO in the middle of
> * a chunk, but that is not a problem as mddev->reshape_position
> @@ -4618,14 +4607,13 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
> struct r10bio *r10_bio;
> sector_t next, safe, last;
> int max_sectors;
> - int nr_sectors;
> int s;
> struct md_rdev *rdev;
> int need_flush = 0;
> struct bio *blist;
> struct bio *bio, *read_bio;
> int sectors_done = 0;
> - struct page **pages;
> + struct folio *folio;
>
> if (sector_nr == 0) {
> /* If restarting in the middle, skip the initial sectors */
> @@ -4741,7 +4729,7 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
> return sectors_done;
> }
>
> - read_bio = bio_alloc_bioset(rdev->bdev, RESYNC_PAGES, REQ_OP_READ,
> + read_bio = bio_alloc_bioset(rdev->bdev, 1, REQ_OP_READ,
> GFP_KERNEL, &mddev->bio_set);
> read_bio->bi_iter.bi_sector = (r10_bio->devs[r10_bio->read_slot].addr
> + rdev->data_offset);
> @@ -4805,32 +4793,23 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
> blist = b;
> }
>
> - /* Now add as many pages as possible to all of these bios. */
> + /* Now add folio to all of these bios. */
>
> - nr_sectors = 0;
> - pages = get_resync_pages(r10_bio->devs[0].bio)->pages;
> - for (s = 0 ; s < max_sectors; s += PAGE_SIZE >> 9) {
> - struct page *page = pages[s / (PAGE_SIZE >> 9)];
> - int len = (max_sectors - s) << 9;
> - if (len > PAGE_SIZE)
> - len = PAGE_SIZE;
> - for (bio = blist; bio ; bio = bio->bi_next) {
> - if (WARN_ON(!bio_add_page(bio, page, len, 0))) {
> - bio->bi_status = BLK_STS_RESOURCE;
> - bio_endio(bio);
> - return sectors_done;
> - }
> + folio = get_resync_folio(r10_bio->devs[0].bio)->folio;
> + for (bio = blist; bio ; bio = bio->bi_next) {
> + if (WARN_ON(!bio_add_folio(bio, folio, max_sectors, 0))) {
> + bio->bi_status = BLK_STS_RESOURCE;
> + bio_endio(bio);
> + return sectors_done;
> }
> - sector_nr += len >> 9;
> - nr_sectors += len >> 9;
> }
> - r10_bio->sectors = nr_sectors;
> + r10_bio->sectors = max_sectors >> 9;
>
> /* Now submit the read */
> atomic_inc(&r10_bio->remaining);
> read_bio->bi_next = NULL;
> submit_bio_noacct(read_bio);
> - sectors_done += nr_sectors;
> + sectors_done += max_sectors;
> if (sector_nr <= last)
> goto read_more;
>
> @@ -4932,8 +4911,8 @@ static int handle_reshape_read_error(struct mddev *mddev,
> struct r10conf *conf = mddev->private;
> struct r10bio *r10b;
> int slot = 0;
> - int idx = 0;
> - struct page **pages;
> + int sect = 0;
> + struct folio *folio;
>
> r10b = kmalloc(struct_size(r10b, devs, conf->copies), GFP_NOIO);
> if (!r10b) {
> @@ -4941,8 +4920,8 @@ static int handle_reshape_read_error(struct mddev *mddev,
> return -ENOMEM;
> }
>
> - /* reshape IOs share pages from .devs[0].bio */
> - pages = get_resync_pages(r10_bio->devs[0].bio)->pages;
> + /* reshape IOs share folio from .devs[0].bio */
> + folio = get_resync_folio(r10_bio->devs[0].bio)->folio;
>
> r10b->sector = r10_bio->sector;
> __raid10_find_phys(&conf->prev, r10b);
> @@ -4958,19 +4937,19 @@ static int handle_reshape_read_error(struct mddev *mddev,
> while (!success) {
> int d = r10b->devs[slot].devnum;
> struct md_rdev *rdev = conf->mirrors[d].rdev;
> - sector_t addr;
> if (rdev == NULL ||
> test_bit(Faulty, &rdev->flags) ||
> !test_bit(In_sync, &rdev->flags))
> goto failed;
>
> - addr = r10b->devs[slot].addr + idx * PAGE_SIZE;
> atomic_inc(&rdev->nr_pending);
> - success = sync_page_io(rdev,
> - addr,
> - s << 9,
> - pages[idx],
> - REQ_OP_READ, false);
> + success = sync_folio_io(rdev,
> + r10b->devs[slot].addr +
> + sect,
> + s << 9,
> + sect << 9,
> + folio,
> + REQ_OP_READ, false);
> rdev_dec_pending(rdev, mddev);
> if (success)
> break;
> @@ -4989,7 +4968,7 @@ static int handle_reshape_read_error(struct mddev *mddev,
> return -EIO;
> }
> sectors -= s;
> - idx++;
> + sect += s;
> }
> kfree(r10b);
> return 0;
> --
> 2.39.2
>
^ permalink raw reply
* Re: [PATCH 04/15] md/raid1: use folio for tmppage
From: Li Nan @ 2026-01-20 11:51 UTC (permalink / raw)
To: Xiao Ni, linan666
Cc: song, yukuai, linux-raid, linux-kernel, yangerkun, yi.zhang
In-Reply-To: <e6f77ede-444b-4d5c-9353-b18f84376ee7@redhat.com>
在 2026/1/20 11:38, Xiao Ni 写道:
>
> 在 2026/1/19 11:20, Xiao Ni 写道:
>> On Wed, Dec 17, 2025 at 8:11 PM <linan666@huaweicloud.com> wrote:
>>> From: Li Nan <linan122@huawei.com>
>>>
>>> Convert tmppage to tmpfolio and use it throughout in raid1.
>>>
>>> Signed-off-by: Li Nan <linan122@huawei.com>
>>> ---
>>> drivers/md/raid1.h | 2 +-
>>> drivers/md/raid1.c | 18 ++++++++++--------
>>> 2 files changed, 11 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/md/raid1.h b/drivers/md/raid1.h
>>> index c98d43a7ae99..d480b3a8c2c4 100644
>>> --- a/drivers/md/raid1.h
>>> +++ b/drivers/md/raid1.h
>>> @@ -101,7 +101,7 @@ struct r1conf {
>>> /* temporary buffer to synchronous IO when attempting to repair
>>> * a read error.
>>> */
>>> - struct page *tmppage;
>>> + struct folio *tmpfolio;
>>>
>>> /* When taking over an array from a different personality, we
>>> store
>>> * the new thread here until we fully activate the array.
>>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>>> index 407925951299..43453f1a04f4 100644
>>> --- a/drivers/md/raid1.c
>>> +++ b/drivers/md/raid1.c
>>> @@ -2417,8 +2417,8 @@ static void fix_read_error(struct r1conf *conf,
>>> struct r1bio *r1_bio)
>>> rdev->recovery_offset >= sect + s)) &&
>>> rdev_has_badblock(rdev, sect, s) == 0) {
>>> atomic_inc(&rdev->nr_pending);
>>> - if (sync_page_io(rdev, sect, s<<9,
>>> - conf->tmppage, REQ_OP_READ,
>>> false))
>>> + if (sync_folio_io(rdev, sect, s<<9, 0,
>>> + conf->tmpfolio, REQ_OP_READ,
>>> false))
>>> success = 1;
>>> rdev_dec_pending(rdev, mddev);
>>> if (success)
>>> @@ -2447,7 +2447,8 @@ static void fix_read_error(struct r1conf *conf,
>>> struct r1bio *r1_bio)
>>> !test_bit(Faulty, &rdev->flags)) {
>>> atomic_inc(&rdev->nr_pending);
>>> r1_sync_page_io(rdev, sect, s,
>>> - conf->tmppage,
>>> REQ_OP_WRITE);
>>> +
>>> folio_page(conf->tmpfolio, 0),
>>> + REQ_OP_WRITE);
>>> rdev_dec_pending(rdev, mddev);
>>> }
>>> }
>>> @@ -2461,7 +2462,8 @@ static void fix_read_error(struct r1conf *conf,
>>> struct r1bio *r1_bio)
>>> !test_bit(Faulty, &rdev->flags)) {
>>> atomic_inc(&rdev->nr_pending);
>>> if (r1_sync_page_io(rdev, sect, s,
>>> - conf->tmppage,
>>> REQ_OP_READ)) {
>>> +
>>> folio_page(conf->tmpfolio, 0),
>>> + REQ_OP_READ)) {
>>> atomic_add(s,
>>> &rdev->corrected_errors);
>>> pr_info("md/raid1:%s: read
>>> error corrected (%d sectors at %llu on %pg)\n",
>>> mdname(mddev), s,
>>> @@ -3120,8 +3122,8 @@ static struct r1conf *setup_conf(struct mddev *mddev)
>>> if (!conf->mirrors)
>>> goto abort;
>>>
>>> - conf->tmppage = alloc_page(GFP_KERNEL);
>>> - if (!conf->tmppage)
>>> + conf->tmpfolio = folio_alloc(GFP_KERNEL, 0);
>>> + if (!conf->tmpfolio)
>>> goto abort;
>>>
>>> r1bio_size = offsetof(struct r1bio, bios[mddev->raid_disks * 2]);
>>> @@ -3196,7 +3198,7 @@ static struct r1conf *setup_conf(struct mddev *mddev)
>>> if (conf) {
>>> mempool_destroy(conf->r1bio_pool);
>>> kfree(conf->mirrors);
>>> - safe_put_page(conf->tmppage);
>>> + folio_put(conf->tmpfolio);
>>> kfree(conf->nr_pending);
>>> kfree(conf->nr_waiting);
>>> kfree(conf->nr_queued);
>>> @@ -3310,7 +3312,7 @@ static void raid1_free(struct mddev *mddev, void
>>> *priv)
>>>
>>> mempool_destroy(conf->r1bio_pool);
>>> kfree(conf->mirrors);
>>> - safe_put_page(conf->tmppage);
>>> + folio_put(conf->tmpfolio);
>>> kfree(conf->nr_pending);
>>> kfree(conf->nr_waiting);
>>> kfree(conf->nr_queued);
>>> --
>>> 2.39.2
>>>
>> Hi Nan
>>
>> Same question for patch04 and patch05, tmpage is used in read io path.
>> From the cover letter, this patch set wants to resolve the multi pages
>> in sync io path. Is it better to keep them for your future patch set?
>>
>> Best Regards
>> Xiao
>>
>> Xiao
>
>
> After reading patch06, I understand here. r1_sync_page_io needs to change
> to r1_sync_folio_io to handle sync read error. Please ignore my above
> comments. patch04 and patch05 look good to me.
>
> Best Regards
>
> Xiao
>
Thanks for your patient review.
--
Thanks,
Nan
^ permalink raw reply
* Re: [PATCH 03/15] md: use folio for bb_folio
From: Li Nan @ 2026-01-20 11:49 UTC (permalink / raw)
To: Xiao Ni, linan666
Cc: song, yukuai, linux-raid, linux-kernel, yangerkun, yi.zhang
In-Reply-To: <CALTww28tZDgBVy=G=doJQ3yfWtuiLk5QoMMyL1cmveacXHLynA@mail.gmail.com>
在 2026/1/19 11:03, Xiao Ni 写道:
> On Wed, Dec 17, 2025 at 8:11 PM <linan666@huaweicloud.com> wrote:
>>
>> From: Li Nan <linan122@huawei.com>
>>
>> Convert bio_page to bio_folio and use it throughout.
>>
>> Signed-off-by: Li Nan <linan122@huawei.com>
>> ---
>> drivers/md/md.h | 3 ++-
>> drivers/md/md.c | 25 +++++++++++++------------
>> 2 files changed, 15 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/md/md.h b/drivers/md/md.h
>> index 410f8a6b75e7..aa6d9df50fd0 100644
>> --- a/drivers/md/md.h
>> +++ b/drivers/md/md.h
>> @@ -144,7 +144,8 @@ struct md_rdev {
>> struct block_device *bdev; /* block device handle */
>> struct file *bdev_file; /* Handle from open for bdev */
>>
>> - struct page *sb_page, *bb_page;
>> + struct page *sb_page;
>> + struct folio *bb_folio;
>> int sb_loaded;
>> __u64 sb_events;
>> sector_t data_offset; /* start of data in array */
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 9dfd6f8da5b8..0732bbcdb95d 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -1073,9 +1073,9 @@ void md_rdev_clear(struct md_rdev *rdev)
>> rdev->sb_start = 0;
>> rdev->sectors = 0;
>> }
>> - if (rdev->bb_page) {
>> - put_page(rdev->bb_page);
>> - rdev->bb_page = NULL;
>> + if (rdev->bb_folio) {
>> + folio_put(rdev->bb_folio);
>> + rdev->bb_folio = NULL;
>> }
>> badblocks_exit(&rdev->badblocks);
>> }
>> @@ -1909,9 +1909,10 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
>>
>> rdev->desc_nr = le32_to_cpu(sb->dev_number);
>>
>> - if (!rdev->bb_page) {
>> - rdev->bb_page = alloc_page(GFP_KERNEL);
>> - if (!rdev->bb_page)
>> + if (!rdev->bb_folio) {
>> + rdev->bb_folio = folio_alloc(GFP_KERNEL, 0);
>> +
>> + if (!rdev->bb_folio)
>> return -ENOMEM;
>> }
>> if ((le32_to_cpu(sb->feature_map) & MD_FEATURE_BAD_BLOCKS) &&
>> @@ -1930,10 +1931,10 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
>> if (offset == 0)
>> return -EINVAL;
>> bb_sector = (long long)offset;
>> - if (!sync_page_io(rdev, bb_sector, sectors << 9,
>> - rdev->bb_page, REQ_OP_READ, true))
>> + if (!sync_folio_io(rdev, bb_sector, sectors << 9, 0,
>> + rdev->bb_folio, REQ_OP_READ, true))
>> return -EIO;
>> - bbp = (__le64 *)page_address(rdev->bb_page);
>> + bbp = (__le64 *)folio_address(rdev->bb_folio);
>> rdev->badblocks.shift = sb->bblog_shift;
>> for (i = 0 ; i < (sectors << (9-3)) ; i++, bbp++) {
>> u64 bb = le64_to_cpu(*bbp);
>> @@ -2300,7 +2301,7 @@ static void super_1_sync(struct mddev *mddev, struct md_rdev *rdev)
>> md_error(mddev, rdev);
>> else {
>> struct badblocks *bb = &rdev->badblocks;
>> - __le64 *bbp = (__le64 *)page_address(rdev->bb_page);
>> + __le64 *bbp = (__le64 *)folio_address(rdev->bb_folio);
>> u64 *p = bb->page;
>> sb->feature_map |= cpu_to_le32(MD_FEATURE_BAD_BLOCKS);
>> if (bb->changed) {
>> @@ -2953,7 +2954,7 @@ void md_update_sb(struct mddev *mddev, int force_change)
>> md_write_metadata(mddev, rdev,
>> rdev->badblocks.sector,
>> rdev->badblocks.size << 9,
>> - rdev->bb_page, 0);
>> + folio_page(rdev->bb_folio, 0), 0);
>> rdev->badblocks.size = 0;
>> }
>>
>> @@ -3809,7 +3810,7 @@ int md_rdev_init(struct md_rdev *rdev)
>> rdev->sb_events = 0;
>> rdev->last_read_error = 0;
>> rdev->sb_loaded = 0;
>> - rdev->bb_page = NULL;
>> + rdev->bb_folio = NULL;
>> atomic_set(&rdev->nr_pending, 0);
>> atomic_set(&rdev->read_errors, 0);
>> atomic_set(&rdev->corrected_errors, 0);
>> --
>> 2.39.2
>>
>
> Hi Nan
>
> Bad block page is only one single page. I don't think it's necessary
> to use folio here. And it uses folio_page to get the page again. Or do
> you plan to replace all page apis to folio apis? Looking through all
> patches, sync_page_io is not removed. In patch02, it says sync_page_io
> will be removed. So maybe it's better to switch bb_page to bb_folio in
> your second patch set? And this patch set only focuses on replacing
> sync pages with folio. It's my 2 cents point. If you think it's better
> to change the bad block page here, I'm still ok.
>
> Best Regards
> Xiao
>
Hi Xiao,
Thanks for your review. Move it to next patch set is fine. I will delete
this patch in v2.
--
Thanks,
Nan
^ permalink raw reply
* [PATCH] md/bitmap: fix GPF in write_page caused by resize race
From: Jack Wang @ 2026-01-20 10:24 UTC (permalink / raw)
To: song, yukuai, linux-raid, linux-kernel; +Cc: stable
A General Protection Fault occurs in write_page() during array resize:
RIP: 0010:write_page+0x22b/0x3c0 [md_mod]
This is a use-after-free race between bitmap_daemon_work() and
__bitmap_resize(). The daemon iterates over `bitmap->storage.filemap`
without locking, while the resize path frees that storage via
md_bitmap_file_unmap(). `quiesce()` does not stop the md thread,
allowing concurrent access to freed pages.
Fix by holding `mddev->bitmap_info.mutex` during the bitmap update.
Closes: https://lore.kernel.org/linux-raid/CAMGffE=Mbfp=7xD_hYxXk1PAaCZNSEAVeQGKGy7YF9f2S4=NEA@mail.gmail.com/T/#u
Cc: stable@vger.kernel.org
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
drivers/md/md-bitmap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 84b7e2af6dba..7bb56d0491a2 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -2453,6 +2453,7 @@ static int __bitmap_resize(struct bitmap *bitmap, sector_t blocks,
memcpy(page_address(store.sb_page),
page_address(bitmap->storage.sb_page),
sizeof(bitmap_super_t));
+ mutex_lock(&bitmap->mddev->bitmap_info.mutex);
spin_lock_irq(&bitmap->counts.lock);
md_bitmap_file_unmap(&bitmap->storage);
bitmap->storage = store;
@@ -2560,7 +2561,7 @@ static int __bitmap_resize(struct bitmap *bitmap, sector_t blocks,
set_page_attr(bitmap, i, BITMAP_PAGE_DIRTY);
}
spin_unlock_irq(&bitmap->counts.lock);
-
+ mutex_unlock(&bitmap->mddev->bitmap_info.mutex);
if (!init) {
__bitmap_unplug(bitmap);
bitmap->mddev->pers->quiesce(bitmap->mddev, 0);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 04/15] md/raid1: use folio for tmppage
From: Xiao Ni @ 2026-01-20 3:38 UTC (permalink / raw)
To: linan666; +Cc: song, yukuai, linux-raid, linux-kernel, yangerkun, yi.zhang
In-Reply-To: <CALTww2-bCOHsK=iXqkTokFBdG=kxc6NsdgtyfWPXBaSX6pmcAA@mail.gmail.com>
在 2026/1/19 11:20, Xiao Ni 写道:
> On Wed, Dec 17, 2025 at 8:11 PM <linan666@huaweicloud.com> wrote:
>> From: Li Nan <linan122@huawei.com>
>>
>> Convert tmppage to tmpfolio and use it throughout in raid1.
>>
>> Signed-off-by: Li Nan <linan122@huawei.com>
>> ---
>> drivers/md/raid1.h | 2 +-
>> drivers/md/raid1.c | 18 ++++++++++--------
>> 2 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/md/raid1.h b/drivers/md/raid1.h
>> index c98d43a7ae99..d480b3a8c2c4 100644
>> --- a/drivers/md/raid1.h
>> +++ b/drivers/md/raid1.h
>> @@ -101,7 +101,7 @@ struct r1conf {
>> /* temporary buffer to synchronous IO when attempting to repair
>> * a read error.
>> */
>> - struct page *tmppage;
>> + struct folio *tmpfolio;
>>
>> /* When taking over an array from a different personality, we store
>> * the new thread here until we fully activate the array.
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index 407925951299..43453f1a04f4 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -2417,8 +2417,8 @@ static void fix_read_error(struct r1conf *conf, struct r1bio *r1_bio)
>> rdev->recovery_offset >= sect + s)) &&
>> rdev_has_badblock(rdev, sect, s) == 0) {
>> atomic_inc(&rdev->nr_pending);
>> - if (sync_page_io(rdev, sect, s<<9,
>> - conf->tmppage, REQ_OP_READ, false))
>> + if (sync_folio_io(rdev, sect, s<<9, 0,
>> + conf->tmpfolio, REQ_OP_READ, false))
>> success = 1;
>> rdev_dec_pending(rdev, mddev);
>> if (success)
>> @@ -2447,7 +2447,8 @@ static void fix_read_error(struct r1conf *conf, struct r1bio *r1_bio)
>> !test_bit(Faulty, &rdev->flags)) {
>> atomic_inc(&rdev->nr_pending);
>> r1_sync_page_io(rdev, sect, s,
>> - conf->tmppage, REQ_OP_WRITE);
>> + folio_page(conf->tmpfolio, 0),
>> + REQ_OP_WRITE);
>> rdev_dec_pending(rdev, mddev);
>> }
>> }
>> @@ -2461,7 +2462,8 @@ static void fix_read_error(struct r1conf *conf, struct r1bio *r1_bio)
>> !test_bit(Faulty, &rdev->flags)) {
>> atomic_inc(&rdev->nr_pending);
>> if (r1_sync_page_io(rdev, sect, s,
>> - conf->tmppage, REQ_OP_READ)) {
>> + folio_page(conf->tmpfolio, 0),
>> + REQ_OP_READ)) {
>> atomic_add(s, &rdev->corrected_errors);
>> pr_info("md/raid1:%s: read error corrected (%d sectors at %llu on %pg)\n",
>> mdname(mddev), s,
>> @@ -3120,8 +3122,8 @@ static struct r1conf *setup_conf(struct mddev *mddev)
>> if (!conf->mirrors)
>> goto abort;
>>
>> - conf->tmppage = alloc_page(GFP_KERNEL);
>> - if (!conf->tmppage)
>> + conf->tmpfolio = folio_alloc(GFP_KERNEL, 0);
>> + if (!conf->tmpfolio)
>> goto abort;
>>
>> r1bio_size = offsetof(struct r1bio, bios[mddev->raid_disks * 2]);
>> @@ -3196,7 +3198,7 @@ static struct r1conf *setup_conf(struct mddev *mddev)
>> if (conf) {
>> mempool_destroy(conf->r1bio_pool);
>> kfree(conf->mirrors);
>> - safe_put_page(conf->tmppage);
>> + folio_put(conf->tmpfolio);
>> kfree(conf->nr_pending);
>> kfree(conf->nr_waiting);
>> kfree(conf->nr_queued);
>> @@ -3310,7 +3312,7 @@ static void raid1_free(struct mddev *mddev, void *priv)
>>
>> mempool_destroy(conf->r1bio_pool);
>> kfree(conf->mirrors);
>> - safe_put_page(conf->tmppage);
>> + folio_put(conf->tmpfolio);
>> kfree(conf->nr_pending);
>> kfree(conf->nr_waiting);
>> kfree(conf->nr_queued);
>> --
>> 2.39.2
>>
> Hi Nan
>
> Same question for patch04 and patch05, tmpage is used in read io path.
> From the cover letter, this patch set wants to resolve the multi pages
> in sync io path. Is it better to keep them for your future patch set?
>
> Best Regards
> Xiao
>
> Xiao
After reading patch06, I understand here. r1_sync_page_io needs to
change to r1_sync_folio_io to handle sync read error. Please ignore my
above comments. patch04 and patch05 look good to me.
Best Regards
Xiao
^ permalink raw reply
* Re: [BUG] md: race between bitmap_daemon_work and __bitmap_resize leading to use-after-free
From: Jinpu Wang @ 2026-01-19 19:05 UTC (permalink / raw)
To: yukuai; +Cc: linux-raid, Song Liu, open list
In-Reply-To: <b1824ba1-a051-4c4f-bbf9-c28fb225edfc@fnnas.com>
Hi Kuai,
On Mon, Jan 19, 2026 at 5:44 PM Yu Kuai <yukuai@fnnas.com> wrote:
>
> Hi,
>
> 在 2026/1/19 23:14, Jinpu Wang 写道:
> > We are looking for suggestions on the best way to synchronize this. It
> > seems we need to either: a) Ensure the md thread's daemon work is
> > stopped/flushed before
> >
> > __bitmap_resize proceeds with unmapping. b) Protect bitmap->storage
> > replacement with a lock that
> > bitmap_daemon_work also respects.
> >
> > Any thoughts on the preferred approach?
>
> create/free/resize and access bitmap other than IO path should all be
> protected with mddev->bitmap_info.mutex.
Thx for the suggestion, I will work on a fix.
>
> --
> Thansk,
> Kuai
Jinpu
^ permalink raw reply
* Re: [BUG] md: race between bitmap_daemon_work and __bitmap_resize leading to use-after-free
From: Yu Kuai @ 2026-01-19 16:44 UTC (permalink / raw)
To: Jinpu Wang, linux-raid, Song Liu, open list, yukuai
In-Reply-To: <CAMGffE=Mbfp=7xD_hYxXk1PAaCZNSEAVeQGKGy7YF9f2S4=NEA@mail.gmail.com>
Hi,
在 2026/1/19 23:14, Jinpu Wang 写道:
> We are looking for suggestions on the best way to synchronize this. It
> seems we need to either: a) Ensure the md thread's daemon work is
> stopped/flushed before
>
> __bitmap_resize proceeds with unmapping. b) Protect bitmap->storage
> replacement with a lock that
> bitmap_daemon_work also respects.
>
> Any thoughts on the preferred approach?
create/free/resize and access bitmap other than IO path should all be
protected with mddev->bitmap_info.mutex.
--
Thansk,
Kuai
^ permalink raw reply
* [BUG] md: race between bitmap_daemon_work and __bitmap_resize leading to use-after-free
From: Jinpu Wang @ 2026-01-19 15:14 UTC (permalink / raw)
To: linux-raid, yukuai, Song Liu, open list
Hello folks,
We are seeing a general protection fault in the md bitmap code during
array resize operations. This appears to be a race condition between
the bitmap daemon work and the bitmap resize code path in kernel 6.1
(and likely later versions).
[Crash Details]
The crash occurs at write_page+0x22b when dereferencing page_buffers(page).
general protection fault, probably for non-canonical address
0xc2f57c2ef374586f: 0000 [#1] PREEMPT SMP
CPU: 18 PID: 1598035 Comm: md53_raid1 Kdump: loaded Tainted: G
O 6.1.118-pserver
RIP: 0010:write_page+0x22b/0x3c0 [md_mod]
Code: f0 ff 83 f0 00 00 00 e8 13 6d a3 cb 48 85 db 74 cb 48 8b 53 28
49 8b b6 40 03 00 00 48 85 d2 0f 84 41 01 00 00 49 8b 44 24 70 <49> 8b
7d 20 b9 00 10 00 00 48 83 e8 01 48 39 c7 0f 84 da 00 00 00
RSP: 0018:ffffa82f3b857c40 EFLAGS: 00010246
RAX: 0000000000000001 RBX: ffff99abc0e39400 RCX: 0000000000000000
RDX: ffff99bfc21a3c00 RSI: 0000000000000008 RDI: ffff9a2c9ce358c0
RBP: ffff99ac72048018 R08: 0000000000000000 R09: ffff99ac720482c0
R10: 0000000000000000 R11: 0000000000000000 R12: ffff99b151373e00
R13: c2f57c2ef374586f R14: ffff99ac72048000 R15: ffff99abc0e394f0
Call Trace:
<TASK>
? exc_general_protection+0x222/0x4b0
? asm_exc_general_protection+0x22/0x30
? write_page+0x22b/0x3c0 [md_mod]
bitmap_daemon_work+0x26b/0x3a0 [md_mod]
md_check_recovery+0x58/0x5d0 [md_mod]
raid1d+0x8e/0x1940 [raid1]
[Analysis]
The root cause is a use-after-free race between
__bitmap_resize() and
bitmap_daemon_work().
bitmap_daemon_work() (running in the md thread) iterates over
bitmap->storage.filemap[] and calls
write_page():
for (j = 0; j < bitmap->storage.file_pages; j++) {
if (bitmap->storage.filemap && ...) {
write_page(bitmap, bitmap->storage.filemap[j], 0);
}
}
Crucially, this access to filemap[j] is done without holding any lock
that would prevent the storage from being replaced and freed.
__bitmap_resize() (triggered by resize ioctl) replaces the bitmap storage:
spin_lock_irq(&bitmap->counts.lock);
md_bitmap_file_unmap(&bitmap->storage); // Frees old pages and kfrees filemap
bitmap->storage = store;
spin_unlock_irq(&bitmap->counts.lock);
Even though
__bitmap_resize() calls quiesce(), this only suspends normal I/O. It
does NOT stop the md thread itself, which continues to run and can
enter md_check_recovery() -> bitmap_daemon_work().
[Race Window]
Thread 1 (md thread) reads a page pointer from
bitmap->storage.filemap[j]. Simultaneously, Thread 2 (resize) calls
md_bitmap_file_unmap(), which calls
free_buffers(page) and
kfree(filemap). When Thread 1 enters
write_page(), it dereferences the now-freed page/buffer_head,
resulting in the GPF.
The current locking (counts.lock) protects the bitmap counters but not
the bitmap->storage structure itself during the transition to
write_page.
We are looking for suggestions on the best way to synchronize this. It
seems we need to either: a) Ensure the md thread's daemon work is
stopped/flushed before
__bitmap_resize proceeds with unmapping. b) Protect bitmap->storage
replacement with a lock that
bitmap_daemon_work also respects.
Any thoughts on the preferred approach?
Best regards,
Jinpu Wang @ IONOS Cloud
^ permalink raw reply
* Re: [PATCH v5 07/12] md: support to align bio to limits
From: Yu Kuai @ 2026-01-19 9:15 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-raid, linan122, xni, dan.carpenter, Martin K. Petersen,
yukuai
In-Reply-To: <aW3q1SnU4gN9Ahba@infradead.org>
Hi,
在 2026/1/19 16:27, Christoph Hellwig 写道:
> On Mon, Jan 19, 2026 at 03:43:34PM +0800, Yu Kuai wrote:
>>>> 64k boundary is still necessary for small IO.
>>> What do you mean with "necessary for small IO"?
>> io_min and io_opt is quite similar in mdraid, IO aligned with io_opt
>> can get the best bandwidth, and IO aligned with io_min can get the
>> best iops. Currently chunk_sectors is the same as io_min, and
>> bio_split_rw() will try to align IO to io_min. I don't think we want
>> to remove this behavior.
> I'm still confused. Let's go back to your example:
>
> 32 disks raid5 array with chunksize=64k.
>
> Let's look at writes first:
>
> Each I/O that is full aligned to 31 * 64k can be handled without a
> read-modify-write cycle, so splitting I/O at that boundary makes perfect
> sense. Below that there really should not me much difference, i.e.
> splitting anything at the 64k boundary is not useful. So you want the
> chunk_sectors to apply at the 31 * 64k boundary, and the io_opt as well.
> And probably io_min too. (all just looking at writes).
This sounds reasonable, however, I'm not 100% sure split at 64k boundary
is not useful, I must run some tests to confirm. This behavior exist for
quite a long time.
>
> For non-degradead reads, not much should matter. All reads should be
> reasonably efficient, splitting 64k boundaries is going to make the
> implementation trivial, but will make your rely heavily on plugging
> below, and also means you use quite a lot of lower bios.
Correct, BTW, even if we don't split at 64k boundary in bio_split_rw(), raid5
will stil try to split at 64k boundary in chunk_aligned_read(), and this do
rely hevily on plugging below.
BTW, current plug limit really is too low for huge array, like 32+ member disks,
only 32 requests and at most 128k per request. However, I still can't find better
solution other than simply increase the limits.
>
> For degraded reads, each I/O will always read 31 * 64k. Splitting at
> 31 * 64k makes the implementation much easier.
I don't feel this is correct, each I/O will be handled by stripes, so a
4k read to the removed disk will only need to read 4k from other disks.
Anyway, this does not matter.
>
> I guess you want different boundaries for reads and writes?
Yes, this is still a potential demand, I'll test and take a detailed look
at other personalities.
>
> Note that io_opt and io_min really just are values for the caller and
> not affect splitting decisions themselves. Of course the underlying
> factors should be related.
Thanks for the explanation, I'll feedback soon after testing.
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH v5 12/12] md: fix abnormal io_opt from member disks
From: Christoph Hellwig @ 2026-01-19 8:36 UTC (permalink / raw)
To: Yu Kuai
Cc: Christoph Hellwig, Coly Li, linux-raid, linan122, xni,
dan.carpenter
In-Reply-To: <9642ed5f-1e60-47dd-a333-abc5cb26ebe1@fnnas.com>
On Mon, Jan 19, 2026 at 03:24:19PM +0800, Yu Kuai wrote:
> >> E.g. a spinning hard drive connect to this HBA card reports its max_sectors as 32767 sectors.
> >> This is around 16MB and too large for normal hard drive.
> > Which is larger than what we'd expect for the HDD itself, where it
> > should be around 1MB. But HBAs do weird stuff, so it might actually
> > be correct here. Have you talked to the mpt3sas maintainers?
>
> We CC them in several previous threads, however, they're not responding. :(
In that case we'll have to assume the value is intended. Especially
if the HBA is running in RAID mode, which would be only sensible path
to modify this value anyway.
^ permalink raw reply
* Re: [PATCH v5 07/12] md: support to align bio to limits
From: Christoph Hellwig @ 2026-01-19 8:27 UTC (permalink / raw)
To: Yu Kuai
Cc: Christoph Hellwig, linux-raid, linan122, xni, dan.carpenter,
Martin K. Petersen
In-Reply-To: <8b44bd64-efc8-428a-ad2f-9a9fcda786a1@fnnas.com>
On Mon, Jan 19, 2026 at 03:43:34PM +0800, Yu Kuai wrote:
> >> 64k boundary is still necessary for small IO.
> > What do you mean with "necessary for small IO"?
>
> io_min and io_opt is quite similar in mdraid, IO aligned with io_opt
> can get the best bandwidth, and IO aligned with io_min can get the
> best iops. Currently chunk_sectors is the same as io_min, and
> bio_split_rw() will try to align IO to io_min. I don't think we want
> to remove this behavior.
I'm still confused. Let's go back to your example:
32 disks raid5 array with chunksize=64k.
Let's look at writes first:
Each I/O that is full aligned to 31 * 64k can be handled without a
read-modify-write cycle, so splitting I/O at that boundary makes perfect
sense. Below that there really should not me much difference, i.e.
splitting anything at the 64k boundary is not useful. So you want the
chunk_sectors to apply at the 31 * 64k boundary, and the io_opt as well.
And probably io_min too. (all just looking at writes).
For non-degradead reads, not much should matter. All reads should be
reasonably efficient, splitting 64k boundaries is going to make the
implementation trivial, but will make your rely heavily on plugging
below, and also means you use quite a lot of lower bios.
For degraded reads, each I/O will always read 31 * 64k. Splitting at
31 * 64k makes the implementation much easier.
I guess you want different boundaries for reads and writes?
Note that io_opt and io_min really just are values for the caller and
not affect splitting decisions themselves. Of course the underlying
factors should be related.
^ permalink raw reply
* Re: [PATCH v5 07/12] md: support to align bio to limits
From: Yu Kuai @ 2026-01-19 7:43 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-raid, linan122, xni, dan.carpenter, yukuai
In-Reply-To: <aW3cxxnbRmon9aYJ@infradead.org>
在 2026/1/19 15:27, Christoph Hellwig 写道:
> On Mon, Jan 19, 2026 at 03:21:14PM +0800, Yu Kuai wrote:
>> Hi，
>>
>> 在 2026/1/19 14:47, Christoph Hellwig 写道:
>>> On Sun, Jan 18, 2026 at 07:40:23PM +0800, Yu Kuai wrote:
>>>> No, the chunk_sectors and io_opt are different, and align io to io_opt
>>>> is not a general idea, for now this is the only requirement in mdraid.
>>> The chunk size was added for (hardware) devices that require I/O split at
>>> a fixed granularity for performance reasons. Which seems to e exactly
>>> what you want here.
>>>
>>> This has nothing to do with max_sectors.
>> For example, 32 disks raid5 array with chunksize=64k, currently the queue
>> limits are:
>>
>> chunk_sectors = 64k
>> io_min = 64k
>> io_opt = 64 * 31k
>> max_sectors = 1M
>>
>> It's correct to split I/O at 64k boundary to avoid performance issues, however
>> split at 64 *31k boundary is what we want to get best bandwidth.
>>
>> So, if we simply changes chunk_sectors to 64 * 31k, it will be incorrect, because
>> 64k boundary is still necessary for small IO.
> What do you mean with "necessary for small IO"?
io_min and io_opt is quite similar in mdraid, IO aligned with io_opt can get the best
bandwidth, and IO aligned with io_min can get the best iops. Currently chunk_sectors
is the same as io_min, and bio_split_rw() will try to align IO to io_min. I don't think
we want to remove this behavior.
>
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH v5 07/12] md: support to align bio to limits
From: Christoph Hellwig @ 2026-01-19 7:27 UTC (permalink / raw)
To: Yu Kuai; +Cc: Christoph Hellwig, linux-raid, linan122, xni, dan.carpenter
In-Reply-To: <f7ba27c9-fc7b-44d5-9ddd-2bfb370e29d3@fnnas.com>
On Mon, Jan 19, 2026 at 03:21:14PM +0800, Yu Kuai wrote:
> Hi，
>
> 在 2026/1/19 14:47, Christoph Hellwig 写道:
> > On Sun, Jan 18, 2026 at 07:40:23PM +0800, Yu Kuai wrote:
> >> No, the chunk_sectors and io_opt are different, and align io to io_opt
> >> is not a general idea, for now this is the only requirement in mdraid.
> > The chunk size was added for (hardware) devices that require I/O split at
> > a fixed granularity for performance reasons. Which seems to e exactly
> > what you want here.
> >
> > This has nothing to do with max_sectors.
>
> For example, 32 disks raid5 array with chunksize=64k, currently the queue
> limits are:
>
> chunk_sectors = 64k
> io_min = 64k
> io_opt = 64 * 31k
> max_sectors = 1M
>
> It's correct to split I/O at 64k boundary to avoid performance issues, however
> split at 64 *31k boundary is what we want to get best bandwidth.
>
> So, if we simply changes chunk_sectors to 64 * 31k, it will be incorrect, because
> 64k boundary is still necessary for small IO.
What do you mean with "necessary for small IO"?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox