Re: [PATCH 2/2] loop: Perform __loop_clr_fd() after disk->open_mutex is dropped.

[Bart Van Assche <bvanassche@acm.org>]

On 9/11/26 6:22 PM, Tetsuo Handa wrote:
> On 2026/09/12 10:02, Bart Van Assche wrote:
>> On 9/11/26 3:18 PM, Tetsuo Handa wrote:
>>> On 2026/09/12 5:03, Bart Van Assche wrote:
>>>> On 9/10/26 2:44 AM, Tetsuo Handa wrote:
>>>>> On 2026/09/10 4:33, Bart Van Assche wrote:
>>>>>> __loop_clr_fd() is queued from the lo_post_release() callback and hence
>>>>>> may be called concurrently with or after another thread has called
>>>>>> bdev_open(). Hence, lo->lo->state should be checked instead of assuming
>>>>>> that it equals Lo_rundown.
>>>>>
>>>>> No, __loop_clr_fd() is queued from the lo_release() callback.
>>>>
>>>> Yes, it is *queued* from the lo_release() callback function but there is
>>>> no guarantee that __loop_clr_fd() has started before bdev_open() is
>>>> called again.
>>>
>>> Since lo->lo_state was set to Lo_rundown by lo_release(), lo_open() will return -ENXIO.
>>> What can go wrong if bdev_open() is called again before __loop_clr_fd() starts?
>>
>> This breaks LO_FLAGS_AUTOCLEAR, isn't it?
> 
> Why do you think so?
> 
>    App1                   App2                   system_long_wq
>    Calls lo_release().
>      Schedules __loop_clr_fd().
>                           Calls lo_open() but fails with -ENXIO.
>                                                  Starts __loop_clr_fd().
>    Calls lo_post_release().
>      Starts waiting for completion of __loop_clr_fd().
>                                                  Finishes __loop_clr_fd().
>      Finishes waiting for completion of __loop_clr_fd().
> 
>    Calls lo_open() again and succeeds.
>                           Calls lo_open() again and succeeds.
> 
> App1's open() after close() is succeeding.
> App2's open() being temporarily failing with -ENXIO should be acceptable.
> If App2 wants to avoid repeatedly failing with -ENXIO, App2 should use
> ioctl(LOOP_CTL_GET_FREE) before open().

Userspace applications and tests (such as mount/umount, losetup, 
systemd, and xfstests) expect that:
1. Teardown of a loop device with LO_FLAGS_AUTOCLEAR completes
    synchronously during the final close() (in lo_release()), releasing
    the backing file references (fput()).
2. Calling open() immediately after close() succeeds (allowing the loop
    device to be immediately reallocated, reopened, or configured in
    Lo_unbound state).

There is evidence of this in the history of drivers/block/loop.c:
* The Asynchronous Autoclear Regression & Revert (Commits 322c4293ecc5
   and bf23747ee053) In December 2021, commit 322c4293ecc5
   ("loop: make autoclear operation asynchronous") attempted to break a
   circular lock dependency by offloading autoclear (__loop_clr_fd())
   from lo_release() to a workqueue (system_long_wq).
* In February 2022, commit bf23747ee053 ("loop: revert 'make autoclear
   operation asynchronous'") had to revert that change after xfstests
   broke. The commit message explicitly states:

   "The kernel test robot is reporting that xfstest which does

     umount ext2 on xfs
     umount xfs

   sequence started failing, for commit 322c4293ecc58110 ("loop: make
   autoclear operation asynchronous") removed a guarantee that fput() of
   backing file is processed before lo_release() from close() returns to
   user mode."

   When autoclear was asynchronous, close() returned before the device
   was unbound and before the backing file was released, breaking
   immediate reuse and subsequent filesystem unmounts.

Bart.