[v4,28/68] fscache: Provide a function to note the release of a page

Commit Message

David Howells Dec. 22, 2021, 11:20 p.m. UTC

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 <[e-mail]>
cc: [e-mail]
Link: https://lore.kernel.org/r/[e-mail]/ # v1
Link: https://lore.kernel.org/r/[e-mail]/ # v2
Link: https://lore.kernel.org/r/[e-mail]/ # v3
---

 include/linux/fscache.h |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Jeff Layton Jan. 6, 2022, 3:55 p.m. UTC | #1

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 <[e-mail]>
> cc: [e-mail]
> Link: https://lore.kernel.org/r/[e-mail]/ # v1
> Link: https://lore.kernel.org/r/[e-mail]/ # v2
> Link: https://lore.kernel.org/r/[e-mail]/ # 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 <[e-mail]>

David Howells Jan. 6, 2022, 4:26 p.m. UTC | #2

Jeff Layton <[e-mail]> 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

Patch

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 */