Re: [PATCH] ceph: do not fill fscache for RWF_DONTCACHE writeback

[Max Kellermann]

On Thu, Apr 2, 2026 at 9:44 PM Viacheslav Dubeyko <vdubeyko@xxxxxxxxxx> wrote:
> The Ceph write_begin path calls netfs_write_begin() but does not pass
> IOCB_DONTCACHE through to trigger __folio_set_dropbehind. So,
> folio_test_dropbehind() would never be true on the Ceph write path right now.
> Does it make sense?

Yes, see:

> > ---
> > Note: this is an additional feature on top of my Ceph-DONTCACHE patch,
> > see https://lore.kernel.org/ceph-devel/20260401053109.1861724-1-max.kellermann@xxxxxxxxx/

The code in this patch is not reachable until my RWF_DONTCACHE patch
is merged as well.

> Are you sure that caching should be always true? All other calls checks that
> ceph_is_cache_enabled():
>
> bool caching = ceph_is_cache_enabled(inode);

This function is only called if caching is enabled.

>
> > +     *len = 0;
> > +}
>
>
> The ceph_folio_is_cacheable() and ceph_flush_fscache_write() are out of
> CONFIG_CEPH_FSCACHE. It doesn't look right.

All of the old code is out of CONFIG_CEPH_FSCACHE, too. Does the old
code look right?

> > @@ -1412,11 +1427,14 @@ int ceph_submit_write(struct address_space *mapping,
> >       bool caching = ceph_is_cache_enabled(inode);
> >       u64 offset;
> >       u64 len;
> > +     u64 cache_offset, cache_len;
>
> Why do you need to introduce the cache_offset and cache_len? We already have
> offset and len.

These keep track of the region that should be submitted to fscache.
Folios without "dropbehind" need to be excluded from that.

> >  new_request:
> >       offset = ceph_fscrypt_page_offset(ceph_wbc->pages[0]);
> >       len = ceph_wbc->wsize;
> > +     cache_offset = 0;
>
> Is it correct initialization? Frankly speaking, I don't quite follow why we need
> such initialization.

Technically, cache_offset does not need to be initialized as long as
cache_len is zero because then its value is never used. Would you feel
more comfortable if I remove the unnecessary initializer? I wasn't
sure which approach would raise fewer eyebrows.

-- 
Max Kellermann
Principal Architect
Hosting Technology

cm4all | Im Mediapark 6a | 50670 Köln | Germany
General information about the company can be found here:
https://www.cm4all.com/impressum
A member of the IONOS Group