fuse: deliver fdclose to the migrated fd before dropping it - #4837
Draft
ThalesBarretto wants to merge 2 commits into
Draft
ThalesBarretto wants to merge 2 commits into
ThalesBarretto wants to merge 2 commits into
Conversation
fd_close() (added with the open-behind rewrite, db95388) tells the xlators of the graph an fd belongs to that the application's handle is closing; open-behind uses it to cancel a deferred open and drop the fd reference and the open stub it keeps for it. fuse_release() calls it on the base fd only. After a graph switch the base fd's ctx owns activefd, an fd created on the new graph by fuse_migrate_fd_open() whose open went through the new graph's open-behind and was deferred (lazy-open is the default; a small-file read is served by quick-read above it, and the close-time FLUSH is answered locally, so nothing triggers it). fuse_fd_ctx_destroy() then just fd_unref()s activefd: open-behind's two references keep it alive, and with it the fuse fd ctx and the new-graph inode, until the file is opened again through that graph or the process exits. Every open file migrated across a 'volume set' with performance.open-behind on (the default) leaks this way. Call fd_close() on the active fd before the fd_unref() in fuse_fd_ctx_destroy(), and likewise on the previous active fd that a second migration abandons in fuse_migrate_fd_open(). Reproducible on any volume with open-behind on: keep N files open on a FUSE mount, run 'volume set <vol> performance.stat-prefetch off', close them, take a client statedump: fd_t active-count = N and gf_fuse_mt_fd_ctx_t num_allocs = N (0 with open-behind off, 0 with this change). Fixes: gluster#4836 Signed-off-by: Thales Antunes de Oliveira Barretto <thales.barretto.git@gmail.com>
Regression test for the fdclose delivery to the migrated fd: keep five files open on a FUSE mount across a graph switch (open-behind on, the default), close them without any other fop, and expect the client statedump to show fd_t active-count 0 and no gf_fuse_mt_fd_ctx_t allocations. FUSE performs the switch on the next request, so the test pokes the mount with stat(2) after the volume set. Counters read from the mount log are relative to a baseline taken after the mount because the log persists across tests. Updates: gluster#4836 Signed-off-by: Thales Antunes de Oliveira Barretto <thales.barretto.git@gmail.com>
ThalesBarretto
marked this pull request as draft
September 22, 2026 10:01
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fuse: deliver fdclose to the migrated fd before dropping it
Problem
fd_close()(added with the open-behind rewrite,db95388706) tells the xlators of the graph an fd belongsto that the application's handle is closing; open-behind uses it to cancel a deferred open and drop the fd
reference and the open stub it keeps for it.
fuse_release()calls it on the base fd only. After a graphswitch the base fd's ctx owns
activefd, an fd created on the new graph byfuse_migrate_fd_open()whoseopen went through the new graph's open-behind and was deferred (
lazy-openis the default; a small-file readis served by quick-read above it, and the close-time FLUSH is answered locally, so nothing triggers it).
fuse_fd_ctx_destroy()then justfd_unref()s activefd: open-behind's two references keep it alive, andwith it the fuse fd ctx and the new-graph inode, until the file is opened again through that graph or the
process exits. Every open file migrated across a
volume setwithperformance.open-behind on(the default)and then closed without a further fd fop leaks this way.
Fix
fuse_fd_ctx_destroy():fd_close(activefd)beforefd_unref(activefd).fd_close()walks the fd's owngraph, so the new graph's
ob_fdclose()runs, cancels the deferred open and drops both references; thefollowing unref reaches zero and
fd_destroy()runs the normal.releasecbks.fuse_migrate_fd_open(): likewisefd_close(old_activefd)before thefd_unref()that abandons theprevious active fd on a re-migration (the fsync in
fuse_migrate_fd()already prevents a leak there; thiskeeps the protocol uniform and balances open-behind's
open_counton the abandoned graph).Composes with #4490, which moves
fuse_fd_ctx_destroy()into the.release/.releasedircbks and keepscalling it — verified on a stock arm carrying #4490 on top of this change: 10 files across a switch, close, 0 live
fds / 0 fuse ctxs / itable active 1.
Reproduction and A/B
Stock builds of devel
a482a8578aand of this branch (default./configure: tcmalloc, no mem-pools, no debug),volume with
performance.open-behind on; N files held open O_RDWR on a FUSE mount, onevolume set, the mountpoked with
stat(FUSE switches graphs on the next request), the handles closed, then a client statedump. On atcmalloc build there is no
fd_tmem-pool, so the count is the fuse xlator's memory accounting (onegf_common_mt_fd_ctxarray per livefd_t), plus the inode table fuse dumps natively.active_size− root) before / afterpreadimmediately (quick-read cache), closepread3 s later (reaches open-behind), closeStatedump of the devel client after all ten handles were closed:
With this change the same run gives
num_allocsabsent (0) for both types andactive_size=1.Test
tests/bugs/fuse/fd-migration-open-behind-leak.t: five files held open, one switch (poked), closed withoutany other fop, expects the fuse xlator's
gf_common_mt_fd_ctxallocation count (live fds; exists on every build,unlike the
fd_tmem-pool) and itsgf_fuse_mt_fd_ctx_tcount to be 0 in the mount statedump. Fails on stockdevel on exactly those two assertions (
Got "5" instead of "0"), passes with the change.The gfapi side of the same gap (gfapi never calls
fd_close()at all) is #3977; a separate PR addresses it.Fixes: #4836