On 02/05/2024 09:03, Anshuman Khandual wrote: > > > On 5/2/24 13:00, Ryan Roberts wrote: >> On 02/05/2024 03:43, Anshuman Khandual wrote: >>> Hello Ryan, >>> >>> On 5/1/24 20:14, Ryan Roberts wrote: >>>> An invalidated pmd should still cause pmd_leaf() to return true. Let's >>>> test for that to ensure all arches remain consistent. >>> >>> This test definitely makes sense. >>> >>>> >>>> Signed-off-by: Ryan Roberts <ryan.roberts@xxxxxxx> >>>> --- >>>> >>>> Hi Andrew, >>>> >>>> This applies on top of v6.9-rc5. It came out of a discussion with Catalin around >>>> the pmd_mkinvalid() bug (the fix for which I just posted). I've run the new test >>>> on both arm64 and x86_64. >>> >>> Right, works on arm64. >>> >>>> >>>> Thanks, >>>> Ryan >>>> >>>> mm/debug_vm_pgtable.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c >>>> index 65c19025da3d..57e9cb0820ab 100644 >>>> --- a/mm/debug_vm_pgtable.c >>>> +++ b/mm/debug_vm_pgtable.c >>>> @@ -981,6 +981,7 @@ static void __init pmd_thp_tests(struct pgtable_debug_args *args) >>>> #ifndef __HAVE_ARCH_PMDP_INVALIDATE >>>> WARN_ON(!pmd_trans_huge(pmd_mkinvalid(pmd_mkhuge(pmd)))); >>>> WARN_ON(!pmd_present(pmd_mkinvalid(pmd_mkhuge(pmd)))); >>>> + WARN_ON(!pmd_leaf(pmd_mkinvalid(pmd_mkhuge(pmd)))); >>>> #endif /* __HAVE_ARCH_PMDP_INVALIDATE */ >>>> } >>> >>> Should not we update descriptions in Documentation/mm/arch_pgtable_helpers.rst >>> asserting that pmd_mkinvalid() also preserves pmd_leaf() ? >> >> Thanks for the review! >> >> We don't document that pmd_mkinvalid() preserves pmd_present() and >> pmd_trans_huge() so I wasn't sure how much detail was appropriate in that >> document - its pretty light at the moment. > > For all other helpers documentation has been light but pxd_mkinvalid() is turning > out to be a special case though. > >> >> If you think this is valuable (and isn't clear enough from the test) then I can >> add something. But as you say in the other patch, it would then start >> conflicting with that. I'd prefer to just put this in as-is to avoid the mess. > > Sure, fair enough. I will try and update how pmd_mkinvalid() preserves pmd_leaf(), > pmd_present(), and pmd_trans_huge() at a later point. Otherwise this patch itself > LGTM and runs fine on arm64. > > Reviewed-by: Anshuman Khandual <anshuman.khandual@xxxxxxx> Thanks!