Re: [PATCH v6 3/3] selftests/mm: add new selftests for KSM

[Stefan Roesch <shr@devkernel.io>]

David Hildenbrand <david@redhat.com> writes:

> On 12.04.23 05:16, Stefan Roesch wrote:
>> This adds three new tests to the selftests for KSM.  These tests use the
>> new prctl API's to enable and disable KSM.
>> 1) add new prctl flags to prctl header file in tools dir
>>     This adds the new prctl flags to the include file prct.h in the
>>     tools directory.  This makes sure they are available for testing.
>> 2) add KSM prctl merge test
>>     This adds the -t option to the ksm_tests program.  The -t flag
>>     allows to specify if it should use madvise or prctl ksm merging.
>> 3) add KSM get merge type test
>>     This adds the -G flag to the ksm_tests program to query the KSM
>>     status with prctl after KSM has been enabled with prctl.
>> 4) add KSM fork test
>>     Add fork test to verify that the MMF_VM_MERGE_ANY flag is inherited
>>     by the child process.
>> 5) add two functions for debugging merge outcome
>>     This adds two functions to report the metrics in /proc/self/ksm_stat
>>     and /sys/kernel/debug/mm/ksm.
>> The debugging can be enabled with the following command line:
>> make -C tools/testing/selftests TARGETS="mm" --keep-going \
>>          EXTRA_CFLAGS=-DDEBUG=1
>
> Would it make sense to instead have a "-D" (if still unused) runtime options to
> print this data? Dead code that's not compiled is a bit unfortunate as it can
> easily bit-rot.
>
>

In the next version I'll add -d option. I'll add a global debug variable
for this. Otherwise we would need to pass down the debug option several
levels.

>
> This patch essentially does two things
>
> 1) Add the option to run all tests/benchmarks with the PRCTL instead of MADVISE
>
> 2) Add some functional KSM tests for the new PRCTL (fork, enabling works,
> disabling works).
>
> The latter should rather go into ksm_functional_tests().
>
> [...]
>
>>   -static int check_ksm_unmerge(int mapping, int prot, int timeout, size_t
>> page_size)
>> +/* Verify that prctl ksm flag is inherited. */
>> +static int check_ksm_fork(void)
>> +{
>> +	int rc = KSFT_FAIL;
>> +	pid_t child_pid;
>> +
>> +	if (prctl(PR_SET_MEMORY_MERGE, 1)) {
>> +		perror("prctl");
>> +		return KSFT_FAIL;
>> +	}
>> +
>> +	child_pid = fork();
>> +	if (child_pid == 0) {
>> +		int is_on = prctl(PR_GET_MEMORY_MERGE, 0);
>> +
>> +		if (!is_on)
>> +			exit(KSFT_FAIL);
>> +
>> +		exit(KSFT_PASS);
>> +	}
>> +
>> +	if (child_pid < 0)
>> +		goto out;
>> +
>> +	if (waitpid(child_pid, &rc, 0) < 0)
>> +		rc = KSFT_FAIL;
>> +
>> +	if (prctl(PR_SET_MEMORY_MERGE, 0)) {
>> +		perror("prctl");
>> +		rc = KSFT_FAIL;
>> +	}
>> +
>> +out:
>> +	if (rc == KSFT_PASS)
>> +		printf("OK\n");
>> +	else
>> +		printf("Not OK\n");
>> +
>> +	return rc;
>> +}
>> +
>> +static int check_ksm_get_merge_type(void)
>> +{
>> +	if (prctl(PR_SET_MEMORY_MERGE, 1)) {
>> +		perror("prctl set");
>> +		return 1;
>> +	}
>> +
>> +	int is_on = prctl(PR_GET_MEMORY_MERGE, 0);
>> +
>> +	if (prctl(PR_SET_MEMORY_MERGE, 0)) {
>> +		perror("prctl set");
>> +		return 1;
>> +	}
>> +
>> +	int is_off = prctl(PR_GET_MEMORY_MERGE, 0);
>> +
>> +	if (is_on && is_off) {
>> +		printf("OK\n");
>> +		return KSFT_PASS;
>> +	}
>> +
>> +	printf("Not OK\n");
>> +	return KSFT_FAIL;
>> +}
>
> Yes, these two are better located in ksm_functional_tests() to just run them
> both automatically when the test is executed.

I moved the check_ksm_get_merge_type() and check_ksm_fork() to the
ksm_functional_test executable. The change will be in the next version.