[PATCH v3 6/7] mm/memfd_luo: remove folio from page cache when accounting fails

Chenghao Duan posted 7 patches 1 week ago
[PATCH v3 6/7] mm/memfd_luo: remove folio from page cache when accounting fails
Posted by Chenghao Duan 1 week ago
In memfd_luo_retrieve_folios(), when shmem_inode_acct_blocks() fails
after successfully adding the folio to the page cache, the code jumps
to unlock_folio without removing the folio from the page cache.

This leaves the folio permanently abandoned in the page cache:
- The folio was added via shmem_add_to_page_cache() which set up
  mapping, index, and incremented nrpages/shmem stats.
- folio_unlock() and folio_put() do not remove it from the cache.
- folio_add_lru() was never called, so it cannot be reclaimed.

Fix by adding a remove_from_cache label that calls filemap_remove_folio()
before unlocking, matching the error handling pattern in
shmem_alloc_and_add_folio().

This issue was identified by the AI review.
https://sashiko.dev/#/patchset/20260323110747.193569-1-duanchenghao@kylinos.cn

Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
---
 mm/memfd_luo.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mm/memfd_luo.c b/mm/memfd_luo.c
index b4cea3670689..f8e8f99b1848 100644
--- a/mm/memfd_luo.c
+++ b/mm/memfd_luo.c
@@ -446,7 +446,7 @@ static int memfd_luo_retrieve_folios(struct file *file,
 		if (err) {
 			pr_err("shmem: failed to account folio index %ld(%ld pages): %d\n",
 			       i, npages, err);
-			goto unlock_folio;
+			goto remove_from_cache;
 		}
 
 		nr_added_pages += npages;
@@ -459,6 +459,8 @@ static int memfd_luo_retrieve_folios(struct file *file,
 
 	return 0;
 
+remove_from_cache:
+	filemap_remove_folio(folio);
 unlock_folio:
 	folio_unlock(folio);
 	folio_put(folio);
-- 
2.25.1
Re: [PATCH v3 6/7] mm/memfd_luo: remove folio from page cache when accounting fails
Posted by Pratyush Yadav 10 hours ago
On Thu, Mar 26 2026, Chenghao Duan wrote:

> In memfd_luo_retrieve_folios(), when shmem_inode_acct_blocks() fails
> after successfully adding the folio to the page cache, the code jumps
> to unlock_folio without removing the folio from the page cache.
>
> This leaves the folio permanently abandoned in the page cache:
> - The folio was added via shmem_add_to_page_cache() which set up
>   mapping, index, and incremented nrpages/shmem stats.
> - folio_unlock() and folio_put() do not remove it from the cache.
> - folio_add_lru() was never called, so it cannot be reclaimed.

This is just not true. The folio is _not_ "permanently abandoned" in the
page cache. When fput() is called by memfd_luo_retrieve(), it will
eventually call shmem_undo_range() on the whole mapping and free all the
folios in there.

I went and looked at shmem_undo_range() and the accompanying accounting
logic, and all that seems to be impervious to this type of superfluous
folio in the filemap. Main reason being that shmem_recalc_inode()
directly uses mapping->nrpages after truncation so even if you don't
account for the folio, as long as you get rid of the whole file (which
we do) it doesn't matter.

I think the only place I can see this causing trouble is maybe in LRU
accounting, though I really don't understand how any of that works so
dunno.

Anyway, I do think this patch is worth having. It keeps the filemap
clean and gets rid of the need of this complex reasoning to figure out
if this is safe.

So I think the commit message needs reworking. Perhaps something like
the below:

    mm/memfd_luo: remove folio from page cache when accounting fails

    In memfd_luo_retrieve_folios(), when shmem_inode_acct_blocks() fails
    after successfully adding the folio to the page cache, the code jumps
    to unlock_folio without removing the folio from the page cache.

    While the folio eventually will be freed when the file is released by
    memfd_luo_retrieve(), it is a good idea to directly remove a folio that
    was not fully added to the file. This avoids the possibility of
    accounting mismatches in shmem or filemap core.

    Fix by adding a remove_from_cache label that calls filemap_remove_folio()
    before unlocking, matching the error handling pattern in
    shmem_alloc_and_add_folio().

    This issue was identified by the AI review.
    https://sashiko.dev/#/patchset/20260323110747.193569-1-duanchenghao@kylinos.cn

With that,

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

>
> Fix by adding a remove_from_cache label that calls filemap_remove_folio()
> before unlocking, matching the error handling pattern in
> shmem_alloc_and_add_folio().
>
> This issue was identified by the AI review.
> https://sashiko.dev/#/patchset/20260323110747.193569-1-duanchenghao@kylinos.cn
>
> Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
[...]

-- 
Regards,
Pratyush Yadav
Re: [PATCH v3 6/7] mm/memfd_luo: remove folio from page cache when accounting fails
Posted by Andrew Morton 4 hours ago
On Thu, 02 Apr 2026 11:52:57 +0000 Pratyush Yadav <pratyush@kernel.org> wrote:

> So I think the commit message needs reworking. Perhaps something like
> the below:
>
> ...
>
> With that,
> 
> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

Thanks, I did this:

From: Chenghao Duan <duanchenghao@kylinos.cn>
Subject: mm/memfd_luo: remove folio from page cache when accounting fails
Date: Thu, 26 Mar 2026 16:47:26 +0800

In memfd_luo_retrieve_folios(), when shmem_inode_acct_blocks() fails
after successfully adding the folio to the page cache, the code jumps
to unlock_folio without removing the folio from the page cache.

While the folio eventually will be freed when the file is released by
memfd_luo_retrieve(), it is a good idea to directly remove a folio that
was not fully added to the file.  This avoids the possibility of
accounting mismatches in shmem or filemap core.

Fix by adding a remove_from_cache label that calls
filemap_remove_folio() before unlocking, matching the error handling
pattern in shmem_alloc_and_add_folio().

This issue was identified by AI review:
https://sashiko.dev/#/patchset/20260323110747.193569-1-duanchenghao@kylinos.cn

[pratyush@kernel.org: changelog alterations]
  Link: https://lkml.kernel.org/r/2vxzzf3lfujq.fsf@kernel.org
Link: https://lkml.kernel.org/r/20260326084727.118437-7-duanchenghao@kylinos.cn
Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
Reviewed-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
Cc: Haoran Jiang <jianghaoran@kylinos.cn>
Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/memfd_luo.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

--- a/mm/memfd_luo.c~mm-memfd_luo-remove-folio-from-page-cache-when-accounting-fails
+++ a/mm/memfd_luo.c
@@ -461,7 +461,7 @@ static int memfd_luo_retrieve_folios(str
 		if (err) {
 			pr_err("shmem: failed to account folio index %ld(%ld pages): %d\n",
 			       i, npages, err);
-			goto unlock_folio;
+			goto remove_from_cache;
 		}
 
 		nr_added_pages += npages;
@@ -474,6 +474,8 @@ static int memfd_luo_retrieve_folios(str
 
 	return 0;
 
+remove_from_cache:
+	filemap_remove_folio(folio);
 unlock_folio:
 	folio_unlock(folio);
 	folio_put(folio);
_
Re: [PATCH v3 6/7] mm/memfd_luo: remove folio from page cache when accounting fails
Posted by Pasha Tatashin 20 hours ago
On Thu, Mar 26, 2026 at 4:48 AM Chenghao Duan <duanchenghao@kylinos.cn> wrote:
>
> In memfd_luo_retrieve_folios(), when shmem_inode_acct_blocks() fails
> after successfully adding the folio to the page cache, the code jumps
> to unlock_folio without removing the folio from the page cache.
>
> This leaves the folio permanently abandoned in the page cache:
> - The folio was added via shmem_add_to_page_cache() which set up
>   mapping, index, and incremented nrpages/shmem stats.
> - folio_unlock() and folio_put() do not remove it from the cache.
> - folio_add_lru() was never called, so it cannot be reclaimed.
>
> Fix by adding a remove_from_cache label that calls filemap_remove_folio()
> before unlocking, matching the error handling pattern in
> shmem_alloc_and_add_folio().
>
> This issue was identified by the AI review.
> https://sashiko.dev/#/patchset/20260323110747.193569-1-duanchenghao@kylinos.cn
>
> Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
> ---
>  mm/memfd_luo.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/mm/memfd_luo.c b/mm/memfd_luo.c
> index b4cea3670689..f8e8f99b1848 100644
> --- a/mm/memfd_luo.c
> +++ b/mm/memfd_luo.c
> @@ -446,7 +446,7 @@ static int memfd_luo_retrieve_folios(struct file *file,
>                 if (err) {
>                         pr_err("shmem: failed to account folio index %ld(%ld pages): %d\n",
>                                i, npages, err);
> -                       goto unlock_folio;
> +                       goto remove_from_cache;
>                 }
>
>                 nr_added_pages += npages;
> @@ -459,6 +459,8 @@ static int memfd_luo_retrieve_folios(struct file *file,
>
>         return 0;
>
> +remove_from_cache:
> +       filemap_remove_folio(folio);
>  unlock_folio:

Reviewed-by: Pasha Tatashin <pasha.tatashin@soleen.com>

Thanks,
Pasha