Re: [PATCH] mmu_notifiers: Notify on pte permission upgrades

From: Alistair Popple
Date: Mon May 22 2023 - 20:39:14 EST



Catalin Marinas <catalin.marinas@xxxxxxx> writes:

> On Mon, May 22, 2023 at 04:37:25PM +1000, Alistair Popple wrote:
>> diff --git a/mm/memory.c b/mm/memory.c
>> index f526b9152bef..0ac78c6a232c 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -2098,6 +2098,7 @@ static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr,
>> struct mm_struct *mm = vma->vm_mm;
>> pte_t *pte, entry;
>> spinlock_t *ptl;
>> + bool changed = false;
>>
>> pte = get_locked_pte(mm, addr, &ptl);
>> if (!pte)
>> @@ -2120,8 +2121,10 @@ static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr,
>> }
>> entry = pte_mkyoung(*pte);
>> entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>> - if (ptep_set_access_flags(vma, addr, pte, entry, 1))
>> + if (ptep_set_access_flags(vma, addr, pte, entry, 1)) {
>> update_mmu_cache(vma, addr, pte);
>> + changed = true;
>> + }
>> }
>> goto out_unlock;
>> }
>
> I haven't checked all the corner cases but can we not have a
> ptep_set_access_flags_notify() that handles this (and the huge
> equivalent)? It matches the other API like ptep_clear_flush_notify().

Good suggestion, thanks! I can make the implementations architecture
specific too which helps with filtering on platforms that don't need
it. I had assumed the invalidate_range() callbacks could sleep and
therefore couldn't be called under PTL. However
ptep_clear_flush_notify() already calls invalidate_range() under PTL, so
we already assume drivers don't sleep in the invalidate_range()
callbacks.