Re: [PATCH v2] fuse: honor desc offset in readahead reads

[Haofeng Li]

Hi Miklos,

Thanks for the pointer.  I looked into the cases raised by Sashiko, and
all three findings are valid.

More precisely, these are partial-folio rather than large-folio-specific
issues.  The relevant condition is that a folio contains multiple
filesystem blocks.  Large folios are one way to reach that condition,
but fuseblk with blksize < PAGE_SIZE is another.

On the current tree, regular FUSE and virtiofs use PAGE_SIZE as the
filesystem block size, and FUSE does not opt its mappings into large
folio allocation with mapping_set_large_folios().  Therefore, with
order-0 folios, iomap_adjust_read_range() cannot skip leading blocks and
produce a non-zero descriptor offset.  Unless the test tree enabled
large folios for FUSE separately, this likely explains why fsx-linux did
not exercise this path.  fsx-linux does verify data contents, so it
should detect the corruption if the partial-folio state is reached.

The currently reachable in-tree case is fuseblk with blksize < PAGE_SIZE,
after commit 93570c652b80 ("fuse: remove fc->blkbits workaround for
partial writes").  A partial buffered write can leave leading blocks
uptodate while later blocks still require readahead.

The three findings are:

1. In the /dev/fuse copy path, fuse_copy_folio() zeroes the entire folio
   when page_zeroing is set and count < folio_size().  For a partial-folio
   descriptor, this destroys the leading uptodate or dirty blocks that
   iomap intentionally skipped.  It should zero only the unfilled
   descriptor range:

       [offset + count, offset + length)

2. The virtiofs scatterlist starts at descs[i].offset, but its short-read
   completion zeroes the absolute range [len, thislen).  For a non-zero
   descriptor offset, the correct range is:

       [offset + len, offset + thislen)

   The existing WARN_ON(descs[i].offset) reflects the old zero-offset
   assumption, which is no longer valid for partial-folio reads.

3. At LLONG_MAX, fuse_send_readpages() decrements both the request count
   and the last descriptor length.  iomap has already accounted the
   original descriptor length, while fuse_readpages_end() completes only
   the decremented length.  With an iomap_folio_state,
   read_bytes_pending is left at one and the folio is never unlocked.
   This affects both values of the async argument, since both completion
   paths go through fuse_readpages_end().  The boundary is narrow, but it
   is deterministically testable by extending the generic/525 scenario
   to a multi-block folio configuration.  The transfer count may be
   reduced, but the original descriptor length needs to be retained for
   iomap completion.

These issues predate this patch: non-zero descriptor offsets were already
recorded and used on the reply side.  This patch fixes the independent
request-position mismatch so that the requested file range agrees with
the descriptor offset; it does not change descriptor construction or the
copy and completion handling above.

I will prepare the three additional fixes as separate patches.  I can
either send them as follow-ups or respin this patch as part of a series,
whichever you prefer.

Thanks,
Haofeng