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, I've moved 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@xxxxxxxxx> Fixes: 3448914e8cc5 ("loop: Add LOOP_CONFIGURE ioctl") --- v1: https://lore.kernel.org/linux-block/20230221222847.607096-1-hi@xxxxxxxxx/ v1 was an RFC, because I was looking for advice on how to handle distinguishing between LOOP_SET_FD with non-zero max_part (in which case partscan will be true, but a uevent should not be emitted), and LOOP_CONFIGURE (where a uevent should be emitted). I didn't hear anything, but I did some experimentation of my own, and adding a partscan_uevent parameter to distinguish between LOOP_SET_FD and LOOP_CONFIGURE feels like the least bad solution to me. drivers/block/loop.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/drivers/block/loop.c b/drivers/block/loop.c index 839373451c2b..f00a0209b522 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -992,7 +992,8 @@ loop_set_status_from_info(struct loop_device *lo, static int loop_configure(struct loop_device *lo, fmode_t mode, struct block_device *bdev, - const struct loop_config *config) + const struct loop_config *config, + bool partscan_uevent) { struct file *file = fget(config->fd); struct inode *inode; @@ -1110,15 +1111,21 @@ static int loop_configure(struct loop_device *lo, fmode_t mode, clear_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state); loop_global_unlock(lo, is_loop); - if (partscan) - loop_reread_partitions(lo); if (!(mode & FMODE_EXCL)) bd_abort_claiming(bdev, loop_configure); + /* + * Now that we are done, reread the partitions with uevent + * re-enabled if appropriate to let userspace know about the + * changes. + */ + dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), !partscan_uevent); + if (partscan) + loop_reread_partitions(lo); + dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0); + 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; out_unlock: @@ -1130,6 +1136,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); + dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0); goto done; } @@ -1547,7 +1554,7 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode, memset(&config, 0, sizeof(config)); config.fd = arg; - return loop_configure(lo, mode, bdev, &config); + return loop_configure(lo, mode, bdev, &config, false); } case LOOP_CONFIGURE: { struct loop_config config; @@ -1555,7 +1562,7 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode, if (copy_from_user(&config, argp, sizeof(config))) return -EFAULT; - return loop_configure(lo, mode, bdev, &config); + return loop_configure(lo, mode, bdev, &config, true); } case LOOP_CHANGE_FD: return loop_change_fd(lo, bdev, arg); -- 2.37.1