* [PATCH v2 0/2] mm/ksm: fix ksm exec support for prctl
@ 2024-03-22 6:09 Jinjiang Tu
2024-03-22 6:09 ` [PATCH v2 1/2] " Jinjiang Tu
2024-03-22 6:09 ` [PATCH v2 2/2] selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec Jinjiang Tu
0 siblings, 2 replies; 12+ messages in thread
From: Jinjiang Tu @ 2024-03-22 6:09 UTC (permalink / raw)
To: akpm, david, shr, hannes, riel, wangkefeng.wang, sunnanyong,
linux-mm
Cc: tujinjiang
commit 3c6f33b7273a ("mm/ksm: support fork/exec for prctl") inherits
MMF_VM_MERGE_ANY flag when a task calls execve(). Howerver, it doesn't
create the mm_slot, so ksmd will not try to scan this task. The first
patch fixes the issue.
The second patch extend the selftests of ksm to verfity the deduplication
really happens after fork/exec inherits ths KSM setting.
Changelog since v1:
- Add ksm cleanup in __bprm_mm_init() when error occurs.
- Add some comment.
- Extend the selftests of ksm fork/exec.
Jinjiang Tu (2):
mm/ksm: fix ksm exec support for prctl
selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec
fs/exec.c | 10 +++
include/linux/ksm.h | 13 +++
.../selftests/mm/ksm_functional_tests.c | 79 +++++++++++++++++--
3 files changed, 96 insertions(+), 6 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl 2024-03-22 6:09 [PATCH v2 0/2] mm/ksm: fix ksm exec support for prctl Jinjiang Tu @ 2024-03-22 6:09 ` Jinjiang Tu 2024-03-22 9:02 ` David Hildenbrand ` (2 more replies) 2024-03-22 6:09 ` [PATCH v2 2/2] selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec Jinjiang Tu 1 sibling, 3 replies; 12+ messages in thread From: Jinjiang Tu @ 2024-03-22 6:09 UTC (permalink / raw) To: akpm, david, shr, hannes, riel, wangkefeng.wang, sunnanyong, linux-mm Cc: tujinjiang commit 3c6f33b7273a ("mm/ksm: support fork/exec for prctl") inherits MMF_VM_MERGE_ANY flag when a task calls execve(). Howerver, it doesn't create the mm_slot, so ksmd will not try to scan this task. To fix it, allocate and add the mm_slot to ksm_mm_head in __bprm_mm_init() when the mm has MMF_VM_MERGE_ANY flag. Fixes: 3c6f33b7273a ("mm/ksm: support fork/exec for prctl") Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com> --- fs/exec.c | 10 ++++++++++ include/linux/ksm.h | 13 +++++++++++++ 2 files changed, 23 insertions(+) diff --git a/fs/exec.c b/fs/exec.c index ff6f26671cfc..66202d016a0a 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -67,6 +67,7 @@ #include <linux/time_namespace.h> #include <linux/user_events.h> #include <linux/rseq.h> +#include <linux/ksm.h> #include <linux/uaccess.h> #include <asm/mmu_context.h> @@ -267,6 +268,13 @@ static int __bprm_mm_init(struct linux_binprm *bprm) goto err_free; } + /* + * Need to be called with mmap write lock + * held, to avoid race with ksmd. + */ + if (ksm_execve(mm)) + goto err_ksm; + /* * Place the stack at the largest stack address the architecture * supports. Later, we'll move this to an appropriate place. We don't @@ -288,6 +296,8 @@ static int __bprm_mm_init(struct linux_binprm *bprm) bprm->p = vma->vm_end - sizeof(void *); return 0; err: + ksm_exit(mm); +err_ksm: mmap_write_unlock(mm); err_free: bprm->vma = NULL; diff --git a/include/linux/ksm.h b/include/linux/ksm.h index 401348e9f92b..7e2b1de3996a 100644 --- a/include/linux/ksm.h +++ b/include/linux/ksm.h @@ -59,6 +59,14 @@ static inline int ksm_fork(struct mm_struct *mm, struct mm_struct *oldmm) return 0; } +static inline int ksm_execve(struct mm_struct *mm) +{ + if (test_bit(MMF_VM_MERGE_ANY, &mm->flags)) + return __ksm_enter(mm); + + return 0; +} + static inline void ksm_exit(struct mm_struct *mm) { if (test_bit(MMF_VM_MERGEABLE, &mm->flags)) @@ -107,6 +115,11 @@ static inline int ksm_fork(struct mm_struct *mm, struct mm_struct *oldmm) return 0; } +static inline int ksm_execve(struct mm_struct *mm) +{ + return 0; +} + static inline void ksm_exit(struct mm_struct *mm) { } -- 2.25.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl 2024-03-22 6:09 ` [PATCH v2 1/2] " Jinjiang Tu @ 2024-03-22 9:02 ` David Hildenbrand 2024-03-25 2:24 ` Jinjiang Tu 2024-03-24 0:03 ` kernel test robot 2024-03-25 5:44 ` Dan Carpenter 2 siblings, 1 reply; 12+ messages in thread From: David Hildenbrand @ 2024-03-22 9:02 UTC (permalink / raw) To: Jinjiang Tu, akpm, shr, hannes, riel, wangkefeng.wang, sunnanyong, linux-mm On 22.03.24 07:09, Jinjiang Tu wrote: > commit 3c6f33b7273a ("mm/ksm: support fork/exec for prctl") inherits > MMF_VM_MERGE_ANY flag when a task calls execve(). Howerver, it doesn't > create the mm_slot, so ksmd will not try to scan this task. > > To fix it, allocate and add the mm_slot to ksm_mm_head in __bprm_mm_init() > when the mm has MMF_VM_MERGE_ANY flag. > > Fixes: 3c6f33b7273a ("mm/ksm: support fork/exec for prctl") > Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com> > --- > fs/exec.c | 10 ++++++++++ > include/linux/ksm.h | 13 +++++++++++++ > 2 files changed, 23 insertions(+) > > diff --git a/fs/exec.c b/fs/exec.c > index ff6f26671cfc..66202d016a0a 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -67,6 +67,7 @@ > #include <linux/time_namespace.h> > #include <linux/user_events.h> > #include <linux/rseq.h> > +#include <linux/ksm.h> > > #include <linux/uaccess.h> > #include <asm/mmu_context.h> > @@ -267,6 +268,13 @@ static int __bprm_mm_init(struct linux_binprm *bprm) > goto err_free; > } > > + /* > + * Need to be called with mmap write lock > + * held, to avoid race with ksmd. > + */ > + if (ksm_execve(mm)) > + goto err_ksm; > + But now, would we revert what insert_vm_struct() did? We're freeing the VMA later, but we might have accounted memory. What would be cleaner is doing the ksm_execve() before the insert_vm_struct(), and then cleaning up in case insert_vm_struct() failed. -- Cheers, David / dhildenb ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl 2024-03-22 9:02 ` David Hildenbrand @ 2024-03-25 2:24 ` Jinjiang Tu 2024-03-25 8:33 ` David Hildenbrand 0 siblings, 1 reply; 12+ messages in thread From: Jinjiang Tu @ 2024-03-25 2:24 UTC (permalink / raw) To: David Hildenbrand, akpm, shr, hannes, riel, wangkefeng.wang, sunnanyong, linux-mm 在 2024/3/22 17:02, David Hildenbrand 写道: > On 22.03.24 07:09, Jinjiang Tu wrote: >> commit 3c6f33b7273a ("mm/ksm: support fork/exec for prctl") inherits >> MMF_VM_MERGE_ANY flag when a task calls execve(). Howerver, it doesn't >> create the mm_slot, so ksmd will not try to scan this task. >> >> To fix it, allocate and add the mm_slot to ksm_mm_head in >> __bprm_mm_init() >> when the mm has MMF_VM_MERGE_ANY flag. >> >> Fixes: 3c6f33b7273a ("mm/ksm: support fork/exec for prctl") >> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com> >> --- >> fs/exec.c | 10 ++++++++++ >> include/linux/ksm.h | 13 +++++++++++++ >> 2 files changed, 23 insertions(+) >> >> diff --git a/fs/exec.c b/fs/exec.c >> index ff6f26671cfc..66202d016a0a 100644 >> --- a/fs/exec.c >> +++ b/fs/exec.c >> @@ -67,6 +67,7 @@ >> #include <linux/time_namespace.h> >> #include <linux/user_events.h> >> #include <linux/rseq.h> >> +#include <linux/ksm.h> >> #include <linux/uaccess.h> >> #include <asm/mmu_context.h> >> @@ -267,6 +268,13 @@ static int __bprm_mm_init(struct linux_binprm >> *bprm) >> goto err_free; >> } >> + /* >> + * Need to be called with mmap write lock >> + * held, to avoid race with ksmd. >> + */ >> + if (ksm_execve(mm)) >> + goto err_ksm; >> + > > But now, would we revert what insert_vm_struct() did? > > We're freeing the VMA later, but we might have accounted memory. > > > What would be cleaner is doing the ksm_execve() before the > insert_vm_struct(), and then cleaning up in case insert_vm_struct() > failed. In fact, ksm_execve() has been called before the insert_vm_struct() in this patch. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl 2024-03-25 2:24 ` Jinjiang Tu @ 2024-03-25 8:33 ` David Hildenbrand 0 siblings, 0 replies; 12+ messages in thread From: David Hildenbrand @ 2024-03-25 8:33 UTC (permalink / raw) To: Jinjiang Tu, akpm, shr, hannes, riel, wangkefeng.wang, sunnanyong, linux-mm On 25.03.24 03:24, Jinjiang Tu wrote: > > 在 2024/3/22 17:02, David Hildenbrand 写道: >> On 22.03.24 07:09, Jinjiang Tu wrote: >>> commit 3c6f33b7273a ("mm/ksm: support fork/exec for prctl") inherits >>> MMF_VM_MERGE_ANY flag when a task calls execve(). Howerver, it doesn't >>> create the mm_slot, so ksmd will not try to scan this task. >>> >>> To fix it, allocate and add the mm_slot to ksm_mm_head in >>> __bprm_mm_init() >>> when the mm has MMF_VM_MERGE_ANY flag. >>> >>> Fixes: 3c6f33b7273a ("mm/ksm: support fork/exec for prctl") >>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com> >>> --- >>> fs/exec.c | 10 ++++++++++ >>> include/linux/ksm.h | 13 +++++++++++++ >>> 2 files changed, 23 insertions(+) >>> >>> diff --git a/fs/exec.c b/fs/exec.c >>> index ff6f26671cfc..66202d016a0a 100644 >>> --- a/fs/exec.c >>> +++ b/fs/exec.c >>> @@ -67,6 +67,7 @@ >>> #include <linux/time_namespace.h> >>> #include <linux/user_events.h> >>> #include <linux/rseq.h> >>> +#include <linux/ksm.h> >>> #include <linux/uaccess.h> >>> #include <asm/mmu_context.h> >>> @@ -267,6 +268,13 @@ static int __bprm_mm_init(struct linux_binprm >>> *bprm) >>> goto err_free; >>> } >>> + /* >>> + * Need to be called with mmap write lock >>> + * held, to avoid race with ksmd. >>> + */ >>> + if (ksm_execve(mm)) >>> + goto err_ksm; >>> + >> >> But now, would we revert what insert_vm_struct() did? >> >> We're freeing the VMA later, but we might have accounted memory. >> >> >> What would be cleaner is doing the ksm_execve() before the >> insert_vm_struct(), and then cleaning up in case insert_vm_struct() >> failed. > In fact, ksm_execve() has been called before the insert_vm_struct() in > this patch. > Ahh, I missed that. Indeed, that works then. -- Cheers, David / dhildenb ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl 2024-03-22 6:09 ` [PATCH v2 1/2] " Jinjiang Tu 2024-03-22 9:02 ` David Hildenbrand @ 2024-03-24 0:03 ` kernel test robot 2024-03-25 5:44 ` Dan Carpenter 2 siblings, 0 replies; 12+ messages in thread From: kernel test robot @ 2024-03-24 0:03 UTC (permalink / raw) To: Jinjiang Tu, akpm, david, shr, hannes, riel, wangkefeng.wang, sunnanyong, linux-mm Cc: llvm, oe-kbuild-all, tujinjiang Hi Jinjiang, kernel test robot noticed the following build warnings: [auto build test WARNING on akpm-mm/mm-everything] url: https://github.com/intel-lab-lkp/linux/commits/Jinjiang-Tu/mm-ksm-fix-ksm-exec-support-for-prctl/20240322-141317 base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything patch link: https://lore.kernel.org/r/20240322060947.3254967-2-tujinjiang%40huawei.com patch subject: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl config: s390-allnoconfig (https://download.01.org/0day-ci/archive/20240324/202403240716.8B7CiDbr-lkp@intel.com/config) compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 23de3862dce582ce91c1aa914467d982cb1a73b4) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240324/202403240716.8B7CiDbr-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202403240716.8B7CiDbr-lkp@intel.com/ All warnings (new ones prefixed by >>): In file included from fs/exec.c:30: In file included from include/linux/mm.h:2211: include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion] 514 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_" | ~~~~~~~~~~~ ^ ~~~ >> fs/exec.c:275:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] 275 | if (ksm_execve(mm)) | ^~~~~~~~~~~~~~ fs/exec.c:305:9: note: uninitialized use occurs here 305 | return err; | ^~~ fs/exec.c:275:2: note: remove the 'if' if its condition is always false 275 | if (ksm_execve(mm)) | ^~~~~~~~~~~~~~~~~~~ 276 | goto err_ksm; | ~~~~~~~~~~~~ fs/exec.c:257:9: note: initialize the variable 'err' to silence this warning 257 | int err; | ^ | = 0 2 warnings generated. vim +275 fs/exec.c 254 255 static int __bprm_mm_init(struct linux_binprm *bprm) 256 { 257 int err; 258 struct vm_area_struct *vma = NULL; 259 struct mm_struct *mm = bprm->mm; 260 261 bprm->vma = vma = vm_area_alloc(mm); 262 if (!vma) 263 return -ENOMEM; 264 vma_set_anonymous(vma); 265 266 if (mmap_write_lock_killable(mm)) { 267 err = -EINTR; 268 goto err_free; 269 } 270 271 /* 272 * Need to be called with mmap write lock 273 * held, to avoid race with ksmd. 274 */ > 275 if (ksm_execve(mm)) 276 goto err_ksm; 277 278 /* 279 * Place the stack at the largest stack address the architecture 280 * supports. Later, we'll move this to an appropriate place. We don't 281 * use STACK_TOP because that can depend on attributes which aren't 282 * configured yet. 283 */ 284 BUILD_BUG_ON(VM_STACK_FLAGS & VM_STACK_INCOMPLETE_SETUP); 285 vma->vm_end = STACK_TOP_MAX; 286 vma->vm_start = vma->vm_end - PAGE_SIZE; 287 vm_flags_init(vma, VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP); 288 vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); 289 290 err = insert_vm_struct(mm, vma); 291 if (err) 292 goto err; 293 294 mm->stack_vm = mm->total_vm = 1; 295 mmap_write_unlock(mm); 296 bprm->p = vma->vm_end - sizeof(void *); 297 return 0; 298 err: 299 ksm_exit(mm); 300 err_ksm: 301 mmap_write_unlock(mm); 302 err_free: 303 bprm->vma = NULL; 304 vm_area_free(vma); 305 return err; 306 } 307 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl 2024-03-22 6:09 ` [PATCH v2 1/2] " Jinjiang Tu 2024-03-22 9:02 ` David Hildenbrand 2024-03-24 0:03 ` kernel test robot @ 2024-03-25 5:44 ` Dan Carpenter 2024-03-25 6:33 ` Jinjiang Tu 2 siblings, 1 reply; 12+ messages in thread From: Dan Carpenter @ 2024-03-25 5:44 UTC (permalink / raw) To: oe-kbuild, Jinjiang Tu, akpm, david, shr, hannes, riel, wangkefeng.wang, sunnanyong, linux-mm Cc: lkp, oe-kbuild-all, tujinjiang Hi Jinjiang, kernel test robot noticed the following build warnings: url: https://github.com/intel-lab-lkp/linux/commits/Jinjiang-Tu/mm-ksm-fix-ksm-exec-support-for-prctl/20240322-141317 base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything patch link: https://lore.kernel.org/r/20240322060947.3254967-2-tujinjiang%40huawei.com patch subject: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl config: openrisc-randconfig-r081-20240322 (https://download.01.org/0day-ci/archive/20240324/202403240146.Pv4gVc5N-lkp@intel.com/config) compiler: or1k-linux-gcc (GCC) 13.2.0 If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> | Closes: https://lore.kernel.org/r/202403240146.Pv4gVc5N-lkp@intel.com/ smatch warnings: fs/exec.c:305 __bprm_mm_init() error: uninitialized symbol 'err'. vim +/err +305 fs/exec.c b6a2fea39318e43 Ollie Wild 2007-07-19 255 static int __bprm_mm_init(struct linux_binprm *bprm) b6a2fea39318e43 Ollie Wild 2007-07-19 256 { eaccbfa564e48c8 Luiz Fernando N. Capitulino 2009-01-06 257 int err; b6a2fea39318e43 Ollie Wild 2007-07-19 258 struct vm_area_struct *vma = NULL; b6a2fea39318e43 Ollie Wild 2007-07-19 259 struct mm_struct *mm = bprm->mm; b6a2fea39318e43 Ollie Wild 2007-07-19 260 490fc053865c9cc Linus Torvalds 2018-07-21 261 bprm->vma = vma = vm_area_alloc(mm); b6a2fea39318e43 Ollie Wild 2007-07-19 262 if (!vma) eaccbfa564e48c8 Luiz Fernando N. Capitulino 2009-01-06 263 return -ENOMEM; bfd40eaff5abb9f Kirill A. Shutemov 2018-07-26 264 vma_set_anonymous(vma); b6a2fea39318e43 Ollie Wild 2007-07-19 265 d8ed45c5dcd455f Michel Lespinasse 2020-06-08 266 if (mmap_write_lock_killable(mm)) { f268dfe905d4682 Michal Hocko 2016-05-23 267 err = -EINTR; f268dfe905d4682 Michal Hocko 2016-05-23 268 goto err_free; f268dfe905d4682 Michal Hocko 2016-05-23 269 } b6a2fea39318e43 Ollie Wild 2007-07-19 270 d282f6b19afd1a9 Jinjiang Tu 2024-03-22 271 /* d282f6b19afd1a9 Jinjiang Tu 2024-03-22 272 * Need to be called with mmap write lock d282f6b19afd1a9 Jinjiang Tu 2024-03-22 273 * held, to avoid race with ksmd. d282f6b19afd1a9 Jinjiang Tu 2024-03-22 274 */ d282f6b19afd1a9 Jinjiang Tu 2024-03-22 275 if (ksm_execve(mm)) d282f6b19afd1a9 Jinjiang Tu 2024-03-22 276 goto err_ksm; "err" not set before the goto. d282f6b19afd1a9 Jinjiang Tu 2024-03-22 277 b6a2fea39318e43 Ollie Wild 2007-07-19 278 /* b6a2fea39318e43 Ollie Wild 2007-07-19 279 * Place the stack at the largest stack address the architecture b6a2fea39318e43 Ollie Wild 2007-07-19 280 * supports. Later, we'll move this to an appropriate place. We don't b6a2fea39318e43 Ollie Wild 2007-07-19 281 * use STACK_TOP because that can depend on attributes which aren't b6a2fea39318e43 Ollie Wild 2007-07-19 282 * configured yet. b6a2fea39318e43 Ollie Wild 2007-07-19 283 */ aacb3d17a73f644 Michal Hocko 2011-07-26 284 BUILD_BUG_ON(VM_STACK_FLAGS & VM_STACK_INCOMPLETE_SETUP); b6a2fea39318e43 Ollie Wild 2007-07-19 285 vma->vm_end = STACK_TOP_MAX; b6a2fea39318e43 Ollie Wild 2007-07-19 286 vma->vm_start = vma->vm_end - PAGE_SIZE; 1c71222e5f2393b Suren Baghdasaryan 2023-01-26 287 vm_flags_init(vma, VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP); 3ed75eb8f1cd895 Coly Li 2007-10-18 288 vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); 462e635e5b73ba9 Tavis Ormandy 2010-12-09 289 b6a2fea39318e43 Ollie Wild 2007-07-19 290 err = insert_vm_struct(mm, vma); eaccbfa564e48c8 Luiz Fernando N. Capitulino 2009-01-06 291 if (err) b6a2fea39318e43 Ollie Wild 2007-07-19 292 goto err; b6a2fea39318e43 Ollie Wild 2007-07-19 293 b6a2fea39318e43 Ollie Wild 2007-07-19 294 mm->stack_vm = mm->total_vm = 1; d8ed45c5dcd455f Michel Lespinasse 2020-06-08 295 mmap_write_unlock(mm); b6a2fea39318e43 Ollie Wild 2007-07-19 296 bprm->p = vma->vm_end - sizeof(void *); b6a2fea39318e43 Ollie Wild 2007-07-19 297 return 0; b6a2fea39318e43 Ollie Wild 2007-07-19 298 err: d282f6b19afd1a9 Jinjiang Tu 2024-03-22 299 ksm_exit(mm); d282f6b19afd1a9 Jinjiang Tu 2024-03-22 300 err_ksm: d8ed45c5dcd455f Michel Lespinasse 2020-06-08 301 mmap_write_unlock(mm); f268dfe905d4682 Michal Hocko 2016-05-23 302 err_free: b6a2fea39318e43 Ollie Wild 2007-07-19 303 bprm->vma = NULL; 3928d4f5ee37cdc Linus Torvalds 2018-07-21 304 vm_area_free(vma); b6a2fea39318e43 Ollie Wild 2007-07-19 @305 return err; b6a2fea39318e43 Ollie Wild 2007-07-19 306 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl 2024-03-25 5:44 ` Dan Carpenter @ 2024-03-25 6:33 ` Jinjiang Tu 0 siblings, 0 replies; 12+ messages in thread From: Jinjiang Tu @ 2024-03-25 6:33 UTC (permalink / raw) To: Dan Carpenter, oe-kbuild, akpm, david, shr, hannes, riel, wangkefeng.wang, sunnanyong, linux-mm Cc: lkp, oe-kbuild-all 在 2024/3/25 13:44, Dan Carpenter 写道: > Hi Jinjiang, > > kernel test robot noticed the following build warnings: > > url: https://github.com/intel-lab-lkp/linux/commits/Jinjiang-Tu/mm-ksm-fix-ksm-exec-support-for-prctl/20240322-141317 > base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything > patch link: https://lore.kernel.org/r/20240322060947.3254967-2-tujinjiang%40huawei.com > patch subject: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl > config: openrisc-randconfig-r081-20240322 (https://download.01.org/0day-ci/archive/20240324/202403240146.Pv4gVc5N-lkp@intel.com/config) > compiler: or1k-linux-gcc (GCC) 13.2.0 > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > | Closes: https://lore.kernel.org/r/202403240146.Pv4gVc5N-lkp@intel.com/ > > smatch warnings: > fs/exec.c:305 __bprm_mm_init() error: uninitialized symbol 'err'. > > vim +/err +305 fs/exec.c > > b6a2fea39318e43 Ollie Wild 2007-07-19 255 static int __bprm_mm_init(struct linux_binprm *bprm) > b6a2fea39318e43 Ollie Wild 2007-07-19 256 { > eaccbfa564e48c8 Luiz Fernando N. Capitulino 2009-01-06 257 int err; > b6a2fea39318e43 Ollie Wild 2007-07-19 258 struct vm_area_struct *vma = NULL; > b6a2fea39318e43 Ollie Wild 2007-07-19 259 struct mm_struct *mm = bprm->mm; > b6a2fea39318e43 Ollie Wild 2007-07-19 260 > 490fc053865c9cc Linus Torvalds 2018-07-21 261 bprm->vma = vma = vm_area_alloc(mm); > b6a2fea39318e43 Ollie Wild 2007-07-19 262 if (!vma) > eaccbfa564e48c8 Luiz Fernando N. Capitulino 2009-01-06 263 return -ENOMEM; > bfd40eaff5abb9f Kirill A. Shutemov 2018-07-26 264 vma_set_anonymous(vma); > b6a2fea39318e43 Ollie Wild 2007-07-19 265 > d8ed45c5dcd455f Michel Lespinasse 2020-06-08 266 if (mmap_write_lock_killable(mm)) { > f268dfe905d4682 Michal Hocko 2016-05-23 267 err = -EINTR; > f268dfe905d4682 Michal Hocko 2016-05-23 268 goto err_free; > f268dfe905d4682 Michal Hocko 2016-05-23 269 } > b6a2fea39318e43 Ollie Wild 2007-07-19 270 > d282f6b19afd1a9 Jinjiang Tu 2024-03-22 271 /* > d282f6b19afd1a9 Jinjiang Tu 2024-03-22 272 * Need to be called with mmap write lock > d282f6b19afd1a9 Jinjiang Tu 2024-03-22 273 * held, to avoid race with ksmd. > d282f6b19afd1a9 Jinjiang Tu 2024-03-22 274 */ > d282f6b19afd1a9 Jinjiang Tu 2024-03-22 275 if (ksm_execve(mm)) > d282f6b19afd1a9 Jinjiang Tu 2024-03-22 276 goto err_ksm; > > "err" not set before the goto. The code should be: err = ksm_execve(mm); if (err) goto err_ksm; I will fix in the next version. > > d282f6b19afd1a9 Jinjiang Tu 2024-03-22 277 > b6a2fea39318e43 Ollie Wild 2007-07-19 278 /* > b6a2fea39318e43 Ollie Wild 2007-07-19 279 * Place the stack at the largest stack address the architecture > b6a2fea39318e43 Ollie Wild 2007-07-19 280 * supports. Later, we'll move this to an appropriate place. We don't > b6a2fea39318e43 Ollie Wild 2007-07-19 281 * use STACK_TOP because that can depend on attributes which aren't > b6a2fea39318e43 Ollie Wild 2007-07-19 282 * configured yet. > b6a2fea39318e43 Ollie Wild 2007-07-19 283 */ > aacb3d17a73f644 Michal Hocko 2011-07-26 284 BUILD_BUG_ON(VM_STACK_FLAGS & VM_STACK_INCOMPLETE_SETUP); > b6a2fea39318e43 Ollie Wild 2007-07-19 285 vma->vm_end = STACK_TOP_MAX; > b6a2fea39318e43 Ollie Wild 2007-07-19 286 vma->vm_start = vma->vm_end - PAGE_SIZE; > 1c71222e5f2393b Suren Baghdasaryan 2023-01-26 287 vm_flags_init(vma, VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP); > 3ed75eb8f1cd895 Coly Li 2007-10-18 288 vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); > 462e635e5b73ba9 Tavis Ormandy 2010-12-09 289 > b6a2fea39318e43 Ollie Wild 2007-07-19 290 err = insert_vm_struct(mm, vma); > eaccbfa564e48c8 Luiz Fernando N. Capitulino 2009-01-06 291 if (err) > b6a2fea39318e43 Ollie Wild 2007-07-19 292 goto err; > b6a2fea39318e43 Ollie Wild 2007-07-19 293 > b6a2fea39318e43 Ollie Wild 2007-07-19 294 mm->stack_vm = mm->total_vm = 1; > d8ed45c5dcd455f Michel Lespinasse 2020-06-08 295 mmap_write_unlock(mm); > b6a2fea39318e43 Ollie Wild 2007-07-19 296 bprm->p = vma->vm_end - sizeof(void *); > b6a2fea39318e43 Ollie Wild 2007-07-19 297 return 0; > b6a2fea39318e43 Ollie Wild 2007-07-19 298 err: > d282f6b19afd1a9 Jinjiang Tu 2024-03-22 299 ksm_exit(mm); > d282f6b19afd1a9 Jinjiang Tu 2024-03-22 300 err_ksm: > d8ed45c5dcd455f Michel Lespinasse 2020-06-08 301 mmap_write_unlock(mm); > f268dfe905d4682 Michal Hocko 2016-05-23 302 err_free: > b6a2fea39318e43 Ollie Wild 2007-07-19 303 bprm->vma = NULL; > 3928d4f5ee37cdc Linus Torvalds 2018-07-21 304 vm_area_free(vma); > b6a2fea39318e43 Ollie Wild 2007-07-19 @305 return err; > b6a2fea39318e43 Ollie Wild 2007-07-19 306 } > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec 2024-03-22 6:09 [PATCH v2 0/2] mm/ksm: fix ksm exec support for prctl Jinjiang Tu 2024-03-22 6:09 ` [PATCH v2 1/2] " Jinjiang Tu @ 2024-03-22 6:09 ` Jinjiang Tu 2024-03-22 11:43 ` David Hildenbrand 1 sibling, 1 reply; 12+ messages in thread From: Jinjiang Tu @ 2024-03-22 6:09 UTC (permalink / raw) To: akpm, david, shr, hannes, riel, wangkefeng.wang, sunnanyong, linux-mm Cc: tujinjiang This extends test_prctl_fork() and test_prctl_fork_exec() to make sure that deduplication really happens, instead of only test the MMF_VM_MERGE_ANY flag is set. Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com> --- .../selftests/mm/ksm_functional_tests.c | 79 +++++++++++++++++-- 1 file changed, 73 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c index d615767e396b..01999aab2e37 100644 --- a/tools/testing/selftests/mm/ksm_functional_tests.c +++ b/tools/testing/selftests/mm/ksm_functional_tests.c @@ -146,6 +146,54 @@ static int ksm_unmerge(void) return 0; } +static int child_test_merge(void) +{ + const unsigned int size = 2 * MiB; + char *map; + int ret = -1; + + /* Stabilize accounting by disabling KSM completely. */ + if (ksm_unmerge()) { + ksft_print_msg("Disabling (unmerging) KSM failed\n"); + return ret; + } + + if (get_my_merging_pages() > 0) { + ksft_print_msg("Still pages merged\n"); + return ret; + } + + map = mmap(NULL, size, PROT_READ|PROT_WRITE, + MAP_PRIVATE|MAP_ANON, -1, 0); + if (map == MAP_FAILED) { + ksft_print_msg("mmap() failed\n"); + return ret; + } + + /* Don't use THP. Ignore if THP are not around on a kernel. */ + if (madvise(map, size, MADV_NOHUGEPAGE) && errno != EINVAL) { + ksft_print_msg("MADV_NOHUGEPAGE failed\n"); + goto unmap; + } + + memset(map, 0x1c, size); + + if (ksm_merge()) { + ksft_print_msg("Running KSM failed\n"); + goto unmap; + } + + if (get_my_merging_pages() <= 0) { + ksft_print_msg("Fail to merge\n"); + goto unmap; + } + + ret = 0; +unmap: + munmap(map, size); + return ret; +} + static char *mmap_and_merge_range(char val, unsigned long size, int prot, bool use_prctl) { @@ -458,7 +506,11 @@ static void test_prctl_fork(void) child_pid = fork(); if (!child_pid) { - exit(prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0)); + if (prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0) != 1) + exit(-1); + if (child_test_merge() != 0) + exit(-2); + exit(0); } else if (child_pid < 0) { ksft_test_result_fail("fork() failed\n"); return; @@ -467,8 +519,14 @@ static void test_prctl_fork(void) if (waitpid(child_pid, &status, 0) < 0) { ksft_test_result_fail("waitpid() failed\n"); return; - } else if (WEXITSTATUS(status) != 1) { - ksft_test_result_fail("unexpected PR_GET_MEMORY_MERGE result in child\n"); + } + + status = WEXITSTATUS(status); + if (status != 0) { + if (status == -1) + ksft_test_result_fail("unexpected PR_GET_MEMORY_MERGE result in child\n"); + else + ksft_test_result_fail("fail to merge in child\n"); return; } @@ -483,7 +541,13 @@ static void test_prctl_fork(void) static int ksm_fork_exec_child(void) { /* Test if KSM is enabled for the process. */ - return prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0) == 1; + if (prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0) != 1) + return -1; + + if (child_test_merge() != 0) + return -2; + + return 0; } static void test_prctl_fork_exec(void) @@ -517,9 +581,12 @@ static void test_prctl_fork_exec(void) if (waitpid(child_pid, &status, 0) > 0) { if (WIFEXITED(status)) { status = WEXITSTATUS(status); - if (status) { + if (status == -1) { ksft_test_result_fail("KSM not enabled\n"); return; + } else if (status == -2) { + ksft_test_result_fail("fail to merge in child\n"); + return; } } else { ksft_test_result_fail("program didn't terminate normally\n"); @@ -599,7 +666,7 @@ int main(int argc, char **argv) int err; if (argc > 1 && !strcmp(argv[1], FORK_EXEC_CHILD_PRG_NAME)) { - exit(ksm_fork_exec_child() == 1 ? 0 : 1); + exit(ksm_fork_exec_child()); } #ifdef __NR_userfaultfd -- 2.25.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec 2024-03-22 6:09 ` [PATCH v2 2/2] selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec Jinjiang Tu @ 2024-03-22 11:43 ` David Hildenbrand 2024-03-25 2:24 ` Jinjiang Tu 0 siblings, 1 reply; 12+ messages in thread From: David Hildenbrand @ 2024-03-22 11:43 UTC (permalink / raw) To: Jinjiang Tu, akpm, shr, hannes, riel, wangkefeng.wang, sunnanyong, linux-mm On 22.03.24 07:09, Jinjiang Tu wrote: > This extends test_prctl_fork() and test_prctl_fork_exec() to make sure > that deduplication really happens, instead of only test the > MMF_VM_MERGE_ANY flag is set. > > Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com> > --- > .../selftests/mm/ksm_functional_tests.c | 79 +++++++++++++++++-- > 1 file changed, 73 insertions(+), 6 deletions(-) > > diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c > index d615767e396b..01999aab2e37 100644 > --- a/tools/testing/selftests/mm/ksm_functional_tests.c > +++ b/tools/testing/selftests/mm/ksm_functional_tests.c > @@ -146,6 +146,54 @@ static int ksm_unmerge(void) > return 0; > } > > +static int child_test_merge(void) > +{ > + const unsigned int size = 2 * MiB; > + char *map; > + int ret = -1; > + > + /* Stabilize accounting by disabling KSM completely. */ > + if (ksm_unmerge()) { > + ksft_print_msg("Disabling (unmerging) KSM failed\n"); > + return ret; > + } > + > + if (get_my_merging_pages() > 0) { > + ksft_print_msg("Still pages merged\n"); > + return ret; > + } > + > + map = mmap(NULL, size, PROT_READ|PROT_WRITE, > + MAP_PRIVATE|MAP_ANON, -1, 0); > + if (map == MAP_FAILED) { > + ksft_print_msg("mmap() failed\n"); > + return ret; > + } > + > + /* Don't use THP. Ignore if THP are not around on a kernel. */ > + if (madvise(map, size, MADV_NOHUGEPAGE) && errno != EINVAL) { > + ksft_print_msg("MADV_NOHUGEPAGE failed\n"); > + goto unmap; > + } > + > + memset(map, 0x1c, size); > + > + if (ksm_merge()) { > + ksft_print_msg("Running KSM failed\n"); > + goto unmap; > + } > + > + if (get_my_merging_pages() <= 0) { > + ksft_print_msg("Fail to merge\n"); > + goto unmap; > + } Looks like all you want is use mmap_and_merge_range(), but neither setting the prctl nor madvise(). Two alternatives: 1) switching from "bool use_prctl" to an enum like enum ksm_merge_mode { KSM_MERGE_PRCTL KSM_MERGE_MADVISE, KSM_MERGE_NONE, /* PRCTL already set */ }; Then, you can simply use mmap_and_merge_range(0x1c, 2 * MiB, PROT_READ|PROT_WRITE, KSM_MERGE_NONE); 2) With "bool use_prctl", before doing the prctl(PR_SET_MEMORY_MERGE, ...), check if it is already enabled. As we do that already in ksm_fork_exec_child(), and fail if it isn't set, that should work. Then, you can simply use mmap_and_merge_range(0x1c, 2 * MiB, PROT_READ|PROT_WRITE, true); here. > + > + ret = 0; > +unmap: > + munmap(map, size); > + return ret; > +} > + > static char *mmap_and_merge_range(char val, unsigned long size, int prot, > bool use_prctl) > { > @@ -458,7 +506,11 @@ static void test_prctl_fork(void) > > child_pid = fork(); > if (!child_pid) { > - exit(prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0)); > + if (prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0) != 1) > + exit(-1); > + if (child_test_merge() != 0) > + exit(-2); > + exit(0); > } else if (child_pid < 0) { > ksft_test_result_fail("fork() failed\n"); > return; > @@ -467,8 +519,14 @@ static void test_prctl_fork(void) > if (waitpid(child_pid, &status, 0) < 0) { > ksft_test_result_fail("waitpid() failed\n"); > return; > - } else if (WEXITSTATUS(status) != 1) { > - ksft_test_result_fail("unexpected PR_GET_MEMORY_MERGE result in child\n"); > + } > + > + status = WEXITSTATUS(status); > + if (status != 0) { > + if (status == -1) > + ksft_test_result_fail("unexpected PR_GET_MEMORY_MERGE result in child\n"); > + else > + ksft_test_result_fail("fail to merge in child\n"); > return; > } > > @@ -483,7 +541,13 @@ static void test_prctl_fork(void) > static int ksm_fork_exec_child(void) > { > /* Test if KSM is enabled for the process. */ > - return prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0) == 1; > + if (prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0) != 1) > + return -1; > + > + if (child_test_merge() != 0) You can drop the "!=0". But maybe, you can just inline the call to mmap_and_merge_range() here. > + return -2; > + > + return 0; -- Cheers, David / dhildenb ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec 2024-03-22 11:43 ` David Hildenbrand @ 2024-03-25 2:24 ` Jinjiang Tu 2024-03-25 8:38 ` David Hildenbrand 0 siblings, 1 reply; 12+ messages in thread From: Jinjiang Tu @ 2024-03-25 2:24 UTC (permalink / raw) To: David Hildenbrand, akpm, shr, hannes, riel, wangkefeng.wang, sunnanyong, linux-mm 在 2024/3/22 19:43, David Hildenbrand 写道: > On 22.03.24 07:09, Jinjiang Tu wrote: >> This extends test_prctl_fork() and test_prctl_fork_exec() to make sure >> that deduplication really happens, instead of only test the >> MMF_VM_MERGE_ANY flag is set. >> >> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com> >> --- >> .../selftests/mm/ksm_functional_tests.c | 79 +++++++++++++++++-- >> 1 file changed, 73 insertions(+), 6 deletions(-) >> >> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c >> b/tools/testing/selftests/mm/ksm_functional_tests.c >> index d615767e396b..01999aab2e37 100644 >> --- a/tools/testing/selftests/mm/ksm_functional_tests.c >> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c >> @@ -146,6 +146,54 @@ static int ksm_unmerge(void) >> return 0; >> } >> +static int child_test_merge(void) >> +{ >> + const unsigned int size = 2 * MiB; >> + char *map; >> + int ret = -1; >> + >> + /* Stabilize accounting by disabling KSM completely. */ >> + if (ksm_unmerge()) { >> + ksft_print_msg("Disabling (unmerging) KSM failed\n"); >> + return ret; >> + } >> + >> + if (get_my_merging_pages() > 0) { >> + ksft_print_msg("Still pages merged\n"); >> + return ret; >> + } >> + >> + map = mmap(NULL, size, PROT_READ|PROT_WRITE, >> + MAP_PRIVATE|MAP_ANON, -1, 0); >> + if (map == MAP_FAILED) { >> + ksft_print_msg("mmap() failed\n"); >> + return ret; >> + } >> + >> + /* Don't use THP. Ignore if THP are not around on a kernel. */ >> + if (madvise(map, size, MADV_NOHUGEPAGE) && errno != EINVAL) { >> + ksft_print_msg("MADV_NOHUGEPAGE failed\n"); >> + goto unmap; >> + } >> + >> + memset(map, 0x1c, size); >> + >> + if (ksm_merge()) { >> + ksft_print_msg("Running KSM failed\n"); >> + goto unmap; >> + } >> + >> + if (get_my_merging_pages() <= 0) { >> + ksft_print_msg("Fail to merge\n"); >> + goto unmap; >> + } > > Looks like all you want is use mmap_and_merge_range(), but neither > setting the prctl nor madvise(). > > Two alternatives: > > 1) switching from "bool use_prctl" to an enum like > > enum ksm_merge_mode { > KSM_MERGE_PRCTL > KSM_MERGE_MADVISE, > KSM_MERGE_NONE, /* PRCTL already set */ > }; > > Then, you can simply use mmap_and_merge_range(0x1c, 2 * MiB, > PROT_READ|PROT_WRITE, KSM_MERGE_NONE); I have considered this before. But, mmap_and_merge_range() calls ksft_test_result_fail() when error occurs, ksft_test_result_fail() prints prefixed with ksft_fail count. When mmap_and_merge_range() is called in the child process, the ksft_fail isn't consisent with the parent process due to the global variable ksft_fail is CoWed. As a result, ksft_print_msg() is intended to be called in child process. Maybe, We could introduce a macro ksm_print() to control which function is called according to ksm_merge_mode : #define ksm_print(mode, fmt, ...) do { \ if ((mode) == KSM_MERGE_NONE) \ ksft_print_msg(fmt, ##__VA_ARGS__);\ else \ ksft_test_result_fail(fmt, ##__VA_ARGS__);\ } while (0) > > 2) With "bool use_prctl", before doing the prctl(PR_SET_MEMORY_MERGE, > ...), check if it is already enabled. > > As we do that already in ksm_fork_exec_child(), and fail if it isn't > set, that should work. > > Then, you can simply use mmap_and_merge_range(0x1c, 2 * MiB, > PROT_READ|PROT_WRITE, true); > > here. > >> + >> + ret = 0; >> +unmap: >> + munmap(map, size); >> + return ret; >> +} >> + >> static char *mmap_and_merge_range(char val, unsigned long size, int >> prot, >> bool use_prctl) >> { >> @@ -458,7 +506,11 @@ static void test_prctl_fork(void) >> child_pid = fork(); >> if (!child_pid) { >> - exit(prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0)); >> + if (prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0) != 1) >> + exit(-1); >> + if (child_test_merge() != 0) >> + exit(-2); >> + exit(0); >> } else if (child_pid < 0) { >> ksft_test_result_fail("fork() failed\n"); >> return; >> @@ -467,8 +519,14 @@ static void test_prctl_fork(void) >> if (waitpid(child_pid, &status, 0) < 0) { >> ksft_test_result_fail("waitpid() failed\n"); >> return; >> - } else if (WEXITSTATUS(status) != 1) { >> - ksft_test_result_fail("unexpected PR_GET_MEMORY_MERGE result >> in child\n"); >> + } >> + >> + status = WEXITSTATUS(status); >> + if (status != 0) { >> + if (status == -1) >> + ksft_test_result_fail("unexpected PR_GET_MEMORY_MERGE >> result in child\n"); >> + else >> + ksft_test_result_fail("fail to merge in child\n"); >> return; >> } >> @@ -483,7 +541,13 @@ static void test_prctl_fork(void) >> static int ksm_fork_exec_child(void) >> { >> /* Test if KSM is enabled for the process. */ >> - return prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0) == 1; >> + if (prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0) != 1) >> + return -1; >> + >> + if (child_test_merge() != 0) > > You can drop the "!=0". But maybe, you can just inline the call to > mmap_and_merge_range() here. > >> + return -2; >> + >> + return 0; > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec 2024-03-25 2:24 ` Jinjiang Tu @ 2024-03-25 8:38 ` David Hildenbrand 0 siblings, 0 replies; 12+ messages in thread From: David Hildenbrand @ 2024-03-25 8:38 UTC (permalink / raw) To: Jinjiang Tu, akpm, shr, hannes, riel, wangkefeng.wang, sunnanyong, linux-mm On 25.03.24 03:24, Jinjiang Tu wrote: > > 在 2024/3/22 19:43, David Hildenbrand 写道: >> On 22.03.24 07:09, Jinjiang Tu wrote: >>> This extends test_prctl_fork() and test_prctl_fork_exec() to make sure >>> that deduplication really happens, instead of only test the >>> MMF_VM_MERGE_ANY flag is set. >>> >>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com> >>> --- >>> .../selftests/mm/ksm_functional_tests.c | 79 +++++++++++++++++-- >>> 1 file changed, 73 insertions(+), 6 deletions(-) >>> >>> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c >>> b/tools/testing/selftests/mm/ksm_functional_tests.c >>> index d615767e396b..01999aab2e37 100644 >>> --- a/tools/testing/selftests/mm/ksm_functional_tests.c >>> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c >>> @@ -146,6 +146,54 @@ static int ksm_unmerge(void) >>> return 0; >>> } >>> +static int child_test_merge(void) >>> +{ >>> + const unsigned int size = 2 * MiB; >>> + char *map; >>> + int ret = -1; >>> + >>> + /* Stabilize accounting by disabling KSM completely. */ >>> + if (ksm_unmerge()) { >>> + ksft_print_msg("Disabling (unmerging) KSM failed\n"); >>> + return ret; >>> + } >>> + >>> + if (get_my_merging_pages() > 0) { >>> + ksft_print_msg("Still pages merged\n"); >>> + return ret; >>> + } >>> + >>> + map = mmap(NULL, size, PROT_READ|PROT_WRITE, >>> + MAP_PRIVATE|MAP_ANON, -1, 0); >>> + if (map == MAP_FAILED) { >>> + ksft_print_msg("mmap() failed\n"); >>> + return ret; >>> + } >>> + >>> + /* Don't use THP. Ignore if THP are not around on a kernel. */ >>> + if (madvise(map, size, MADV_NOHUGEPAGE) && errno != EINVAL) { >>> + ksft_print_msg("MADV_NOHUGEPAGE failed\n"); >>> + goto unmap; >>> + } >>> + >>> + memset(map, 0x1c, size); >>> + >>> + if (ksm_merge()) { >>> + ksft_print_msg("Running KSM failed\n"); >>> + goto unmap; >>> + } >>> + >>> + if (get_my_merging_pages() <= 0) { >>> + ksft_print_msg("Fail to merge\n"); >>> + goto unmap; >>> + } >> >> Looks like all you want is use mmap_and_merge_range(), but neither >> setting the prctl nor madvise(). >> >> Two alternatives: >> >> 1) switching from "bool use_prctl" to an enum like >> >> enum ksm_merge_mode { >> KSM_MERGE_PRCTL >> KSM_MERGE_MADVISE, >> KSM_MERGE_NONE, /* PRCTL already set */ >> }; >> >> Then, you can simply use mmap_and_merge_range(0x1c, 2 * MiB, >> PROT_READ|PROT_WRITE, KSM_MERGE_NONE); > I have considered this before. But, mmap_and_merge_range() calls > ksft_test_result_fail() when error occurs, ksft_test_result_fail() > prints prefixed with ksft_fail count. When mmap_and_merge_range() is > called in the child process, the ksft_fail isn't consisent with the > parent process due to the global variable ksft_fail is CoWed. As a > result, ksft_print_msg() is intended to be called in child process. > > Maybe, We could introduce a macro ksm_print() to control which function > is called according to ksm_merge_mode : As an alternative, convert all ksft_test_result_fail() in there into ksft_print_msg(), and in the callers of mmap_and_merge_range(), do something like map = mmap_and_merge_range() ... if (map == MAP_FAILED) { ksft_test_result_fail("Merging memory failed"); return; } -- Cheers, David / dhildenb ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-03-25 8:38 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-22 6:09 [PATCH v2 0/2] mm/ksm: fix ksm exec support for prctl Jinjiang Tu 2024-03-22 6:09 ` [PATCH v2 1/2] " Jinjiang Tu 2024-03-22 9:02 ` David Hildenbrand 2024-03-25 2:24 ` Jinjiang Tu 2024-03-25 8:33 ` David Hildenbrand 2024-03-24 0:03 ` kernel test robot 2024-03-25 5:44 ` Dan Carpenter 2024-03-25 6:33 ` Jinjiang Tu 2024-03-22 6:09 ` [PATCH v2 2/2] selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec Jinjiang Tu 2024-03-22 11:43 ` David Hildenbrand 2024-03-25 2:24 ` Jinjiang Tu 2024-03-25 8:38 ` David Hildenbrand
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).