[PATCHSET 1/5] xfs_scrub: fixes and cleanups for inode iteration

Darrick J. Wong
2025-02-06T22:29:36Z

Hi all,

Christoph and I were investigating some performance problems in
xfs_scrub on filesystems that have a lot of rtgroups, and we noticed
several problems and inefficiencies in the existing inode iteration
code.

The first observation is that two of the three callers of
scrub_all_inodes (phases 5 and 6) just want to walk all the user files
in the filesystem. They don't care about metadir directories, and they
don't care about matching inumbers data to bulkstat data for the purpose
of finding broken files. The third caller (phase 3) does, so it makes
more sense to create a much simpler iterator for phase 5 and 6 that only
calls bulkstat.

But then I started noticing other problems in the phase 3 inode
iteration code -- if the per-inumbers bulkstat iterator races with other
threads that are creating or deleting files we can walk off the end of
the bulkstat array, we can miss newly allocated files, miss old
allocated inodes if there are newly allocated ones, pointlessly try to
scan deleted files, and redundantly scan files from another inobt
record.

These races rarely happen, but they all need fixing.

If you're going to start using this code, I strongly recommend pulling
from my git trees, which are linked below.

With a bit of luck, this should all go splendidly.
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=scrub-inode-iteration-fixes
---
Commits in this patchset:
* libxfs: unmap xmbuf pages to avoid disaster
* libxfs: mark xmbuf_{un,}map_page static
* man: document new XFS_BULK_IREQ_METADIR flag to bulkstat
* libfrog: wrap handle construction code
* xfs_scrub: don't report data loss in unlinked inodes twice
* xfs_scrub: call bulkstat directly if we're only scanning user files
* xfs_scrub: remove flags argument from scrub_scan_all_inodes
* xfs_scrub: selectively re-run bulkstat after re-running inumbers
* xfs_scrub: actually iterate all the bulkstat records
* xfs_scrub: don't double-scan inodes during phase 3
* xfs_scrub: don't (re)set the bulkstat request icount incorrectly
* xfs_scrub: don't complain if bulkstat fails
* xfs_scrub: return early from bulkstat_for_inumbers if no bulkstat data
* xfs_scrub: don't blow away new inodes in bulkstat_single_step
* xfs_scrub: hoist the phase3 bulkstat single stepping code
* xfs_scrub: ignore freed inodes when single-stepping during phase 3
* xfs_scrub: try harder to fill the bulkstat array with bulkstat()
---
include/cache.h | 6
io/parent.c | 9 -
libfrog/Makefile | 1
libfrog/bitmask.h | 6
libfrog/handle_priv.h | 55 ++++
libxfs/buf_mem.c | 159 +++++++++---
libxfs/buf_mem.h | 3
libxfs/cache.c | 11 +
man/man2/ioctl_xfs_bulkstat.2 | 8 +
scrub/common.c | 9 -
scrub/inodes.c | 552 ++++++++++++++++++++++++++++++++++++-----
scrub/inodes.h | 12 +
scrub/phase3.c | 7 -
scrub/phase5.c | 14 -
scrub/phase6.c | 14 +
spaceman/health.c | 9 -
16 files changed, 726 insertions(+), 149 deletions(-)
create mode 100644 libfrog/handle_priv.h