[PATCH] powerpc/mm: Add _PAGE_SAO to _PAGE_CACHE_CTL mask
Alexey Kardashevskiy
aik at ozlabs.ru
Fri Feb 1 16:02:10 AEDT 2019
On 31/01/2019 00:35, Michael Ellerman wrote:
> Reza Arbab <arbab at linux.ibm.com> writes:
>
>> On Tue, Jan 29, 2019 at 08:37:28PM +0530, Aneesh Kumar K.V wrote:
>>> Not sure what the fix is about. We set the related hash pte flags via
>>>
>>> if ((pteflags & _PAGE_CACHE_CTL) == _PAGE_TOLERANT)
>>> rflags |= HPTE_R_I;
>>> else if ((pteflags & _PAGE_CACHE_CTL) == _PAGE_NON_IDEMPOTENT)
>>> rflags |= (HPTE_R_I | HPTE_R_G);
>>> else if ((pteflags & _PAGE_CACHE_CTL) == _PAGE_SAO)
>>> rflags |= (HPTE_R_W | HPTE_R_I | HPTE_R_M);
>>
>> Again, nothing broken here, just a code readability thing. As Alexey
>> (and Charlie) noted, given the above it is a little confusing to define
>> _PAGE_CACHE_CTL this way:
>>
>> #define _PAGE_CACHE_CTL (_PAGE_NON_IDEMPOTENT | _PAGE_TOLERANT)
>
> Yeah that's confusing I agree.
>
> It's not really a maintainability thing, because those bits are in the
> architecture, so they can't change.
>
>> I like Alexey's idea, maybe just use a literal?
>>
>> #define _PAGE_CACHE_CTL 0x30
>
> I prefer your original patch. It serves as documentation on what values
> we expect to see in that field.
As documentation, it gives an idea that there can be both
_PAGE_NON_IDEMPOTENT and _PAGE_TOLERANT set which is not true. Putting
possible values in the comment next to "#define _PAGE_CACHE_CTL" will
document it properly imho.
--
Alexey
More information about the Linuxppc-dev
mailing list