Messages in this thread | | | Date | Mon, 25 Aug 2014 19:58:24 +0400 | From | Maxim Patlasov <> | Subject | Re: [PATCH 5/6] fuse: fix synchronous case of fuse_file_put() |
| |
On 08/22/2014 06:08 PM, Miklos Szeredi wrote: > On Thu, Aug 21, 2014 at 6:09 PM, Maxim Patlasov <MPatlasov@parallels.com> wrote: >> If fuse_file_put() is called with sync==true, the user may be blocked for >> a while, until userspace ACKs our FUSE_RELEASE request. This blocking must be >> uninterruptible. Otherwise request could be interrupted, but file association >> in user space remains. >> >> Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com> >> --- >> fs/fuse/file.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/fs/fuse/file.c b/fs/fuse/file.c >> index cd55488..b92143a 100644 >> --- a/fs/fuse/file.c >> +++ b/fs/fuse/file.c >> @@ -136,6 +136,10 @@ static void fuse_file_put(struct fuse_file *ff, bool sync) >> path_put(&req->misc.release.path); >> fuse_put_request(ff->fc, req); >> } else if (sync) { >> + /* Must force. Otherwise request could be interrupted, >> + * but file association in user space remains. >> + */ >> + req->force = 1; >> req->background = 0; >> fuse_request_send(ff->fc, req); >> path_put(&req->misc.release.path); >> > > Some thought needs to go into this: if RELEASE is interrupted, then > we should possibly allow that, effectively backgrounding the request. > > The synchronous nature is just an optimization and we really don't > know where we are being interrupted, possibly in a place which very > much *should* allow interruption.
A fuse daemon who explicitly enables the feature (synchronous release) would definitely want non-interruptible behaviour of last fput. Otherwise, it would face the same problem that the feature tries to resolve: an application was killed and exited, but there is no way to determine why actual processing of RELEASE will be completed.
As for fuseblk mounts, I'm not so sure. I believed the lack of force=1 was a bug and my patch fixes it. If you think it's safer to preserve old behaviour, I could set "force" conditionally. May be you could explain in more details why you think we should allow interruption somewhere. Any examples or use cases? Btw, fuse_flush also uses force=1. Do you concerns deal with it as well?
> > Also fuse really should distinguish fatal and non-fatal interruptions > and handle them accordingly...
Do you think it's worthy to elaborate this in the scope of "synchronous release" feature?
Thanks, Maxim
| |