All Virtuozzo development lists (kernel + QEMU)
 help / color / mirror / Atom feed
* [Devel] [PATCH hci-8.0] fs/fuse: fix fuse file refcount leak on close_wait writeback
@ 2026-09-22 13:53 Liu Kui
  2026-09-22 15:37 ` Konstantin Khorenko
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Liu Kui @ 2026-09-22 13:53 UTC (permalink / raw)
  To: Konstantin Khorenko; +Cc: azaitsev, devel, Liu Kui

Mainline commit 4046d3adcca4 ("fuse: move fuse file initialization to
wpa allocation time") moved acquisition of the wpa's fuse file
reference from submit time (fuse_writepages_send) to wpa allocation
time, and compensated by dropping the reference unconditionally in
fuse_writepage_free().

This breaks the close_wait accounting in the vz tree, which splits the
drop between two functions: fuse_writepage_finish() does
__fuse_file_put() for close_wait (allowing fuse_release() to pass
wait_event(ff->count == 1)), while fuse_writepage_free() puts the
reference only for !close_wait. This split requires that every wpa
owning a reference passes through fuse_writepage_finish() before being
freed. Two paths free a wpa without calling it:

  - fuse_writepage_add(): temp page swap case frees the new wpa
  - fuse_send_writepage(): out_free path frees the aux request chain

Before that commit these paths only ever saw wpas with ia.ff == NULL
(never submitted or promoted), so nothing leaked. After it, every wpa
owns a reference from birth, and on close_wait mounts both paths leak
it: ff->count never returns to 1, fuse_release() hangs forever in
wait_event(), and the fuse file and its FUSE_RELEASE are leaked.

Fix it by reverting to the old ownership model for the writeback path:
a wpa is allocated with ia.ff = NULL and acquires the reference at
submit time (fuse_writepages_send, fuse_writepage_locked) or at aux
promotion time (fuse_writepage_end). Also make fuse_writepage_free()
derive fuse_conn from wpa->inode since ia.ff may now legitimately be
NULL there.

Note: this is effectively a revert of the mainline change for the
writeback path. It can be restored later on a rework of the close_wait
accounting (e.g. dropping the finish/free split in favour of an
unconditional put).

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

Signed-off-by: Liu Kui <kui.liu@virtuozzo.com>
---
Fix a fuse file refcount leak on close_wait mounts introduced by the
rebase to mainline commit 4046d3adcca4 ("fuse: move fuse file
initialization to wpa allocation time"): wpas freed without going
through fuse_writepage_finish() (aux temp-page swap, killed aux chains)
now own a reference nobody drops, so fuse_release() hangs forever in
wait_event(ff->count == 1). Restore the old ownership model (reference
acquired at submit/promotion time) for the writeback path.

https://virtuozzo.atlassian.net/browse/VSTOR-145614
---
 fs/fuse/file.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 180f60fc539a..50d3f8186d39 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2181,7 +2181,7 @@ static ssize_t fuse_splice_write(struct pipe_inode_info *pipe, struct file *out,
 static void fuse_writepage_free(struct fuse_writepage_args *wpa)
 {
 	struct fuse_args_pages *ap = &wpa->ia.ap;
-	struct fuse_conn *fc = wpa->ia.ff->fm->fc;
+	struct fuse_conn *fc = get_fuse_conn(wpa->inode);
 	int i;
 
 	if (wpa->bucket)
@@ -2374,6 +2374,7 @@ static void fuse_writepage_end(struct fuse_mount *fm, struct fuse_args *args,
 
 		wpa->next = next->next;
 		next->next = NULL;
+		next->ia.ff = fuse_file_get(wpa->ia.ff);
 		tree_insert(&fi->writepages, next);
 
 		/*
@@ -2523,7 +2524,7 @@ static struct fuse_writepage_args *fuse_writepage_args_setup(struct folio *folio
 	fuse_write_args_fill(&wpa->ia, ff, inode, folio_pos(folio), 0);
 	wpa->ia.write.in.write_flags |= FUSE_WRITE_CACHE;
 	wpa->inode = inode;
-	wpa->ia.ff = ff;
+	wpa->ia.ff = NULL;
 
 	ap = &wpa->ia.ap;
 	ap->args.in_pages = true;
@@ -2561,6 +2562,7 @@ static int fuse_writepage_locked(struct folio *folio)
 	if (!wpa)
 		goto err_writepage_args;
 
+	wpa->ia.ff = ff;
 	ap = &wpa->ia.ap;
 	ap->num_pages = 1;
 
@@ -2628,6 +2630,7 @@ static void fuse_writepages_send(struct fuse_fill_wb_data *data)
 	int num_pages = wpa->ia.ap.num_pages;
 	int i;
 
+	wpa->ia.ff = fuse_file_get(data->ff);
 	spin_lock(&fi->lock);
 	list_add_tail(&wpa->queue_entry, &fi->queued_writes);
 	fuse_flush_writepages(inode);
@@ -2791,7 +2794,6 @@ static int fuse_writepages_fill(struct folio *folio,
 			folio_redirty_for_writepage(wbc, folio);
 			goto out_unlock;
 		}
-		fuse_file_get(wpa->ia.ff);
 		data->max_pages = 1;
 		ap = &wpa->ia.ap;
 

---
base-commit: 978369605ab89b48b86d12af9f8dda88689ed5ca
change-id: 20260922-vstor-145614-10a4ac579905

Best regards,
--  
Liu Kui <kui.liu@virtuozzo.com>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Devel] [PATCH hci-8.0] fs/fuse: fix fuse file refcount leak on close_wait writeback
  2026-09-22 13:53 [Devel] [PATCH hci-8.0] fs/fuse: fix fuse file refcount leak on close_wait writeback Liu Kui
@ 2026-09-22 15:37 ` Konstantin Khorenko
  2026-09-22 15:54 ` Konstantin Khorenko
  2026-09-22 18:54 ` Konstantin Khorenko
  2 siblings, 0 replies; 4+ messages in thread
From: Konstantin Khorenko @ 2026-09-22 15:37 UTC (permalink / raw)
  To: Liu Kui; +Cc: azaitsev, devel

On 9/22/26 15:53, Liu Kui wrote:
> Mainline commit 4046d3adcca4 ("fuse: move fuse file initialization to
> wpa allocation time") moved acquisition of the wpa's fuse file
> reference from submit time (fuse_writepages_send) to wpa allocation
> time, and compensated by dropping the reference unconditionally in
> fuse_writepage_free().
> 
> This breaks the close_wait accounting in the vz tree, which splits the
> drop between two functions: fuse_writepage_finish() does
> __fuse_file_put() for close_wait (allowing fuse_release() to pass
> wait_event(ff->count == 1)), while fuse_writepage_free() puts the
> reference only for !close_wait. This split requires that every wpa
> owning a reference passes through fuse_writepage_finish() before being
> freed. Two paths free a wpa without calling it:
> 
>   - fuse_writepage_add(): temp page swap case frees the new wpa
>   - fuse_send_writepage(): out_free path frees the aux request chain
> 
> Before that commit these paths only ever saw wpas with ia.ff == NULL
> (never submitted or promoted), so nothing leaked. After it, every wpa
> owns a reference from birth, and on close_wait mounts both paths leak
> it: ff->count never returns to 1, fuse_release() hangs forever in
> wait_event(), and the fuse file and its FUSE_RELEASE are leaked.
> 
> Fix it by reverting to the old ownership model for the writeback path:
> a wpa is allocated with ia.ff = NULL and acquires the reference at
> submit time (fuse_writepages_send, fuse_writepage_locked) or at aux
> promotion time (fuse_writepage_end). Also make fuse_writepage_free()
> derive fuse_conn from wpa->inode since ia.ff may now legitimately be
> NULL there.
> 
> Note: this is effectively a revert of the mainline change for the
> writeback path. It can be restored later on a rework of the close_wait
> accounting (e.g. dropping the finish/free split in favour of an
> unconditional put).
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-145614

  Fixes: 6b5ddd8253eb ("fuse: Wait for real writeback to finish on file")
  Feature: vStorage

?


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Devel] [PATCH hci-8.0] fs/fuse: fix fuse file refcount leak on close_wait writeback
  2026-09-22 13:53 [Devel] [PATCH hci-8.0] fs/fuse: fix fuse file refcount leak on close_wait writeback Liu Kui
  2026-09-22 15:37 ` Konstantin Khorenko
@ 2026-09-22 15:54 ` Konstantin Khorenko
  2026-09-22 18:54 ` Konstantin Khorenko
  2 siblings, 0 replies; 4+ messages in thread
From: Konstantin Khorenko @ 2026-09-22 15:54 UTC (permalink / raw)
  To: Liu Kui; +Cc: azaitsev, devel

  Reverting to the old model brings back its latent defect.

  The defect. An aux request takes two fields from different files:

  - write.in.fh and args->ff are filled from the ff at fill time (fs/fuse/file.c:2791, fs/fuse/file.c:1478).
  - The reference taken at promotion is on the primary request's ff (fs/fuse/file.c:2377).

  While the aux sits in the chain, it holds no reference on its fill-time ff. With close_wait this means fuse_release of
  that file passes its wait_event and frees the ff.

  Scenario:

  1. ffA writes a page, the primary wpa holds a reference on ffA.
  2. ffA is closed, fuse_release blocks in wait_event, ffA is removed from write_files.
  3. ffB writes the same page and ends up as an aux with args->ff = ffB and ffB's fh.
  4. ffB is closed. The aux holds no reference, so release passes, FUSE_RELEASE is sent, ffB is freed.
  5. The primary completes, the aux is promoted with a reference on ffA and goes out.

  Consequences at step 5:

  - fuse_request_queue_background dereferences the already freed ffB (fs/fuse/dev.c:663).
  - The request goes out with an fh userspace has already released.

  Status. This is pre-existing vz9 behaviour, the patch does not introduce it. But the mainline model, where every wpa holds
  a reference on the ff whose fh it uses, closed it as a side effect. This patch takes that away.

  An alternative of roughly the same size fixes both bugs:

  - Keep the mainline model: the reference is taken when the wpa is allocated.
  - Drop the put from fuse_writepage_finish.
  - In fuse_writepage_free call fuse_release_ff(wpa->inode, wpa->ia.ff). It already does the close_wait split with wake_up
    under fi->lock.

  All three fuse_writepage_free callers run without fi->lock, so this fits without reordering anything.
  You mention this option yourself in the "Note:", so this is not something new,

  the question is - are we ok to revert to the old ms scheme without that global rework?

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 9/22/26 15:53, Liu Kui wrote:
> Mainline commit 4046d3adcca4 ("fuse: move fuse file initialization to
> wpa allocation time") moved acquisition of the wpa's fuse file
> reference from submit time (fuse_writepages_send) to wpa allocation
> time, and compensated by dropping the reference unconditionally in
> fuse_writepage_free().
> 
> This breaks the close_wait accounting in the vz tree, which splits the
> drop between two functions: fuse_writepage_finish() does
> __fuse_file_put() for close_wait (allowing fuse_release() to pass
> wait_event(ff->count == 1)), while fuse_writepage_free() puts the
> reference only for !close_wait. This split requires that every wpa
> owning a reference passes through fuse_writepage_finish() before being
> freed. Two paths free a wpa without calling it:
> 
>   - fuse_writepage_add(): temp page swap case frees the new wpa
>   - fuse_send_writepage(): out_free path frees the aux request chain
> 
> Before that commit these paths only ever saw wpas with ia.ff == NULL
> (never submitted or promoted), so nothing leaked. After it, every wpa
> owns a reference from birth, and on close_wait mounts both paths leak
> it: ff->count never returns to 1, fuse_release() hangs forever in
> wait_event(), and the fuse file and its FUSE_RELEASE are leaked.
> 
> Fix it by reverting to the old ownership model for the writeback path:
> a wpa is allocated with ia.ff = NULL and acquires the reference at
> submit time (fuse_writepages_send, fuse_writepage_locked) or at aux
> promotion time (fuse_writepage_end). Also make fuse_writepage_free()
> derive fuse_conn from wpa->inode since ia.ff may now legitimately be
> NULL there.
> 
> Note: this is effectively a revert of the mainline change for the
> writeback path. It can be restored later on a rework of the close_wait
> accounting (e.g. dropping the finish/free split in favour of an
> unconditional put).
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-145614
> 
> Signed-off-by: Liu Kui <kui.liu@virtuozzo.com>
> ---
> Fix a fuse file refcount leak on close_wait mounts introduced by the
> rebase to mainline commit 4046d3adcca4 ("fuse: move fuse file
> initialization to wpa allocation time"): wpas freed without going
> through fuse_writepage_finish() (aux temp-page swap, killed aux chains)
> now own a reference nobody drops, so fuse_release() hangs forever in
> wait_event(ff->count == 1). Restore the old ownership model (reference
> acquired at submit/promotion time) for the writeback path.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-145614
> ---
>  fs/fuse/file.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 180f60fc539a..50d3f8186d39 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -2181,7 +2181,7 @@ static ssize_t fuse_splice_write(struct pipe_inode_info *pipe, struct file *out,
>  static void fuse_writepage_free(struct fuse_writepage_args *wpa)
>  {
>  	struct fuse_args_pages *ap = &wpa->ia.ap;
> -	struct fuse_conn *fc = wpa->ia.ff->fm->fc;
> +	struct fuse_conn *fc = get_fuse_conn(wpa->inode);
>  	int i;
>  
>  	if (wpa->bucket)
> @@ -2374,6 +2374,7 @@ static void fuse_writepage_end(struct fuse_mount *fm, struct fuse_args *args,
>  
>  		wpa->next = next->next;
>  		next->next = NULL;
> +		next->ia.ff = fuse_file_get(wpa->ia.ff);
>  		tree_insert(&fi->writepages, next);
>  
>  		/*
> @@ -2523,7 +2524,7 @@ static struct fuse_writepage_args *fuse_writepage_args_setup(struct folio *folio
>  	fuse_write_args_fill(&wpa->ia, ff, inode, folio_pos(folio), 0);
>  	wpa->ia.write.in.write_flags |= FUSE_WRITE_CACHE;
>  	wpa->inode = inode;
> -	wpa->ia.ff = ff;
> +	wpa->ia.ff = NULL;
>  
>  	ap = &wpa->ia.ap;
>  	ap->args.in_pages = true;
> @@ -2561,6 +2562,7 @@ static int fuse_writepage_locked(struct folio *folio)
>  	if (!wpa)
>  		goto err_writepage_args;
>  
> +	wpa->ia.ff = ff;
>  	ap = &wpa->ia.ap;
>  	ap->num_pages = 1;
>  
> @@ -2628,6 +2630,7 @@ static void fuse_writepages_send(struct fuse_fill_wb_data *data)
>  	int num_pages = wpa->ia.ap.num_pages;
>  	int i;
>  
> +	wpa->ia.ff = fuse_file_get(data->ff);
>  	spin_lock(&fi->lock);
>  	list_add_tail(&wpa->queue_entry, &fi->queued_writes);
>  	fuse_flush_writepages(inode);
> @@ -2791,7 +2794,6 @@ static int fuse_writepages_fill(struct folio *folio,
>  			folio_redirty_for_writepage(wbc, folio);
>  			goto out_unlock;
>  		}
> -		fuse_file_get(wpa->ia.ff);
>  		data->max_pages = 1;
>  		ap = &wpa->ia.ap;
>  
> 
> ---
> base-commit: 978369605ab89b48b86d12af9f8dda88689ed5ca
> change-id: 20260922-vstor-145614-10a4ac579905
> 
> Best regards,
> --  
> Liu Kui <kui.liu@virtuozzo.com>
> 

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Devel] [PATCH hci-8.0] fs/fuse: fix fuse file refcount leak on close_wait writeback
  2026-09-22 13:53 [Devel] [PATCH hci-8.0] fs/fuse: fix fuse file refcount leak on close_wait writeback Liu Kui
  2026-09-22 15:37 ` Konstantin Khorenko
  2026-09-22 15:54 ` Konstantin Khorenko
@ 2026-09-22 18:54 ` Konstantin Khorenko
  2 siblings, 0 replies; 4+ messages in thread
From: Konstantin Khorenko @ 2026-09-22 18:54 UTC (permalink / raw)
  To: Liu Kui; +Cc: devel, azaitsev

On Tue, 22 Sep 2026 21:53:40 +0800, Liu Kui wrote:
> fs/fuse: fix fuse file refcount leak on close_wait writeback

Applied to branch-rh10-6.12.0-211.52.1.22.x.vz10-ovz, will be released in rh10-6.12.0-211.52.1.22.2.vz10, thanks!

[1/1] fs/fuse: fix fuse file refcount leak on close_wait writeback
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/ac3b8c47f41f

-- 
Konstantin Khorenko <khorenko@virtuozzo.com>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-22 18:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-22 13:53 [Devel] [PATCH hci-8.0] fs/fuse: fix fuse file refcount leak on close_wait writeback Liu Kui
2026-09-22 15:37 ` Konstantin Khorenko
2026-09-22 15:54 ` Konstantin Khorenko
2026-09-22 18:54 ` Konstantin Khorenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.