Re: [PATCH v2 1/5] mm/memory-failure: keep the folio, not the poisoned subpage, locked across split

["David Hildenbrand (Arm)" <david@kernel.org>]

On 7/15/26 12:42, Kiryl Shutsemau wrote:
> On Tue, Jul 14, 2026 at 01:31:54PM -0400, Zi Yan wrote:
>> On Tue Jul 14, 2026 at 12:40 PM EDT, Kiryl Shutsemau wrote:
>>>
>>> I saw this option too, but I wound rather not go this path.
>>>
>>> iput() still can lead to inode eviction an bunch of random filesystem
>>> complexity under us. I don't think we want to think about other
>>> fs-related locking issues in split context.
>>
>> Your reasoning makes sense to me. Let's ignore this option.
>>
>> For your patch 2, we might want something like below to avoid over
>> rejecting splits. WDYT?
>>
>> offset = folio_page_idx(folio, lock_at);
>>
>> if (split_type == SPLIT_TYPE_UNIFORM)
>> 	lock_at_index = folio->index + round_down(offset, 1UL << new_order);
>> else
>> 	/* @lock_at in non uniform split is always @folio */
>> 	lock_at_index = folio->index;
>>
>> if (lock_at_index >= end) {
>> 	ret = -EBUSY;
>> 	goto out_unlock;
>> }
>>
> 
> Right. With the -EBUSY condition growing this hairy -- and having to stay
> correct for non-uniform splits too -- just moving i_mmap_unlock_read() out
> of the window looks more attractive.
> 
> This is really Hao's original patch with the reasoning corrected, so I kept
> him as author. v3 below.
> 
> ----------------------------------------------------------------------
> 
> From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
> Subject: [PATCH v3] mm/huge_memory: unlock i_mmap_rwsem before releasing
>  after-split folios
> 
> __folio_split() keeps dereferencing the mapping after the split:
> shmem_uncharge(mapping->host) and remap_page() while the folios are still
> frozen/locked, and i_mmap_unlock_read(mapping) at the very end, after the
> after-split folios have been unlocked and freed.
> 
> Nothing holds an inode reference across that. The split relies on @folio
> -- which the beyond-EOF drop loop never removes, as it starts at
> folio_next(folio) -- staying locked and in the page cache to hold off
> eviction. But the unlock loop unlocks @folio before i_mmap_unlock_read()
> runs. If the caller's @lock_at is a tail beyond EOF, as memory_failure()
> passes when splitting a poisoned tail of a shmem THP that reaches past
> i_size during truncation, it too is gone from the page cache; so once
> @folio is unlocked no locked, in-cache folio pins the inode, and a
> concurrent final iput() can evict and RCU-free it before
> i_mmap_unlock_read() touches i_mmap_rwsem:
> 
>   BUG: KASAN: slab-use-after-free in __up_read+0x634/0x790
>    i_mmap_unlock_read include/linux/fs.h:537 [inline]
>    __folio_split+0x732/0x1640 mm/huge_memory.c:4100
>    try_to_split_thp_page+0xab/0x390 mm/memory-failure.c:1675
>    memory_failure+0x1394/0x26e0 mm/memory-failure.c:2470
> 
>   Freed by task 4601:
>    shmem_free_in_core_inode+0x54/0xb0 mm/shmem.c:5177
>    evict+0x57f/0xac0 fs/inode.c:870
> 
> Do every mapping dereference while @folio still pins the inode: drop
> i_mmap_rwsem right after remap_page(), before the loop that unlocks and
> frees the after-split folios, and clear @mapping so the exit path does not
> unlock it again. shmem_uncharge() and remap_page() already run before that
> point, so after this nothing past the unlock loop touches the inode or the
> mapping.
> 
> This is now a rule the split depends on, alongside keeping @folio frozen
> until the page cache is updated: no inode or mapping dereference once the
> after-split folios start being unlocked.
> 
> Reported-by: Hao Zhang <zhanghao1@kylinos.cn>
> Closes: https://lore.kernel.org/linux-mm/20260710071344.GA106129@zh-pc
> Fixes: baa355fd3314 ("thp: file pages support for split_huge_page()")
> Cc: <stable@vger.kernel.org>
> Co-developed-by: Hao Zhang <zhanghao1@kylinos.cn>
> Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> ---
>  mm/huge_memory.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 2bccb0a53a0a..abaea34ef558 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -4109,6 +4109,18 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
> 
>  	remap_page(folio, 1 << old_order, ttu_flags);
> 
> +	/*
> +	 * Drop the mapping while the inode is still pinned. @folio stays
> +	 * locked and present in the page cache until the loop below, so
> +	 * eviction cannot free the inode yet; @lock_at is not enough, it may
> +	 * be a tail beyond EOF that the split already dropped from the page
> +	 * cache. Nothing past this point may touch the inode or the mapping.
> +	 */
> +	if (mapping) {
> +		i_mmap_unlock_read(mapping);
> +		mapping = NULL;
> +	}
> +
>  	/*
>  	 * Unlock all after-split folios except the one containing
>  	 * @lock_at page. If @folio is not split, it will be kept locked.

LGTM

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David