Re: [RESEND PATCH v3 4/5] mm/mprotect: do not flush on permission promotion

[Nadav Amit]

> On Mar 11, 2022, at 11:07 AM, Nadav Amit <nadav.amit@xxxxxxxxx> wrote:
> 
> From: Nadav Amit <namit@xxxxxxxxxx>
> 
> Currently, using mprotect() to unprotect a memory region or uffd to
> unprotect a memory region causes a TLB flush. At least on x86, as
> protection is promoted, no TLB flush is needed.
> 
> Add an arch-specific pte_may_need_flush() which tells whether a TLB
> flush is needed based on the old PTE and the new one. Implement an x86
> pte_may_need_flush().
> 
> For x86, besides the simple logic that PTE protection promotion or
> changes of software bits does require a flush, also add logic that
> considers the dirty-bit. If the dirty-bit is clear and write-protect is
> set, no TLB flush is needed, as x86 updates the dirty-bit atomically
> on write, and if the bit is clear, the PTE is reread.
> 
> 

[snip]

> + */
> +static inline bool pte_needs_flush(pte_t oldpte, pte_t newpte)
> +{
> +	/* !PRESENT -> * ; no need for flush */
> +	if (!pte_present(oldpte))
> +		return false;
> +
> +	/* PRESENT -> !PRESENT ; needs flush */
> +	if (!pte_present(newpte))
> +		return true;
> +
> +	/* PFN changed ; needs flush */
> +	if (pte_pfn(oldpte) != pte_pfn(newpte))
> +		return true;
> +
> +	return pte_flags_need_flush(pte_flags(oldpte), pte_flags(newpte),
> +				    _PAGE_ACCESSED);
> +}


Looking again at this patch, I think the PRESENT -> !PRESENT is
not needed:

1. It might trigger unnecessary flushes for PROT_NONE (or NUMA)
2. Real PRESENT -> !PRESENT is already checked by
   pte_flags_need_flush().


Let me know if I am missing something. Otherwise I will change it
for v4.