Re: [PATCH V2] ublk: fix deadlock when reading partition table

[Ming Lei <ming.lei@redhat.com>]

On Tue, Dec 16, 2025 at 08:19:15PM -0700, Jens Axboe wrote:
> On 12/16/25 8:09 PM, Ming Lei wrote:
> > On Tue, Dec 16, 2025 at 10:57:25AM -0700, Jens Axboe wrote:
> >> On 12/16/25 8:03 AM, Jens Axboe wrote:
> >>>> The issue for ublk is actually triggered by something abnormal: submit AIO
> >>>> & close(ublk disk) in client application, then fput() is called when the
> >>>> submitted AIO is done, it will cause deferred fput handler to wq for any block
> >>>> IO completed from irq handler.
> >>>
> >>> My suggested logic is something ala this in bdev_release():
> >>>
> >>> 	if (current->flags & PF_KTHREAD) {
> >>> 		mutex_lock(&disk->open_mutex);
> >>> 	} else {
> >>> 		if (!mutex_trylock(&disk->open_mutex)) {
> >>> 			deferred_put(file);
> >>> 			return;
> >>> 		}
> >>> 	}
> >>>
> >>> and that's about it.
> >>
> >> I took a look at the bug report, and now it makes more sense to me -
> >> this is an aio only issue, as it does fput() from ->bi_end_io() context.
> >> That's pretty nasty, as you don't really know what context that might
> >> be, both in terms of irq/bh state, but also in terms of locks. The
> >> former fput() does work around.
> >>
> >> Why isn't the fix something as simple as the below, with your comment
> >> added on top? I'm not aware of anyone else that would do fput off
> >> ->bi_end_io, so we migt as well treat the source of the issue rather
> >> than work around it in ublk. THAT makes a lot more sense to me.
> > 
> > It doesn't matter if fput is called from ->bi_end_io() directly, it can
> > be triggered on io-uring indirectly too, in which fput() is called from
> > __io_submit_flush_completions() in case of non-registerd file.
> 
> Because of the work-around in io_req_post_cqe()? Or just because of
> !DEFER_TASKRUN?

When fput() is called from __io_submit_flush_completions(), its release
handler will be deferred to run task work, where the current task
is blocked because of ->open_mutex.

It is actually one ublk specific issue which relies on the current task
for handling IO and providing forward progress, so cause deadlock since
reading partition table(with ->open_mutex) requires the task for handling IO.

> 
> The real problem is holding ->open_mutex over IO, and then also
> requiring it to put the file as well. bdev_release() should be able to
> work-around that, rather than need anyone to paper around it.

deferred bdev_release is not safe, for example of suggestion:

        if (current->flags & PF_KTHREAD) {
                mutex_lock(&disk->open_mutex);
        } else {
                if (!mutex_trylock(&disk->open_mutex)) {
                        deferred_put(file);
                        return;
                }
        }

deferred_put(file) will cause disk released after returning to userspace.

Yes, __fput_deferred() allows that in case of in_interrupt(), which usually
means one abnormal application(close(disk) before completing/handling IO),
but it will cause normal application to release disk after returning to
userspace, it may cause -EBUSY for following syscall.


Thanks,
Ming