Messages in this thread | | | Date | Mon, 10 Aug 2020 15:37:17 -0400 | From | Vivek Goyal <> | Subject | Re: [PATCH v2 18/20] fuse: Release file in process context |
| |
On Mon, Aug 10, 2020 at 10:29:13AM +0200, Miklos Szeredi wrote: > On Fri, Aug 7, 2020 at 9:55 PM Vivek Goyal <vgoyal@redhat.com> wrote: > > > > fuse_file_put(sync) can be called with sync=true/false. If sync=true, > > it waits for release request response and then calls iput() in the > > caller's context. If sync=false, it does not wait for release request > > response, frees the fuse_file struct immediately and req->end function > > does the iput(). > > > > iput() can be a problem with DAX if called in req->end context. If this > > is last reference to inode (VFS has let go its reference already), then > > iput() will clean DAX mappings as well and send REMOVEMAPPING requests > > and wait for completion. (All the the worker thread context which is > > processing fuse replies from daemon on the host). > > > > That means it blocks worker thread and it stops processing further > > replies and system deadlocks. > > Is this reasoning specific to DAX? Seems to me this is a virtio-fs > specific issue.
Hi Miklos,
I am looking at this patch closely and maybe we don't need it, given current code.
Reason being that looks like for virtiofs now we set ctx->destroy=true and that sets fc->destroy. And if that's set, we don't schedule async req in fuse_release_common();
fuse_release_common() { fuse_file_put(ff, ff->fc->destroy, isdir); }
And if we don't schedule async file put, we don't run the risk of blocking worker thread in evict_inode() with DAX enabled.
I ran blogbench multiple times with this patch reverted and I can't reproduce the deadlock.
So we can drop this patch for now. If need be I will fix it as post merge patch.
Thanks Vivek
| |