Re: [PATCH] mm: clear pmd in pmdp_invalidate()

[David Hildenbrand (Arm)]

On 11/4/26 12:12, Guilherme Giacomo Simoes wrote:
> David, I haved immersed inside this problem on last days, and how sashiko-bot
> very well apointed, I'm haved doing a wrong duplicated clear on pmd's. I not
> notice that the pmd_establish() already called the page_table_check_pmd_set()
> wich in turn call __page_table_check_pmd_clear(), my bad.
> 
> At first time, I thinked the problem is a duplicated
> page_table_check_pmd_set(), but with a LLM helpful I ruled out this hypothesis,
> because I not found any obvious call to page_table_check_pmd_set() before
> remove_migration_pte(). At second time, I suspicious that maybe we are haved a
> race condition, that make no sense...
> 
> After a lot of debug and tests, I found a thing.. And I attempt to test this.
> If you see inside arch/riscv/include/asm/pgtable.h, the pmd_mkinvalid() do:
> ```
> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
> {
> 	return __pmd(pmd_val(pmd) & ~(_PAGE_PRESENT|_PAGE_PROT_NONE));
> }
> ```
> 
> but , the pmd_user_accessible_page() check `pmd_leaf(pmd) && pmd_user(pmd);`
> and pmd_leaf():
> ```
> static inline bool pmd_leaf(pmd_t pmd)
> {
> 	return pmd_present(pmd) && (pmd_val(pmd) & _PAGE_LEAF);
> }
> ```
> 
> and the pmd_present():
> ```
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> static inline int pmd_present(pmd_t pmd)
> {
> 	/*
> 	 * Checking for _PAGE_LEAF is needed too because:
> 	 * When splitting a THP, split_huge_page() will temporarily clear
> 	 * the present bit, in this situation, pmd_present() and
> 	 * pmd_trans_huge() still needs to return true.
> 	 */
> 	return (pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROT_NONE | _PAGE_LEAF));
> }
> #else
> static inline int pmd_present(pmd_t pmd)
> {
> 	return (pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROT_NONE));
> }
> #endif
> ```
> 
> And the TRANSPARENT HUGE PAGE , is exactly a page that I was allocated with
> mmap() (see my previous emails with my repro.c).
> 
> So, I change the pmd_mkinvalid() adding PAGE_LEAF flag:
> ```
> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
> {
> 	return __pmd(pmd_val(pmd) & ~(_PAGE_PRESENT|_PAGE_PROT_NONE|_PAGE_LEAF));
> }
> ```
> 
> And this work for my repro.c.
> The problem here is what the documentation of pmd_present() say:
> ```
> 	/*
> 	 * Checking for _PAGE_LEAF is needed too because:
> 	 * When splitting a THP, split_huge_page() will temporarily clear
> 	 * the present bit, in this situation, pmd_present() and
> 	 * pmd_trans_huge() still needs to return true.
> 	 */
> ```
> 
> So the documentation say that if a THP is splitting, this should return true
> event though PAGE_PRESENT and PAGE_PROT_NONE is cleared.
> If you think about this, maybe make sense.
> 
> And after some searchs I saw that was already fixed by Yunhui Cui
> <cuiyunhui@xxxxxxxxxxxxx>
> https://lore.kernel.org/all/20260523042052.35476-1-cuiyunhui@xxxxxxxxxxxxx/

I remember that Zi is currently also fixing some pmd_mkinvalid() implementations.

Is there anything to fix from your side, or does the above fix already resolve
your problem?

-- 
Cheers,

David