[Devel] [PATCH hci-8.1 3/3] fs/fuse: use wait_var_event to wait for fuse file references

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

On close_wait mounts, fuse_release_close_wait() needs to wait for all
asynchronous references to the fuse file to be dropped before sending
FUSE_RELEASE. This wait on fi->page_waitq was inherited from the past
implementation and is no longer appropriate as the wait condition
evolved:

 - fi->page_waitq is meant to serve page-under-writeback waiters and
   fuse_set_nowrite()'s writectr waiters; now it also has to serve the
   ff reference waiter, so every wakeup can wake unrelated waiters.

 - the wake is decoupled from the decrement, so every path dropping an
   ff reference must remember to wake fi->page_waitq, and because of
   this we grew two variants of the stock fuse_file_put():
   __fuse_file_put() and fuse_release_ff(). This is bug prone, and a
   misuse can leave the releaser blocked forever.

Instead of waiting on fi->page_waitq, a simple wait_var_event() on
&ff->count serves the purpose best: the matching wake_up_var() uses
the address purely as a hash key and never dereferences it, so the
wakeup can be folded into fuse_file_put() itself. With that, both
__fuse_file_put() and fuse_release_ff() become redundant and are
removed. This cleanup also makes the code less divergent from
mainline.

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

Signed-off-by: Liu Kui <kui.liu@virtuozzo.com>
---
 fs/fuse/file.c                     | 69 +++++++++++++++++++++-----------------
 fs/fuse/fuse_i.h                   |  2 +-
 fs/fuse/kio/pcs/pcs_fuse_kdirect.c |  3 +-
 3 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index d7c9ea8a7c45..74c8d925e2fd 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -24,6 +24,7 @@
 #include <linux/fiemap.h>
 #include <linux/jhash.h>
 #include <linux/file.h>
+#include <linux/wait_bit.h>
 
 unsigned int hash_seed_r;
 module_param(hash_seed_r, uint, 0644);
@@ -140,12 +141,23 @@ static void fuse_release_end(struct fuse_mount *fm, struct fuse_args *args,
 	kfree(ra);
 }
 
-static void fuse_file_put(struct fuse_file *ff, bool sync)
+void fuse_file_put(struct fuse_file *ff, bool sync)
 {
+	/* read before the drop: ff may not be touched after a non-final one */
+	bool close_wait = ff->fm->fc->close_wait;
+
 	if (refcount_dec_and_test(&ff->count)) {
 		struct fuse_release_args *ra = &ff->args->release_args;
 		struct fuse_args *args = (ra ? &ra->args : NULL);
 
+		/*
+		 * On close_wait the final put must come from a synchronous
+		 * caller: fuse_release() waits for ff->count == 1 while
+		 * holding its own reference, so an asynchronous completion
+		 * can never drop the last one.
+		 */
+		WARN_ON(!sync && close_wait);
+
 		if (ra && ra->inode)
 			fuse_file_io_release(ff, ra->inode);
 
@@ -165,14 +177,25 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
 				fuse_release_end(ff->fm, args, -ENOTCONN);
 		}
 		kfree(ff);
+		return;
 	}
-}
 
-static void __fuse_file_put(struct fuse_file *ff)
-{
-	if (refcount_dec_and_test(&ff->count))
-		BUG();
+	/*
+	 * ff may already be freed by a racing final put -- don't touch it
+	 * past this point.
+	 */
+	if (close_wait) {
+		/*
+		 * Pairs with wait_var_event() in fuse_release_close_wait():
+		 * the wake_up_var() contract requires a full barrier between
+		 * the counter update and the wakeup, and refcount_dec()
+		 * alone is only release-ordered.
+		 */
+		smp_mb__after_atomic();
+		wake_up_var(&ff->count);
+	}
 }
+EXPORT_SYMBOL_GPL(fuse_file_put);
 
 struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
 				 unsigned int open_flags, bool isdir)
@@ -557,9 +580,10 @@ static void fuse_release_close_wait(struct inode *inode, struct file *file)
 	/*
 	 * 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.
+	 * is sent by the releaser itself. Woken by the wake_up_var() in
+	 * fuse_file_put().
 	 */
-	wait_event(fi->page_waitq, refcount_read(&ff->count) == 1);
+	wait_var_event(&ff->count, refcount_read(&ff->count) == 1);
 
 	inode_lock(inode);
 	fuse_sync_writes(inode);
@@ -1252,22 +1276,6 @@ static int fuse_read_folio(struct file *file, struct folio *folio)
 	return err;
 }
 
-void fuse_release_ff(struct inode *inode, struct fuse_file *ff)
-{
-	if (ff) {
-		if (ff->fm->fc->close_wait) {
-			struct fuse_inode *fi = get_fuse_inode(inode);
-			spin_lock(&fi->lock);
-			__fuse_file_put(ff);
-			wake_up(&fi->page_waitq);
-			spin_unlock(&fi->lock);
-		} else {
-			fuse_file_put(ff, false);
-		}
-	}
-}
-EXPORT_SYMBOL_GPL(fuse_release_ff);
-
 void fuse_revoke_readpages(struct fuse_file *ff)
 {
 	struct fuse_io_args *ia;
@@ -1330,7 +1338,7 @@ static void fuse_readpages_end(struct fuse_mount *fm, struct fuse_args *args,
 	fuse_invalidate_atime(inode);
 
 	if (ff)
-		fuse_release_ff(inode, ff);
+		fuse_file_put(ff, false);
 
 	fuse_io_free(ia);
 }
@@ -2189,7 +2197,7 @@ static void fuse_writepage_free(struct fuse_writepage_args *wpa)
 	for (i = 0; i < ap->num_pages; i++)
 		__free_page(ap->pages[i]);
 
-	fuse_release_ff(wpa->inode, wpa->ia.ff);
+	fuse_file_put(wpa->ia.ff, false);
 
 	kfree(ap->pages);
 	kfree(wpa);
@@ -2572,7 +2580,7 @@ static int fuse_writepage_locked(struct folio *folio)
 	return 0;
 
 err_writepage_args:
-	fuse_release_ff(inode, ff);
+	fuse_file_put(ff, false);
 err_nofile:
 	folio_put(tmp_folio);
 err:
@@ -2859,7 +2867,8 @@ static int fuse_writepages(struct address_space *mapping,
 		WARN_ON(!data.wpa->ia.ap.num_pages);
 		fuse_writepages_send(&data);
 	}
-	fuse_release_ff(inode, data.ff);
+	if (data.ff)
+		fuse_file_put(data.ff, false);
 
 	kfree(data.orig_pages);
 out:
@@ -3929,7 +3938,7 @@ static int fuse_request_fiemap(struct inode *inode, u32 cur_max,
 
 	ap.pages = fuse_pages_alloc(fc->max_pages, GFP_KERNEL, &ap.descs);
 	if (!ap.pages) {
-		fuse_release_ff(inode, ff);
+		fuse_file_put(ff, false);
 		return -ENOMEM;
 	}
 
@@ -4021,7 +4030,7 @@ static int fuse_request_fiemap(struct inode *inode, u32 cur_max,
 		ap.pages[allocated] = NULL;
 	}
 	kfree(ap.pages);
-	fuse_release_ff(inode, ff);
+	fuse_file_put(ff, false);
 	return err;
 }
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index f6da643b9c86..e94d40458af3 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1673,7 +1673,7 @@ int fuse_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 
 struct fuse_file *__fuse_write_file_get(struct fuse_inode *fi);
 struct fuse_file *fuse_file_get(struct fuse_file *ff);
-void fuse_release_ff(struct inode *inode, struct fuse_file *ff);
+void fuse_file_put(struct fuse_file *ff, bool sync);
 struct fuse_req *fuse_dev_find_request(unsigned int fd, u64 unique);
 
 void fuse_revoke_readpages(struct fuse_file *ff);
diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
index efbab75654c8..5064198a7b4c 100644
--- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
+++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
@@ -728,7 +728,8 @@ static int submit_size_grow(struct inode *inode, unsigned long long size)
 	args.out_args[0].value = &outarg;
 
 	err = fuse_simple_request(fm, &args);
-	fuse_release_ff(inode, ff);
+	if (ff)
+		fuse_file_put(ff, false);
 
 	return err;
 

-- 
Git-155)

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