[PATCH v2 0/2] mm/ksm: fix ksm exec support for prctl

[PATCH v2 0/2] mm/ksm: fix ksm exec support for prctl

[Jinjiang Tu]

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

[PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl

[Jinjiang Tu]

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

[PATCH v2 2/2] selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec

[Jinjiang Tu]

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

Re: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl

[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.

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 2/2] selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec

[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);

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

Re: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl

[kernel test robot]

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

Re: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl

[Jinjiang Tu]

在 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.

Re: [PATCH v2 2/2] selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec

[Jinjiang Tu]

在 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;
>
>

Re: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl

[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.

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

Re: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl

[Jinjiang Tu]

在 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  }
>

Re: [PATCH v2 1/2] mm/ksm: fix ksm exec support for prctl

[David Hildenbrand]

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

Re: [PATCH v2 2/2] selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec

[David Hildenbrand]

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