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

[Pratyush Yadav]

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@xxxxxxxxxx

With that,

Reviewed-by: Pratyush Yadav <pratyush@xxxxxxxxxx>

>
> 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@xxxxxxxxxx
>
> Signed-off-by: Chenghao Duan <duanchenghao@xxxxxxxxxx>
[...]

-- 
Regards,
Pratyush Yadav