On Wed, Mar 15, 2023 at 08:48:40AM -0700, Christoph Hellwig wrote: > On Sun, Mar 12, 2023 at 07:10:31PM +0000, Alyssa Ross wrote: > > + * 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); > > What worries me here is that you move the partition re-read out of > the exclusive claim, which is another potentially user visible > change (and user visible behavior changes are a field of landmines > in loop as you have noticed). Makes sense. > But in the end we only need to suppress the events until Lo_Bound > is set. So something like the patch below that reduces the no even > critical section might do the job? If you say so! I had trouble understanding which parts of the function uevents needed to be suppressed for, so I was trying to move as little as possible out of that section. What happens next? I'm still getting up to speed on the kernel development process — will you submit this as a patch with a patch body and a S-o-b? Or am I supposed to do something with it? I know enough to know that I should give you a: Tested-by: Alyssa Ross <hi@xxxxxxxxx> > 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)
Attachment:
signature.asc
Description: PGP signature
