hi, Lorenzo, On Wed, Oct 09, 2024 at 10:24:58PM +0100, Lorenzo Stoakes wrote: > On Wed, Oct 09, 2024 at 02:44:30PM +0800, Oliver Sang wrote: > [snip] > > > > > > I will look into this now, if I provide patches would you be able to test > > > them using the same boxes? It'd be much appreciated! > > > > sure! that's our pleasure! > > > > Hi Oliver, > > Thanks so much for this, could you give the below a try? I've not tried to > seriously test it locally yet, so it'd be good to set your test machines on > it. > > If this doesn't help it suggests call stack/branching might be a thing here > in which case I have other approaches I can take before we have to > duplicate this code. > > This patch is against the mm-unstable branch in Andrew's tree [0] but > hopefully should apply fine to Linus's too. > > [0]:https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/ > > Thanks again! you are welcome! I found the patch could be applied directly on cacded5e42, so I did it. this is our normal practice that we want to avoid impacts from other commits. but if your patch should reply on some new patches in mm-unstable or mainline, please let me know. I could reapply and retest. I mentioned patch base since I found by my applyment upon cacded5e42, your patch seems not have obvious performance impact, still have similar regression. for brief, I just list 2 examples here. all tests and full data are attached as fc21959f74bc11-cacded5e42b960-2e71337ac26478 (1) model: Sapphire Rapids nr_node: 2 nr_cpu: 224 memory: 512G brand: Intel(R) Xeon(R) Platinum 8480CTDX ========================================================================================= compiler/cpufreq_governor/kconfig/rootfs/tbox_group/test/testcase/testtime: gcc-12/performance/x86_64-rhel-8.3/debian-12-x86_64-20240206.cgz/lkp-spr-2sp4/brk_test/aim9/300s commit: fc21959f74bc11 ("mm: abstract vma_expand() to use vma_merge_struct") cacded5e42b960 ("mm: avoid using vma_merge() for new VMAs") 2e71337ac26478 ("mm: explicitly enable an expand-only merge mode for brk()") fc21959f74bc1138 cacded5e42b9609b07b22d80c10 2e71337ac2647889d3d9d76a5ce ---------------- --------------------------- --------------------------- %stddev %change %stddev %change %stddev \ | \ | \ 3540976 -6.4% 3314159 -6.7% 3302864 aim9.brk_test.ops_per_sec (2) which is using same Ivy Bridge-EP in our original report (test machine: 48 threads 2 sockets Intel(R) Xeon(R) CPU E5-2697 v2 @ 2.70GHz (Ivy Bridge-EP) with 64G memory) ========================================================================================= compiler/cpufreq_governor/kconfig/rootfs/tbox_group/test/testcase/testtime: gcc-12/performance/x86_64-rhel-8.3/debian-12-x86_64-20240206.cgz/lkp-ivb-2ep2/brk_test/aim9/300s commit: fc21959f74bc11 ("mm: abstract vma_expand() to use vma_merge_struct") cacded5e42b960 ("mm: avoid using vma_merge() for new VMAs") 2e71337ac26478 ("mm: explicitly enable an expand-only merge mode for brk()") fc21959f74bc1138 cacded5e42b9609b07b22d80c10 2e71337ac2647889d3d9d76a5ce ---------------- --------------------------- --------------------------- %stddev %change %stddev %change %stddev \ | \ | \ 1322908 -5.0% 1256536 -4.1% 1268145 aim9.brk_test.ops_per_sec > > Best, Lorenzo > > > ----8<---- > From 7eb4aa421b357668bc44405c58b0444abf44334a Mon Sep 17 00:00:00 2001 > From: Lorenzo Stoakes > Date: Wed, 9 Oct 2024 21:57:03 +0100 > Subject: [PATCH] mm: explicitly enable an expand-only merge mode for brk() > > Try to do less work on brk() to improve perf. > --- > mm/mmap.c | 1 + > mm/vma.c | 25 ++++++++++++++++--------- > mm/vma.h | 11 +++++++++++ > 3 files changed, 28 insertions(+), 9 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index 02f7b45c3076..c2c68ef45a3b 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -1740,6 +1740,7 @@ static int do_brk_flags(struct vma_iterator *vmi, struct vm_area_struct *vma, > if (vma && vma->vm_end == addr) { > VMG_STATE(vmg, mm, vmi, addr, addr + len, flags, PHYS_PFN(addr)); > > + vmg.mode = VMA_MERGE_MODE_EXPAND_ONLY; > vmg.prev = vma; > vma_iter_next_range(vmi); > > diff --git a/mm/vma.c b/mm/vma.c > index 749c4881fd60..f525a0750c41 100644 > --- a/mm/vma.c > +++ b/mm/vma.c > @@ -561,6 +561,7 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg) > unsigned long end = vmg->end; > pgoff_t pgoff = vmg->pgoff; > pgoff_t pglen = PHYS_PFN(end - start); > + bool expand_only = vmg_mode_expand_only(vmg); > bool can_merge_left, can_merge_right; > > mmap_assert_write_locked(vmg->mm); > @@ -575,7 +576,7 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg) > return NULL; > > can_merge_left = can_vma_merge_left(vmg); > - can_merge_right = can_vma_merge_right(vmg, can_merge_left); > + can_merge_right = !expand_only && can_vma_merge_right(vmg, can_merge_left); > > /* If we can merge with the next VMA, adjust vmg accordingly. */ > if (can_merge_right) { > @@ -603,13 +604,18 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg) > return vmg->vma; > } > > - /* If expansion failed, reset state. Allows us to retry merge later. */ > - vmg->vma = NULL; > - vmg->start = start; > - vmg->end = end; > - vmg->pgoff = pgoff; > - if (vmg->vma == prev) > - vma_iter_set(vmg->vmi, start); > + /* > + * Unless in expand only case and expansion failed, reset state. > + * Allows us to retry merge later. > + */ > + if (!expand_only) { > + vmg->vma = NULL; > + vmg->start = start; > + vmg->end = end; > + vmg->pgoff = pgoff; > + if (vmg->vma == prev) > + vma_iter_set(vmg->vmi, start); > + } > > return NULL; > } > @@ -641,7 +647,8 @@ int vma_expand(struct vma_merge_struct *vmg) > mmap_assert_write_locked(vmg->mm); > > vma_start_write(vma); > - if (next && (vma != next) && (vmg->end == next->vm_end)) { > + if (!vmg_mode_expand_only(vmg) && next && > + (vma != next) && (vmg->end == next->vm_end)) { > int ret; > > remove_next = true; > diff --git a/mm/vma.h b/mm/vma.h > index 82354fe5edd0..14224b36a979 100644 > --- a/mm/vma.h > +++ b/mm/vma.h > @@ -52,6 +52,11 @@ struct vma_munmap_struct { > unsigned long data_vm; > }; > > +enum vma_merge_mode { > + VMA_MERGE_MODE_NORMAL, > + VMA_MERGE_MODE_EXPAND_ONLY, > +}; > + > enum vma_merge_state { > VMA_MERGE_START, > VMA_MERGE_ERROR_NOMEM, > @@ -75,9 +80,15 @@ struct vma_merge_struct { > struct mempolicy *policy; > struct vm_userfaultfd_ctx uffd_ctx; > struct anon_vma_name *anon_name; > + enum vma_merge_mode mode; > enum vma_merge_state state; > }; > > +static inline bool vmg_mode_expand_only(struct vma_merge_struct *vmg) > +{ > + return vmg->mode == VMA_MERGE_MODE_EXPAND_ONLY; > +} > + > static inline bool vmg_nomem(struct vma_merge_struct *vmg) > { > return vmg->state == VMA_MERGE_ERROR_NOMEM; > -- > 2.46.2