Re: [RFC v1 1/3] mm/mmu_notifier: Add a new notifier for mapping updates (new pages)

Jason Gunthorpe
2023-07-19T00:24:47Z

On Wed, Jul 19, 2023 at 12:05:29AM +0000, Kasireddy, Vivek wrote:

> > If there is no change to the PTEs then it is hard to see why this
> > would be part of a mmu_notifier.
> IIUC, the PTEs do get changed but only when a new page is faulted in.
> For shmem, it looks like the PTEs are updated in handle_pte_fault()
> after shmem_fault() gets called and for hugetlbfs, this seems to
> happen in hugetlb_fault().

That sounds about right

> Instead of introducing a new notifier, I did think about reusing
> (or overloading) .change_pte() but I did not fully understand the impact
> it would have on KVM, the only user of .change_pte().

Yes, change_pte will be called, I think, but under various locks. Why
would you need to change it?

What you are doing here doesn't make any sense within the design of
mmu_notifiers, eg:

> @ -2164,8 +2165,12 @@ static vm_fault_t shmem_fault(struct vm_fault *vmf)
> gfp, vma, vmf, &ret);
> if (err)
> return vmf_error(err);
> - if (folio)
> + if (folio) {
> vmf->page = folio_file_page(folio, vmf->pgoff);
> + if (ret == VM_FAULT_LOCKED)
> + mmu_notifier_update_mapping(vma->vm_mm, vmf->address,
> + page_to_pfn(vmf->page));
> + }
> return ret;

Hasn't even updated the PTEs yet, but it is invoking a callback??

Jason