Matt Benjamin has uploaded this change for review.

View Change

mdcache: fix a refcnt underrun w/FSAL_RGW triggered by readdir readahead

The following is a summary written by Claude Code (Opus 4.6 1M), which
performed in depth debugging, subsequent to fixing a broken invariant
to delete fh-lru entries immediately upon reaching lru_refcnt==0 (this
was a regression introduced in earlier refactoring).

The current proposed fix is also generated, but is not any of the fixes
the model originally proposed:

1. I rejected introducing a "dir-skip" argument that would have (I think)
prevented any mdcache entry representing a directory from being reaped
if any readdir was going on (yikes)

2. I accepted the concept of declining to reap entries that belong to
a directory currently being populated--however, I introduced the idea
of providing the being-populated check as a callback, making
it fine grained, and permitting alternate skip routines in future.

Note that the problem specifically affects different traversals that
are executing concurrently, it's not a problem that can be addressed
in the scope of a single traversal.

Model:

 Problem: During concurrent readdir on a large directory, multiple
  ganesha worker threads populate chunks simultaneously. The mdcache
  LRU reaper reclaims entries that have dropped to sentinel refcnt
  (after their dirents are unchunked by chunk cleanup).  When the same
  file is re-encountered in a subsequent readdir batch,
  mdcache_new_entry can't find the old entry (already unhashed during
  reap) and creates a new one — with a new FSAL sub_handle holding an
  independent ref on the same underlying RGWFileHandle. This
  oscillation creates multiple entries for the same file, and when
  multiple entries' sub_handles are released between lookups, the
  RGWFileHandle's cohort_lru refcnt drops to zero, triggering deletion
  and a subsequent use-after-free crash.

  Fix: A caller-supplied callback mechanism in the LRU reaper allows
  mdcache_populate_dir_chunk to veto reaping of entries that belong to
  a directory with an active populate. Three pieces:

  1. populate_origin (per entry): tags each entry with the directory
   it was created for during readdir populate, so the reaper can
   identify entries belonging to a specific directory.
   2. populate_count (per directory): atomic counter incremented when
   mdcache_populate_dir_chunk starts, decremented when it returns. A
   positive count means at least one thread is actively populating
   this directory.  3. mdcache_lru_reap_check_cb (callback): threaded
   from mdcache_new_entry through mdcache_lru_get to the reaper. The
   readdir populate path passes skip_populate_dir, which vetoes
   reaping any entry whose populate_origin matches the directory being
   populated, as long as that directory's populate_count >
   0. Non-readdir callers pass NULL (no veto). The callback design
   avoids hardcoding readdir-specific logic in the LRU reaper and is
   extensible to other use cases.
  Effect: entries created during readdir survive long enough for
  mdcache_find_keyed_reason to find them on the next pass, taking the
  "found existing" path in mdcache_new_entry — which properly releases
  the duplicate sub_handle without creating a second independent ref
  on the RGWFileHandle.

Change-Id: I24f4847ffddc9ff3b52b4fd21e9dec80a2dc3d39
Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
Assisted-by: Claude Code, Opus 4.6 1M
---
M src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_handle.c
M src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_helpers.c
M src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_int.h
M src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c
M src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.h
5 files changed, 86 insertions(+), 34 deletions(-)

git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/81/1248681/1

To view, visit change 1248681. To unsubscribe, or for help writing mail filters, visit settings.