> On Jan 3, 2023, at 9:42 PM, Ming Lei <ming.lei@xxxxxxxxxx> wrote: > > 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? I don’t think the splat is the issue. The issue is the blocking behavior, which is both unconditional and unbounded in time, and (worse) takes place without relinquishing the locks. wait_event_timeout() is therefore not a valid solution IMHO. > >> >> 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. I understand your considerations. My intuition is that just as umount cannot be done while a file is opened and would return -EBUSY, so should deleting the ublock while the ublk is in use. So as I see it, there are 2 possible options for proper deletion of ublk, and actually both can be implemented and distinguished with a new flag (UBLK_F_*): 1. Blocking - similar to the way it is done today, but (hopefully) without holding locks, and with using wait_event_interruptible() instead of wait_event() to allow interruption (and return EINTR if interrupted). 2. Best-effort - returning EBUSY if it cannot be removed. I can imagine use-cases for both, and it would also allow you not to change ubdsrv if you choose so. Does it make sense?
