Re: [PATCH] mm: thp: Deny THP for guest_memfd and secretmem in file_thp_enabled()

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

On 2/11/26 00:00, Ackerley Tng wrote:
> "David Hildenbrand (Arm)" <david@kernel.org> writes:
> 
>>>
>>> I could give this a shot. 5.15.199 doesn't have AS_INACCESSIBLE. Should
>>> we backport AS_INACCESSIBLE there or could the fix for 5.15.199 just be
>>> special-casing secretmem like you suggested below?
>>
>> Yes. If there is no guest_memfd we wouldn't need it.
>>
> 
> Seems like on 5.15.199 there's a hugepage_vma_check(), which will return
> false since secretmem has vma->vm_ops defined [1], so secretmem VMAs are
> skipped.

Are you sure? We check for CONFIG_READ_ONLY_THP_FOR_FS before that:

	/* Only regular file is valid */
	if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && vma->vm_file &&
	    (vm_flags & VM_EXEC)) {
		struct inode *inode = vma->vm_file->f_inode;

		return !inode_is_open_for_write(inode) &&
			S_ISREG(inode->i_mode);
	}


So if you have VM_EXEC on the VMA (mmaped with PROT_EXEC), it would work.
I think secretmem sets SB_I_NOEXEC, which prevents that. Same for guest_memfd.

v6.6.123 still has that VM_EXEC check in file_thp_enabled().

The check was dropped in commit:

commit 7fbb5e188248c50f737720825da1864ce42536d1
Author: Fangrui Song <i@maskray.me>
Date:   Tue Dec 19 21:41:23 2023 -0800

     mm: remove VM_EXEC requirement for THP eligibility
     
     Commit e6be37b2e7bd ("mm/huge_memory.c: add missing read-only THP checking
     in transparent_hugepage_enabled()") introduced the VM_EXEC requirement,
     which is not strictly needed.
     
     lld's default --rosegment option and GNU ld's -z separate-code option
     (default on Linux/x86 since binutils 2.31) create a read-only PT_LOAD
     segment without the PF_X flag, which should be eligible for THP.


So that one broke secretmem.


So when we fix it, we should

Fixes: 7fbb5e188248 ("mm: remove VM_EXEC requirement for THP eligibility")


What about the following:

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 44ff8a648afd..9fbe5c28a6bc 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -94,6 +94,9 @@ static inline bool file_thp_enabled(struct vm_area_struct *vma)
  
         inode = file_inode(vma->vm_file);
  
+       if (IS_ANON_FILE(inode))
+               return false;
+
         return !inode_is_open_for_write(inode) && S_ISREG(inode->i_mode);
  }
  


-- 
Cheers,

David