Re: [PATCH] userfaultfd: mark uffd_wp regardless of VM_WRITE flag

[Nadav Amit <nadav.amit@gmail.com>]

> On Feb 17, 2022, at 6:23 PM, Nadav Amit <nadav.amit@gmail.com> wrote:
> 
>> PS: I always think here the VM_SOFTDIRTY check is wrong, IMHO it should be:
>> 
>>       if (dirty_accountable && pte_dirty(ptent) &&
>>                       (pte_soft_dirty(ptent) ||
>>                       (vma->vm_flags & VM_SOFTDIRTY))) {
>>               ptent = pte_mkwrite(ptent);
>>       }

I know it is off-topic (not directly related to my patch), but
I tried to understand the logic - both of the existing code and of
your suggested change - and I failed.

IIUC dirty_accountable (whose value is taken from
vma_wants_writenotify()) means that the writes *should* be tracked,
and therefore the page should remain read-only.

So basically the condition should have been based on
!dirty_accountable, i.e. the inverted value of dirty_accountable.

The problem is that dirty_accountable also reflects VM_SOFTDIRTY
considerations, so looking on the PTE does not tell you whether
the PTE should remain write-protected even if it is dirty.

Am I missing something?