Re: [PATCH RFC] mm: arm64: advance pte for contpte_ptep_set_access_flags

[David Hildenbrand <david@redhat.com>]

On 05.09.24 09:20, Ryan Roberts wrote:
> On 05/09/2024 04:27, Barry Song wrote:
>> On Thu, Sep 5, 2024 at 3:50 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>>
>>> On 04/09/2024 16:13, Will Deacon wrote:
>>>> (Adding Ryan, since you're asking him a question!)
>>>
>>> Thanks, Will!
>>>
>>> Afraid I don't do a good job of monitoring the list; I'm guessing there are
>>> automated ways to filter for mentions of my name so I catch this sort of thing
>>> in future?
>>
>> It's not your fault. I just realized that, for some unknown reason, I forgot to
>> CC you.
> 
> No worries. I was just asking if there is a general approach that people take to
> monitor mail that they are not explicitly cc'ed on, but I guess that's a bit off
> topic.

I do something slightly different: I filter mails based on keywords. For 
example, whenever a patch contains page_mapcount(), folio_mapcount(), 
folio_mapped(), ... it gets put into a separate "mapcount" folder :) 
Same regarding file names.

For you, it would probably make sense to filter for contpte or sth like 
that :)

[...]

>>>
>>> The code is correct and working as intended, AFAICT. But I accept that this is
>>> not exactly obvious. I'd be happy to Rb your proposed change if you feel it
>>> clarifies things.
>>
>> If this is the case, I'd rather add some comments instead in v2?
>>
>> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
>> index a3edced29ac1..55107d27d3f8 100644
>> --- a/arch/arm64/mm/contpte.c
>> +++ b/arch/arm64/mm/contpte.c
>> @@ -421,6 +421,12 @@ int contpte_ptep_set_access_flags(struct
>> vm_area_struct *vma,
>>                  ptep = contpte_align_down(ptep);
>>                  start_addr = addr = ALIGN_DOWN(addr, CONT_PTE_SIZE);
>>
>> +               /*
>> +                * We are not advancing entry because __ptep_set_access_flags()
>> +                * only consumes access flags from entry. And since we
>> have checked
>> +                * for the whole contpte block and returned early, pte_same()
>> +                * within __ptep_set_access_flags() is likely false.
>> +                */
>>                  for (i = 0; i < CONT_PTES; i++, ptep++, addr += PAGE_SIZE)
>>                          __ptep_set_access_flags(vma, addr, ptep, entry, 0);
> 
> LGTM:
> 
> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
> 

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb