[PATCH] FUSE: add the async option for the flush/release operation

[PATCH] FUSE: add the async option for the flush/release operation

[Enke Chen]

Hi, Miklos:

This patch adds the async option for the flush/release operation in FUSE.

The async flush/release option allows a FUSE-based application to be terminated
without being blocked in the flush/release operation even in the presence of
complex external interactions. In addition, the async operation can be more
efficient when a large number of fuse-based files is involved.

---
Deadlock Example:

    Process A is a multi-threaded application that interacts with Process B,
    a FUSE-server.


               UNIX-domain socket
    App (A)  -----------------------  FUSE-server (B)
       |                                   |
       |                                   |
       |                                   |
       +-----------------------------------+
               open/flush/release


    When the FUSE-server receives an open and flush/release operations from
    Process A, it would in turn interact with Process A (e.g., coordinating
    shared memory allocation and de-allocation) using the connection-oriented
    UNIX-domain socket.

    A deadlock occurs when Process A is terminating:

      1) As part of process termination (i.e., do_exit() in the kernel), it
         would send "flush/release" to Process B, and wait for its reply due
         to the synchronous nature of the operation.

      2) When Process B receives the "flush/release" request, it would in turn
         send a message to Process A (over the UNIX-domain channel) and wait
         for its reply.

      3) As Process A is terminating, it may not be able to reply to Process B,
         resulting in a deadlock.

   The async flush/release option offers a simple and robust solution to the
   deadlock issue.

   With the async flush/release operation, all the files and sockets in Process
   A can be closed without being blocked, which in turn would un-block the
   operation in Process B using the UNIX-domain socket.
---

Signed-off-by: Enke Chen <enkechen@cisco.com>

Version: 4.7.0_next_20160805

 fs/fuse/file.c            |   39 +++++++++++++++++++++++++++------------
 fs/fuse/fuse_i.h          |    4 ++++
 fs/fuse/inode.c           |    4 +++-
 include/uapi/linux/fuse.h |    7 ++++++-
 4 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index f394aff..7dd144f 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -273,7 +273,8 @@ void fuse_release_common(struct file *file, int opcode)
 	 * synchronous RELEASE is allowed (and desirable) in this case
 	 * because the server can be trusted not to screw up.
 	 */
-	fuse_file_put(ff, ff->fc->destroy_req != NULL);
+	fuse_file_put(ff, (ff->fc->destroy_req != NULL) &&
+		      !ff->fc->async_flush);
 }
 
 static int fuse_open(struct inode *inode, struct file *file)
@@ -394,13 +395,19 @@ static void fuse_sync_writes(struct inode *inode)
 	fuse_release_nowrite(inode);
 }
 
+static void fuse_flush_end(struct fuse_conn *fc, struct fuse_req *req)
+{
+	if (req->out.h.error == -ENOSYS)
+		fc->no_flush = 1;
+}
+
 static int fuse_flush(struct file *file, fl_owner_t id)
 {
 	struct inode *inode = file_inode(file);
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_file *ff = file->private_data;
 	struct fuse_req *req;
-	struct fuse_flush_in inarg;
+	struct fuse_flush_in *inarg;
 	int err;
 
 	if (is_bad_inode(inode))
@@ -423,20 +430,28 @@ static int fuse_flush(struct file *file, fl_owner_t id)
 
 	req = fuse_get_req_nofail_nopages(fc, file);
 	memset(&inarg, 0, sizeof(inarg));
-	inarg.fh = ff->fh;
-	inarg.lock_owner = fuse_lock_owner_id(fc, id);
+	inarg = &req->misc.flush_in;
+	inarg->fh = ff->fh;
+	inarg->lock_owner = fuse_lock_owner_id(fc, id);
 	req->in.h.opcode = FUSE_FLUSH;
 	req->in.h.nodeid = get_node_id(inode);
 	req->in.numargs = 1;
-	req->in.args[0].size = sizeof(inarg);
-	req->in.args[0].value = &inarg;
-	__set_bit(FR_FORCE, &req->flags);
-	fuse_request_send(fc, req);
-	err = req->out.h.error;
-	fuse_put_request(fc, req);
-	if (err == -ENOSYS) {
-		fc->no_flush = 1;
+	req->in.args[0].size = sizeof(struct fuse_flush_in);
+	req->in.args[0].value = inarg;
+	if (fc->async_flush) {
+		req->end = fuse_flush_end;
+		__set_bit(FR_BACKGROUND, &req->flags);
+		fuse_request_send_background(fc, req);
 		err = 0;
+	} else {
+		__set_bit(FR_FORCE, &req->flags);
+		fuse_request_send(fc, req);
+		err = req->out.h.error;
+		fuse_put_request(fc, req);
+		if (err == -ENOSYS) {
+			fc->no_flush = 1;
+			err = 0;
+		}
 	}
 	return err;
 }
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index d98d8cc..f212cdd 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -350,6 +350,7 @@ struct fuse_req {
 			struct fuse_req *next;
 		} write;
 		struct fuse_notify_retrieve_in retrieve_in;
+		struct fuse_flush_in flush_in;
 	} misc;
 
 	/** page vector */
@@ -624,6 +625,9 @@ struct fuse_conn {
 	/** Is lseek not implemented by fs? */
 	unsigned no_lseek:1;
 
+	/** Does the filesystem want async flush? */
+	unsigned async_flush:1;
+
 	/** The number of requests waiting for completion */
 	atomic_t num_waiting;
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 4e05b51..2d031b1 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -910,6 +910,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_req *req)
 				fc->writeback_cache = 1;
 			if (arg->flags & FUSE_PARALLEL_DIROPS)
 				fc->parallel_dirops = 1;
+			if (arg->flags & FUSE_ASYNC_FLUSH)
+				fc->async_flush = 1;
 			if (arg->time_gran && arg->time_gran <= 1000000000)
 				fc->sb->s_time_gran = arg->time_gran;
 		} else {
@@ -941,7 +943,7 @@ static void fuse_send_init(struct fuse_conn *fc, struct fuse_req *req)
 		FUSE_FLOCK_LOCKS | FUSE_HAS_IOCTL_DIR | FUSE_AUTO_INVAL_DATA |
 		FUSE_DO_READDIRPLUS | FUSE_READDIRPLUS_AUTO | FUSE_ASYNC_DIO |
 		FUSE_WRITEBACK_CACHE | FUSE_NO_OPEN_SUPPORT |
-		FUSE_PARALLEL_DIROPS;
+		FUSE_PARALLEL_DIROPS | FUSE_ASYNC_FLUSH;
 	req->in.h.opcode = FUSE_INIT;
 	req->in.numargs = 1;
 	req->in.args[0].size = sizeof(*arg);
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 27e1736..76087d3 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -108,6 +108,9 @@
  *
  *  7.25
  *  - add FUSE_PARALLEL_DIROPS
+ *
+ *  7.26
+ *  - add FUSE_ASYNC_FLUSH
  */
 
 #ifndef _LINUX_FUSE_H
@@ -143,7 +146,7 @@
 #define FUSE_KERNEL_VERSION 7
 
 /** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 25
+#define FUSE_KERNEL_MINOR_VERSION 26
 
 /** The node ID of the root inode */
 #define FUSE_ROOT_ID 1
@@ -238,6 +241,7 @@ struct fuse_file_lock {
  * FUSE_WRITEBACK_CACHE: use writeback cache for buffered writes
  * FUSE_NO_OPEN_SUPPORT: kernel supports zero-message opens
  * FUSE_PARALLEL_DIROPS: allow parallel lookups and readdir
+ * FUSE_ASYNC_FLUSH: asynchronous flush and release
  */
 #define FUSE_ASYNC_READ		(1 << 0)
 #define FUSE_POSIX_LOCKS	(1 << 1)
@@ -258,6 +262,7 @@ struct fuse_file_lock {
 #define FUSE_WRITEBACK_CACHE	(1 << 16)
 #define FUSE_NO_OPEN_SUPPORT	(1 << 17)
 #define FUSE_PARALLEL_DIROPS    (1 << 18)
+#define FUSE_ASYNC_FLUSH	(1 << 19)
 
 /**
  * CUSE INIT request/reply flags

Re: [PATCH] FUSE: add the async option for the flush/release operation

[Miklos Szeredi]

On Wed, Aug 10, 2016 at 5:26 AM, Enke Chen <enkechen@cisco.com> wrote:
> Hi, Miklos:
>
> This patch adds the async option for the flush/release operation in FUSE.
>
> The async flush/release option allows a FUSE-based application to be terminated
> without being blocked in the flush/release operation even in the presence of
> complex external interactions. In addition, the async operation can be more
> efficient when a large number of fuse-based files is involved.
>
> ---
> Deadlock Example:
>
>     Process A is a multi-threaded application that interacts with Process B,
>     a FUSE-server.
>
>
>                UNIX-domain socket
>     App (A)  -----------------------  FUSE-server (B)
>        |                                   |
>        |                                   |
>        |                                   |
>        +-----------------------------------+
>                open/flush/release

Why would the fuse server want to communicate with the app (using
other than the filesystem)?

Thanks,
Miklos

Re: [PATCH] FUSE: add the async option for the flush/release operation

[Enke Chen]

Hi, Miklos:

On 8/9/16 11:52 PM, Miklos Szeredi wrote:
> On Wed, Aug 10, 2016 at 5:26 AM, Enke Chen <enkechen@cisco.com> wrote:
>> Hi, Miklos:
>>
>> This patch adds the async option for the flush/release operation in FUSE.
>>
>> The async flush/release option allows a FUSE-based application to be terminated
>> without being blocked in the flush/release operation even in the presence of
>> complex external interactions. In addition, the async operation can be more
>> efficient when a large number of fuse-based files is involved.
>>
>> ---
>> Deadlock Example:
>>
>>     Process A is a multi-threaded application that interacts with Process B,
>>     a FUSE-server.
>>
>>
>>                UNIX-domain socket
>>     App (A)  -----------------------  FUSE-server (B)
>>        |                                   |
>>        |                                   |
>>        |                                   |
>>        +-----------------------------------+
>>                open/flush/release
> 
> Why would the fuse server want to communicate with the app (using
> other than the filesystem)?

In this particular case, the other communication channel is used to coordinate
the allocation (with "open") and de-alocation (with "flush/release") of the
shared memory associated with the opened "file".

In general an application may have special handling for the "flush/release"
operation that involve external interactions with one or more other processes,
and that is where this "async" operation can help.

IMO it would be even better if the "async" operation can be made the default so
folks do not need to worry about this types of deadlocks.  From reading of the
code, it seems that FUSE does "async" release under certain conditions already.

Thanks.  -- Enke

Re: [PATCH] FUSE: add the async option for the flush/release operation

[Miklos Szeredi]

On Wed, Aug 10, 2016 at 6:50 PM, Enke Chen <enkechen@cisco.com> wrote:
> Hi, Miklos:
>
> On 8/9/16 11:52 PM, Miklos Szeredi wrote:
>> On Wed, Aug 10, 2016 at 5:26 AM, Enke Chen <enkechen@cisco.com> wrote:
>>> Hi, Miklos:
>>>
>>> This patch adds the async option for the flush/release operation in FUSE.
>>>
>>> The async flush/release option allows a FUSE-based application to be terminated
>>> without being blocked in the flush/release operation even in the presence of
>>> complex external interactions. In addition, the async operation can be more
>>> efficient when a large number of fuse-based files is involved.
>>>
>>> ---
>>> Deadlock Example:
>>>
>>>     Process A is a multi-threaded application that interacts with Process B,
>>>     a FUSE-server.
>>>
>>>
>>>                UNIX-domain socket
>>>     App (A)  -----------------------  FUSE-server (B)
>>>        |                                   |
>>>        |                                   |
>>>        |                                   |
>>>        +-----------------------------------+
>>>                open/flush/release
>>
>> Why would the fuse server want to communicate with the app (using
>> other than the filesystem)?
>
> In this particular case, the other communication channel is used to coordinate
> the allocation (with "open") and de-alocation (with "flush/release") of the
> shared memory associated with the opened "file".
>
> In general an application may have special handling for the "flush/release"
> operation that involve external interactions with one or more other processes,

Sure, it can interact with other processes, but *not* with the process
accessing the filesystem.  There are no end of possible deadlocks that
way, and it goes straight against the design philosophy of kernel
interfaces.

Maybe I'm missing something, but this sure looks like a case of bad
system design.   You need to give more details to convince me
otherwise.

> and that is where this "async" operation can help.
>
> IMO it would be even better if the "async" operation can be made the default so
> folks do not need to worry about this types of deadlocks.  From reading of the
> code, it seems that FUSE does "async" release under certain conditions already.

Release being async is okay, and is the default for non-fuseblk
mounts.  For fuseblk it is not the default, because it could cause
problems:

  5a18ec176c93 ("fuse: fix hang of single threaded fuseblk filesystem")

We could make release optionally async for fuseblk.

Flush being async is not okay, it needs to return an error value.  If
the filesystem does not want to return an error value, it may omit a
flush implementation completely.

Thanks,
Miklos

Re: [PATCH] FUSE: add the async option for the flush/release operation

[Enke Chen]

Hi, Miklos:

Thanks for your reply and explanation. Please see my comments below.

On 8/15/16 2:36 AM, Miklos Szeredi wrote:
> On Wed, Aug 10, 2016 at 6:50 PM, Enke Chen <enkechen@cisco.com> wrote:
>> Hi, Miklos:
>>
>> On 8/9/16 11:52 PM, Miklos Szeredi wrote:
>>> On Wed, Aug 10, 2016 at 5:26 AM, Enke Chen <enkechen@cisco.com> wrote:
>>>> Hi, Miklos:
>>>>
>>>> This patch adds the async option for the flush/release operation in FUSE.
>>>>
>>>> The async flush/release option allows a FUSE-based application to be terminated
>>>> without being blocked in the flush/release operation even in the presence of
>>>> complex external interactions. In addition, the async operation can be more
>>>> efficient when a large number of fuse-based files is involved.
>>>>
>>>> ---
>>>> Deadlock Example:
>>>>
>>>>     Process A is a multi-threaded application that interacts with Process B,
>>>>     a FUSE-server.
>>>>
>>>>
>>>>                UNIX-domain socket
>>>>     App (A)  -----------------------  FUSE-server (B)
>>>>        |                                   |
>>>>        |                                   |
>>>>        |                                   |
>>>>        +-----------------------------------+
>>>>                open/flush/release
>>>
>>> Why would the fuse server want to communicate with the app (using
>>> other than the filesystem)?
>>
>> In this particular case, the other communication channel is used to coordinate
>> the allocation (with "open") and de-alocation (with "flush/release") of the
>> shared memory associated with the opened "file".
>>
>> In general an application may have special handling for the "flush/release"
>> operation that involve external interactions with one or more other processes,
> 
> Sure, it can interact with other processes, but *not* with the process
> accessing the filesystem.  There are no end of possible deadlocks that
> way, and it goes straight against the design philosophy of kernel
> interfaces.
> 
> Maybe I'm missing something, but this sure looks like a case of bad
> system design.   You need to give more details to convince me
> otherwise.

On the "system design", I agree and I certainly prefer simpler external
interactions among processes.  But we do not always know what libs would
do, and when another interaction would be introduced.

> 
>> and that is where this "async" operation can help.
>>
>> IMO it would be even better if the "async" operation can be made the default so
>> folks do not need to worry about this types of deadlocks.  From reading of the
>> code, it seems that FUSE does "async" release under certain conditions already.
> 
> Release being async is okay, and is the default for non-fuseblk
> mounts.  For fuseblk it is not the default, because it could cause
> problems:
> 
>   5a18ec176c93 ("fuse: fix hang of single threaded fuseblk filesystem")
> 
> We could make release optionally async for fuseblk.
> 
> Flush being async is not okay, it needs to return an error value.  If
> the filesystem does not want to return an error value, it may omit a
> flush implementation completely.

I was not aware that the release operation is already async by default for
non-fuseblk mounts. That is really what we need in order to break the deadlock
in the example. (I had the "flush" operation there for the sake of completeness,
and not out of necessity.)

The deadlock I described was an old problem from several years ago. I just
re-ran the test with the newer kernel (3.14 and 4.7), and confirmed that the
issue is gone with the "release" operation being async.  The deadlock was also
reproduced after changing the release operation from "async" to "sync" in the
fuse module.

So the patch is no longer needed unless we want to modify it to support the
"async release" for the fuseblk.

Thanks again.  -- Enke