| Message ID | 164021525963.640689.9264556596205140044.stgit@warthog.procyon.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fscache, cachefiles: Rewrite | expand |
On Wed, 2021-12-22 at 23:20 +0000, David Howells wrote: > Provide a function to be called from a network filesystem's releasepage > method to indicate that a page has been released that might have been a > reflection of data upon the server - and now that data must be reloaded > from the server or the cache. > > This is used to end an optimisation for empty files, in particular files > that have just been created locally, whereby we know there cannot yet be > any data that we would need to read from the server or the cache. > > Signed-off-by: David Howells <dhowells@redhat.com> > cc: linux-cachefs@redhat.com > Link: https://lore.kernel.org/r/163819617128.215744.4725572296135656508.stgit@warthog.procyon.org.uk/ # v1 > Link: https://lore.kernel.org/r/163906920354.143852.7511819614661372008.stgit@warthog.procyon.org.uk/ # v2 > Link: https://lore.kernel.org/r/163967128061.1823006.611781655060034988.stgit@warthog.procyon.org.uk/ # v3 > --- > > include/linux/fscache.h | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/include/linux/fscache.h b/include/linux/fscache.h > index 18e725671594..28ce258c1f87 100644 > --- a/include/linux/fscache.h > +++ b/include/linux/fscache.h > @@ -607,4 +607,20 @@ static inline void fscache_clear_inode_writeback(struct fscache_cookie *cookie, > } > } > > +/** > + * fscache_note_page_release - Note that a netfs page got released > + * @cookie: The cookie corresponding to the file > + * > + * Note that a page that has been copied to the cache has been released. This > + * means that future reads will need to look in the cache to see if it's there. > + */ > +static inline > +void fscache_note_page_release(struct fscache_cookie *cookie) > +{ > + if (cookie && > + test_bit(FSCACHE_COOKIE_HAVE_DATA, &cookie->flags) && > + test_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags)) > + clear_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags); > +} > + > #endif /* _LINUX_FSCACHE_H */ > > Is this logic correct? FSCACHE_COOKIE_HAVE_DATA gets set in cachefiles_write_complete, but will that ever be called on a cookie that has no data? Will we ever call cachefiles_write at all when there is no data to be written? -- Jeff Layton <jlayton@kernel.org>
Jeff Layton <jlayton@kernel.org> wrote: > > +/** > > + * fscache_note_page_release - Note that a netfs page got released > > + * @cookie: The cookie corresponding to the file > > + * > > + * Note that a page that has been copied to the cache has been released. This > > + * means that future reads will need to look in the cache to see if it's there. > > + */ > > +static inline > > +void fscache_note_page_release(struct fscache_cookie *cookie) > > +{ > > + if (cookie && > > + test_bit(FSCACHE_COOKIE_HAVE_DATA, &cookie->flags) && > > + test_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags)) > > + clear_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags); > > +} > > + > > #endif /* _LINUX_FSCACHE_H */ > > > > > > Is this logic correct? > > FSCACHE_COOKIE_HAVE_DATA gets set in cachefiles_write_complete, but will > that ever be called on a cookie that has no data? Will we ever call > cachefiles_write at all when there is no data to be written? FSCACHE_COOKIE_NO_DATA_TO_READ is set if we have no data in the cache yet (ie. the backing file lookup was negative, the file is 0 length or the cookie got invalidated). It means that we have no data in the cache, not that the file is necessarily empty on the server. FSCACHE_COOKIE_HAVE_DATA is set once we've stored data in the backing file. From that point on, we have data we *could* read - however, it's covered by pages in the netfs pagecache until at such time one of those covering pages is released. So if we've written data to the cache (HAVE_DATA) and there wasn't any data in the cache when we started (NO_DATA_TO_READ), it may no longer be true that we can skip reading from the cache. Read skipping is done by cachefiles_prepare_read(). Note that I'm not doing tracking on a per-page basis, but only on a per-file basis. David
diff --git a/include/linux/fscache.h b/include/linux/fscache.h index 18e725671594..28ce258c1f87 100644 --- a/include/linux/fscache.h +++ b/include/linux/fscache.h @@ -607,4 +607,20 @@ static inline void fscache_clear_inode_writeback(struct fscache_cookie *cookie, } } +/** + * fscache_note_page_release - Note that a netfs page got released + * @cookie: The cookie corresponding to the file + * + * Note that a page that has been copied to the cache has been released. This + * means that future reads will need to look in the cache to see if it's there. + */ +static inline +void fscache_note_page_release(struct fscache_cookie *cookie) +{ + if (cookie && + test_bit(FSCACHE_COOKIE_HAVE_DATA, &cookie->flags) && + test_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags)) + clear_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags); +} + #endif /* _LINUX_FSCACHE_H */
Provide a function to be called from a network filesystem's releasepage method to indicate that a page has been released that might have been a reflection of data upon the server - and now that data must be reloaded from the server or the cache. This is used to end an optimisation for empty files, in particular files that have just been created locally, whereby we know there cannot yet be any data that we would need to read from the server or the cache. Signed-off-by: David Howells <dhowells@redhat.com> cc: linux-cachefs@redhat.com Link: https://lore.kernel.org/r/163819617128.215744.4725572296135656508.stgit@warthog.procyon.org.uk/ # v1 Link: https://lore.kernel.org/r/163906920354.143852.7511819614661372008.stgit@warthog.procyon.org.uk/ # v2 Link: https://lore.kernel.org/r/163967128061.1823006.611781655060034988.stgit@warthog.procyon.org.uk/ # v3 --- include/linux/fscache.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)