> On Feb 17, 2022, at 6:23 PM, Nadav Amit <nadav.amit@xxxxxxxxx> 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?
