Hi, Kefeng,
On Thu, Apr 18, 2024 at 08:06:41PM +0800, Kefeng Wang wrote:
> Add userfaultfd_wp() check in vmf_orig_pte_uffd_wp() to avoid the
> unnecessary pte_marker_entry_uffd_wp() in most pagefault, difference
> as shows below from perf data of lat_pagefault, note, the function
> vmf_orig_pte_uffd_wp() is not inlined in the two kernel versions.
>
> perf report -i perf.data.before | grep vmf
> 0.17% 0.13% lat_pagefault [kernel.kallsyms] [k] vmf_orig_pte_uffd_wp.part.0.isra.0
> perf report -i perf.data.after | grep vmf
Any real number to share too besides the perf greps? I meant, even if perf
report will not report such function anymore, it doesn't mean it'll be
faster, and how much it improves?
Now we're switching from pte_marker_uffd_wp() check into a vma flag check.
I think it makes more sense to compare the number rather than the perf
reports, as the vma flag check instructions will be buried under other
entries IIUC.
Thanks,
>
> In addition, directly call vmf_orig_pte_uffd_wp() in do_anonymous_page()
> and set_pte_range() to save a uffd_wp variable.
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
> ---
> v2: update changelog
>
> mm/memory.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 5ae2409d3cb9..2cf54def3995 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -116,6 +116,8 @@ static bool vmf_orig_pte_uffd_wp(struct vm_fault *vmf)
> {
> if (!(vmf->flags & FAULT_FLAG_ORIG_PTE_VALID))
> return false;
> + if (!userfaultfd_wp(vmf->vma))
> + return false;
>
> return pte_marker_uffd_wp(vmf->orig_pte);
> }
> @@ -4388,7 +4390,6 @@ static struct folio *alloc_anon_folio(struct vm_fault *vmf)
> */
> static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
> {
> - bool uffd_wp = vmf_orig_pte_uffd_wp(vmf);
> struct vm_area_struct *vma = vmf->vma;
> unsigned long addr = vmf->address;
> struct folio *folio;
> @@ -4488,7 +4489,7 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
> folio_add_new_anon_rmap(folio, vma, addr);
> folio_add_lru_vma(folio, vma);
> setpte:
> - if (uffd_wp)
> + if (vmf_orig_pte_uffd_wp(vmf))
> entry = pte_mkuffd_wp(entry);
> set_ptes(vma->vm_mm, addr, vmf->pte, entry, nr_pages);
>
> @@ -4663,7 +4664,6 @@ void set_pte_range(struct vm_fault *vmf, struct folio *folio,
> struct page *page, unsigned int nr, unsigned long addr)
> {
> struct vm_area_struct *vma = vmf->vma;
> - bool uffd_wp = vmf_orig_pte_uffd_wp(vmf);
> bool write = vmf->flags & FAULT_FLAG_WRITE;
> bool prefault = in_range(vmf->address, addr, nr * PAGE_SIZE);
> pte_t entry;
> @@ -4678,7 +4678,7 @@ void set_pte_range(struct vm_fault *vmf, struct folio *folio,
>
> if (write)
> entry = maybe_mkwrite(pte_mkdirty(entry), vma);
> - if (unlikely(uffd_wp))
> + if (unlikely(vmf_orig_pte_uffd_wp(vmf)))
> entry = pte_mkuffd_wp(entry);
> /* copy-on-write page */
> if (write && !(vma->vm_flags & VM_SHARED)) {
> --
> 2.27.0
>
--
Peter Xu
