Reza Arbab <ar...@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. cheers
