Re: [PATCH v12 0/5] netfs, cachefiles: Changes for next, primarily occupancy tracking-related

[Petr Chmelar <petr.chmelar@innovatrics.com>]

Hi David,

The cover letter leaves non-contiguous objects, which still use
SEEK_DATA/SEEK_HOLE, for a later series.  We hit that case badly in
production, so here are numbers in case they help with its priority.

Objects get into that state when a large file is read at random
offsets, as training jobs do with data sets on NFS.  Each read is
copied to the cache at its own offset, and with other files being
cached at the same time most pieces end up in separate XFS extents.  The
object is BACKFS_MAP from the first discontiguous write and stays so
once it has filled in.

SEEK_HOLE has no upper bound.  On XFS, iomap_seek_hole() calls
xfs_seek_iomap_begin() once per extent until it finds a hole, and in a
fully populated object the only hole is EOF.  cachefiles probes the
occupancy at least once per read request, so a sequential read of such
an object walks the rest of its extent list for every readahead batch.

On our NFS clients (Ubuntu, v6.8, cache on XFS), a sequential read of a
fully cached 184 GB object with 1,080,732 extents ran at 0.4 MB/s.  In
a 15 s trace of one of the affected readers, each SEEK_HOLE took 64 ms
to 1 s, XFS did 58.5 million extent lookups, and nothing was read from
the server.  After xfs_fsr brought the object down to a few hundred
extents, a traced read of it ran at 369 MB/s from the cache.  On
another machine, v6.14 does the same walk at about 220 ns per extent,
against 520 ns on v6.8.

It's the same on vfs-7.4.netfs (9f8b64f66).  In QEMU (TCG, 4 vCPUs),
over an NFSv4.2 loopback mount with fsc and the cache on XFS, a 1 GiB
object filled by random 64 KiB reads, while a second file is filled
the same way, ends up with about 16,300 extents.  A sequential read of
it after dropping the page cache makes 4,098 SEEK_HOLE calls that take
176 s in total, and runs at 4.3 MiB/s.

To confirm that the walk is the whole problem, I wrote a naive
workaround with LLM assistance.  I don't know the cachefiles and netfs
internals well, so take it as a probe rather than a proposal:

 - Each cachefiles_object remembers the last region that SEEK_DATA/
   SEEK_HOLE found to be contiguous data.  cachefiles_query_occupancy()
   answers a query that starts inside that region without seeking, and
   overlapping or abutting regions merge.  A sequential read then pays
   for one walk instead of one per request.

 - The region is forgotten after every place where cachefiles removes
   data from a backing file: the punch on low space in
   __cachefiles_prepare_write(), the padding clear and the truncation in
   cachefiles_collect_write(), cachefiles_shorten_object() and
   invalidation.  A sequence number sampled before the seeks stops a
   query that raced with a removal from recording what it saw, and
   cookie->inval_counter ties the region to the backing file.

In the same test, the workaround makes one SEEK_HOLE call taking 61 ms,
and the read runs at 17 MiB/s.  A debug build that checked every
remembered answer against a fresh SEEK_DATA/SEEK_HOLE saw no mismatch in
more than 7,000 checks over NFS and 9p.  It only helps fully or mostly
populated objects read in order; random reads of a partly cached object
still seek.

I expect the right fix is for cachefiles to track what it has written
itself, as CONTENT_ALL now does for contiguous files, rather than
something like the above.  I can send the workaround if it's useful as
a reference, and I'm happy to test whatever you plan for this case on
our workload.

While testing it I hit a data corruption on partly cached objects that
bisects to patch 4/5 of this series and reproduces without the
workaround; I've reported it in a reply to that patch:

  https://lore.kernel.org/r/20261011.cachefiles-partial-zeros.pchmelar@innovatrics.com

The numbers on vfs-7.4.netfs and the debug cross-check were also
produced with LLM assistance.

Thanks,
Petr