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>
