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

Reply via email to