On 2022/03/25 2:23, Christoph Hellwig wrote:
> On Thu, Mar 24, 2022 at 11:24:43PM +0900, Tetsuo Handa wrote:
>> On 2022/03/24 23:13, Jan Kara wrote:
>>>> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
>>>> index b3170e8cdbe95..e1eb925d3f855 100644
>>>> --- a/drivers/block/loop.c
>>>> +++ b/drivers/block/loop.c
>>>> @@ -1244,7 +1244,7 @@ static int loop_clr_fd(struct loop_device *lo)
>>>> * <dev>/do something like mkfs/losetup -d <dev> causing the losetup -d
>>>> * command to fail with EBUSY.
>>>> */
>>>> - if (atomic_read(&lo->lo_refcnt) > 1) {
>>>> + if (disk_openers(lo->lo_disk) > 1) {
>>
>> This is sometimes "if (0) {" due to not holding disk->open_mutex.
>
> Yes, as clearly documented in the commit log. In fact turning it
> into an explicit if 0 (that is removing this code) might be a not
> bad idea either - the code was added to work around a
>
> if (lo->lo_refcnt > 1) /* we needed one fd for the ioctl */
> return -EBUSY;
>
> that already did not make much sense to start with (but goes
> back to before git history).
>
> But for now I'd really prefer to stop moving the goalpost further and
> further.
Then, why not kill this code?
drivers/block/loop.c | 29 +++++++----------------------
1 file changed, 7 insertions(+), 22 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 3e636a75c83a..26c8808d02d0 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1205,30 +1205,15 @@ static int loop_clr_fd(struct loop_device *lo)
err = mutex_lock_killable(&lo->lo_mutex);
if (err)
return err;
- if (lo->lo_state != Lo_bound) {
- mutex_unlock(&lo->lo_mutex);
- return -ENXIO;
- }
- /*
- * If we've explicitly asked to tear down the loop device,
- * and it has an elevated reference count, set it for auto-teardown when
- * the last reference goes away. This stops $!~#$@ udev from
- * preventing teardown because it decided that it needs to run blkid on
- * the loopback device whenever they appear. xfstests is notorious for
- * failing tests because blkid via udev races with a losetup
- * <dev>/do something like mkfs/losetup -d <dev> causing the losetup -d
- * command to fail with EBUSY.
- */
- if (atomic_read(&lo->lo_refcnt) > 1) {
- lo->lo_flags |= LO_FLAGS_AUTOCLEAR;
- mutex_unlock(&lo->lo_mutex);
- return 0;
- }
- lo->lo_state = Lo_rundown;
+ if (lo->lo_state != Lo_bound)
+ err = -ENXIO;
+ else
+ lo->lo_state = Lo_rundown;
mutex_unlock(&lo->lo_mutex);
- __loop_clr_fd(lo, false);
- return 0;
+ if (!err)
+ __loop_clr_fd(lo, false);
+ return err;
}
static int
--
2.32.0
