Re: [PATCH v1] mm/debug_vm_pgtable: Test pmd_leaf() behavior with pmd_mkinvalid()

[Ryan Roberts <ryan.roberts@arm.com>]

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@arm.com>
>> ---
>>
>> 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.

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.