Re: [PATCH v2 06/47] mm/filemap: read mapping->a_ops->free_folio under i_lock

[Zi Yan]

On Thu Oct 29, 2026 at 4:33 PM EDT, David Hildenbrand (Arm) wrote:
> From: Josef Bacik <josef@xxxxxxxxxxxxxx>
>
> memory_failure() -> ... -> filemap_remove_folio() can read mapping->a_ops
> after the inode has been freed. Once the folio is deleted from the
> pagecache it no longer pins the mapping, so after i_lock is dropped
> e.g., a concurrent final iput() can evict and free the inode before
> filemap_free_folio() looks up mapping->a_ops->free_folio.
>
> Most callers hold a reference on the inode, but not all.
>
> Documentation/filesystems/vfs.rst explicitly states that ->free_folio()
> must not assume the original address_space mapping still exists.
>
> Fix it by loading ->free_folio while i_lock is still held, similar to
> what folio_unmap_invalidate() does.
>
> Inline filemap_free_folio() into its remaining caller such that
> delete_from_page_cache_batch() also just loads the callback before
> dropping the i_lock.
>
> The bug was found by code inspection and has not been reproduced.
>
> Fix generated by an LLM and sent by Josef Bacik in private.
>
> Fixes: 59c66c5f8c4f ("mm: factor out page cache page freeing into a separate function")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> [ david: get rid of filemap_free_folio() entirely; improve patch description ]
> Reviewed-by: Rik van Riel <riel@xxxxxxxxxxx>
> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> ---
>  mm/filemap.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
>

Makes sense.

Reviewed-by: Zi Yan <ziy@xxxxxxxxxx>


-- 
Best Regards,
Yan, Zi