* Re: [PATCH v2] raid6: arm64: add SVE optimized implementation for syndrome generation
From: Demian Shulhan @ 2026-03-31 13:18 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Ard Biesheuvel, Mark Rutland, Song Liu, Yu Kuai, Will Deacon,
Catalin Marinas, Mark Brown, linux-arm-kernel, robin.murphy,
Li Nan, linux-raid, linux-kernel
In-Reply-To: <20260331063659.GA2061@lst.de>
Hi all,
Ard, your questions regarding real-world I/O bottlenecks and SVE power
efficiency versus raw throughput are entirely valid. I agree that
introducing SVE support requires solid real-world data to justify the
added complexity.
Due to my current workload, I won't be able to run the necessary
hardware tests and prepare the benchmark code immediately. I will get
back to the list in about 1 week with the requested source code,
unmangled test results, and further analysis.
Thanks!
вт, 31 бер. 2026 р. о 09:37 Christoph Hellwig <hch@lst.de> пише:
>
> On Mon, Mar 30, 2026 at 06:39:49PM +0200, Ard Biesheuvel wrote:
> > I think the results are impressive, but I'd like to better understand
> > its implications on a real-world scenario. Is this code only a
> > bottleneck when rebuilding an array?
>
> The syndrome generation is run every time you write data to a RAID6
> array, and if you do partial stripe writes it (or rather the XOR
> variant) is run twice. So this is the most performance critical
> path for writing to RAID6.
>
> Rebuild usually runs totally different code, but can end up here as well
> when both parity disks are lost.
>
> > > Furthermore, as Christoph suggested, I tested scalability on wider
> > > arrays since the default kernel benchmark is hardcoded to 8 disks,
> > > which doesn't give the unrolled SVE loop enough data to shine. On a
> > > 16-disk array, svex4 hits 15.1 GB/s compared to 8.0 GB/s for neonx4.
> > > On a 24-disk array, while neonx4 chokes and drops to 7.8 GB/s, svex4
> > > maintains a stable 15.0 GB/s — effectively doubling the throughput.
> >
> > Does this mean the kernel benchmark is no longer fit for purpose? If
> > it cannot distinguish between implementations that differ in performance
> > by a factor of 2, I don't think we can rely on it to pick the optimal one.
>
> It is not good, and we should either fix it or run more than one.
> The current setup is not really representative of real-life array.
> It also leads to wrong selections on x86, but only at the which unroll
> level to pick level, and only for minor differences so far. I plan
> to add this to the next version of the raid6 lib patches.
>
^ permalink raw reply
* Re: [PATCH v2 0/5] xor/arm: Replace vectorized version with intrinsics
From: Christoph Hellwig @ 2026-03-31 15:16 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-raid, linux-arm-kernel, linux-crypto, Ard Biesheuvel,
Christoph Hellwig, Russell King, Arnd Bergmann, Eric Biggers
In-Reply-To: <20260331074940.55502-7-ardb+git@google.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
I think some of the intrinsics patches were also in your crc64 series,
so I'm not sure how to be merge this.
^ permalink raw reply
* Re: [PATCH v2 0/5] xor/arm: Replace vectorized version with intrinsics
From: Ard Biesheuvel @ 2026-03-31 15:26 UTC (permalink / raw)
To: Christoph Hellwig, Ard Biesheuvel
Cc: linux-raid, linux-arm-kernel, linux-crypto, Russell King,
Arnd Bergmann, Eric Biggers
In-Reply-To: <20260331151653.GA8011@lst.de>
On Tue, 31 Mar 2026, at 17:16, Christoph Hellwig wrote:
> Looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
Thanks.
> I think some of the intrinsics patches were also in your crc64 series,
> so I'm not sure how to be merge this.
The first patch is used by both series, yes. If this is good to go, we might as well just merge it, and defer the crc work (or at least the 32-bit ARM specific changes) to the next cycle. I am in no particular hurry with any of this, so whatever works for other people is fine with me.
The RAID pieces are going through akpm's tree, right?
Eric, any preferences? (assuming you are on board with the CRC64 changes in the first place)
^ permalink raw reply
* Re: [PATCH v2 0/5] xor/arm: Replace vectorized version with intrinsics
From: Christoph Hellwig @ 2026-03-31 15:28 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Christoph Hellwig, Ard Biesheuvel, linux-raid, linux-arm-kernel,
linux-crypto, Russell King, Arnd Bergmann, Eric Biggers
In-Reply-To: <2176aab5-2167-4cdf-9090-0f8e0a6fb5a0@app.fastmail.com>
On Tue, Mar 31, 2026 at 05:26:33PM +0200, Ard Biesheuvel wrote:
> The RAID pieces are going through akpm's tree, right?
Yes.
^ permalink raw reply
* Re: [RFC PATCH] md/raid5: Fix UAF on IO across the reshape position
From: Benjamin Marzinski @ 2026-03-31 16:36 UTC (permalink / raw)
To: Xiao Ni; +Cc: Yu Kuai, Song Liu, Li Nan, linux-raid, dm-devel, Nigel Croxon
In-Reply-To: <CALTww2-SE6b7bQ93ZbJ3N_Xxgrm4b8rv6Sukjt5cUVNHScPFRg@mail.gmail.com>
On Tue, Mar 31, 2026 at 10:24:48AM +0800, Xiao Ni wrote:
> On Tue, Mar 31, 2026 at 3:16 AM Benjamin Marzinski <bmarzins@redhat.com> wrote:
> >
> > On Mon, Mar 30, 2026 at 11:44:54PM +0800, Xiao Ni wrote:
> > > On Tue, Mar 24, 2026 at 6:58 AM Benjamin Marzinski <bmarzins@redhat.com> wrote:
> > > >
> > > > If make_stripe_request() returns STRIPE_WAIT_RESHAPE,
> > > > raid5_make_request() will free the cloned bio. But raid5_make_request()
> > > > can call make_stripe_request() multiple times, writing to the various
> > > > stripes. If that bio got added to the toread or towrite lists of a
> > > > stripe disk in an earlier call to make_stripe_request(), then it's not
> > > > safe to just free the bio if a later part of it is found to cross the
> > > > reshape position. Doing so can lead to a UAF error, when bio_endio()
> > > > is called on the bio for the earlier stripes.
> > > >
> > > > Instead, raid5_make_request() needs to wait until all parts of the bio
> > > > have called bio_endio(). To do this, bios that cross the reshape
> > > > position while the reshape can't make progress are flagged as needing a
> > > > retry, and mddev tracks the number of bios needing a retry which have
> > > > not yet completed. When raid5_make_request() has a bio that failed
> > > > make_stripe_request() with STRIPE_WAIT_RESHAPE, it waits for this
> > > > counter to reach zero. When the bio_endio() is called for the last time
> > > > on a bio needing a retry, it decrements mddev's count of outstanding
> > > > bios needing a retry. This guarantees that raid5_make_request() doesn't
> > > > return until the cloned bio needing a retry for io across the reshape
> > > > boundary is safely cleaned up.
> > > >
> > > > There is a simple reproducer available at [1]. Compile the kernel with
> > > > KASAN for more useful reporting when the error is triggered (this is not
> > > > necessary to see the bug).
> > > >
> > > > [1] https://gist.github.com/bmarzins/e48598824305cf2171289e47d7241fa5
> > > >
> > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > > > ---
> > > >
> > > > I've tested this for regressions with the lvm2-testsuite raid tests. I
> > > > have not run any md-specific tests on it.
> > > >
> > > >
> > > > drivers/md/md.c | 30 +++++++++---------------------
> > > > drivers/md/md.h | 5 ++++-
> > > > drivers/md/raid5.c | 8 +++++++-
> > > > 3 files changed, 20 insertions(+), 23 deletions(-)
> > > >
> > > > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > > > index 3ce6f9e9d38e..5ec116b9da32 100644
> > > > --- a/drivers/md/md.c
> > > > +++ b/drivers/md/md.c
> > > > @@ -776,9 +776,11 @@ int mddev_init(struct mddev *mddev)
> > > > atomic_set(&mddev->active, 1);
> > > > atomic_set(&mddev->openers, 0);
> > > > atomic_set(&mddev->sync_seq, 0);
> > > > + atomic_set(&mddev->pending_retry_bios, 0);
> > > > spin_lock_init(&mddev->lock);
> > > > init_waitqueue_head(&mddev->sb_wait);
> > > > init_waitqueue_head(&mddev->recovery_wait);
> > > > + init_waitqueue_head(&mddev->retry_bios_wait);
> > > > mddev->reshape_position = MaxSector;
> > > > mddev->reshape_backwards = 0;
> > > > mddev->last_sync_action = ACTION_IDLE;
> > > > @@ -9218,6 +9220,7 @@ static void md_end_clone_io(struct bio *bio)
> > > > struct md_io_clone *md_io_clone = bio->bi_private;
> > > > struct bio *orig_bio = md_io_clone->orig_bio;
> > > > struct mddev *mddev = md_io_clone->mddev;
> > > > + unsigned int must_retry = md_io_clone->must_retry;
> > > >
> > > > if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
> > > > md_bitmap_end(mddev, md_io_clone);
> > > > @@ -9229,7 +9232,11 @@ static void md_end_clone_io(struct bio *bio)
> > > > bio_end_io_acct(orig_bio, md_io_clone->start_time);
> > > >
> > > > bio_put(bio);
> > > > - bio_endio(orig_bio);
> > > > + if (unlikely(must_retry)) {
> > > > + if (atomic_dec_and_test(&mddev->pending_retry_bios))
> > > > + wake_up(&mddev->retry_bios_wait);
> > > > + } else
> > > > + bio_endio(orig_bio);
> > > > percpu_ref_put(&mddev->active_io);
> > > > }
> > > >
> > > > @@ -9243,6 +9250,7 @@ static void md_clone_bio(struct mddev *mddev, struct bio **bio)
> > > > md_io_clone = container_of(clone, struct md_io_clone, bio_clone);
> > > > md_io_clone->orig_bio = *bio;
> > > > md_io_clone->mddev = mddev;
> > > > + md_io_clone->must_retry = 0;
> > > > if (blk_queue_io_stat(bdev->bd_disk->queue))
> > > > md_io_clone->start_time = bio_start_io_acct(*bio);
> > > >
> > > > @@ -9265,26 +9273,6 @@ void md_account_bio(struct mddev *mddev, struct bio **bio)
> > > > }
> > > > EXPORT_SYMBOL_GPL(md_account_bio);
> > > >
> > > > -void md_free_cloned_bio(struct bio *bio)
> > > > -{
> > > > - struct md_io_clone *md_io_clone = bio->bi_private;
> > > > - struct bio *orig_bio = md_io_clone->orig_bio;
> > > > - struct mddev *mddev = md_io_clone->mddev;
> > > > -
> > > > - if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
> > > > - md_bitmap_end(mddev, md_io_clone);
> > > > -
> > > > - if (bio->bi_status && !orig_bio->bi_status)
> > > > - orig_bio->bi_status = bio->bi_status;
> > > > -
> > > > - if (md_io_clone->start_time)
> > > > - bio_end_io_acct(orig_bio, md_io_clone->start_time);
> > > > -
> > > > - bio_put(bio);
> > > > - percpu_ref_put(&mddev->active_io);
> > > > -}
> > > > -EXPORT_SYMBOL_GPL(md_free_cloned_bio);
> > > > -
> > > > /* md_allow_write(mddev)
> > > > * Calling this ensures that the array is marked 'active' so that writes
> > > > * may proceed without blocking. It is important to call this before
> > > > diff --git a/drivers/md/md.h b/drivers/md/md.h
> > > > index ac84289664cd..49a231f11676 100644
> > > > --- a/drivers/md/md.h
> > > > +++ b/drivers/md/md.h
> > > > @@ -626,6 +626,9 @@ struct mddev {
> > > >
> > > > /* The sequence number for sync thread */
> > > > atomic_t sync_seq;
> > > > +
> > > > + wait_queue_head_t retry_bios_wait;
> > > > + atomic_t pending_retry_bios;
> > > > };
> > > >
> > > > enum recovery_flags {
> > > > @@ -877,6 +880,7 @@ struct md_io_clone {
> > > > sector_t offset;
> > > > unsigned long sectors;
> > > > enum stat_group rw;
> > > > + unsigned int must_retry;
> > > > struct bio bio_clone;
> > > > };
> > > >
> > > > @@ -917,7 +921,6 @@ extern void md_finish_reshape(struct mddev *mddev);
> > > > void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
> > > > struct bio *bio, sector_t start, sector_t size);
> > > > void md_account_bio(struct mddev *mddev, struct bio **bio);
> > > > -void md_free_cloned_bio(struct bio *bio);
> > > >
> > > > extern bool __must_check md_flush_request(struct mddev *mddev, struct bio *bio);
> > > > void md_write_metadata(struct mddev *mddev, struct md_rdev *rdev,
> > > > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > > > index a8e8d431071b..fb78a757f2fd 100644
> > > > --- a/drivers/md/raid5.c
> > > > +++ b/drivers/md/raid5.c
> > > > @@ -6217,7 +6217,13 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
> > > >
> > > > mempool_free(ctx, conf->ctx_pool);
> > > > if (res == STRIPE_WAIT_RESHAPE) {
> > > > - md_free_cloned_bio(bi);
> > > > + struct md_io_clone *md_io_clone = bi->bi_private;
> > > > +
> > > > + md_io_clone->must_retry = 1;
> > > > + atomic_inc(&mddev->pending_retry_bios);
> > > > + bio_endio(bi);
> > > > + wait_event(mddev->retry_bios_wait,
> > > > + atomic_read(&mddev->pending_retry_bios)==0);
> > >
> > > Hi Ben
> > >
> > > There is a problem here. The new counter pending_retry_bios above
> > > doesn't represent the bios which have been added to stripes. So uaf
> > > still can happen.
> >
> > I don't think it can. That counter won't be zero until there are no bios
> > that need to get retried. So we way end up waiting too long, but we
> > shouldn't ever end up not waiting long enough.
>
> Ah, yes, you're right. I thought wrongly. The counter tracks multiple
> threads that encounter BLK_STS_RESOURCE. For each thread,
> md_end_clone_bio is called only when all bios return. But why do
> different threads need to sync at raid5_make_request?
>
>
> >
> > > Do we need to add a new counter?
> >
> > I did it that way because I wanted to avoid growing md_io_clone
> > ("must_retry" fit nicely in a hole in the structure, so it didn't take
> > up any extra space). But if you are fine with adding the extra
>
> Nice design.
>
> > reshape_completion pointer to md_io_clone, then your way avoids the
> > chance that we might wait when we don't need to.
>
> If you mention the threads sync here, yes.
I can certainly add comments explaining why I left off the READ_ONCE(),
WRITE_ONCE() (actually, there's no real reason not to keep the
WRITE_ONCE, since that's not in the fast path) if you think it makes
sense to leave them off.
I'm fine with either my or your code for to deal with the waiting,
whichever you think makes more sense. Since this is a very hard bug to
hit, and the fix involves adding extra data to every bio and extra code
in the end_io function that all the raid bios call, I wanted to make the
impact as small as possible. But md_io_clone is still 8 bytes away from
filling up 3 cachelines (at 64 bytes a cacheline), so there's space to
add the reshape_completion pointer without increaseing the number of
cachelines, which means that your solution for waiting is probably the
way to go, unless you think something else might end up needing that
space.
-Ben
> >
> > > Because
> > > md_end_clone_io is only called when all bios return. How about
> > > something like:
> > >
> > > md_end_clone_io
> > > + if (unlikely(READ_ONCE(md_io_clone->waiting_reshape))) {
> > > + complete(md_io_clone->reshape_completion);
> > > + } else {
> > > + bio_endio(orig_bio);
> > > + }
> > >
> > > raid5_make_request:
> > >
> > > if (res == STRIPE_WAIT_RESHAPE) {
> > > - md_free_cloned_bio(bi);
> > > + struct md_io_clone *md_io_clone = bi->bi_private;
> > > + struct completion done;
> > > +
> > > + init_completion(&done);
> > > + md_io_clone->reshape_completion = &done;
> > > + WRITE_ONCE(md_io_clone->waiting_reshape, 1);
> > > +
> > > + bio_endio(bi);
> > > +
> > > + wait_for_completion(&done);
> >
> > This looks fine.
> >
> > I'm pretty sure that the READ_ONCE() and WRITE_ONCE() are unnecessary.
> > First, theres no chance that these operations will ever happen at the
> > same time and we're just writing a 1, so there's no chance of tearing.
>
> For the first point, I have a question. They can't happen
> simultaneously. But the cache of different CPUs may contain different
> values.
>
> >
> > The compiler can't reorder setting md_io_clone->waiting_reshape to afer
> > bio_endio(), since bio_endio can free md_io_clone. Also the compiler
> > can't reorder reading it to before calling md_end_clone_io() from
> > bio_endio(), obviously.
>
> Thanks very much for pointing this out.
>
> >
> > If the bio was never chained, then there can't be a race, since only
> > raid5_make_request() will be calling bio_endio(). waiting_reshape will
> > be read by the same process that sets it.
> >
> > If the bio was chained, then like you said, md_end_clone_io() will only
> > get called by the final caller of bio_endio(). This is determined by
> > bio_remaining_done(), which guarantees a full memory barrier for
> > atomic_dec_and_test(&bio->__bi_remaining).
>
> Nice analysis! Thanks.
>
> >
> > Since we know the compiler will keep these instructions on the proper
> > size of a full memory barrier, I'm pretty sure that this is concurrency
> > race free, even without the READ_ONCE() and WRITE_ONCE(). Like with
> > trying to avoid growing md_io_clone, I was trying to keep
> > md_end_clone_io() as fast as possible (at least on architectures where
> > READ_ONCE can have more of a performance impact) Again, if I'm
> > overthinking this (or if my concurrency argument is flawed) feel free to
> > ignore me.
>
> Thanks for your patience in explaining the concurrency problem. I
> agree with you. And it's better to add comments that describe the
> reason and it will help people understand it better.
>
> By the way, the md raid maintainer's email has changed. I fixed the
> email address in to list.
>
> Regards
> Xiao
>
> >
> > -Ben
> >
> > > Best Regards
> > > Xiao
> > >
> > >
> > > > return false;
> > > > }
> > >
> > > >
> > > > --
> > > > 2.53.0
> > > >
> >
^ permalink raw reply
* Re: 4 TiB WD SN700 drives drop off of Sonnet Card
From: Jani Partanen @ 2026-03-31 21:45 UTC (permalink / raw)
To: Justin Piszcz, LKML, linux-nvme, linux-raid
In-Reply-To: <CAO9zADxDnb5OVVVZWj64ac70SACH=-G5TC+2F+vfuDmLbZECQA@mail.gmail.com>
Hi, if you are running them with 4K LBA format, switch back to 512k. 4K
LBA cause issues to many WD drives. WD also know this, its been reported
years ago and it never got firmware fix so its quite safe to say that
they have harware flaw what cause it and cannot be fixed by firmware.
If you are running your drives with 512k LBA, then this is some other issue.
// Jani
On 01/03/2026 13.39, Justin Piszcz wrote:
> Hello,
>
> Kernel: 6.12.73
> Distribution: Debian Stable Trixie
>
> There are long outstanding issues with these WD Red SN700 4TiB drives
> with an ASUS FlahsStor Gen1.
>
> I have 16 drives on two x8 Sonnet boards and the same issue is
> occurring across two different platforms. Are these drives completely
> defective when used in a software raid configuration?
>
> Does anyone have a specific kernel parameter to make these drives work
> without dropping off the bus— or is it time to retire/replace these
> drives? On various forums the below parameters had been tested
> previously and did not help.
>
> [Sun Mar 1 00:30:58 2026] nvme nvme0: controller is down; will reset:
> CSTS=0xffffffff, PCI_STATUS=0xffff
> [Sun Mar 1 00:30:58 2026] nvme nvme0: Does your device have a faulty
> power saving mode enabled?
> [Sun Mar 1 00:30:58 2026] nvme nvme0: Try
> "nvme_core.default_ps_max_latency_us=0 pcie_aspm=off pcie_port_pm=off"
> and report a bug
> [Sun Mar 1 00:31:03 2026] nvme 0000:03:00.0: Unable to change power
> state from D3cold to D0, device inaccessible
> [Sun Mar 1 00:31:03 2026] nvme nvme0: Disabling device after reset failure: -19
> [Sun Mar 1 00:31:03 2026] md/raid:md0: Disk failure on nvme0n1p1,
> disabling device.
>
> Justin
>
^ permalink raw reply
* Re: [RFC PATCH] md/raid5: Fix UAF on IO across the reshape position
From: Xiao Ni @ 2026-04-01 0:22 UTC (permalink / raw)
To: Benjamin Marzinski
Cc: Yu Kuai, Song Liu, Li Nan, linux-raid, dm-devel, Nigel Croxon
In-Reply-To: <acv4A6WayrNbQEax@redhat.com>
On Wed, Apr 1, 2026 at 12:36 AM Benjamin Marzinski <bmarzins@redhat.com> wrote:
>
> On Tue, Mar 31, 2026 at 10:24:48AM +0800, Xiao Ni wrote:
> > On Tue, Mar 31, 2026 at 3:16 AM Benjamin Marzinski <bmarzins@redhat.com> wrote:
> > >
> > > On Mon, Mar 30, 2026 at 11:44:54PM +0800, Xiao Ni wrote:
> > > > On Tue, Mar 24, 2026 at 6:58 AM Benjamin Marzinski <bmarzins@redhat.com> wrote:
> > > > >
> > > > > If make_stripe_request() returns STRIPE_WAIT_RESHAPE,
> > > > > raid5_make_request() will free the cloned bio. But raid5_make_request()
> > > > > can call make_stripe_request() multiple times, writing to the various
> > > > > stripes. If that bio got added to the toread or towrite lists of a
> > > > > stripe disk in an earlier call to make_stripe_request(), then it's not
> > > > > safe to just free the bio if a later part of it is found to cross the
> > > > > reshape position. Doing so can lead to a UAF error, when bio_endio()
> > > > > is called on the bio for the earlier stripes.
> > > > >
> > > > > Instead, raid5_make_request() needs to wait until all parts of the bio
> > > > > have called bio_endio(). To do this, bios that cross the reshape
> > > > > position while the reshape can't make progress are flagged as needing a
> > > > > retry, and mddev tracks the number of bios needing a retry which have
> > > > > not yet completed. When raid5_make_request() has a bio that failed
> > > > > make_stripe_request() with STRIPE_WAIT_RESHAPE, it waits for this
> > > > > counter to reach zero. When the bio_endio() is called for the last time
> > > > > on a bio needing a retry, it decrements mddev's count of outstanding
> > > > > bios needing a retry. This guarantees that raid5_make_request() doesn't
> > > > > return until the cloned bio needing a retry for io across the reshape
> > > > > boundary is safely cleaned up.
> > > > >
> > > > > There is a simple reproducer available at [1]. Compile the kernel with
> > > > > KASAN for more useful reporting when the error is triggered (this is not
> > > > > necessary to see the bug).
> > > > >
> > > > > [1] https://gist.github.com/bmarzins/e48598824305cf2171289e47d7241fa5
> > > > >
> > > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > > > > ---
> > > > >
> > > > > I've tested this for regressions with the lvm2-testsuite raid tests. I
> > > > > have not run any md-specific tests on it.
> > > > >
> > > > >
> > > > > drivers/md/md.c | 30 +++++++++---------------------
> > > > > drivers/md/md.h | 5 ++++-
> > > > > drivers/md/raid5.c | 8 +++++++-
> > > > > 3 files changed, 20 insertions(+), 23 deletions(-)
> > > > >
> > > > > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > > > > index 3ce6f9e9d38e..5ec116b9da32 100644
> > > > > --- a/drivers/md/md.c
> > > > > +++ b/drivers/md/md.c
> > > > > @@ -776,9 +776,11 @@ int mddev_init(struct mddev *mddev)
> > > > > atomic_set(&mddev->active, 1);
> > > > > atomic_set(&mddev->openers, 0);
> > > > > atomic_set(&mddev->sync_seq, 0);
> > > > > + atomic_set(&mddev->pending_retry_bios, 0);
> > > > > spin_lock_init(&mddev->lock);
> > > > > init_waitqueue_head(&mddev->sb_wait);
> > > > > init_waitqueue_head(&mddev->recovery_wait);
> > > > > + init_waitqueue_head(&mddev->retry_bios_wait);
> > > > > mddev->reshape_position = MaxSector;
> > > > > mddev->reshape_backwards = 0;
> > > > > mddev->last_sync_action = ACTION_IDLE;
> > > > > @@ -9218,6 +9220,7 @@ static void md_end_clone_io(struct bio *bio)
> > > > > struct md_io_clone *md_io_clone = bio->bi_private;
> > > > > struct bio *orig_bio = md_io_clone->orig_bio;
> > > > > struct mddev *mddev = md_io_clone->mddev;
> > > > > + unsigned int must_retry = md_io_clone->must_retry;
> > > > >
> > > > > if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
> > > > > md_bitmap_end(mddev, md_io_clone);
> > > > > @@ -9229,7 +9232,11 @@ static void md_end_clone_io(struct bio *bio)
> > > > > bio_end_io_acct(orig_bio, md_io_clone->start_time);
> > > > >
> > > > > bio_put(bio);
> > > > > - bio_endio(orig_bio);
> > > > > + if (unlikely(must_retry)) {
> > > > > + if (atomic_dec_and_test(&mddev->pending_retry_bios))
> > > > > + wake_up(&mddev->retry_bios_wait);
> > > > > + } else
> > > > > + bio_endio(orig_bio);
> > > > > percpu_ref_put(&mddev->active_io);
> > > > > }
> > > > >
> > > > > @@ -9243,6 +9250,7 @@ static void md_clone_bio(struct mddev *mddev, struct bio **bio)
> > > > > md_io_clone = container_of(clone, struct md_io_clone, bio_clone);
> > > > > md_io_clone->orig_bio = *bio;
> > > > > md_io_clone->mddev = mddev;
> > > > > + md_io_clone->must_retry = 0;
> > > > > if (blk_queue_io_stat(bdev->bd_disk->queue))
> > > > > md_io_clone->start_time = bio_start_io_acct(*bio);
> > > > >
> > > > > @@ -9265,26 +9273,6 @@ void md_account_bio(struct mddev *mddev, struct bio **bio)
> > > > > }
> > > > > EXPORT_SYMBOL_GPL(md_account_bio);
> > > > >
> > > > > -void md_free_cloned_bio(struct bio *bio)
> > > > > -{
> > > > > - struct md_io_clone *md_io_clone = bio->bi_private;
> > > > > - struct bio *orig_bio = md_io_clone->orig_bio;
> > > > > - struct mddev *mddev = md_io_clone->mddev;
> > > > > -
> > > > > - if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
> > > > > - md_bitmap_end(mddev, md_io_clone);
> > > > > -
> > > > > - if (bio->bi_status && !orig_bio->bi_status)
> > > > > - orig_bio->bi_status = bio->bi_status;
> > > > > -
> > > > > - if (md_io_clone->start_time)
> > > > > - bio_end_io_acct(orig_bio, md_io_clone->start_time);
> > > > > -
> > > > > - bio_put(bio);
> > > > > - percpu_ref_put(&mddev->active_io);
> > > > > -}
> > > > > -EXPORT_SYMBOL_GPL(md_free_cloned_bio);
> > > > > -
> > > > > /* md_allow_write(mddev)
> > > > > * Calling this ensures that the array is marked 'active' so that writes
> > > > > * may proceed without blocking. It is important to call this before
> > > > > diff --git a/drivers/md/md.h b/drivers/md/md.h
> > > > > index ac84289664cd..49a231f11676 100644
> > > > > --- a/drivers/md/md.h
> > > > > +++ b/drivers/md/md.h
> > > > > @@ -626,6 +626,9 @@ struct mddev {
> > > > >
> > > > > /* The sequence number for sync thread */
> > > > > atomic_t sync_seq;
> > > > > +
> > > > > + wait_queue_head_t retry_bios_wait;
> > > > > + atomic_t pending_retry_bios;
> > > > > };
> > > > >
> > > > > enum recovery_flags {
> > > > > @@ -877,6 +880,7 @@ struct md_io_clone {
> > > > > sector_t offset;
> > > > > unsigned long sectors;
> > > > > enum stat_group rw;
> > > > > + unsigned int must_retry;
> > > > > struct bio bio_clone;
> > > > > };
> > > > >
> > > > > @@ -917,7 +921,6 @@ extern void md_finish_reshape(struct mddev *mddev);
> > > > > void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
> > > > > struct bio *bio, sector_t start, sector_t size);
> > > > > void md_account_bio(struct mddev *mddev, struct bio **bio);
> > > > > -void md_free_cloned_bio(struct bio *bio);
> > > > >
> > > > > extern bool __must_check md_flush_request(struct mddev *mddev, struct bio *bio);
> > > > > void md_write_metadata(struct mddev *mddev, struct md_rdev *rdev,
> > > > > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > > > > index a8e8d431071b..fb78a757f2fd 100644
> > > > > --- a/drivers/md/raid5.c
> > > > > +++ b/drivers/md/raid5.c
> > > > > @@ -6217,7 +6217,13 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
> > > > >
> > > > > mempool_free(ctx, conf->ctx_pool);
> > > > > if (res == STRIPE_WAIT_RESHAPE) {
> > > > > - md_free_cloned_bio(bi);
> > > > > + struct md_io_clone *md_io_clone = bi->bi_private;
> > > > > +
> > > > > + md_io_clone->must_retry = 1;
> > > > > + atomic_inc(&mddev->pending_retry_bios);
> > > > > + bio_endio(bi);
> > > > > + wait_event(mddev->retry_bios_wait,
> > > > > + atomic_read(&mddev->pending_retry_bios)==0);
> > > >
> > > > Hi Ben
> > > >
> > > > There is a problem here. The new counter pending_retry_bios above
> > > > doesn't represent the bios which have been added to stripes. So uaf
> > > > still can happen.
> > >
> > > I don't think it can. That counter won't be zero until there are no bios
> > > that need to get retried. So we way end up waiting too long, but we
> > > shouldn't ever end up not waiting long enough.
> >
> > Ah, yes, you're right. I thought wrongly. The counter tracks multiple
> > threads that encounter BLK_STS_RESOURCE. For each thread,
> > md_end_clone_bio is called only when all bios return. But why do
> > different threads need to sync at raid5_make_request?
> >
> >
> > >
> > > > Do we need to add a new counter?
> > >
> > > I did it that way because I wanted to avoid growing md_io_clone
> > > ("must_retry" fit nicely in a hole in the structure, so it didn't take
> > > up any extra space). But if you are fine with adding the extra
> >
> > Nice design.
> >
> > > reshape_completion pointer to md_io_clone, then your way avoids the
> > > chance that we might wait when we don't need to.
> >
> > If you mention the threads sync here, yes.
>
> I can certainly add comments explaining why I left off the READ_ONCE(),
> WRITE_ONCE() (actually, there's no real reason not to keep the
> WRITE_ONCE, since that's not in the fast path) if you think it makes
> sense to leave them off.
>
> I'm fine with either my or your code for to deal with the waiting,
> whichever you think makes more sense. Since this is a very hard bug to
> hit, and the fix involves adding extra data to every bio and extra code
> in the end_io function that all the raid bios call, I wanted to make the
> impact as small as possible. But md_io_clone is still 8 bytes away from
> filling up 3 cachelines (at 64 bytes a cacheline), so there's space to
> add the reshape_completion pointer without increaseing the number of
> cachelines, which means that your solution for waiting is probably the
> way to go, unless you think something else might end up needing that
> space.
Thanks. After thinking, as you said, this is a corner case. So both
ways are fine with me also. Let's wait for Kuai's decision.
Regards
Xiao
>
> -Ben
>
> > >
> > > > Because
> > > > md_end_clone_io is only called when all bios return. How about
> > > > something like:
> > > >
> > > > md_end_clone_io
> > > > + if (unlikely(READ_ONCE(md_io_clone->waiting_reshape))) {
> > > > + complete(md_io_clone->reshape_completion);
> > > > + } else {
> > > > + bio_endio(orig_bio);
> > > > + }
> > > >
> > > > raid5_make_request:
> > > >
> > > > if (res == STRIPE_WAIT_RESHAPE) {
> > > > - md_free_cloned_bio(bi);
> > > > + struct md_io_clone *md_io_clone = bi->bi_private;
> > > > + struct completion done;
> > > > +
> > > > + init_completion(&done);
> > > > + md_io_clone->reshape_completion = &done;
> > > > + WRITE_ONCE(md_io_clone->waiting_reshape, 1);
> > > > +
> > > > + bio_endio(bi);
> > > > +
> > > > + wait_for_completion(&done);
> > >
> > > This looks fine.
> > >
> > > I'm pretty sure that the READ_ONCE() and WRITE_ONCE() are unnecessary.
> > > First, theres no chance that these operations will ever happen at the
> > > same time and we're just writing a 1, so there's no chance of tearing.
> >
> > For the first point, I have a question. They can't happen
> > simultaneously. But the cache of different CPUs may contain different
> > values.
> >
> > >
> > > The compiler can't reorder setting md_io_clone->waiting_reshape to afer
> > > bio_endio(), since bio_endio can free md_io_clone. Also the compiler
> > > can't reorder reading it to before calling md_end_clone_io() from
> > > bio_endio(), obviously.
> >
> > Thanks very much for pointing this out.
> >
> > >
> > > If the bio was never chained, then there can't be a race, since only
> > > raid5_make_request() will be calling bio_endio(). waiting_reshape will
> > > be read by the same process that sets it.
> > >
> > > If the bio was chained, then like you said, md_end_clone_io() will only
> > > get called by the final caller of bio_endio(). This is determined by
> > > bio_remaining_done(), which guarantees a full memory barrier for
> > > atomic_dec_and_test(&bio->__bi_remaining).
> >
> > Nice analysis! Thanks.
> >
> > >
> > > Since we know the compiler will keep these instructions on the proper
> > > size of a full memory barrier, I'm pretty sure that this is concurrency
> > > race free, even without the READ_ONCE() and WRITE_ONCE(). Like with
> > > trying to avoid growing md_io_clone, I was trying to keep
> > > md_end_clone_io() as fast as possible (at least on architectures where
> > > READ_ONCE can have more of a performance impact) Again, if I'm
> > > overthinking this (or if my concurrency argument is flawed) feel free to
> > > ignore me.
> >
> > Thanks for your patience in explaining the concurrency problem. I
> > agree with you. And it's better to add comments that describe the
> > reason and it will help people understand it better.
> >
> > By the way, the md raid maintainer's email has changed. I fixed the
> > email address in to list.
> >
> > Regards
> > Xiao
> >
> > >
> > > -Ben
> > >
> > > > Best Regards
> > > > Xiao
> > > >
> > > >
> > > > > return false;
> > > > > }
> > > >
> > > > >
> > > > > --
> > > > > 2.53.0
> > > > >
> > >
>
^ permalink raw reply
* Re: [PATCH] md: fix array_state=clear sysfs deadlock
From: Li Nan @ 2026-04-02 2:09 UTC (permalink / raw)
To: Yu Kuai, song; +Cc: xni, linux-raid, linux-kernel, yangerkun@huawei.com
In-Reply-To: <20260330055213.3976052-1-yukuai@fnnas.com>
在 2026/3/30 13:52, Yu Kuai 写道:
> From: Yu Kuai <yukuai3@huawei.com>
>
> When "clear" is written to array_state, md_attr_store() breaks sysfs
> active protection so the array can delete itself from its own sysfs
> store method.
>
> However, md_attr_store() currently drops the mddev reference before
> calling sysfs_unbreak_active_protection(). Once do_md_stop(..., 0)
> has made the mddev eligible for delayed deletion, the temporary
> kobject reference taken by sysfs_break_active_protection() can become
> the last kobject reference protecting the md kobject.
>
> That allows sysfs_unbreak_active_protection() to drop the last
> kobject reference from the current sysfs writer context. kobject
> teardown then recurses into kernfs removal while the current sysfs
> node is still being unwound, and lockdep reports recursive locking on
> kn->active with kernfs_drain() in the call chain.
>
> Reproducer on an existing level:
> 1. Create an md0 linear array and activate it:
> mknod /dev/md0 b 9 0
> echo none > /sys/block/md0/md/metadata_version
> echo linear > /sys/block/md0/md/level
> echo 1 > /sys/block/md0/md/raid_disks
> echo "$(cat /sys/class/block/sdb/dev)" > /sys/block/md0/md/new_dev
> echo "$(($(cat /sys/class/block/sdb/size) / 2))" > \
> /sys/block/md0/md/dev-sdb/size
> echo 0 > /sys/block/md0/md/dev-sdb/slot
> echo active > /sys/block/md0/md/array_state
> 2. Wait briefly for the array to settle, then clear it:
> sleep 2
> echo clear > /sys/block/md0/md/array_state
>
> The warning looks like:
>
> WARNING: possible recursive locking detected
> bash/588 is trying to acquire lock:
> (kn->active#65) at __kernfs_remove+0x157/0x1d0
> but task is already holding lock:
> (kn->active#65) at sysfs_unbreak_active_protection+0x1f/0x40
> ...
> Call Trace:
> kernfs_drain
> __kernfs_remove
> kernfs_remove_by_name_ns
> sysfs_remove_group
> sysfs_remove_groups
> __kobject_del
> kobject_put
> md_attr_store
> kernfs_fop_write_iter
> vfs_write
> ksys_write
>
> Restore active protection before mddev_put() so the extra sysfs
> kobject reference is dropped while the mddev is still held alive. The
> actual md kobject deletion is then deferred until after the sysfs
> write path has fully returned.
>
> Fixes: 9e59d609763f ("md: call del_gendisk in control path")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
> drivers/md/md.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 521d9b34cd9e..02efe9700256 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6130,10 +6130,16 @@ md_attr_store(struct kobject *kobj, struct attribute *attr,
> }
> spin_unlock(&all_mddevs_lock);
> rv = entry->store(mddev, page, length);
> - mddev_put(mddev);
>
> + /*
> + * For "array_state=clear", dropping the extra kobject reference from
> + * sysfs_break_active_protection() can trigger md kobject deletion.
> + * Restore active protection before mddev_put() so deletion happens
> + * after the sysfs write path fully unwinds.
> + */
> if (kn)
> sysfs_unbreak_active_protection(kn);
> + mddev_put(mddev);
>
> return rv;
> }
LGTM
--
Thanks,
Nan
^ permalink raw reply
* Re: [PATCH] md: wake raid456 reshape waiters before suspend
From: Li Nan @ 2026-04-02 2:53 UTC (permalink / raw)
To: Yu Kuai, song, linux-raid; +Cc: linux-kernel, yangerkun@huawei.com
In-Reply-To: <20260327140729.2030564-1-yukuai@fnnas.com>
在 2026/3/27 22:07, Yu Kuai 写道:
> During raid456 reshape, direct IO across the reshape position can sleep
> in raid5_make_request() waiting for reshape progress while still
> holding an active_io reference. If userspace then freezes reshape and
> writes md/suspend_lo or md/suspend_hi, mddev_suspend() kills active_io
> and waits for all in-flight IO to drain.
>
> This can deadlock: the IO needs reshape progress to continue, but the
> reshape thread is already frozen, so the active_io reference is never
> dropped and suspend never completes.
>
> raid5_prepare_suspend() already wakes wait_for_reshape for dm-raid. Do
> the same for normal md suspend when reshape is already interrupted, so
> waiting raid456 IO can abort, drop its reference, and let suspend
> finish.
>
> The mdadm test tests/25raid456-reshape-deadlock reproduces the hang.
>
> Fixes: 714d20150ed8 ("md: add new helpers to suspend/resume array")
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
> ---
> drivers/md/md.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 65c85973092a..3ae5f36b5baf 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -489,6 +489,17 @@ int mddev_suspend(struct mddev *mddev, bool interruptible)
> }
>
> percpu_ref_kill(&mddev->active_io);
> +
> + /*
> + * RAID456 IO can sleep in wait_for_reshape while still holding an
> + * active_io reference. If reshape is already interrupted or frozen,
> + * wake those waiters so they can abort and drop the reference instead
> + * of deadlocking suspend.
> + */
> + if (mddev->pers && mddev->pers->prepare_suspend &&
> + reshape_interrupted(mddev))
> + mddev->pers->prepare_suspend(mddev);
> +
> if (interruptible)
> err = wait_event_interruptible(mddev->sb_wait,
> percpu_ref_is_zero(&mddev->active_io));
If I only freeze raid5 without suspend, the IO still hangs and cannot
complete. Should we fix this scenario?
--
Thanks,
Nan
^ permalink raw reply
* [PATCH] md/raid5: fix soft lockup in retry_aligned_read()
From: Chia-Ming Chang @ 2026-04-02 6:14 UTC (permalink / raw)
To: song, yukuai
Cc: linan122, shli, neil, linux-raid, linux-kernel, Chia-Ming Chang,
stable, FengWei Shih
When retry_aligned_read() encounters an overlapped stripe, it releases
the stripe via raid5_release_stripe() which puts it on the lockless
released_stripes llist. In the next raid5d loop iteration,
release_stripe_list() drains the stripe onto handle_list (since
STRIPE_HANDLE is set by the original IO), but retry_aligned_read()
runs before handle_active_stripes() and removes the stripe from
handle_list via find_get_stripe() -> list_del_init(). This prevents
handle_stripe() from ever processing the stripe to resolve the
overlap, causing an infinite loop and soft lockup.
Fix this by using __release_stripe() with temp_inactive_list instead
of raid5_release_stripe() in the failure path, so the stripe does not
go through the released_stripes llist. This allows raid5d to break out
of its loop, and the overlap will be resolved when the stripe is
eventually processed by handle_stripe().
Fixes: 773ca82fa1ee ("raid5: make release_stripe lockless")
Cc: stable@vger.kernel.org
Signed-off-by: FengWei Shih <dannyshih@synology.com>
Signed-off-by: Chia-Ming Chang <chiamingc@synology.com>
---
drivers/md/raid5.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index a8e8d431071b..335d2b6b1079 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6641,7 +6641,13 @@ static int retry_aligned_read(struct r5conf *conf, struct bio *raid_bio,
}
if (!add_stripe_bio(sh, raid_bio, dd_idx, 0, 0)) {
- raid5_release_stripe(sh);
+ int hash;
+
+ spin_lock_irq(&conf->device_lock);
+ hash = sh->hash_lock_index;
+ __release_stripe(conf, sh,
+ &conf->temp_inactive_list[hash]);
+ spin_unlock_irq(&conf->device_lock);
conf->retry_read_aligned = raid_bio;
conf->retry_read_offset = scnt;
return handled;
--
2.34.1
Disclaimer: The contents of this e-mail message and any attachments are confidential and are intended solely for addressee. The information may also be legally privileged. This transmission is sent in trust, for the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please immediately notify the sender by reply e-mail or phone and delete this message and its attachments, if any.
^ permalink raw reply related
* [PATCH] md/raid5: validate payload size before accessing journal metadata
From: Junrui Luo @ 2026-04-04 7:44 UTC (permalink / raw)
To: Song Liu, Yu Kuai, Li Nan, Shaohua Li
Cc: Song Liu, linux-raid, linux-kernel, Yuhao Jiang, stable,
Junrui Luo
r5c_recovery_analyze_meta_block() and
r5l_recovery_verify_data_checksum_for_mb() iterate over payloads in a
journal metadata block using on-disk payload size fields without
validating them against the remaining space in the metadata block.
A corrupted journal contains payload sizes extending beyond the PAGE_SIZE
boundary can cause out-of-bounds reads when accessing payload fields or
computing offsets.
Add bounds validation for each payload type to ensure the full payload
fits within meta_size before processing.
Fixes: b4c625c67362 ("md/r5cache: r5cache recovery: part 1")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
drivers/md/raid5-cache.c | 48 +++++++++++++++++++++++++++++++++---------------
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 66b10cbda96d..7b7546bfa21f 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -2002,15 +2002,27 @@ r5l_recovery_verify_data_checksum_for_mb(struct r5l_log *log,
return -ENOMEM;
while (mb_offset < le32_to_cpu(mb->meta_size)) {
+ sector_t payload_len;
+
payload = (void *)mb + mb_offset;
payload_flush = (void *)mb + mb_offset;
if (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_DATA) {
+ payload_len = sizeof(struct r5l_payload_data_parity) +
+ (sector_t)sizeof(__le32) *
+ (le32_to_cpu(payload->size) >> (PAGE_SHIFT - 9));
+ if (mb_offset + payload_len > le32_to_cpu(mb->meta_size))
+ goto mismatch;
if (r5l_recovery_verify_data_checksum(
log, ctx, page, log_offset,
payload->checksum[0]) < 0)
goto mismatch;
} else if (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_PARITY) {
+ payload_len = sizeof(struct r5l_payload_data_parity) +
+ (sector_t)sizeof(__le32) *
+ (le32_to_cpu(payload->size) >> (PAGE_SHIFT - 9));
+ if (mb_offset + payload_len > le32_to_cpu(mb->meta_size))
+ goto mismatch;
if (r5l_recovery_verify_data_checksum(
log, ctx, page, log_offset,
payload->checksum[0]) < 0)
@@ -2023,22 +2035,18 @@ r5l_recovery_verify_data_checksum_for_mb(struct r5l_log *log,
payload->checksum[1]) < 0)
goto mismatch;
} else if (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_FLUSH) {
- /* nothing to do for R5LOG_PAYLOAD_FLUSH here */
+ payload_len = sizeof(struct r5l_payload_flush) +
+ (sector_t)le32_to_cpu(payload_flush->size);
+ if (mb_offset + payload_len > le32_to_cpu(mb->meta_size))
+ goto mismatch;
} else /* not R5LOG_PAYLOAD_DATA/PARITY/FLUSH */
goto mismatch;
- if (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_FLUSH) {
- mb_offset += sizeof(struct r5l_payload_flush) +
- le32_to_cpu(payload_flush->size);
- } else {
- /* DATA or PARITY payload */
+ if (le16_to_cpu(payload->header.type) != R5LOG_PAYLOAD_FLUSH) {
log_offset = r5l_ring_add(log, log_offset,
le32_to_cpu(payload->size));
- mb_offset += sizeof(struct r5l_payload_data_parity) +
- sizeof(__le32) *
- (le32_to_cpu(payload->size) >> (PAGE_SHIFT - 9));
}
-
+ mb_offset += payload_len;
}
put_page(page);
@@ -2089,6 +2097,7 @@ r5c_recovery_analyze_meta_block(struct r5l_log *log,
log_offset = r5l_ring_add(log, ctx->pos, BLOCK_SECTORS);
while (mb_offset < le32_to_cpu(mb->meta_size)) {
+ sector_t payload_len;
int dd;
payload = (void *)mb + mb_offset;
@@ -2097,6 +2106,12 @@ r5c_recovery_analyze_meta_block(struct r5l_log *log,
if (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_FLUSH) {
int i, count;
+ payload_len = sizeof(struct r5l_payload_flush) +
+ (sector_t)le32_to_cpu(payload_flush->size);
+ if (mb_offset + payload_len >
+ le32_to_cpu(mb->meta_size))
+ return -EINVAL;
+
count = le32_to_cpu(payload_flush->size) / sizeof(__le64);
for (i = 0; i < count; ++i) {
stripe_sect = le64_to_cpu(payload_flush->flush_stripes[i]);
@@ -2110,12 +2125,17 @@ r5c_recovery_analyze_meta_block(struct r5l_log *log,
}
}
- mb_offset += sizeof(struct r5l_payload_flush) +
- le32_to_cpu(payload_flush->size);
+ mb_offset += payload_len;
continue;
}
/* DATA or PARITY payload */
+ payload_len = sizeof(struct r5l_payload_data_parity) +
+ (sector_t)sizeof(__le32) *
+ (le32_to_cpu(payload->size) >> (PAGE_SHIFT - 9));
+ if (mb_offset + payload_len > le32_to_cpu(mb->meta_size))
+ return -EINVAL;
+
stripe_sect = (le16_to_cpu(payload->header.type) == R5LOG_PAYLOAD_DATA) ?
raid5_compute_sector(
conf, le64_to_cpu(payload->location), 0, &dd,
@@ -2180,9 +2200,7 @@ r5c_recovery_analyze_meta_block(struct r5l_log *log,
log_offset = r5l_ring_add(log, log_offset,
le32_to_cpu(payload->size));
- mb_offset += sizeof(struct r5l_payload_data_parity) +
- sizeof(__le32) *
- (le32_to_cpu(payload->size) >> (PAGE_SHIFT - 9));
+ mb_offset += payload_len;
}
return 0;
---
base-commit: 7aaa8047eafd0bd628065b15757d9b48c5f9c07d
change-id: 20260404-fixes-ef0f9985384d
Best regards,
--
Junrui Luo <moonafterrain@outlook.com>
^ permalink raw reply related
* Re: [PATCH] md: fix array_state=clear sysfs deadlock
From: Yu Kuai @ 2026-04-07 5:02 UTC (permalink / raw)
To: song; +Cc: linan122, xni, linux-raid, linux-kernel, yukuai
In-Reply-To: <20260330055213.3976052-1-yukuai@fnnas.com>
在 2026/3/30 13:52, Yu Kuai 写道:
> From: Yu Kuai<yukuai3@huawei.com>
>
> When "clear" is written to array_state, md_attr_store() breaks sysfs
> active protection so the array can delete itself from its own sysfs
> store method.
>
> However, md_attr_store() currently drops the mddev reference before
> calling sysfs_unbreak_active_protection(). Once do_md_stop(..., 0)
> has made the mddev eligible for delayed deletion, the temporary
> kobject reference taken by sysfs_break_active_protection() can become
> the last kobject reference protecting the md kobject.
>
> That allows sysfs_unbreak_active_protection() to drop the last
> kobject reference from the current sysfs writer context. kobject
> teardown then recurses into kernfs removal while the current sysfs
> node is still being unwound, and lockdep reports recursive locking on
> kn->active with kernfs_drain() in the call chain.
>
> Reproducer on an existing level:
> 1. Create an md0 linear array and activate it:
> mknod /dev/md0 b 9 0
> echo none > /sys/block/md0/md/metadata_version
> echo linear > /sys/block/md0/md/level
> echo 1 > /sys/block/md0/md/raid_disks
> echo "$(cat /sys/class/block/sdb/dev)" > /sys/block/md0/md/new_dev
> echo "$(($(cat /sys/class/block/sdb/size) / 2))" > \
> /sys/block/md0/md/dev-sdb/size
> echo 0 > /sys/block/md0/md/dev-sdb/slot
> echo active > /sys/block/md0/md/array_state
> 2. Wait briefly for the array to settle, then clear it:
> sleep 2
> echo clear > /sys/block/md0/md/array_state
>
> The warning looks like:
>
> WARNING: possible recursive locking detected
> bash/588 is trying to acquire lock:
> (kn->active#65) at __kernfs_remove+0x157/0x1d0
> but task is already holding lock:
> (kn->active#65) at sysfs_unbreak_active_protection+0x1f/0x40
> ...
> Call Trace:
> kernfs_drain
> __kernfs_remove
> kernfs_remove_by_name_ns
> sysfs_remove_group
> sysfs_remove_groups
> __kobject_del
> kobject_put
> md_attr_store
> kernfs_fop_write_iter
> vfs_write
> ksys_write
>
> Restore active protection before mddev_put() so the extra sysfs
> kobject reference is dropped while the mddev is still held alive. The
> actual md kobject deletion is then deferred until after the sysfs
> write path has fully returned.
>
> Fixes: 9e59d609763f ("md: call del_gendisk in control path")
> Signed-off-by: Yu Kuai<yukuai3@huawei.com>
> ---
> drivers/md/md.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
Applied to md-7.1
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid0: use kvzalloc/kvfree for strip_zone and devlist allocations
From: Yu Kuai @ 2026-04-07 5:06 UTC (permalink / raw)
To: Gregory Price, song, linan122
Cc: linux-raid, linux-kernel, linux-mm, syzbot+924649752adf0d3ac9dd,
akpm, yukuai
In-Reply-To: <20260308234202.3118119-1-gourry@gourry.net>
Hi,
在 2026/3/9 7:42, Gregory Price 写道:
> syzbot reported a WARNING at mm/page_alloc.c:__alloc_frozen_pages_noprof()
> triggered by create_strip_zones() in the RAID0 driver.
>
> When raid_disks is large, the allocation size exceeds MAX_PAGE_ORDER (4MB
> on x86), causing WARN_ON_ONCE_GFP(order > MAX_PAGE_ORDER).
>
> Convert the strip_zone and devlist allocations from kzalloc/kzalloc_objs to
> kvzalloc/kvzalloc_objs, which first attempts a contiguous allocation with
> __GFP_NOWARN and then falls back to vmalloc for large sizes. Convert the
> corresponding kfree calls to kvfree.
>
> Both arrays are pure metadata lookup tables (arrays of pointers and zone
> descriptors) accessed only via indexing, so they do not require physically
> contiguous memory.
>
> Reported-by:syzbot+924649752adf0d3ac9dd@syzkaller.appspotmail.com
Reported-by should be followed by Closes tag, applied tom md-7.1 with following tag:
Closes:[syzbot] [mm?] WARNING in create_strip_zones - syzbot <https://lore.kernel.org/all/69adaba8.a00a0220.b130.0005.GAE@google.com/>
> Signed-off-by: Gregory Price<gourry@gourry.net>
> ---
> drivers/md/raid0.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md: remove unused static md_wq workqueue
From: Yu Kuai @ 2026-04-07 5:26 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, linux-raid, linux-kernel, yukuai
In-Reply-To: <20260328193522.3624-1-abd.masalkhi@gmail.com>
在 2026/3/29 3:35, Abd-Alrhman Masalkhi 写道:
> The md_wq workqueue is defined as static and initialized in md_init(),
> but it is not used anywhere within md.c.
>
> All asynchronous and deferred work in this file is handled via
> md_misc_wq or dedicated md threads.
>
> Fixes: b75197e86e6d3 ("md: Remove flush handling")
> Signed-off-by: Abd-Alrhman Masalkhi<abd.masalkhi@gmail.com>
> ---
> drivers/md/md.c | 8 --------
> 1 file changed, 8 deletions(-)
Applied to md-7.1
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: validate payload size before accessing journal metadata
From: Yu Kuai @ 2026-04-07 5:28 UTC (permalink / raw)
To: Junrui Luo, Song Liu, Li Nan, Shaohua Li
Cc: Song Liu, linux-raid, linux-kernel, Yuhao Jiang, stable, yukuai
In-Reply-To: <SYBPR01MB78815E78D829BB86CD7C8015AF5FA@SYBPR01MB7881.ausprd01.prod.outlook.com>
Hi,
在 2026/4/4 15:44, Junrui Luo 写道:
> r5c_recovery_analyze_meta_block() and
> r5l_recovery_verify_data_checksum_for_mb() iterate over payloads in a
> journal metadata block using on-disk payload size fields without
> validating them against the remaining space in the metadata block.
>
> A corrupted journal contains payload sizes extending beyond the PAGE_SIZE
> boundary can cause out-of-bounds reads when accessing payload fields or
> computing offsets.
>
> Add bounds validation for each payload type to ensure the full payload
> fits within meta_size before processing.
>
> Fixes: b4c625c67362 ("md/r5cache: r5cache recovery: part 1")
> Reported-by: Yuhao Jiang<danisjiang@gmail.com>
I didn't found a report mail from patchwork, so I remove this tag
> Cc:stable@vger.kernel.org
> Signed-off-by: Junrui Luo<moonafterrain@outlook.com>
> ---
> drivers/md/raid5-cache.c | 48 +++++++++++++++++++++++++++++++++---------------
> 1 file changed, 33 insertions(+), 15 deletions(-)
Applied to md-7.1
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH v3 0/3] md/md-llbitmap: add CleanUnwritten support
From: Yu Kuai @ 2026-04-07 5:29 UTC (permalink / raw)
To: linux-raid, song; +Cc: linan122, xni, yukuai
In-Reply-To: <20260323054644.3351791-1-yukuai@fnnas.com>
在 2026/3/23 13:46, Yu Kuai 写道:
> Patch 1 improves compatibility with older mdadm versions by detecting
> on-disk bitmap version and falling back to the correct bitmap_ops when
> there's a version mismatch.
>
> Patch 2 adds support for proactive XOR parity building in RAID-456 arrays.
> This allows users to pre-build parity for unwritten regions via sysfs
> before any user data is written, which can improve write performance for
> workloads that will eventually use all storage. New states (CleanUnwritten,
> NeedSyncUnwritten, SyncingUnwritten) are added to track these regions
> separately from normal dirty/syncing states.
>
> Patch 3 optimizes initial array sync for RAID-456 arrays on devices that
> support write_zeroes with unmap. By zeroing all disks upfront, parity is
> automatically consistent (0 XOR 0 = 0), allowing the bitmap to be
> initialized to BitCleanUnwritten and skipping the initial sync entirely.
> This significantly reduces array initialization time on modern NVMe SSDs.
>
> Yu Kuai (3):
> md: add fallback to correct bitmap_ops on version mismatch
> md/md-llbitmap: add CleanUnwritten state for RAID-5 proactive parity
> building
> md/md-llbitmap: optimize initial sync with write_zeroes_unmap support
>
> drivers/md/md-llbitmap.c | 202 ++++++++++++++++++++++++++++++++++++---
> drivers/md/md.c | 111 ++++++++++++++++++++-
> 2 files changed, 299 insertions(+), 14 deletions(-)
Applied to md-7.1
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH v3 1/1] md/raid1: serialize overlap io for writemostly disk
From: Yu Kuai @ 2026-04-07 5:30 UTC (permalink / raw)
To: Xiao Ni; +Cc: ncroxon, linux-raid, yukuai
In-Reply-To: <20260324072501.59865-1-xni@redhat.com>
在 2026/3/24 15:24, Xiao Ni 写道:
> Previously, using wait_event() would wake up all waiters simultaneously,
> and they would compete for the tree lock. The bio which gets the lock
> first will be handled, so the write sequence cannot be guaranteed.
>
> For example:
> bio1(100,200)
> bio2(150,200)
> bio3(150,300)
>
> The write sequence of fast device is bio1,bio2,bio3. But the write sequence
> of slow device could be bio1,bio3,bio2 due to lock competition. This causes
> data corruption.
>
> Replace waitqueue with a fifo list to guarantee the write sequence. And it
> also needs to iterate the list when removing one entry. If not, it may miss
> the opportunity to wake up the waiting io.
>
> For example:
> bio1(1,3), bio2(2,4)
> bio3(5,7), bio4(6,8)
> These four bios are in the same bucket. bio1 and bio3 are inserted into
> the rbtree. bio2 and bio4 are added to the waiting list and bio2 is the
> first one. bio3 returns from slow disk and tries to wake up the waiting
> bios. bio2 is removed from the list and will be handled. But bio1 hasn't
> finished. So bio2 will be added into waiting list again. Then bio1 returns
> from slow disk and wakes up waiting bios. bio4 is removed from the list
> and will be handled. Now bio1, bio3 and bio4 all finish and bio2 is left
> on the waiting list. So it needs to iterate the waiting list to wake up
> the right bio.
>
> Signed-off-by: Xiao Ni<xni@redhat.com>
> ---
> v2: use prepare_to_wait_exclusive
> v3: return back to self managed fifo list to find the right waiting node
> drivers/md/md.c | 1 -
> drivers/md/md.h | 5 ++++-
> drivers/md/raid1.c | 45 ++++++++++++++++++++++++++++++++++-----------
> 3 files changed, 38 insertions(+), 13 deletions(-)
Applied to md-7.1
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md: wake raid456 reshape waiters before suspend
From: Yu Kuai @ 2026-04-07 5:32 UTC (permalink / raw)
To: song, linux-raid; +Cc: linan122, linux-kernel, yukuai
In-Reply-To: <20260327140729.2030564-1-yukuai@fnnas.com>
在 2026/3/27 22:07, Yu Kuai 写道:
> During raid456 reshape, direct IO across the reshape position can sleep
> in raid5_make_request() waiting for reshape progress while still
> holding an active_io reference. If userspace then freezes reshape and
> writes md/suspend_lo or md/suspend_hi, mddev_suspend() kills active_io
> and waits for all in-flight IO to drain.
>
> This can deadlock: the IO needs reshape progress to continue, but the
> reshape thread is already frozen, so the active_io reference is never
> dropped and suspend never completes.
>
> raid5_prepare_suspend() already wakes wait_for_reshape for dm-raid. Do
> the same for normal md suspend when reshape is already interrupted, so
> waiting raid456 IO can abort, drop its reference, and let suspend
> finish.
>
> The mdadm test tests/25raid456-reshape-deadlock reproduces the hang.
>
> Fixes: 714d20150ed8 ("md: add new helpers to suspend/resume array")
> Signed-off-by: Yu Kuai<yukuai@fnnas.com>
> ---
> drivers/md/md.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
Applied to md-7.1
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH] md: wake raid456 reshape waiters before suspend
From: Yu Kuai @ 2026-04-07 5:33 UTC (permalink / raw)
To: Li Nan, song, linux-raid; +Cc: linux-kernel, yangerkun@huawei.com, yukuai
In-Reply-To: <3f930776-20e7-f74b-067b-bfb348291a50@huaweicloud.com>
Hi,
在 2026/4/2 10:53, Li Nan 写道:
>
>
> 在 2026/3/27 22:07, Yu Kuai 写道:
>> During raid456 reshape, direct IO across the reshape position can sleep
>> in raid5_make_request() waiting for reshape progress while still
>> holding an active_io reference. If userspace then freezes reshape and
>> writes md/suspend_lo or md/suspend_hi, mddev_suspend() kills active_io
>> and waits for all in-flight IO to drain.
>>
>> This can deadlock: the IO needs reshape progress to continue, but the
>> reshape thread is already frozen, so the active_io reference is never
>> dropped and suspend never completes.
>>
>> raid5_prepare_suspend() already wakes wait_for_reshape for dm-raid. Do
>> the same for normal md suspend when reshape is already interrupted, so
>> waiting raid456 IO can abort, drop its reference, and let suspend
>> finish.
>>
>> The mdadm test tests/25raid456-reshape-deadlock reproduces the hang.
>>
>> Fixes: 714d20150ed8 ("md: add new helpers to suspend/resume array")
>> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
>> ---
>> drivers/md/md.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 65c85973092a..3ae5f36b5baf 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -489,6 +489,17 @@ int mddev_suspend(struct mddev *mddev, bool
>> interruptible)
>> }
>> percpu_ref_kill(&mddev->active_io);
>> +
>> + /*
>> + * RAID456 IO can sleep in wait_for_reshape while still holding an
>> + * active_io reference. If reshape is already interrupted or
>> frozen,
>> + * wake those waiters so they can abort and drop the reference
>> instead
>> + * of deadlocking suspend.
>> + */
>> + if (mddev->pers && mddev->pers->prepare_suspend &&
>> + reshape_interrupted(mddev))
>> + mddev->pers->prepare_suspend(mddev);
>> +
>> if (interruptible)
>> err = wait_event_interruptible(mddev->sb_wait,
>> percpu_ref_is_zero(&mddev->active_io));
>
> If I only freeze raid5 without suspend, the IO still hangs and cannot
> complete. Should we fix this scenario?
Yes, for other path can freeze raid5 directly, we should fix it as well.
>
> --
> Thanks,
> Nan
>
--
Thansk,
Kuai
^ permalink raw reply
* Re: [RFC PATCH] md/raid5: Fix UAF on IO across the reshape position
From: Yu Kuai @ 2026-04-07 6:24 UTC (permalink / raw)
To: Xiao Ni, Benjamin Marzinski
Cc: Yu Kuai, Song Liu, Li Nan, linux-raid, dm-devel, Nigel Croxon,
yukuai
In-Reply-To: <CALTww29gvZBft7-WGwY_-hZCnrxbAK3AAZf-AmGKn7M1t0-0rw@mail.gmail.com>
Hi,
在 2026/4/1 8:22, Xiao Ni 写道:
> On Wed, Apr 1, 2026 at 12:36 AM Benjamin Marzinski <bmarzins@redhat.com> wrote:
>> On Tue, Mar 31, 2026 at 10:24:48AM +0800, Xiao Ni wrote:
>>> On Tue, Mar 31, 2026 at 3:16 AM Benjamin Marzinski <bmarzins@redhat.com> wrote:
>>>> On Mon, Mar 30, 2026 at 11:44:54PM +0800, Xiao Ni wrote:
>>>>> On Tue, Mar 24, 2026 at 6:58 AM Benjamin Marzinski <bmarzins@redhat.com> wrote:
>>>>>> If make_stripe_request() returns STRIPE_WAIT_RESHAPE,
>>>>>> raid5_make_request() will free the cloned bio. But raid5_make_request()
>>>>>> can call make_stripe_request() multiple times, writing to the various
>>>>>> stripes. If that bio got added to the toread or towrite lists of a
>>>>>> stripe disk in an earlier call to make_stripe_request(), then it's not
>>>>>> safe to just free the bio if a later part of it is found to cross the
>>>>>> reshape position. Doing so can lead to a UAF error, when bio_endio()
>>>>>> is called on the bio for the earlier stripes.
>>>>>>
>>>>>> Instead, raid5_make_request() needs to wait until all parts of the bio
>>>>>> have called bio_endio(). To do this, bios that cross the reshape
>>>>>> position while the reshape can't make progress are flagged as needing a
>>>>>> retry, and mddev tracks the number of bios needing a retry which have
>>>>>> not yet completed. When raid5_make_request() has a bio that failed
>>>>>> make_stripe_request() with STRIPE_WAIT_RESHAPE, it waits for this
>>>>>> counter to reach zero. When the bio_endio() is called for the last time
>>>>>> on a bio needing a retry, it decrements mddev's count of outstanding
>>>>>> bios needing a retry. This guarantees that raid5_make_request() doesn't
>>>>>> return until the cloned bio needing a retry for io across the reshape
>>>>>> boundary is safely cleaned up.
>>>>>>
>>>>>> There is a simple reproducer available at [1]. Compile the kernel with
>>>>>> KASAN for more useful reporting when the error is triggered (this is not
>>>>>> necessary to see the bug).
>>>>>>
>>>>>> [1] https://gist.github.com/bmarzins/e48598824305cf2171289e47d7241fa5
>>>>>>
>>>>>> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
>>>>>> ---
>>>>>>
>>>>>> I've tested this for regressions with the lvm2-testsuite raid tests. I
>>>>>> have not run any md-specific tests on it.
>>>>>>
>>>>>>
>>>>>> drivers/md/md.c | 30 +++++++++---------------------
>>>>>> drivers/md/md.h | 5 ++++-
>>>>>> drivers/md/raid5.c | 8 +++++++-
>>>>>> 3 files changed, 20 insertions(+), 23 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>>>>> index 3ce6f9e9d38e..5ec116b9da32 100644
>>>>>> --- a/drivers/md/md.c
>>>>>> +++ b/drivers/md/md.c
>>>>>> @@ -776,9 +776,11 @@ int mddev_init(struct mddev *mddev)
>>>>>> atomic_set(&mddev->active, 1);
>>>>>> atomic_set(&mddev->openers, 0);
>>>>>> atomic_set(&mddev->sync_seq, 0);
>>>>>> + atomic_set(&mddev->pending_retry_bios, 0);
>>>>>> spin_lock_init(&mddev->lock);
>>>>>> init_waitqueue_head(&mddev->sb_wait);
>>>>>> init_waitqueue_head(&mddev->recovery_wait);
>>>>>> + init_waitqueue_head(&mddev->retry_bios_wait);
>>>>>> mddev->reshape_position = MaxSector;
>>>>>> mddev->reshape_backwards = 0;
>>>>>> mddev->last_sync_action = ACTION_IDLE;
>>>>>> @@ -9218,6 +9220,7 @@ static void md_end_clone_io(struct bio *bio)
>>>>>> struct md_io_clone *md_io_clone = bio->bi_private;
>>>>>> struct bio *orig_bio = md_io_clone->orig_bio;
>>>>>> struct mddev *mddev = md_io_clone->mddev;
>>>>>> + unsigned int must_retry = md_io_clone->must_retry;
>>>>>>
>>>>>> if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
>>>>>> md_bitmap_end(mddev, md_io_clone);
>>>>>> @@ -9229,7 +9232,11 @@ static void md_end_clone_io(struct bio *bio)
>>>>>> bio_end_io_acct(orig_bio, md_io_clone->start_time);
>>>>>>
>>>>>> bio_put(bio);
>>>>>> - bio_endio(orig_bio);
>>>>>> + if (unlikely(must_retry)) {
>>>>>> + if (atomic_dec_and_test(&mddev->pending_retry_bios))
>>>>>> + wake_up(&mddev->retry_bios_wait);
>>>>>> + } else
>>>>>> + bio_endio(orig_bio);
>>>>>> percpu_ref_put(&mddev->active_io);
>>>>>> }
>>>>>>
>>>>>> @@ -9243,6 +9250,7 @@ static void md_clone_bio(struct mddev *mddev, struct bio **bio)
>>>>>> md_io_clone = container_of(clone, struct md_io_clone, bio_clone);
>>>>>> md_io_clone->orig_bio = *bio;
>>>>>> md_io_clone->mddev = mddev;
>>>>>> + md_io_clone->must_retry = 0;
>>>>>> if (blk_queue_io_stat(bdev->bd_disk->queue))
>>>>>> md_io_clone->start_time = bio_start_io_acct(*bio);
>>>>>>
>>>>>> @@ -9265,26 +9273,6 @@ void md_account_bio(struct mddev *mddev, struct bio **bio)
>>>>>> }
>>>>>> EXPORT_SYMBOL_GPL(md_account_bio);
>>>>>>
>>>>>> -void md_free_cloned_bio(struct bio *bio)
>>>>>> -{
>>>>>> - struct md_io_clone *md_io_clone = bio->bi_private;
>>>>>> - struct bio *orig_bio = md_io_clone->orig_bio;
>>>>>> - struct mddev *mddev = md_io_clone->mddev;
>>>>>> -
>>>>>> - if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
>>>>>> - md_bitmap_end(mddev, md_io_clone);
>>>>>> -
>>>>>> - if (bio->bi_status && !orig_bio->bi_status)
>>>>>> - orig_bio->bi_status = bio->bi_status;
>>>>>> -
>>>>>> - if (md_io_clone->start_time)
>>>>>> - bio_end_io_acct(orig_bio, md_io_clone->start_time);
>>>>>> -
>>>>>> - bio_put(bio);
>>>>>> - percpu_ref_put(&mddev->active_io);
>>>>>> -}
>>>>>> -EXPORT_SYMBOL_GPL(md_free_cloned_bio);
>>>>>> -
>>>>>> /* md_allow_write(mddev)
>>>>>> * Calling this ensures that the array is marked 'active' so that writes
>>>>>> * may proceed without blocking. It is important to call this before
>>>>>> diff --git a/drivers/md/md.h b/drivers/md/md.h
>>>>>> index ac84289664cd..49a231f11676 100644
>>>>>> --- a/drivers/md/md.h
>>>>>> +++ b/drivers/md/md.h
>>>>>> @@ -626,6 +626,9 @@ struct mddev {
>>>>>>
>>>>>> /* The sequence number for sync thread */
>>>>>> atomic_t sync_seq;
>>>>>> +
>>>>>> + wait_queue_head_t retry_bios_wait;
>>>>>> + atomic_t pending_retry_bios;
>>>>>> };
>>>>>>
>>>>>> enum recovery_flags {
>>>>>> @@ -877,6 +880,7 @@ struct md_io_clone {
>>>>>> sector_t offset;
>>>>>> unsigned long sectors;
>>>>>> enum stat_group rw;
>>>>>> + unsigned int must_retry;
>>>>>> struct bio bio_clone;
>>>>>> };
>>>>>>
>>>>>> @@ -917,7 +921,6 @@ extern void md_finish_reshape(struct mddev *mddev);
>>>>>> void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev,
>>>>>> struct bio *bio, sector_t start, sector_t size);
>>>>>> void md_account_bio(struct mddev *mddev, struct bio **bio);
>>>>>> -void md_free_cloned_bio(struct bio *bio);
>>>>>>
>>>>>> extern bool __must_check md_flush_request(struct mddev *mddev, struct bio *bio);
>>>>>> void md_write_metadata(struct mddev *mddev, struct md_rdev *rdev,
>>>>>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>>>>>> index a8e8d431071b..fb78a757f2fd 100644
>>>>>> --- a/drivers/md/raid5.c
>>>>>> +++ b/drivers/md/raid5.c
>>>>>> @@ -6217,7 +6217,13 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
>>>>>>
>>>>>> mempool_free(ctx, conf->ctx_pool);
>>>>>> if (res == STRIPE_WAIT_RESHAPE) {
>>>>>> - md_free_cloned_bio(bi);
>>>>>> + struct md_io_clone *md_io_clone = bi->bi_private;
>>>>>> +
>>>>>> + md_io_clone->must_retry = 1;
>>>>>> + atomic_inc(&mddev->pending_retry_bios);
>>>>>> + bio_endio(bi);
>>>>>> + wait_event(mddev->retry_bios_wait,
>>>>>> + atomic_read(&mddev->pending_retry_bios)==0);
>>>>> Hi Ben
>>>>>
>>>>> There is a problem here. The new counter pending_retry_bios above
>>>>> doesn't represent the bios which have been added to stripes. So uaf
>>>>> still can happen.
>>>> I don't think it can. That counter won't be zero until there are no bios
>>>> that need to get retried. So we way end up waiting too long, but we
>>>> shouldn't ever end up not waiting long enough.
>>> Ah, yes, you're right. I thought wrongly. The counter tracks multiple
>>> threads that encounter BLK_STS_RESOURCE. For each thread,
>>> md_end_clone_bio is called only when all bios return. But why do
>>> different threads need to sync at raid5_make_request?
>>>
>>>
>>>>> Do we need to add a new counter?
>>>> I did it that way because I wanted to avoid growing md_io_clone
>>>> ("must_retry" fit nicely in a hole in the structure, so it didn't take
>>>> up any extra space). But if you are fine with adding the extra
>>> Nice design.
>>>
>>>> reshape_completion pointer to md_io_clone, then your way avoids the
>>>> chance that we might wait when we don't need to.
>>> If you mention the threads sync here, yes.
>> I can certainly add comments explaining why I left off the READ_ONCE(),
>> WRITE_ONCE() (actually, there's no real reason not to keep the
>> WRITE_ONCE, since that's not in the fast path) if you think it makes
>> sense to leave them off.
>>
>> I'm fine with either my or your code for to deal with the waiting,
>> whichever you think makes more sense. Since this is a very hard bug to
>> hit, and the fix involves adding extra data to every bio and extra code
>> in the end_io function that all the raid bios call, I wanted to make the
>> impact as small as possible. But md_io_clone is still 8 bytes away from
>> filling up 3 cachelines (at 64 bytes a cacheline), so there's space to
>> add the reshape_completion pointer without increaseing the number of
>> cachelines, which means that your solution for waiting is probably the
>> way to go, unless you think something else might end up needing that
>> space.
I think it's not good to add a global counter in mddev, and I agree it's
not good to increase md_io_clone for this corner case.
How about we remove the use of bi_private filed, and replace it with
container_of() to get md_clone_io from md_end_clone_io. And then we can use
this field to record the completion without adding new field.
> Thanks. After thinking, as you said, this is a corner case. So both
> ways are fine with me also. Let's wait for Kuai's decision.
>
> Regards
> Xiao
>> -Ben
>>
>>>>> Because
>>>>> md_end_clone_io is only called when all bios return. How about
>>>>> something like:
>>>>>
>>>>> md_end_clone_io
>>>>> + if (unlikely(READ_ONCE(md_io_clone->waiting_reshape))) {
>>>>> + complete(md_io_clone->reshape_completion);
>>>>> + } else {
>>>>> + bio_endio(orig_bio);
>>>>> + }
>>>>>
>>>>> raid5_make_request:
>>>>>
>>>>> if (res == STRIPE_WAIT_RESHAPE) {
>>>>> - md_free_cloned_bio(bi);
>>>>> + struct md_io_clone *md_io_clone = bi->bi_private;
>>>>> + struct completion done;
>>>>> +
>>>>> + init_completion(&done);
>>>>> + md_io_clone->reshape_completion = &done;
>>>>> + WRITE_ONCE(md_io_clone->waiting_reshape, 1);
>>>>> +
>>>>> + bio_endio(bi);
>>>>> +
>>>>> + wait_for_completion(&done);
>>>> This looks fine.
>>>>
>>>> I'm pretty sure that the READ_ONCE() and WRITE_ONCE() are unnecessary.
>>>> First, theres no chance that these operations will ever happen at the
>>>> same time and we're just writing a 1, so there's no chance of tearing.
>>> For the first point, I have a question. They can't happen
>>> simultaneously. But the cache of different CPUs may contain different
>>> values.
>>>
>>>> The compiler can't reorder setting md_io_clone->waiting_reshape to afer
>>>> bio_endio(), since bio_endio can free md_io_clone. Also the compiler
>>>> can't reorder reading it to before calling md_end_clone_io() from
>>>> bio_endio(), obviously.
>>> Thanks very much for pointing this out.
>>>
>>>> If the bio was never chained, then there can't be a race, since only
>>>> raid5_make_request() will be calling bio_endio(). waiting_reshape will
>>>> be read by the same process that sets it.
>>>>
>>>> If the bio was chained, then like you said, md_end_clone_io() will only
>>>> get called by the final caller of bio_endio(). This is determined by
>>>> bio_remaining_done(), which guarantees a full memory barrier for
>>>> atomic_dec_and_test(&bio->__bi_remaining).
>>> Nice analysis! Thanks.
>>>
>>>> Since we know the compiler will keep these instructions on the proper
>>>> size of a full memory barrier, I'm pretty sure that this is concurrency
>>>> race free, even without the READ_ONCE() and WRITE_ONCE(). Like with
>>>> trying to avoid growing md_io_clone, I was trying to keep
>>>> md_end_clone_io() as fast as possible (at least on architectures where
>>>> READ_ONCE can have more of a performance impact) Again, if I'm
>>>> overthinking this (or if my concurrency argument is flawed) feel free to
>>>> ignore me.
>>> Thanks for your patience in explaining the concurrency problem. I
>>> agree with you. And it's better to add comments that describe the
>>> reason and it will help people understand it better.
>>>
>>> By the way, the md raid maintainer's email has changed. I fixed the
>>> email address in to list.
>>>
>>> Regards
>>> Xiao
>>>
>>>> -Ben
>>>>
>>>>> Best Regards
>>>>> Xiao
>>>>>
>>>>>
>>>>>> return false;
>>>>>> }
>>>>>> --
>>>>>> 2.53.0
>>>>>>
>
--
Thansk,
Kuai
^ permalink raw reply
* [PATCH v2 0/5] md: bitmap grow fixes
From: Su Yue @ 2026-04-07 10:26 UTC (permalink / raw)
To: linux-raid; +Cc: song, xni, linan122, yukuai, heming.zhao, l, Su Yue
Hi, This v2 series is to fixes bugs exposed by mdadm/clustermd-autotest.
The bugs are not cluster md only but also bitmap related.
The series based on v7.0-rc7 passes tests with mdadm v4.6 without regression.
v2:
Add a dummy bitmap operations per Kuai's suggestion.
To Yu Kuai:
A NULL group can't used for internal_bitmap_group since
the entries from internal_bitmap_attrs should be under bitmap.
So instead of sysfs_create_groups(), sysfs_create_group() and sysfs_merge_group()
are still needed /sigh.
Su Yue (5):
md/md-bitmap: call md_bitmap_create,destroy in location_store
md/md-bitmap: add an extra sysfs argument to md_bitmap_create and
destroy
md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset
md: skip ID_BITMAP_NONE when show available bitmap types
md/md-bitmap: remove member group from bitmap_operations
drivers/md/md-bitmap.c | 121 ++++++++++++++++++++++++++++++++++++---
drivers/md/md-bitmap.h | 3 +-
drivers/md/md-llbitmap.c | 13 ++++-
drivers/md/md.c | 55 +++++++++---------
drivers/md/md.h | 2 +
5 files changed, 155 insertions(+), 39 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store
From: Su Yue @ 2026-04-07 10:26 UTC (permalink / raw)
To: linux-raid; +Cc: song, xni, linan122, yukuai, heming.zhao, l, Su Yue
In-Reply-To: <20260407102625.5686-1-glass.su@suse.com>
If bitmap/location is present, mdadm will call update_array_info()
while growing bitmap from none to internal via location_store().
md_bitmap_create() is needed to set mddev->bitmap_ops otherwise
mddev->bitmap_ops->get_stats() in update_array_info() will trigger
kernel NULL pointer dereference.
Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
Signed-off-by: Su Yue <glass.su@suse.com>
---
drivers/md/md-bitmap.c | 11 ++++++++---
drivers/md/md.c | 4 ++--
drivers/md/md.h | 2 ++
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 83378c033c72..2f24aae05552 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -2618,7 +2618,7 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
goto out;
}
- bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev);
mddev->bitmap_info.offset = 0;
if (mddev->bitmap_info.file) {
struct file *f = mddev->bitmap_info.file;
@@ -2653,15 +2653,20 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
goto out;
}
+ /*
+ * lockless bitmap shoudle have set bitmap_id
+ * using bitmap_type, so always ID_BITMAP.
+ */
+ mddev->bitmap_id = ID_BITMAP;
mddev->bitmap_info.offset = offset;
- rv = bitmap_create(mddev);
+ rv = md_bitmap_create(mddev);
if (rv)
goto out;
rv = bitmap_load(mddev);
if (rv) {
mddev->bitmap_info.offset = 0;
- bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev);
goto out;
}
}
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 3ce6f9e9d38e..8b1ecc370ad6 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6447,7 +6447,7 @@ static void md_safemode_timeout(struct timer_list *t)
static int start_dirty_degraded;
-static int md_bitmap_create(struct mddev *mddev)
+int md_bitmap_create(struct mddev *mddev)
{
if (mddev->bitmap_id == ID_BITMAP_NONE)
return -EINVAL;
@@ -6458,7 +6458,7 @@ static int md_bitmap_create(struct mddev *mddev)
return mddev->bitmap_ops->create(mddev);
}
-static void md_bitmap_destroy(struct mddev *mddev)
+void md_bitmap_destroy(struct mddev *mddev)
{
if (!md_bitmap_registered(mddev))
return;
diff --git a/drivers/md/md.h b/drivers/md/md.h
index ac84289664cd..ed69244af00d 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -895,6 +895,8 @@ static inline void safe_put_page(struct page *p)
int register_md_submodule(struct md_submodule_head *msh);
void unregister_md_submodule(struct md_submodule_head *msh);
+int md_bitmap_create(struct mddev *mddev);
+void md_bitmap_destroy(struct mddev *mddev);
extern struct md_thread *md_register_thread(
void (*run)(struct md_thread *thread),
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/5] md/md-bitmap: add an extra sysfs argument to md_bitmap_create and destroy
From: Su Yue @ 2026-04-07 10:26 UTC (permalink / raw)
To: linux-raid; +Cc: song, xni, linan122, yukuai, heming.zhao, l, Su Yue
In-Reply-To: <20260407102625.5686-1-glass.su@suse.com>
For further use, no functional change.
Signed-off-by: Su Yue <glass.su@suse.com>
---
drivers/md/md-bitmap.c | 6 +++---
drivers/md/md.c | 36 ++++++++++++++++++------------------
drivers/md/md.h | 4 ++--
3 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 2f24aae05552..ac06c9647bf0 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -2618,7 +2618,7 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
goto out;
}
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
mddev->bitmap_info.offset = 0;
if (mddev->bitmap_info.file) {
struct file *f = mddev->bitmap_info.file;
@@ -2659,14 +2659,14 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
*/
mddev->bitmap_id = ID_BITMAP;
mddev->bitmap_info.offset = offset;
- rv = md_bitmap_create(mddev);
+ rv = md_bitmap_create(mddev, true);
if (rv)
goto out;
rv = bitmap_load(mddev);
if (rv) {
mddev->bitmap_info.offset = 0;
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
goto out;
}
}
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 8b1ecc370ad6..d3c8f77b4fe3 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -678,7 +678,7 @@ static void active_io_release(struct percpu_ref *ref)
static void no_op(struct percpu_ref *r) {}
-static bool mddev_set_bitmap_ops(struct mddev *mddev)
+static bool mddev_set_bitmap_ops(struct mddev *mddev, bool create_sysfs)
{
struct bitmap_operations *old = mddev->bitmap_ops;
struct md_submodule_head *head;
@@ -703,7 +703,7 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
mddev->bitmap_ops = (void *)head;
xa_unlock(&md_submodule);
- if (!mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
+ if (create_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
if (sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group))
pr_warn("md: cannot register extra bitmap attributes for %s\n",
mdname(mddev));
@@ -721,9 +721,9 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev)
return false;
}
-static void mddev_clear_bitmap_ops(struct mddev *mddev)
+static void mddev_clear_bitmap_ops(struct mddev *mddev, bool remove_sysfs)
{
- if (!mddev_is_dm(mddev) && mddev->bitmap_ops &&
+ if (remove_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops &&
mddev->bitmap_ops->group)
sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
@@ -6447,24 +6447,24 @@ static void md_safemode_timeout(struct timer_list *t)
static int start_dirty_degraded;
-int md_bitmap_create(struct mddev *mddev)
+int md_bitmap_create(struct mddev *mddev, bool create_sysfs)
{
if (mddev->bitmap_id == ID_BITMAP_NONE)
return -EINVAL;
- if (!mddev_set_bitmap_ops(mddev))
+ if (!mddev_set_bitmap_ops(mddev, create_sysfs))
return -ENOENT;
return mddev->bitmap_ops->create(mddev);
}
-void md_bitmap_destroy(struct mddev *mddev)
+void md_bitmap_destroy(struct mddev *mddev, bool remove_sysfs)
{
if (!md_bitmap_registered(mddev))
return;
mddev->bitmap_ops->destroy(mddev);
- mddev_clear_bitmap_ops(mddev);
+ mddev_clear_bitmap_ops(mddev, remove_sysfs);
}
int md_run(struct mddev *mddev)
@@ -6612,7 +6612,7 @@ int md_run(struct mddev *mddev)
}
if (err == 0 && pers->sync_request &&
(mddev->bitmap_info.file || mddev->bitmap_info.offset)) {
- err = md_bitmap_create(mddev);
+ err = md_bitmap_create(mddev, true);
if (err)
pr_warn("%s: failed to create bitmap (%d)\n",
mdname(mddev), err);
@@ -6685,7 +6685,7 @@ int md_run(struct mddev *mddev)
pers->free(mddev, mddev->private);
mddev->private = NULL;
put_pers(pers);
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
return err;
}
EXPORT_SYMBOL_GPL(md_run);
@@ -6702,7 +6702,7 @@ int do_md_run(struct mddev *mddev)
if (md_bitmap_registered(mddev)) {
err = mddev->bitmap_ops->load(mddev);
if (err) {
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
goto out;
}
}
@@ -6903,7 +6903,7 @@ static void __md_stop(struct mddev *mddev)
{
struct md_personality *pers = mddev->pers;
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
mddev_detach(mddev);
spin_lock(&mddev->lock);
mddev->pers = NULL;
@@ -7680,16 +7680,16 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
err = 0;
if (mddev->pers) {
if (fd >= 0) {
- err = md_bitmap_create(mddev);
+ err = md_bitmap_create(mddev, true);
if (!err)
err = mddev->bitmap_ops->load(mddev);
if (err) {
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
fd = -1;
}
} else if (fd < 0) {
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
}
}
@@ -7996,12 +7996,12 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
mddev->bitmap_info.default_offset;
mddev->bitmap_info.space =
mddev->bitmap_info.default_space;
- rv = md_bitmap_create(mddev);
+ rv = md_bitmap_create(mddev, true);
if (!rv)
rv = mddev->bitmap_ops->load(mddev);
if (rv)
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
} else {
struct md_bitmap_stats stats;
@@ -8027,7 +8027,7 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
put_cluster_ops(mddev);
mddev->safemode_delay = DEFAULT_SAFEMODE_DELAY;
}
- md_bitmap_destroy(mddev);
+ md_bitmap_destroy(mddev, true);
mddev->bitmap_info.offset = 0;
}
}
diff --git a/drivers/md/md.h b/drivers/md/md.h
index ed69244af00d..4aaba3d7015c 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -895,8 +895,8 @@ static inline void safe_put_page(struct page *p)
int register_md_submodule(struct md_submodule_head *msh);
void unregister_md_submodule(struct md_submodule_head *msh);
-int md_bitmap_create(struct mddev *mddev);
-void md_bitmap_destroy(struct mddev *mddev);
+int md_bitmap_create(struct mddev *mddev, bool create_sysfs);
+void md_bitmap_destroy(struct mddev *mddev, bool remove_sysfs);
extern struct md_thread *md_register_thread(
void (*run)(struct md_thread *thread),
--
2.53.0
^ permalink raw reply related
* [PATCH v2 3/5] md/md-bitmap: add dummy bitmap ops for none to fix wrong bitmap offset
From: Su Yue @ 2026-04-07 10:26 UTC (permalink / raw)
To: linux-raid; +Cc: song, xni, linan122, yukuai, heming.zhao, l, Su Yue
In-Reply-To: <20260407102625.5686-1-glass.su@suse.com>
Before commit fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
if CONFIG_MD_BITMAP is enabled, both bitmap none, internal and clustered have
the sysfs file bitmap/location.
After the commit, if bitmap is none, bitmap/location doesn't exist anymore.
It breaks 'grow' behavior of a md array of madam with MD_FEATURE_BITMAP_OFFSET.
Take level=mirror and metadata=1.2 as an example:
$ mdadm --create /dev/md0 -f --bitmap=none --raid-devices=2 --level=mirror \
--metadata=1.2 /dev/vdd /dev/vde
$ mdadm --grow /dev/md0 --bitmap=internal
$ cat /sys/block/md0/md/bitmap/location
Before:+8
After: +2
While growing bitmap from none to internal, clustered and llbitmap,
mdadm/Grow.c:Grow_addbitmap() tries to detect bitmap/location first.
1)If bitmap/location exists, it sets bitmap/location after getinfo_super().
2)If bitmap/location doesn't exist, mdadm just calls md_set_array_info() then
mddev->bitmap_info.default_offset will be used.
Situation can be worse if growing none to clustered, bitmap offset of the node
calling `madm --grow` will be changed but the other node are reading bitmap sb from
the old location.
Here introducing a dummy bitmap_operations for ID_BITMAP_NONE to restore sysfs
file bitmap/location for ID_BITMAP_NONE and ID_BITMAP.
location_store() now calls md_bitmap_(create|destroy) with false sysfs parameter then
(add,remove)_internal_bitmap() will be called to manage sysfs entries.
Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of bitmap_ops until creating bitmap")
Suggested-by: Yu Kuai <yukuai@fnnas.com>
Signed-off-by: Su Yue <glass.su@suse.com>
---
drivers/md/md-bitmap.c | 116 ++++++++++++++++++++++++++++++++++++---
drivers/md/md-bitmap.h | 3 +
drivers/md/md-llbitmap.c | 12 ++++
drivers/md/md.c | 16 +++---
4 files changed, 130 insertions(+), 17 deletions(-)
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index ac06c9647bf0..a8a176428c61 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -240,6 +240,11 @@ static bool bitmap_enabled(void *data, bool flush)
bitmap->storage.filemap != NULL;
}
+static bool dummy_bitmap_enabled(void *data, bool flush)
+{
+ return false;
+}
+
/*
* check a page and, if necessary, allocate it (or hijack it if the alloc fails)
*
@@ -2201,6 +2206,11 @@ static int bitmap_create(struct mddev *mddev)
return 0;
}
+static int dummy_bitmap_create(struct mddev *mddev)
+{
+ return 0;
+}
+
static int bitmap_load(struct mddev *mddev)
{
int err = 0;
@@ -2594,6 +2604,9 @@ location_show(struct mddev *mddev, char *page)
return len;
}
+static int create_internal_group(struct mddev *mddev);
+static void remove_internal_group(struct mddev *mddev);
+
static ssize_t
location_store(struct mddev *mddev, const char *buf, size_t len)
{
@@ -2618,7 +2631,8 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
goto out;
}
- md_bitmap_destroy(mddev, true);
+ md_bitmap_destroy(mddev, false);
+ remove_internal_group(mddev);
mddev->bitmap_info.offset = 0;
if (mddev->bitmap_info.file) {
struct file *f = mddev->bitmap_info.file;
@@ -2659,14 +2673,22 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
*/
mddev->bitmap_id = ID_BITMAP;
mddev->bitmap_info.offset = offset;
- rv = md_bitmap_create(mddev, true);
+ rv = md_bitmap_create(mddev, false);
if (rv)
goto out;
+ rv = create_internal_group(mddev);
+ if (rv) {
+ mddev->bitmap_info.offset = 0;
+ bitmap_destroy(mddev);
+ goto out;
+ }
+
rv = bitmap_load(mddev);
if (rv) {
+ remove_internal_group(mddev);
mddev->bitmap_info.offset = 0;
- md_bitmap_destroy(mddev, true);
+ md_bitmap_destroy(mddev, false);
goto out;
}
}
@@ -2960,8 +2982,7 @@ static struct md_sysfs_entry max_backlog_used =
__ATTR(max_backlog_used, S_IRUGO | S_IWUSR,
behind_writes_used_show, behind_writes_used_reset);
-static struct attribute *md_bitmap_attrs[] = {
- &bitmap_location.attr,
+static struct attribute *internal_bitmap_attrs[] = {
&bitmap_space.attr,
&bitmap_timeout.attr,
&bitmap_backlog.attr,
@@ -2972,11 +2993,57 @@ static struct attribute *md_bitmap_attrs[] = {
NULL
};
-static struct attribute_group md_bitmap_group = {
+static struct attribute_group internal_bitmap_group = {
.name = "bitmap",
- .attrs = md_bitmap_attrs,
+ .attrs = internal_bitmap_attrs,
};
+/* Only necessary attrs for compatibility */
+static struct attribute *common_bitmap_attrs[] = {
+ &bitmap_location.attr,
+ NULL
+};
+
+static const struct attribute_group common_bitmap_group = {
+ .name = "bitmap",
+ .attrs = common_bitmap_attrs,
+};
+
+static int create_internal_group(struct mddev *mddev)
+{
+ /*
+ * md_bitmap_group and md_bitmap_common_group are using same name
+ * 'bitmap'.
+ */
+ return sysfs_merge_group(&mddev->kobj, &internal_bitmap_group);
+}
+
+static void remove_internal_group(struct mddev *mddev)
+{
+ sysfs_unmerge_group(&mddev->kobj, &internal_bitmap_group);
+}
+
+static int bitmap_register_groups(struct mddev *mddev)
+{
+ int ret;
+
+ ret = sysfs_create_group(&mddev->kobj, &common_bitmap_group);
+
+ if (ret)
+ return ret;
+
+ ret = sysfs_merge_group(&mddev->kobj, &internal_bitmap_group);
+ if (ret)
+ sysfs_remove_group(&mddev->kobj, &common_bitmap_group);
+
+ return ret;
+}
+
+static void bitmap_unregister_groups(struct mddev *mddev)
+{
+ sysfs_unmerge_group(&mddev->kobj, &internal_bitmap_group);
+}
+
static struct bitmap_operations bitmap_ops = {
.head = {
.type = MD_BITMAP,
@@ -3018,21 +3085,52 @@ static struct bitmap_operations bitmap_ops = {
.set_pages = bitmap_set_pages,
.free = md_bitmap_free,
- .group = &md_bitmap_group,
+ .register_groups = bitmap_register_groups,
+ .unregister_groups = bitmap_unregister_groups,
+};
+
+static int none_bitmap_register_groups(struct mddev *mddev)
+{
+ return sysfs_create_group(&mddev->kobj, &common_bitmap_group);
+}
+
+static struct bitmap_operations none_bitmap_ops = {
+ .head = {
+ .type = MD_BITMAP,
+ .id = ID_BITMAP_NONE,
+ .name = "none",
+ },
+
+ .enabled = dummy_bitmap_enabled,
+ .create = dummy_bitmap_create,
+ .destroy = bitmap_destroy,
+ .load = bitmap_load,
+ .get_stats = bitmap_get_stats,
+ .free = md_bitmap_free,
+
+ .register_groups = none_bitmap_register_groups,
+ .unregister_groups = NULL,
};
int md_bitmap_init(void)
{
+ int ret;
+
md_bitmap_wq = alloc_workqueue("md_bitmap", WQ_MEM_RECLAIM | WQ_UNBOUND,
0);
if (!md_bitmap_wq)
return -ENOMEM;
- return register_md_submodule(&bitmap_ops.head);
+ ret = register_md_submodule(&bitmap_ops.head);
+ if (ret)
+ return ret;
+
+ return register_md_submodule(&none_bitmap_ops.head);
}
void md_bitmap_exit(void)
{
destroy_workqueue(md_bitmap_wq);
unregister_md_submodule(&bitmap_ops.head);
+ unregister_md_submodule(&none_bitmap_ops.head);
}
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index b42a28fa83a0..10bc6854798c 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -125,6 +125,9 @@ struct bitmap_operations {
void (*set_pages)(void *data, unsigned long pages);
void (*free)(void *data);
+ int (*register_groups)(struct mddev *mddev);
+ void (*unregister_groups)(struct mddev *mddev);
+
struct attribute_group *group;
};
diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index bf398d7476b3..9b3ea4f1d268 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1561,6 +1561,16 @@ static struct attribute_group md_llbitmap_group = {
.attrs = md_llbitmap_attrs,
};
+static int llbitmap_register_groups(struct mddev *mddev)
+{
+ return sysfs_create_group(&mddev->kobj, &md_llbitmap_group);
+}
+
+static void llbitmap_unregister_groups(struct mddev *mddev)
+{
+ sysfs_remove_group(&mddev->kobj, &md_llbitmap_group);
+}
+
static struct bitmap_operations llbitmap_ops = {
.head = {
.type = MD_BITMAP,
@@ -1597,6 +1607,8 @@ static struct bitmap_operations llbitmap_ops = {
.dirty_bits = llbitmap_dirty_bits,
.write_all = llbitmap_write_all,
+ .register_groups = llbitmap_register_groups,
+ .unregister_groups = llbitmap_unregister_groups,
.group = &md_llbitmap_group,
};
diff --git a/drivers/md/md.c b/drivers/md/md.c
index d3c8f77b4fe3..55a95b227b83 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -683,8 +683,7 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev, bool create_sysfs)
struct bitmap_operations *old = mddev->bitmap_ops;
struct md_submodule_head *head;
- if (mddev->bitmap_id == ID_BITMAP_NONE ||
- (old && old->head.id == mddev->bitmap_id))
+ if (old && old->head.id == mddev->bitmap_id)
return true;
xa_lock(&md_submodule);
@@ -703,8 +702,8 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev, bool create_sysfs)
mddev->bitmap_ops = (void *)head;
xa_unlock(&md_submodule);
- if (create_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops->group) {
- if (sysfs_create_group(&mddev->kobj, mddev->bitmap_ops->group))
+ if (create_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops->register_groups) {
+ if (mddev->bitmap_ops->register_groups(mddev))
pr_warn("md: cannot register extra bitmap attributes for %s\n",
mdname(mddev));
else
@@ -724,8 +723,8 @@ static bool mddev_set_bitmap_ops(struct mddev *mddev, bool create_sysfs)
static void mddev_clear_bitmap_ops(struct mddev *mddev, bool remove_sysfs)
{
if (remove_sysfs && !mddev_is_dm(mddev) && mddev->bitmap_ops &&
- mddev->bitmap_ops->group)
- sysfs_remove_group(&mddev->kobj, mddev->bitmap_ops->group);
+ mddev->bitmap_ops->unregister_groups)
+ mddev->bitmap_ops->unregister_groups(mddev);
mddev->bitmap_ops = NULL;
}
@@ -6610,8 +6609,9 @@ int md_run(struct mddev *mddev)
(unsigned long long)pers->size(mddev, 0, 0) / 2);
err = -EINVAL;
}
- if (err == 0 && pers->sync_request &&
- (mddev->bitmap_info.file || mddev->bitmap_info.offset)) {
+ if (err == 0 && pers->sync_request) {
+ if (mddev->bitmap_info.offset == 0)
+ mddev->bitmap_id = ID_BITMAP_NONE;
err = md_bitmap_create(mddev, true);
if (err)
pr_warn("%s: failed to create bitmap (%d)\n",
--
2.53.0
^ permalink raw reply related
* [PATCH v2 4/5] md: skip ID_BITMAP_NONE when show available bitmap types
From: Su Yue @ 2026-04-07 10:26 UTC (permalink / raw)
To: linux-raid; +Cc: song, xni, linan122, yukuai, heming.zhao, l, Su Yue
In-Reply-To: <20260407102625.5686-1-glass.su@suse.com>
As none_bitmap_ops is introduced, ID_BITMAP_NONE should be skipped while
iterating md submodules, otherwise:
$ cat /sys/block/md0/md/bitmap_type
[none] bitmap llbitmap [none]
Signed-off-by: Su Yue <glass.su@suse.com>
---
drivers/md/md.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 55a95b227b83..20a953676319 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4269,6 +4269,8 @@ bitmap_type_show(struct mddev *mddev, char *page)
xa_for_each(&md_submodule, i, head) {
if (head->type != MD_BITMAP)
continue;
+ if (head->id == ID_BITMAP_NONE)
+ continue;
if (mddev->bitmap_id == head->id)
len += sprintf(page + len, "[%s] ", head->name);
--
2.53.0
^ permalink raw reply related
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