lkml.org 
[lkml]   [2014]   [Aug]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 0/5] fuse: close file synchronously (v2)
On 08/13/2014 04:44 PM, Miklos Szeredi wrote:
> On Fri, Jun 6, 2014 at 3:27 PM, Maxim Patlasov <MPatlasov@parallels.com> wrote:
>> Hi,
>>
>> There is a long-standing demand for synchronous behaviour of fuse_release:
>>
>> http://sourceforge.net/mailarchive/message.php?msg_id=19343889
>> http://sourceforge.net/mailarchive/message.php?msg_id=29814693
>>
>> A year ago Avati and me explained why such a feature would be useful:
>>
>> http://sourceforge.net/mailarchive/message.php?msg_id=29889055
>> http://sourceforge.net/mailarchive/message.php?msg_id=29867423
>>
>> In short, the problem is that fuse_release (that's called on last user
>> close(2)) sends FUSE_RELEASE to userspace and returns without waiting for
>> ACK from userspace. Consequently, there is a gap when user regards the
>> file released while userspace fuse is still working on it. An attempt to
>> access the file from another node leads to complicated synchronization
>> problems because the first node still "holds" the file.
> Tying RELEASE to close(2) is not going to work. Look at all the
> places that call fput() (or fdput() in recent kernels), those are all
> potential triggers for RELEASE, some realistic, some not quite, but
> all are certainly places that a synchronous release could block
> *instead* of close.
>
> Which just means, that close will still be asynchronous with release
> some of the time. So it's not clear to me what is to be gained from
> this patchset.

The patch-set doesn't tie RELEASE to close(2), it ensures that we report
to user space exactly the last fput(). That's correct because this is
exactly the moment when any file system with sharing mode connected to
open/close must drop sharing mode. This is the case even for some local
filesystems, for example, ntfs-3g.

Could you please look closely at your commit
5a18ec176c934ca1bc9dc61580a5e0e90a9b5733. It actually implemented two
different things: 1) synchronous release and 2) delayed path_put. The
latter was well explained by the comment:

> /*
> * If this is a fuseblk mount, then it's possible that
> * releasing the path will result in releasing the
> * super block and sending the DESTROY request. If
> * the server is single threaded, this would hang.
> * For this reason do the path_put() in a separate
> * thread.
> */

So it's clear why the delay needed and why it's bound to fuseblk
condition. But synchronous close was made under the same condition,
which is obviously wrong. I understand why you made that decision in
2011: otherwise, we could block in a wrong context (last decrement of
ff->count might happen in scope of read-ahead or mmap-ed writeback). But
now, with the approach implemented in this patch-set, this is impossible
-- we wait for completion of all async operations before triggering
synchronous release. Thus the patch-set untie a functionality which
already existed before (synchronous release) from wrong condition
(fuseblk mount) putting it under well-defined control (FUSE_CLOSE_WAIT).

Thanks,
Maxim


\
 
 \ /
  Last update: 2014-08-14 14:41    [W:0.246 / U:0.248 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site