Re: Potential hang on ublk_ctrl_del_dev()

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

On Tue, Jan 03, 2023 at 01:47:37PM -0800, Nadav Amit wrote:
> Hello Ming,
> 
> I am trying the ublk and it seems very exciting.
> 
> However, I encounter an issue when I remove a ublk device that is mounted or
> in use.
> 
> In ublk_ctrl_del_dev(), shouldn’t we *not* wait if ublk_idr_freed() is false?
> It seems to me that it is saner to return -EBUSY in such a case and let
> userspace deal with the results.
> 
> For instance, if I run the following (using ubdsrv):
> 
>  $ mkfs.ext4 /dev/ram0
>  $ ./ublk add -t loop -f /dev/ram0
>  $ sudo mount /dev/ublkb0 tmp
>  $ sudo ./ublk del -a
> 
> ublk_ctrl_del_dev() would not be done until the partition is unmounted, and you
> can get a splat that is similar to the one below.

The splat itself can be avoided easily by replace wait_event with
wait_event_timeout() plus loop, but I guess you think the sync delete
isn't good too?

> 
> What do you say? Would you agree to change the behavior to return -EBUSY?

It is designed in this way from beginning, and I believe it is just for
the sake of simplicity, and one point is that the device number needs
to be freed after 'ublk del' returns.

But if it isn't friendly from user's viewpoint, we can change to return
-EBUSY. One simple solution is to check if the ublk block device
is opened before running any deletion action, if yes, stop to delete it
and return -EBUSY; otherwise go ahead and stop & delete the pair of devices.
And the userspace part(ublk utility) needs update too.

However, -EBUSY isn't perfect too, cause user has to retry the delete
command manually.

thanks,
Ming