> On Feb 17, 2022, at 8:00 PM, Nadav Amit <nadav.amit@xxxxxxxxx> wrote:
>
>
>> 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.
Just as I pressed send, I understood your suggestion.
It is still confusing for me how setting write-permissions for
dirty_accountable PTEs is safe (as long as they are dirty), and
yet in the general case it is not safe. I need to further think
of it.
