* [Devel] [PATCH hci-8.1 1/3] fs/fuse: give num_openers and i_size_unstable each a single role
2026-09-30 5:44 [Devel] [PATCH hci-8.1 0/3] fs/fuse: rework close_wait handling Liu Kui
@ 2026-09-30 5:44 ` Liu Kui
2026-09-30 5:50 ` no-reply
2026-09-30 5:44 ` [Devel] [PATCH hci-8.1 2/3] fs/fuse: move the close_wait handling of fuse_release() into a helper Liu Kui
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Liu Kui @ 2026-09-30 5:44 UTC (permalink / raw)
To: Konstantin Khorenko; +Cc: azaitsev, devel, Liu Kui
Currently fi->num_openers serves two purposes: it helps decide whether
the i_size attribute needs to be taken from the server, and it manages
when kio's file_open/file_close methods need to be called. The former
role already requires the extra i_size_unstable flag to handle lease
revokes, which makes num_openers redundant for that purpose.
Give each field a single role: use i_size_unstable as the "take the
attributes from the server" flag, and make num_openers a pure open
count managing the calls of kio's file_open/file_close. A new inode
starts with the i_size_unstable flag set, an open finding it set
refreshes the attributes and clears it on success, and the last
release sets it back after writeback has drained.
Also fix a bug where a failed refresh could re-trust stale local
attributes.
https://virtuozzo.atlassian.net/browse/VSTOR-147878
Signed-off-by: Liu Kui <kui.liu@virtuozzo.com>
---
fs/fuse/file.c | 21 ++++++++++++++++-----
fs/fuse/fuse_i.h | 4 ++--
fs/fuse/inode.c | 5 +++--
3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index e0efb524cda7..c41d6445c7e5 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -293,8 +293,14 @@ static int fuse_open_close_wait(struct inode *inode, struct file *file)
inode_lock(inode);
spin_lock(&fi->lock);
- if (++fi->num_openers == 1 || fi->i_size_unstable) {
- fi->i_size_unstable = 1;
+ fi->num_openers++;
+ /*
+ * i_size_unstable set means the attributes must be taken from the
+ * server: no open has refreshed them yet (first open), the last
+ * opener is gone, or the files were invalidated by a lease revoke.
+ * Refresh the attributes so ours can be trusted again.
+ */
+ if (fi->i_size_unstable) {
fi->inval_mask = ~0;
spin_unlock(&fi->lock);
err = fuse_update_attributes(inode, file, ~0);
@@ -303,9 +309,10 @@ static int fuse_open_close_wait(struct inode *inode, struct file *file)
err = fc->kio.op->file_open(file, inode);
spin_lock(&fi->lock);
- fi->i_size_unstable = 0;
if (err)
fi->num_openers--;
+ else
+ fi->i_size_unstable = 0;
}
file->f_mode |= FMODE_NOWAIT;
@@ -583,8 +590,12 @@ static int fuse_release(struct inode *inode, struct file *file)
wait_event(fi->page_waitq, refcount_read(&ff->count) == 1);
spin_lock(&fi->lock);
- /* since now we can trust userspace attr.size */
- fi->num_openers--;
+ /*
+ * 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;
spin_unlock(&fi->lock);
} else if (fc->close_wait)
wait_event(fi->page_waitq, refcount_read(&ff->count) == 1);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 21f873337627..f6da643b9c86 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -200,10 +200,10 @@ struct fuse_inode {
/** Reference to backing file in passthrough mode */
struct fuse_backing *fb;
#endif
- /** Mostly to detect very first open */
+ /** Number of opens, drives kio file_open/file_close pairing */
int num_openers;
- /** Even though num_openers>0, trust server i_size */
+ /** Take attributes (i_size) from the server, don't trust ours */
int i_size_unstable;
/** Private kdirect io context */
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index eb2e9a4f8756..4eaa89d9c46c 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -116,7 +116,8 @@ static struct inode *fuse_alloc_inode(struct super_block *sb)
fi->orig_ino = 0;
fi->state = 0;
fi->submount_lookup = NULL;
- fi->i_size_unstable = 0;
+ /* take attributes from the server until the first open */
+ fi->i_size_unstable = 1;
fi->private = NULL;
fi->inval_files_seq = 0;
INIT_LIST_HEAD(&fi->rw_files);
@@ -328,7 +329,7 @@ u32 fuse_get_cache_mask(struct inode *inode)
if (fc->close_wait) {
struct fuse_inode *fi = get_fuse_inode(inode);
- if (!fi->num_openers || fi->i_size_unstable)
+ if (fi->i_size_unstable)
return 0;
}
--
Git-155)
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 8+ messages in thread* [Devel] [PATCH hci-8.1 2/3] fs/fuse: move the close_wait handling of fuse_release() into a helper
2026-09-30 5:44 [Devel] [PATCH hci-8.1 0/3] fs/fuse: rework close_wait handling Liu Kui
2026-09-30 5:44 ` [Devel] [PATCH hci-8.1 1/3] fs/fuse: give num_openers and i_size_unstable each a single role Liu Kui
@ 2026-09-30 5:44 ` Liu Kui
2026-09-30 5:49 ` no-reply
2026-09-30 5:44 ` [Devel] [PATCH hci-8.1 3/3] fs/fuse: use wait_var_event to wait for fuse file references Liu Kui
2026-09-30 15:39 ` [Devel] [PATCH hci-8.1 0/3] fs/fuse: rework close_wait handling Konstantin Khorenko
3 siblings, 1 reply; 8+ messages in thread
From: Liu Kui @ 2026-09-30 5:44 UTC (permalink / raw)
To: Konstantin Khorenko; +Cc: azaitsev, devel, Liu Kui
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
^ permalink raw reply [flat|nested] 8+ messages in thread* [Devel] [PATCH hci-8.1 3/3] fs/fuse: use wait_var_event to wait for fuse file references
2026-09-30 5:44 [Devel] [PATCH hci-8.1 0/3] fs/fuse: rework close_wait handling Liu Kui
2026-09-30 5:44 ` [Devel] [PATCH hci-8.1 1/3] fs/fuse: give num_openers and i_size_unstable each a single role Liu Kui
2026-09-30 5:44 ` [Devel] [PATCH hci-8.1 2/3] fs/fuse: move the close_wait handling of fuse_release() into a helper Liu Kui
@ 2026-09-30 5:44 ` Liu Kui
2026-09-30 5:51 ` no-reply
2026-09-30 15:39 ` [Devel] [PATCH hci-8.1 0/3] fs/fuse: rework close_wait handling Konstantin Khorenko
3 siblings, 1 reply; 8+ messages in thread
From: Liu Kui @ 2026-09-30 5:44 UTC (permalink / raw)
To: Konstantin Khorenko; +Cc: azaitsev, devel, Liu Kui
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
^ permalink raw reply [flat|nested] 8+ messages in thread