Re: [PATCH] ceph: acquire caps for read_folio requests without an rw context

[Ilya Dryomov]

On Fri, Sep 4, 2026 at 6:55 AM Xiubo Li <xiubo.li@xxxxxxxxx> wrote:
>
> Hi Max,
>
> The fix mechanism looks right: a NETFS_READPAGE request issued via
> read_mapping_folio() with no rw context should acquire caps, blocking,
> before hitting the OSD, so it can't race a writer that hasn't flushed
> yet. Two things though:
>
> There is one existing Ceph-internal caller that now lands in the new
> blocking branch: ceph_uninline_data() calls
> read_mapping_folio(inode->i_mapping, 0, file) from ceph_init_file_info()
> (fs/ceph/file.c:262) at open() time, with no rw context. On an
> O_WRONLY open of an inline-data file with no recent readers, RD isn't
> part of the file's wanted caps (__ceph_caps_file_wanted() derives them
> from the used fmode), so __ceph_get_caps(RD) takes the -EUCLEAN path
> into ceph_renew_caps(), which raises the wanted set via
> __ceph_touch_fmode() and issues a synchronous MDS open request. Such an
> open now pays an extra sync MDS round-trip, and open() can newly fail on
> MDS errors. This read also doesn't need caps: the OSD object is not
> created until the uninline CREATE, and this path intentionally relies
> on the -ENOENT -> zero-fill behavior. Could you handle this internal
> caller explicitly (e.g. have ceph_uninline_data()'s read bypass the caps
> check), rather than making every cap-less READPAGE unconditionally
> block?
>
> The blocking acquisition now happens with the folio locked
> (read_folio contract), inverting the usual caps-before-folio-lock order
> of ceph_read_iter()/ceph_filemap_fault(). I checked the obvious cycles
> and didn't find one: a folio that reaches read_folio is clean (dirty =>
> uptodate => read_folio is never called, and netfs_read_folio() diverts
> dirty folios to netfs_read_gaps() before the request is allocated), so
> the Fb-revoke flush/writeback never needs to lock it, and the truncate
> path uses folio_trylock. But please document that invariant in a
> comment, since it's what makes waiting for caps under the folio lock
> safe.
>
> Minor: the new branch hardcodes want = CEPH_CAP_FILE_CACHE, while the
> other read paths add CEPH_CAP_FILE_LAZYIO for lazyio files
> (ceph_filemap_fault(), ceph_splice_read()); it may also be worth
> aligning.
>
> It might also help stable reviewers if the commit message notes that
> current mainline erofs fileio reads via vfs_iocb_iter_read() ->
> ceph_read_iter(), so the trigger is gone there and the patch mainly
> matters for stable kernels and future direct read_mapping_folio()
> callers.

Hi Max,

Did you get a chance to follow up on Xiubo's comments?  Particularly on
whether the data corruption aspect is indeed gone with EROFS switching
away from read_mapping_folio() in the upcoming release:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e587a984d332b76ab89fab546f1594f61f73d5f6

If so, I don't think the Fixes tag applies anymore (although it was
questionable to begin with as this patch is fixing an oversight in
CephFS, not in EROFS).

Thanks,

                Ilya