I am currently working on my master's thesis trying to increase
number of merges of VMAs currently failing because of page offset
incompatibility and difference in their anon_vmas. The following
refactor and added merge call included in this series is just two
smaller upgrades I created along the way.
The rest of the work is still being worked on but was send to mailing
list as a RFC:
https://lore.kernel.org/all/[email protected]/
This patch series is based on tag v5.18-rc2.
Jakub Matěna (2):
mm: refactor of vma_merge()
mm: add merging after mremap resize
mm/mmap.c | 87 +++++++++++++++++++++++------------------------------
mm/mremap.c | 8 +++--
2 files changed, 43 insertions(+), 52 deletions(-)
--
2.35.1
When mremap call results in expansion, it might be possible to merge the
VMA with the next VMA which might become adjacent. This patch adds
vma_merge call after the expansion is done to try and merge.
Signed-off-by: Jakub Matěna <[email protected]>
---
mm/mremap.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mm/mremap.c b/mm/mremap.c
index 303d3290b938..c41237e62156 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -9,6 +9,7 @@
*/
#include <linux/mm.h>
+#include <linux/mm_inline.h>
#include <linux/hugetlb.h>
#include <linux/shm.h>
#include <linux/ksm.h>
@@ -1022,8 +1023,11 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
}
}
- if (vma_adjust(vma, vma->vm_start, addr + new_len,
- vma->vm_pgoff, NULL)) {
+ vma = vma_merge(mm, vma, addr + old_len, addr + new_len,
+ vma->vm_flags, vma->anon_vma, vma->vm_file,
+ vma->vm_pgoff + (old_len >> PAGE_SHIFT), vma_policy(vma),
+ vma->vm_userfaultfd_ctx, anon_vma_name(vma));
+ if (!vma) {
vm_unacct_memory(pages);
ret = -ENOMEM;
goto out;
--
2.35.1
Refactor vma_merge() to make it shorter and more understandable.
Main change is the elimination of code duplicity in the case of
merge next check. This is done by first doing checks and caching
the results before executing the merge itself. The variable 'area' is
divided into 'mid' and 'res' as previously it was used for two purposes,
as the middle VMA between prev and next and also as the result of the
merge itself. Exit paths are also unified.
Signed-off-by: Jakub Matěna <[email protected]>
---
mm/mmap.c | 87 +++++++++++++++++++++++--------------------------------
1 file changed, 37 insertions(+), 50 deletions(-)
diff --git a/mm/mmap.c b/mm/mmap.c
index 3aa839f81e63..bd0dde2ad3e2 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1170,8 +1170,10 @@ struct vm_area_struct *vma_merge(struct mm_struct *mm,
struct anon_vma_name *anon_name)
{
pgoff_t pglen = (end - addr) >> PAGE_SHIFT;
- struct vm_area_struct *area, *next;
- int err;
+ struct vm_area_struct *mid, *next, *res;
+ int err = -1;
+ bool merge_prev = false;
+ bool merge_next = false;
/*
* We later require that vma->vm_flags == vm_flags,
@@ -1181,75 +1183,60 @@ struct vm_area_struct *vma_merge(struct mm_struct *mm,
return NULL;
next = vma_next(mm, prev);
- area = next;
- if (area && area->vm_end == end) /* cases 6, 7, 8 */
+ mid = next;
+ if (next && next->vm_end == end) /* cases 6, 7, 8 */
next = next->vm_next;
/* verify some invariant that must be enforced by the caller */
VM_WARN_ON(prev && addr <= prev->vm_start);
- VM_WARN_ON(area && end > area->vm_end);
+ VM_WARN_ON(mid && end > mid->vm_end);
VM_WARN_ON(addr >= end);
- /*
- * Can it merge with the predecessor?
- */
+ /* Can we merge the predecessor? */
if (prev && prev->vm_end == addr &&
mpol_equal(vma_policy(prev), policy) &&
can_vma_merge_after(prev, vm_flags,
anon_vma, file, pgoff,
vm_userfaultfd_ctx, anon_name)) {
- /*
- * OK, it can. Can we now merge in the successor as well?
- */
- if (next && end == next->vm_start &&
- mpol_equal(policy, vma_policy(next)) &&
- can_vma_merge_before(next, vm_flags,
- anon_vma, file,
- pgoff+pglen,
- vm_userfaultfd_ctx, anon_name) &&
- is_mergeable_anon_vma(prev->anon_vma,
- next->anon_vma, NULL)) {
- /* cases 1, 6 */
- err = __vma_adjust(prev, prev->vm_start,
- next->vm_end, prev->vm_pgoff, NULL,
- prev);
- } else /* cases 2, 5, 7 */
- err = __vma_adjust(prev, prev->vm_start,
- end, prev->vm_pgoff, NULL, prev);
- if (err)
- return NULL;
- khugepaged_enter_vma_merge(prev, vm_flags);
- return prev;
+ merge_prev = true;
}
-
- /*
- * Can this new request be merged in front of next?
- */
+ /* Can we merge the successor? */
if (next && end == next->vm_start &&
mpol_equal(policy, vma_policy(next)) &&
can_vma_merge_before(next, vm_flags,
anon_vma, file, pgoff+pglen,
vm_userfaultfd_ctx, anon_name)) {
+ merge_next = true;
+ }
+ /* Can we merge both the predecessor and the successor? */
+ if (merge_prev && merge_next &&
+ is_mergeable_anon_vma(prev->anon_vma,
+ next->anon_vma, NULL)) { /* cases 1, 6 */
+ err = __vma_adjust(prev, prev->vm_start,
+ next->vm_end, prev->vm_pgoff, NULL,
+ prev);
+ res = prev;
+ } else if (merge_prev) { /* cases 2, 5, 7 */
+ err = __vma_adjust(prev, prev->vm_start,
+ end, prev->vm_pgoff, NULL, prev);
+ res = prev;
+ } else if (merge_next) {
if (prev && addr < prev->vm_end) /* case 4 */
err = __vma_adjust(prev, prev->vm_start,
- addr, prev->vm_pgoff, NULL, next);
- else { /* cases 3, 8 */
- err = __vma_adjust(area, addr, next->vm_end,
- next->vm_pgoff - pglen, NULL, next);
- /*
- * In case 3 area is already equal to next and
- * this is a noop, but in case 8 "area" has
- * been removed and next was expanded over it.
- */
- area = next;
- }
- if (err)
- return NULL;
- khugepaged_enter_vma_merge(area, vm_flags);
- return area;
+ addr, prev->vm_pgoff, NULL, next);
+ else /* cases 3, 8 */
+ err = __vma_adjust(mid, addr, next->vm_end,
+ next->vm_pgoff - pglen, NULL, next);
+ res = next;
}
- return NULL;
+ /*
+ * Cannot merge with predecessor or successor or error in __vma_adjust?
+ */
+ if (err)
+ return NULL;
+ khugepaged_enter_vma_merge(res, vm_flags);
+ return res;
}
/*
--
2.35.1
Hi "Jakub,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v5.18]
[cannot apply to akpm-mm/mm-everything linus/master next-20220527]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Jakub-Mat-na/Refactor-of-vma_merge-and-new-merge-call/20220527-184844
base: 4b0986a3613c92f4ec1bdc7f60ec66fea135991f
config: mips-pic32mzda_defconfig (https://download.01.org/0day-ci/archive/20220527/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 134d7f9a4b97e9035150d970bd9e376043c4577e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/fae2b6e81c73c0517e3be864608d069a2d4fb8b3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jakub-Mat-na/Refactor-of-vma_merge-and-new-merge-call/20220527-184844
git checkout fae2b6e81c73c0517e3be864608d069a2d4fb8b3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> mm/mremap.c:1028:47: error: call to undeclared function 'vma_policy'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
vma->vm_pgoff + (old_len >> PAGE_SHIFT), vma_policy(vma),
^
mm/mremap.c:1028:47: warning: incompatible integer to pointer conversion passing 'int' to parameter of type 'struct mempolicy *' [-Wint-conversion]
vma->vm_pgoff + (old_len >> PAGE_SHIFT), vma_policy(vma),
^~~~~~~~~~~~~~~
include/linux/mm.h:2628:20: note: passing argument to parameter here
struct mempolicy *, struct vm_userfaultfd_ctx, struct anon_vma_name *);
^
1 warning and 1 error generated.
vim +/vma_policy +1028 mm/mremap.c
885
886 /*
887 * Expand (or shrink) an existing mapping, potentially moving it at the
888 * same time (controlled by the MREMAP_MAYMOVE flag and available VM space)
889 *
890 * MREMAP_FIXED option added 5-Dec-1999 by Benjamin LaHaise
891 * This option implies MREMAP_MAYMOVE.
892 */
893 SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
894 unsigned long, new_len, unsigned long, flags,
895 unsigned long, new_addr)
896 {
897 struct mm_struct *mm = current->mm;
898 struct vm_area_struct *vma;
899 unsigned long ret = -EINVAL;
900 bool locked = false;
901 bool downgraded = false;
902 struct vm_userfaultfd_ctx uf = NULL_VM_UFFD_CTX;
903 LIST_HEAD(uf_unmap_early);
904 LIST_HEAD(uf_unmap);
905
906 /*
907 * There is a deliberate asymmetry here: we strip the pointer tag
908 * from the old address but leave the new address alone. This is
909 * for consistency with mmap(), where we prevent the creation of
910 * aliasing mappings in userspace by leaving the tag bits of the
911 * mapping address intact. A non-zero tag will cause the subsequent
912 * range checks to reject the address as invalid.
913 *
914 * See Documentation/arm64/tagged-address-abi.rst for more information.
915 */
916 addr = untagged_addr(addr);
917
918 if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE | MREMAP_DONTUNMAP))
919 return ret;
920
921 if (flags & MREMAP_FIXED && !(flags & MREMAP_MAYMOVE))
922 return ret;
923
924 /*
925 * MREMAP_DONTUNMAP is always a move and it does not allow resizing
926 * in the process.
927 */
928 if (flags & MREMAP_DONTUNMAP &&
929 (!(flags & MREMAP_MAYMOVE) || old_len != new_len))
930 return ret;
931
932
933 if (offset_in_page(addr))
934 return ret;
935
936 old_len = PAGE_ALIGN(old_len);
937 new_len = PAGE_ALIGN(new_len);
938
939 /*
940 * We allow a zero old-len as a special case
941 * for DOS-emu "duplicate shm area" thing. But
942 * a zero new-len is nonsensical.
943 */
944 if (!new_len)
945 return ret;
946
947 if (mmap_write_lock_killable(current->mm))
948 return -EINTR;
949 vma = vma_lookup(mm, addr);
950 if (!vma) {
951 ret = -EFAULT;
952 goto out;
953 }
954
955 if (is_vm_hugetlb_page(vma)) {
956 struct hstate *h __maybe_unused = hstate_vma(vma);
957
958 old_len = ALIGN(old_len, huge_page_size(h));
959 new_len = ALIGN(new_len, huge_page_size(h));
960
961 /* addrs must be huge page aligned */
962 if (addr & ~huge_page_mask(h))
963 goto out;
964 if (new_addr & ~huge_page_mask(h))
965 goto out;
966
967 /*
968 * Don't allow remap expansion, because the underlying hugetlb
969 * reservation is not yet capable to handle split reservation.
970 */
971 if (new_len > old_len)
972 goto out;
973 }
974
975 if (flags & (MREMAP_FIXED | MREMAP_DONTUNMAP)) {
976 ret = mremap_to(addr, old_len, new_addr, new_len,
977 &locked, flags, &uf, &uf_unmap_early,
978 &uf_unmap);
979 goto out;
980 }
981
982 /*
983 * Always allow a shrinking remap: that just unmaps
984 * the unnecessary pages..
985 * __do_munmap does all the needed commit accounting, and
986 * downgrades mmap_lock to read if so directed.
987 */
988 if (old_len >= new_len) {
989 int retval;
990
991 retval = __do_munmap(mm, addr+new_len, old_len - new_len,
992 &uf_unmap, true);
993 if (retval < 0 && old_len != new_len) {
994 ret = retval;
995 goto out;
996 /* Returning 1 indicates mmap_lock is downgraded to read. */
997 } else if (retval == 1)
998 downgraded = true;
999 ret = addr;
1000 goto out;
1001 }
1002
1003 /*
1004 * Ok, we need to grow..
1005 */
1006 vma = vma_to_resize(addr, old_len, new_len, flags);
1007 if (IS_ERR(vma)) {
1008 ret = PTR_ERR(vma);
1009 goto out;
1010 }
1011
1012 /* old_len exactly to the end of the area..
1013 */
1014 if (old_len == vma->vm_end - addr) {
1015 /* can we just expand the current mapping? */
1016 if (vma_expandable(vma, new_len - old_len)) {
1017 long pages = (new_len - old_len) >> PAGE_SHIFT;
1018
1019 if (vma->vm_flags & VM_ACCOUNT) {
1020 if (security_vm_enough_memory_mm(mm, pages)) {
1021 ret = -ENOMEM;
1022 goto out;
1023 }
1024 }
1025
1026 vma = vma_merge(mm, vma, addr + old_len, addr + new_len,
1027 vma->vm_flags, vma->anon_vma, vma->vm_file,
> 1028 vma->vm_pgoff + (old_len >> PAGE_SHIFT), vma_policy(vma),
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Thu, 2 Jun 2022 16:56:41 +0200 Jakub Matěna <[email protected]> wrote:
> Refactor vma_merge() to make it shorter and more understandable.
> Main change is the elimination of code duplicity in the case of
> merge next check. This is done by first doing checks and caching
> the results before executing the merge itself. The variable 'area' is
> divided into 'mid' and 'res' as previously it was used for two purposes,
> as the middle VMA between prev and next and also as the result of the
> merge itself. Exit paths are also unified.
>
Thanks, but unfortunately the code you're working on here has changed
greatly in Liam's mapletree patchset. Could you please take a look at
the mm-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm and decide if you
want to tackle it?