[Devel] [PATCH hci-8.1 2/3] fs/fuse: move the close_wait handling of fuse_release() into a helper

[Liu Kui <kui.liu@virtuozzo.com>]

Move all the close_wait handling of fuse_release() into a new helper
fuse_release_close_wait(), and restore the !close_wait case to the
mainline behavior except the FMODE_WRITE check.

Unify the code paths: unconditionally remove the file from
fi->write_files and wait until we hold the last reference. Decrement
fi->num_openers under the inode lock, which detects the last open.
This fixes two racing releasers both reading num_openers == 0 and
calling kio's file_close twice, possible because the decrement was
done early under fi->lock while the check ran later under the inode
lock:

  releaser A                      releaser B
  ----------                      ----------
  --fi->num_openers  (2 -> 1)
                                  --fi->num_openers  (1 -> 0)
  inode_lock()
  num_openers == 0 -> file_close()
  inode_unlock()
                                  inode_lock()
                                  num_openers == 0 -> file_close()

The atomic decrement under the inode lock also matches the increment
in fuse_open_close_wait() for !writeback_cache files.

https://virtuozzo.atlassian.net/browse/VSTOR-147878

Signed-off-by: Liu Kui <kui.liu@virtuozzo.com>
---
 fs/fuse/file.c | 103 ++++++++++++++++++++++++++-------------------------------
 1 file changed, 46 insertions(+), 57 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index c41d6445c7e5..d7c9ea8a7c45 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -537,82 +537,71 @@ void fuse_file_release(struct inode *inode, struct fuse_file *ff,
 	fuse_file_put(ff, ff->fm->fc->destroy || ff->fm->fc->close_wait);
 }
 
-void fuse_release_common(struct file *file, bool isdir)
-{
-	fuse_file_release(file_inode(file), file->private_data, file->f_flags,
-			  (fl_owner_t) file, isdir);
-}
-
 static void fuse_sync_writes(struct inode *inode);
 
-static int fuse_release(struct inode *inode, struct file *file)
+static void fuse_release_close_wait(struct inode *inode, struct file *file)
 {
 	struct fuse_file *ff = file->private_data;
 	struct fuse_inode *fi = get_fuse_inode(inode);
 	struct fuse_conn *fc = ff->fm->fc;
 
-	set_bit(FUSE_S_CLOSING, &ff->ff_state);
+	/*
+	 * Stop the write rerouting from picking this file, otherwise
+	 * continuous writeback on the inode can keep the reference count
+	 * elevated and the wait below may never finish.
+	 */
 	spin_lock(&fi->lock);
+	list_del_init(&ff->write_entry);
 	spin_unlock(&fi->lock);
 
 	/*
-	 * Dirty pages might remain despite write_inode_now() call from
-	 * fuse_flush() due to writes racing with the close.
+	 * Wait until we are the last one holding a reference, so that all
+	 * in-flight requests on this file have finished and FUSE_RELEASE
+	 * is sent by the releaser itself.
 	 */
-	if (fc->writeback_cache) {
-		if (file->f_mode & FMODE_WRITE) {
-			write_inode_now(inode, 1);
+	wait_event(fi->page_waitq, refcount_read(&ff->count) == 1);
 
-			/* Must remove file from write list. Otherwise it is possible this
-			 * file will get more writeback from another files rerouted via write_files
-			 */
-			spin_lock(&fi->lock);
-			list_del_init(&ff->write_entry);
-			spin_unlock(&fi->lock);
-
-			/* A writeback from another fuse file might come after
-			 * filemap_write_and_wait() above
-			 */
-			if (!fc->close_wait)
-				filemap_write_and_wait(file->f_mapping);
-		} else
-			BUG_ON(!list_empty(&ff->write_entry));
-
-		/* This can livelock. Inode can be open via another file
-		 * and that file can generate continuous writeback.
-		 * I think i_mutex could be taken around this.
-		 *
-		 * For now we replace this with waiting on ff->count,
-		 * it is safe, because we essentially wait only for writeback (and readahead)
-		 * enqueued on this file and it is not going to get new one: it is closing.
-		 */
-		if (fc->close_wait)
-			wait_event(fi->page_waitq, refcount_read(&ff->count) == 1);
+	inode_lock(inode);
+	fuse_sync_writes(inode);
+	fuse_read_dio_wait(fi);
 
+	/* num_openers is serialized by the inode lock, no fi->lock needed */
+	if (--fi->num_openers == 0) {
+		/* take the attributes from the server again */
 		spin_lock(&fi->lock);
-		/*
-		 * Last opener is gone and its writeback has drained: from
-		 * now on take the attributes from the server again.
-		 */
-		if (--fi->num_openers == 0)
-			fi->i_size_unstable = 1;
+		fi->i_size_unstable = 1;
 		spin_unlock(&fi->lock);
-	} else if (fc->close_wait)
-		wait_event(fi->page_waitq, refcount_read(&ff->count) == 1);
-
-	if (fc->kio.op) {
-		/*
-		 * Flush pending requests before FUSE_RELEASE makes userspace
-		 * to drop the lease of the file. Otherwise, they never finish.
-		 */
-		inode_lock(inode);
-		fuse_sync_writes(inode);
-		fuse_read_dio_wait(fi);
 
-		if (fi->num_openers == 0 && fc->kio.op->file_close)
+		if (fc->kio.op && fc->kio.op->file_close)
 			fc->kio.op->file_close(file, inode);
-		inode_unlock(inode);
 	}
+	inode_unlock(inode);
+}
+
+void fuse_release_common(struct file *file, bool isdir)
+{
+	fuse_file_release(file_inode(file), file->private_data, file->f_flags,
+			  (fl_owner_t) file, isdir);
+}
+
+static int fuse_release(struct inode *inode, struct file *file)
+{
+	struct fuse_file *ff = file->private_data;
+	struct fuse_conn *fc = get_fuse_conn(inode);
+
+	/* stop fiemap from picking this file before flushing starts */
+	if (fc->close_wait)
+		set_bit(FUSE_S_CLOSING, &ff->ff_state);
+
+	/*
+	 * Dirty pages might remain despite write_inode_now() call from
+	 * fuse_flush() due to writes racing with the close.
+	 */
+	if (fc->writeback_cache && (file->f_mode & FMODE_WRITE))
+		write_inode_now(inode, 1);
+
+	if (fc->close_wait)
+		fuse_release_close_wait(inode, file);
 
 	fuse_release_common(file, false);
 

-- 
Git-155)

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel