[PATCH] mm/hugetlb: fix folio is still mapped when deleted

[PATCH] mm/hugetlb: fix folio is still mapped when deleted

[Jinjiang Tu]

Migration may be raced with fallocating hole. remove_inode_single_folio
will unmap the folio if the folio is still mapped. However, it's called
without folio lock. If the folio is migrated and the mapped pte has been
converted to migration entry, folio_mapped() returns false, and won't
unmap it. Due to extra refcount held by remove_inode_single_folio,
migration fails, restores migration entry to normal pte, and the folio
is mapped again. As a result, we triggered BUG in filemap_unaccount_folio.

The log is as follows:
 BUG: Bad page cache in process hugetlb  pfn:156c00
 page: refcount:515 mapcount:0 mapping:0000000099fef6e1 index:0x0 pfn:0x156c00
 head: order:9 mapcount:1 entire_mapcount:1 nr_pages_mapped:0 pincount:0
 aops:hugetlbfs_aops ino:dcc dentry name(?):"my_hugepage_file"
 flags: 0x17ffffc00000c1(locked|waiters|head|node=0|zone=2|lastcpupid=0x1fffff)
 page_type: f4(hugetlb)
 page dumped because: still mapped when deleted
 CPU: 1 UID: 0 PID: 395 Comm: hugetlb Not tainted 6.17.0-rc5-00044-g7aac71907bde-dirty #484 NONE
 Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 0.0.0 02/06/2015
 Call Trace:
  <TASK>
  dump_stack_lvl+0x4f/0x70
  filemap_unaccount_folio+0xc4/0x1c0
  __filemap_remove_folio+0x38/0x1c0
  filemap_remove_folio+0x41/0xd0
  remove_inode_hugepages+0x142/0x250
  hugetlbfs_fallocate+0x471/0x5a0
  vfs_fallocate+0x149/0x380

Hold folio lock before checking if the folio is mapped to avold race with
migration.

Fixes: 4aae8d1c051e ("mm/hugetlbfs: unmap pages if page fault raced with hole punch")
Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
---
 fs/hugetlbfs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 09d4baef29cf..d21865d0178a 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -521,10 +521,10 @@ static bool remove_inode_single_folio(struct hstate *h, struct inode *inode,
 	 * the fault mutex.  The mutex will prevent faults
 	 * until we finish removing the folio.
 	 */
+	folio_lock(folio);
 	if (unlikely(folio_mapped(folio)))
 		hugetlb_unmap_file_folio(h, mapping, folio, index);
 
-	folio_lock(folio);
 	/*
 	 * We must remove the folio from page cache before removing
 	 * the region/ reserve map (hugetlb_unreserve_pages).  In
-- 
2.43.0

Re: [PATCH] mm/hugetlb: fix folio is still mapped when deleted

[Matthew Wilcox]

On Thu, Sep 11, 2025 at 09:08:48PM +0800, Jinjiang Tu wrote:
> Migration may be raced with fallocating hole. remove_inode_single_folio
> will unmap the folio if the folio is still mapped. However, it's called
> without folio lock. If the folio is migrated and the mapped pte has been
> converted to migration entry, folio_mapped() returns false, and won't
> unmap it. Due to extra refcount held by remove_inode_single_folio,
> migration fails, restores migration entry to normal pte, and the folio
> is mapped again. As a result, we triggered BUG in filemap_unaccount_folio.

> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 09d4baef29cf..d21865d0178a 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -521,10 +521,10 @@ static bool remove_inode_single_folio(struct hstate *h, struct inode *inode,
>  	 * the fault mutex.  The mutex will prevent faults
>  	 * until we finish removing the folio.
>  	 */
> +	folio_lock(folio);

The comment above is now nonsensical.  Can you correct it, please?

>  	if (unlikely(folio_mapped(folio)))
>  		hugetlb_unmap_file_folio(h, mapping, folio, index);
>  
> -	folio_lock(folio);
>  	/*
>  	 * We must remove the folio from page cache before removing
>  	 * the region/ reserve map (hugetlb_unreserve_pages).  In
> -- 
> 2.43.0
> 
> 

Re: [PATCH] mm/hugetlb: fix folio is still mapped when deleted

[Jinjiang Tu]

[-- Attachment #1: Type: text/plain, Size: 1415 bytes --]


在 2025/9/12 2:13, Matthew Wilcox 写道:
> On Thu, Sep 11, 2025 at 09:08:48PM +0800, Jinjiang Tu wrote:
>> Migration may be raced with fallocating hole. remove_inode_single_folio
>> will unmap the folio if the folio is still mapped. However, it's called
>> without folio lock. If the folio is migrated and the mapped pte has been
>> converted to migration entry, folio_mapped() returns false, and won't
>> unmap it. Due to extra refcount held by remove_inode_single_folio,
>> migration fails, restores migration entry to normal pte, and the folio
>> is mapped again. As a result, we triggered BUG in filemap_unaccount_folio.
>> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
>> index 09d4baef29cf..d21865d0178a 100644
>> --- a/fs/hugetlbfs/inode.c
>> +++ b/fs/hugetlbfs/inode.c
>> @@ -521,10 +521,10 @@ static bool remove_inode_single_folio(struct hstate *h, struct inode *inode,
>>   	 * the fault mutex.  The mutex will prevent faults
>>   	 * until we finish removing the folio.
>>   	 */
>> +	folio_lock(folio);
> The comment above is now nonsensical.  Can you correct it, please?

OK, I will update it.

>
>>   	if (unlikely(folio_mapped(folio)))
>>   		hugetlb_unmap_file_folio(h, mapping, folio, index);
>>   
>> -	folio_lock(folio);
>>   	/*
>>   	 * We must remove the folio from page cache before removing
>>   	 * the region/ reserve map (hugetlb_unreserve_pages).  In
>> -- 
>> 2.43.0
>>
>>

[-- Attachment #2: Type: text/html, Size: 2295 bytes --]