| Message ID | 20230320125430.55367-1-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | loop: LOOP_CONFIGURE: send uevents for partitions | expand |
Alyssa, On 3/20/23 05:54, Christoph Hellwig wrote: > From: Alyssa Ross <hi@alyssa.is> > > LOOP_CONFIGURE is, as far as I understand it, supposed to be a way to > combine LOOP_SET_FD and LOOP_SET_STATUS64 into a single syscall. When > using LOOP_SET_FD+LOOP_SET_STATUS64, a single uevent would be sent for > each partition found on the loop device after the second ioctl(), but > when using LOOP_CONFIGURE, no such uevent was being sent. > > In the old setup, uevents are disabled for LOOP_SET_FD, but not for > LOOP_SET_STATUS64. This makes sense, as it prevents uevents being > sent for a partially configured device during LOOP_SET_FD - they're > only sent at the end of LOOP_SET_STATUS64. But for LOOP_CONFIGURE, > uevents were disabled for the entire operation, so that final > notification was never issued. To fix this, reduce the critical > section to exclude the loop_reread_partitions() call, which causes > the uevents to be issued, to after uevents are re-enabled, matching > the behaviour of the LOOP_SET_FD+LOOP_SET_STATUS64 combination. > > I noticed this because Busybox's losetup program recently changed from > using LOOP_SET_FD+LOOP_SET_STATUS64 to LOOP_CONFIGURE, and this broke > my setup, for which I want a notification from the kernel any time a > new partition becomes available. > > Signed-off-by: Alyssa Ross <hi@alyssa.is> > [hch: reduced the critical section] > Signed-off-by: Christoph Hellwig <hch@lst.de> > Fixes: 3448914e8cc5 ("loop: Add LOOP_CONFIGURE ioctl") > Can you please add the testcase in the blktests to test the above mentioned behavior ? -ck
On Mon, 20 Mar 2023 13:54:30 +0100, Christoph Hellwig wrote: > LOOP_CONFIGURE is, as far as I understand it, supposed to be a way to > combine LOOP_SET_FD and LOOP_SET_STATUS64 into a single syscall. When > using LOOP_SET_FD+LOOP_SET_STATUS64, a single uevent would be sent for > each partition found on the loop device after the second ioctl(), but > when using LOOP_CONFIGURE, no such uevent was being sent. > > In the old setup, uevents are disabled for LOOP_SET_FD, but not for > LOOP_SET_STATUS64. This makes sense, as it prevents uevents being > sent for a partially configured device during LOOP_SET_FD - they're > only sent at the end of LOOP_SET_STATUS64. But for LOOP_CONFIGURE, > uevents were disabled for the entire operation, so that final > notification was never issued. To fix this, reduce the critical > section to exclude the loop_reread_partitions() call, which causes > the uevents to be issued, to after uevents are re-enabled, matching > the behaviour of the LOOP_SET_FD+LOOP_SET_STATUS64 combination. > > [...] Applied, thanks! [1/1] loop: LOOP_CONFIGURE: send uevents for partitions (no commit info) Best regards,
diff --git a/drivers/block/loop.c b/drivers/block/loop.c index 839373451c2b7d..9d61c027185141 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -1010,9 +1010,6 @@ static int loop_configure(struct loop_device *lo, fmode_t mode, /* This is safe, since we have a reference from open(). */ __module_get(THIS_MODULE); - /* suppress uevents while reconfiguring the device */ - dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 1); - /* * If we don't hold exclusive handle for the device, upgrade to it * here to avoid changing device under exclusive owner. @@ -1067,6 +1064,9 @@ static int loop_configure(struct loop_device *lo, fmode_t mode, } } + /* suppress uevents while reconfiguring the device */ + dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 1); + disk_force_media_change(lo->lo_disk, DISK_EVENT_MEDIA_CHANGE); set_disk_ro(lo->lo_disk, (lo->lo_flags & LO_FLAGS_READ_ONLY) != 0); @@ -1109,17 +1109,17 @@ static int loop_configure(struct loop_device *lo, fmode_t mode, if (partscan) clear_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state); + /* enable and uncork uevent now that we are done */ + dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0); + loop_global_unlock(lo, is_loop); if (partscan) loop_reread_partitions(lo); + if (!(mode & FMODE_EXCL)) bd_abort_claiming(bdev, loop_configure); - error = 0; -done: - /* enable and uncork uevent now that we are done */ - dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0); - return error; + return 0; out_unlock: loop_global_unlock(lo, is_loop); @@ -1130,7 +1130,7 @@ static int loop_configure(struct loop_device *lo, fmode_t mode, fput(file); /* This is safe: open() is still holding a reference. */ module_put(THIS_MODULE); - goto done; + return error; } static void __loop_clr_fd(struct loop_device *lo, bool release)