On Thu, 23 Mar 2023 at 12:04, Bernd Schubert <bschubert@xxxxxxx> wrote: > > Thanks for looking at these patches! > > I'm adding in Ming Lei, as I had taken several ideas from ublkm I guess > I also should also explain in the commit messages and code why it is > done that way. > > On 3/23/23 11:27, Miklos Szeredi wrote: > > On Tue, 21 Mar 2023 at 02:11, Bernd Schubert <bschubert@xxxxxxx> wrote: > >> > >> This adds a delayed work queue that runs in intervals > >> to check and to stop the ring if needed. Fuse connection > >> abort now waits for this worker to complete. > > > > This seems like a hack. Can you explain what the problem is? > > > > The first thing I notice is that you store a reference to the task > > that initiated the ring creation. This already looks fishy, as the > > ring could well survive the task (thread) that created it, no? > > You mean the currently ongoing work, where the daemon can be restarted? > Daemon restart will need some work with ring communication, I will take > care of that once we have agreed on an approach. [Also added in Alexsandre]. > > fuse_uring_stop_mon() checks if the daemon process is exiting and and > looks at fc->ring.daemon->flags & PF_EXITING - this is what the process > reference is for. Okay, so you are saying that the lifetime of the ring is bound to the lifetime of the thread that created it? Why is that? I'ts much more common to bind a lifetime of an object to that of an open file. io_uring_setup() will do that for example. It's much easier to hook into the destruction of an open file, than into the destruction of a process (as you've observed). And the way you do it is even more confusing as the ring is destroyed not when the process is destroyed, but when a specific thread is destroyed, making this a thread specific behavior that is probably best avoided. So the obvious solution would be to destroy the ring(s) in fuse_dev_release(). Why wouldn't that work? Thanks, Miklos
