Re: [PATCH mm-unstable v1 1/5] mm: consolidate anonymous folio PTE mapping into helpers

[Zi Yan]

<snip>

>>
>>> -     if (vma->vm_flags & VM_WRITE)
>>> -             entry = pte_mkwrite(pte_mkdirty(entry), vma);
>>
>> OK, this becomes maybe_mkwrite(pte_mkdirty(entry), vma).
>
> Yes, upon further investigation this does seem to slightly change the behavior.

I did not notice it when I was reviewing it. ;)

>
> pte_mkdirty() is now being called unconditionally from the VM_WRITE
> flag. I noticed other callers in the kernel doing this too.
>
> Is it ok to leave the pte_mkdirty() or should I go back to using
> pte_mkwrite with the conditional guarding both mkwrite and mkdirty?


IMHO, it is better to use the conditional guarding way.
We reach here when userspace reads an address (VM_WRITE is not set)
and no zero page is used. Using maybe_mkwrite(pte_mkdirty(entry), vma)
means we will get a dirty PTE pointing to the allocated page but user
only reads from it.

Best Regards,
Yan, Zi