[PATCH 0/2] Use folios for fuse readdir cache

[PATCH 0/2] Use folios for fuse readdir cache

[Matthew Wilcox (Oracle)]

The page cache stores folios, not pages.  While there's no need to use
large folios for this cache (nor advantage, I suspect), using a folio
here gets rid of about ten calls to compound_head() and some uses of
deprecated APIs.  This patch set is only compile-tested.

Matthew Wilcox (Oracle) (2):
  fuse: Use a folio in fuse_add_dirent_to_cache()
  fuse: Use a folio in fuse_readdir_cached()

 fs/fuse/readdir.c | 57 ++++++++++++++++++++++++-----------------------
 1 file changed, 29 insertions(+), 28 deletions(-)

-- 
2.47.2

[PATCH 1/2] fuse: Use a folio in fuse_add_dirent_to_cache()

[Matthew Wilcox (Oracle)]

Retrieve a folio from the page cache and use it throughout.
Removes three hidden calls to compound_head().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/fuse/readdir.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
index c2aae2eef086..09bed488ee35 100644
--- a/fs/fuse/readdir.c
+++ b/fs/fuse/readdir.c
@@ -35,7 +35,7 @@ static void fuse_add_dirent_to_cache(struct file *file,
 	struct fuse_inode *fi = get_fuse_inode(file_inode(file));
 	size_t reclen = FUSE_DIRENT_SIZE(dirent);
 	pgoff_t index;
-	struct page *page;
+	struct folio *folio;
 	loff_t size;
 	u64 version;
 	unsigned int offset;
@@ -62,12 +62,13 @@ static void fuse_add_dirent_to_cache(struct file *file,
 	spin_unlock(&fi->rdc.lock);
 
 	if (offset) {
-		page = find_lock_page(file->f_mapping, index);
+		folio = filemap_lock_folio(file->f_mapping, index);
 	} else {
-		page = find_or_create_page(file->f_mapping, index,
-					   mapping_gfp_mask(file->f_mapping));
+		folio = __filemap_get_folio(file->f_mapping, index,
+				FGP_LOCK | FGP_ACCESSED | FGP_CREAT,
+				mapping_gfp_mask(file->f_mapping));
 	}
-	if (!page)
+	if (!folio)
 		return;
 
 	spin_lock(&fi->rdc.lock);
@@ -76,19 +77,19 @@ static void fuse_add_dirent_to_cache(struct file *file,
 	    WARN_ON(fi->rdc.pos != pos))
 		goto unlock;
 
-	addr = kmap_local_page(page);
+	addr = kmap_local_folio(folio, offset);
+	memcpy(addr, dirent, reclen);
 	if (!offset) {
-		clear_page(addr);
-		SetPageUptodate(page);
+		memset(addr + reclen, 0, PAGE_SIZE - reclen);
+		folio_mark_uptodate(folio);
 	}
-	memcpy(addr + offset, dirent, reclen);
 	kunmap_local(addr);
 	fi->rdc.size = (index << PAGE_SHIFT) + offset + reclen;
 	fi->rdc.pos = dirent->off;
 unlock:
 	spin_unlock(&fi->rdc.lock);
-	unlock_page(page);
-	put_page(page);
+	folio_unlock(folio);
+	folio_put(folio);
 }
 
 static void fuse_readdir_cache_end(struct file *file, loff_t pos)
-- 
2.47.2

[PATCH 2/2] fuse: Use a folio in fuse_readdir_cached()

[Matthew Wilcox (Oracle)]

Retrieve a folio from the page cache and use it throughout.
Removes seven hidden calls to compound_head().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/fuse/readdir.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
index 09bed488ee35..37abf4e484ec 100644
--- a/fs/fuse/readdir.c
+++ b/fs/fuse/readdir.c
@@ -452,7 +452,7 @@ static int fuse_readdir_cached(struct file *file, struct dir_context *ctx)
 	enum fuse_parse_result res;
 	pgoff_t index;
 	unsigned int size;
-	struct page *page;
+	struct folio *folio;
 	void *addr;
 
 	/* Seeked?  If so, reset the cache stream */
@@ -528,42 +528,42 @@ static int fuse_readdir_cached(struct file *file, struct dir_context *ctx)
 	if ((ff->readdir.cache_off & ~PAGE_MASK) == size)
 		return 0;
 
-	page = find_get_page_flags(file->f_mapping, index,
-				   FGP_ACCESSED | FGP_LOCK);
+	folio = __filemap_get_folio(file->f_mapping, index,
+			FGP_ACCESSED | FGP_LOCK, 0);
 	/* Page gone missing, then re-added to cache, but not initialized? */
-	if (page && !PageUptodate(page)) {
-		unlock_page(page);
-		put_page(page);
-		page = NULL;
+	if (folio && !folio_test_uptodate(folio)) {
+		folio_unlock(folio);
+		folio_put(folio);
+		folio = NULL;
 	}
 	spin_lock(&fi->rdc.lock);
-	if (!page) {
+	if (!folio) {
 		/*
-		 * Uh-oh: page gone missing, cache is useless
+		 * Uh-oh: folio gone missing, cache is useless
 		 */
 		if (fi->rdc.version == ff->readdir.version)
 			fuse_rdc_reset(inode);
 		goto retry_locked;
 	}
 
-	/* Make sure it's still the same version after getting the page. */
+	/* Make sure it's still the same version after getting the folio. */
 	if (ff->readdir.version != fi->rdc.version) {
 		spin_unlock(&fi->rdc.lock);
-		unlock_page(page);
-		put_page(page);
+		folio_unlock(folio);
+		folio_put(folio);
 		goto retry;
 	}
 	spin_unlock(&fi->rdc.lock);
 
 	/*
-	 * Contents of the page are now protected against changing by holding
-	 * the page lock.
+	 * Contents of the folio are now protected against changing by holding
+	 * the folio lock.
 	 */
-	addr = kmap_local_page(page);
+	addr = kmap_local_folio(folio, 0);
 	res = fuse_parse_cache(ff, addr, size, ctx);
 	kunmap_local(addr);
-	unlock_page(page);
-	put_page(page);
+	folio_unlock(folio);
+	folio_put(folio);
 
 	if (res == FOUND_ERR)
 		return -EIO;
-- 
2.47.2

Re: [PATCH 1/2] fuse: Use a folio in fuse_add_dirent_to_cache()

[Joanne Koong]

On Fri, Jun 13, 2025 at 9:46 AM Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
>
> Retrieve a folio from the page cache and use it throughout.
> Removes three hidden calls to compound_head().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Joanne Koong <joannelkoong@gmail.com>

> ---
>  fs/fuse/readdir.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
> index c2aae2eef086..09bed488ee35 100644
> --- a/fs/fuse/readdir.c
> +++ b/fs/fuse/readdir.c
> @@ -35,7 +35,7 @@ static void fuse_add_dirent_to_cache(struct file *file,
>         struct fuse_inode *fi = get_fuse_inode(file_inode(file));
>         size_t reclen = FUSE_DIRENT_SIZE(dirent);
>         pgoff_t index;
> -       struct page *page;
> +       struct folio *folio;
>         loff_t size;
>         u64 version;
>         unsigned int offset;
> @@ -62,12 +62,13 @@ static void fuse_add_dirent_to_cache(struct file *file,
>         spin_unlock(&fi->rdc.lock);
>
>         if (offset) {
> -               page = find_lock_page(file->f_mapping, index);
> +               folio = filemap_lock_folio(file->f_mapping, index);
>         } else {
> -               page = find_or_create_page(file->f_mapping, index,
> -                                          mapping_gfp_mask(file->f_mapping));
> +               folio = __filemap_get_folio(file->f_mapping, index,
> +                               FGP_LOCK | FGP_ACCESSED | FGP_CREAT,
> +                               mapping_gfp_mask(file->f_mapping));

nit: in the fuse codebase, the convention for line breaks is for the
next line to have the same indentation as where the previous line's
args start

>         }
> -       if (!page)
> +       if (!folio)
>                 return;

Re: [PATCH 1/2] fuse: Use a folio in fuse_add_dirent_to_cache()

[Joanne Koong]

On Fri, Jun 13, 2025 at 1:14 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Fri, Jun 13, 2025 at 9:46 AM Matthew Wilcox (Oracle)
> <willy@infradead.org> wrote:
> >
> > Retrieve a folio from the page cache and use it throughout.
> > Removes three hidden calls to compound_head().
> >
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>
> Reviewed-by: Joanne Koong <joannelkoong@gmail.com>
>
> > ---
> >  fs/fuse/readdir.c | 23 ++++++++++++-----------
> >  1 file changed, 12 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
> > index c2aae2eef086..09bed488ee35 100644
> > --- a/fs/fuse/readdir.c
> > +++ b/fs/fuse/readdir.c
> > @@ -35,7 +35,7 @@ static void fuse_add_dirent_to_cache(struct file *file,
> >         struct fuse_inode *fi = get_fuse_inode(file_inode(file));
> >         size_t reclen = FUSE_DIRENT_SIZE(dirent);
> >         pgoff_t index;
> > -       struct page *page;
> > +       struct folio *folio;
> >         loff_t size;
> >         u64 version;
> >         unsigned int offset;
> > @@ -62,12 +62,13 @@ static void fuse_add_dirent_to_cache(struct file *file,
> >         spin_unlock(&fi->rdc.lock);
> >
> >         if (offset) {
> > -               page = find_lock_page(file->f_mapping, index);
> > +               folio = filemap_lock_folio(file->f_mapping, index);
> >         } else {
> > -               page = find_or_create_page(file->f_mapping, index,
> > -                                          mapping_gfp_mask(file->f_mapping));
> > +               folio = __filemap_get_folio(file->f_mapping, index,
> > +                               FGP_LOCK | FGP_ACCESSED | FGP_CREAT,
> > +                               mapping_gfp_mask(file->f_mapping));
>
> nit: in the fuse codebase, the convention for line breaks is for the
> next line to have the same indentation as where the previous line's
> args start

Just noticed this, it looks like the filemap_grab_folio() api does the
same thing and can be used here.

>
> >         }
> > -       if (!page)
> > +       if (!folio)

I think this needs to be "if (IS_ERR(folio))" instead

> >                 return;

Re: [PATCH 2/2] fuse: Use a folio in fuse_readdir_cached()

[Joanne Koong]

On Fri, Jun 13, 2025 at 9:46 AM Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
>
> Retrieve a folio from the page cache and use it throughout.
> Removes seven hidden calls to compound_head().
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/fuse/readdir.c | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
> index 09bed488ee35..37abf4e484ec 100644
> --- a/fs/fuse/readdir.c
> +++ b/fs/fuse/readdir.c
> @@ -452,7 +452,7 @@ static int fuse_readdir_cached(struct file *file, struct dir_context *ctx)
>         enum fuse_parse_result res;
>         pgoff_t index;
>         unsigned int size;
> -       struct page *page;
> +       struct folio *folio;
>         void *addr;
>
>         /* Seeked?  If so, reset the cache stream */
> @@ -528,42 +528,42 @@ static int fuse_readdir_cached(struct file *file, struct dir_context *ctx)
>         if ((ff->readdir.cache_off & ~PAGE_MASK) == size)
>                 return 0;
>
> -       page = find_get_page_flags(file->f_mapping, index,
> -                                  FGP_ACCESSED | FGP_LOCK);
> +       folio = __filemap_get_folio(file->f_mapping, index,
> +                       FGP_ACCESSED | FGP_LOCK, 0);
>         /* Page gone missing, then re-added to cache, but not initialized? */

Should this comment also get updated to "Folio gone missing"?

> -       if (page && !PageUptodate(page)) {
> -               unlock_page(page);
> -               put_page(page);
> -               page = NULL;
> +       if (folio && !folio_test_uptodate(folio)) {

I think you meant "if (!(IS_ERR(folio) && ..." here?

> +               folio_unlock(folio);
> +               folio_put(folio);
> +               folio = NULL;
>         }
>         spin_lock(&fi->rdc.lock);
> -       if (!page) {
> +       if (!folio) {

same here

>                 /*
> -                * Uh-oh: page gone missing, cache is useless
> +                * Uh-oh: folio gone missing, cache is useless
>                  */
>                 if (fi->rdc.version == ff->readdir.version)
>                         fuse_rdc_reset(inode);
>                 goto retry_locked;
>         }
>
> -       /* Make sure it's still the same version after getting the page. */
> +       /* Make sure it's still the same version after getting the folio. */
>         if (ff->readdir.version != fi->rdc.version) {
>                 spin_unlock(&fi->rdc.lock);
> -               unlock_page(page);
> -               put_page(page);
> +               folio_unlock(folio);
> +               folio_put(folio);
>                 goto retry;
>         }
>         spin_unlock(&fi->rdc.lock);
>
>         /*
> -        * Contents of the page are now protected against changing by holding
> -        * the page lock.
> +        * Contents of the folio are now protected against changing by holding
> +        * the folio lock.
>          */
> -       addr = kmap_local_page(page);
> +       addr = kmap_local_folio(folio, 0);
>         res = fuse_parse_cache(ff, addr, size, ctx);
>         kunmap_local(addr);
> -       unlock_page(page);
> -       put_page(page);
> +       folio_unlock(folio);
> +       folio_put(folio);
>
>         if (res == FOUND_ERR)
>                 return -EIO;
> --
> 2.47.2
>