Re: [PATCH v17 08/26] x86/mm: Introduce _PAGE_COW

[Dave Hansen]

On 1/21/21 12:16 PM, Yu, Yu-cheng wrote:
> 
>>> @@ -343,6 +349,16 @@ static inline pte_t pte_mkold(pte_t pte)
>>>     static inline pte_t pte_wrprotect(pte_t pte)
>>>   {
>>> +    /*
>>> +     * Blindly clearing _PAGE_RW might accidentally create
>>> +     * a shadow stack PTE (RW=0, Dirty=1).  Move the hardware
>>> +     * dirty value to the software bit.
>>> +     */
>>> +    if (cpu_feature_enabled(X86_FEATURE_SHSTK)) {
>>> +        pte.pte |= (pte.pte & _PAGE_DIRTY) >> _PAGE_BIT_DIRTY <<
>>> _PAGE_BIT_COW;
>>
>> Why the unreadable shifting when you can simply do:
>>
>>                  if (pte.pte & _PAGE_DIRTY)
>>                          pte.pte |= _PAGE_COW;
>>
>> ?
> 
> It clears _PAGE_DIRTY and sets _PAGE_COW.  That is,
> 
> if (pte.pte & _PAGE_DIRTY) {
>     pte.pte &= ~_PAGE_DIRTY;
>     pte.pte |= _PAGE_COW;
> }
> 
> So, shifting makes resulting code more efficient.

Are you sure?

Usually, the compiler is better at making code efficient than humans.  I
find that coding it in the most human-readable way is best unless I
*know* the compiler is unable to generate god code.