Re: [PATCHv2] md: clear MD_CLOSING when array_state_store() bails on interrupted lock
Jinpu Wang <[email protected]> Tue, 4 Aug 2026 20:44:17 +0200
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-raid |
|---|---|
| Message-ID | <CAMGffE=gipMNJkTLoSBj26mGWsTmp2aiFKM3yRe12BdjwWftBw@mail.gmail.com> |
Hi Team, Regarding the pre-existing issue reported by the bot, it is not related to this fix and cannot be triggered in normal cases, so I will skip it for now. Linux RAID folks, do you have a chance to check this patch? It fixes a problem we hit by chance. Thanks, Jinpu On Fri, Jul 17, 2026 at 7:25=E2=80=AFAM <[email protected]> wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issu= e(s) to consider: > > Pre-existing issues: > - [High] Calling `restart_array()` with the `mddev->lock` spinlock held c= an sleep because it clears the disk read-only flag, which triggers a `GFP_K= ERNEL` memory allocation for a uevent. > -- > > --- Patch [1]: [PATCHv2] md: clear MD_CLOSING when array_state_store() ba= ils on interrupted lock --- > commit 3ae9b81fbeee45b412994652c2c3b3ac1b35d177 > Author: Jack Wang <[email protected]> > > md: clear MD_CLOSING when array_state_store() bails on interrupted lock > > This commit ensures that MD_CLOSING is correctly cleared if > mutex_lock_interruptible() fails while writing to array_state. It introdu= ces > a common exit label and a boolean flag to track whether the flag was actu= ally > set by the current thread. > > > diff --git a/drivers/md/md.c b/drivers/md/md.c > > index d1465bcd86c81..7b7a4d9259079 100644 > > --- a/drivers/md/md.c > > +++ b/drivers/md/md.c > [ ... ] > > @@ -4696,7 +4698,7 @@ array_state_store(struct mddev *mddev, const char= *buf, size_t len) > > } > > err =3D mddev_lock(mddev); > > if (err) > > - return err; > > + goto out_clear_closing; > > [Severity: High] > This is a pre-existing issue, but just above this code in array_state_sto= re(), > there is a fast path that can sleep while holding a spinlock. > > If a privileged user sets the md device to read-only using > blockdev --setro /dev/md0 and then writes 'active' to > /sys/block/md0/md/array_state, the code acquires the mddev->lock spinlock > and calls restart_array(): > > drivers/md/md.c:array_state_store() { > ... > spin_lock(&mddev->lock); > if (st =3D=3D active) { > restart_array(mddev); > ... > } > > restart_array() unconditionally calls set_disk_ro() to ensure the disk is > read-write: > > drivers/md/md.c:restart_array() { > ... > set_disk_ro(disk, 0); > ... > } > > Because the disk was set to read-only, set_disk_ro() clears the flag and > triggers a uevent: > > block/genhd.c:set_disk_ro() { > ... > set_disk_ro_uevent(disk, read_only); > ... > } > > This calls into kobject_uevent_env(), which performs an unconditional > GFP_KERNEL allocation: > > lib/kobject_uevent.c:kobject_uevent_env() { > ... > devpath =3D kobject_get_path(kobj, GFP_KERNEL); > ... > } > > Can this sequence cause a sleep-in-atomic bug and potentially lead to a > deadlock or panic? > > -- > Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260717051233.38= [email protected]?part=3D1