Re: [LTP] [PATCH v3 2/2] add ksm test for smart-scan feature

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
> index 7258489ed..c96fe8bfc 100644
> --- a/testcases/kernel/mem/.gitignore
> +++ b/testcases/kernel/mem/.gitignore
> @@ -53,6 +53,7 @@
>  /ksm/ksm04
>  /ksm/ksm05
>  /ksm/ksm06
> +/ksm/ksm07
>  /mem/mem02
>  /mmapstress/mmap-corruption01
>  /mmapstress/mmapstress01
> diff --git a/testcases/kernel/mem/ksm/ksm07.c b/testcases/kernel/mem/ksm/ksm07.c
> new file mode 100644
> index 000000000..16153fdb2
> --- /dev/null
> +++ b/testcases/kernel/mem/ksm/ksm07.c
> @@ -0,0 +1,131 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2010-2017  Red Hat, Inc.
                      ^
		      2023?
> + */
> +/*\
> + * [Description]
> + *
> + * Kernel Samepage Merging (KSM)
> + *
> + * This adds a new ksm (kernel samepage merging) test to evaluate the new
> + * smart scan feature. It allocates a page and fills it with 'a'
> + * characters. It captures the pages_skipped counter, waits for a few
> + * iterations and captures the pages_skipped counter again. The expectation
> + * is that over 50% of the page scans are skipped (There is only one page
> + * that has KSM enabled and it gets scanned during each iteration and it
> + * cannot be de-duplicated).
> + *
> + * Prerequisites:
> + *
> + * 1) ksm and ksmtuned daemons need to be disabled. Otherwise, it could
> + *    distrub the testing as they also change some ksm tunables depends
> + *    on current workloads.

I guess that it would be more user friendly to check if a process with
such name is running and at least print a warning, or even skip the
test.

I suppose that we can add a library function that would loop over
/proc/*/comm and return true if we find a match.

> + */
> +#include <sys/types.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <sys/wait.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include "../include/mem.h"
> +#include "ksm_common.h"
> +
> +/* This test allocates one page, fills the page with a's, captures the
> + * full_scan and pages_skipped counters. Then it makes sure at least 3
> + * full scans have been performed and measures the above counters again.
> + * The expectation is that at least 50% of the pages are skipped.
> + *
> + * To wait for at least 3 scans it uses the wait_ksmd_full_scan() function. In
> + * reality, it will be a lot more scans as the wait_ksmd_full_scan() function
> + * sleeps for one second.
> + */
> +static void create_memory(void)
> +{
> +	int status;
> +	int full_scans_begin;
> +	int full_scans_end;
> +	int pages_skipped_begin;
> +	int pages_skipped_end;
> +	int diff_pages;
> +	int diff_scans;
> +	unsigned long page_size;
> +	char *memory;
> +
> +	/* Apply for the space for memory. */
> +	page_size = sysconf(_SC_PAGE_SIZE);
> +	memory = SAFE_MALLOC(page_size);
> +
> +	for (int i = 0; i < 1; i++) {
> +		memory = SAFE_MMAP(NULL, page_size, PROT_READ|PROT_WRITE,
> +			MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> +#ifdef HAVE_DECL_MADV_MERGEABLE
> +		if (madvise(memory, page_size, MADV_MERGEABLE) == -1)
> +			tst_brk(TBROK|TERRNO, "madvise");
> +#endif
> +	}
> +	memset(memory, 'a', page_size);
> +
> +	tst_res(TINFO, "KSM merging...");
> +	if (access(PATH_KSM "max_page_sharing", F_OK) == 0) {
> +		SAFE_FILE_PRINTF(PATH_KSM "run", "2");
> +	}
> +
> +	/* Set defalut ksm scan values. */
> +	SAFE_FILE_PRINTF(PATH_KSM "run", "1");
> +	SAFE_FILE_PRINTF(PATH_KSM "pages_to_scan", "%ld", 100l);
> +	SAFE_FILE_PRINTF(PATH_KSM "sleep_millisecs", "0");
> +
> +	/* Measure pages skipped aka "smart scan". */
> +	SAFE_FILE_SCANF(PATH_KSM "full_scans", "%d", &full_scans_begin);
> +	SAFE_FILE_SCANF(PATH_KSM "pages_skipped", "%d", &pages_skipped_begin);
> +	wait_ksmd_full_scan();
> +
> +	tst_res(TINFO, "stop KSM.");
> +	SAFE_FILE_PRINTF(PATH_KSM "run", "0");
> +
> +	SAFE_FILE_SCANF(PATH_KSM "full_scans", "%d", &full_scans_end);
> +	SAFE_FILE_SCANF(PATH_KSM "pages_skipped", "%d", &pages_skipped_end);
> +	diff_pages = pages_skipped_end - pages_skipped_begin;
> +	diff_scans = full_scans_end - full_scans_begin;
> +
> +	if (diff_pages < diff_scans * 50 / 100) {
> +		tst_res(TFAIL, "not enough pages have been skipped by smart_scan.");
> +	} else {
> +		tst_res(TPASS, "smart_scan skipped more than 50%% of the pages.");
> +	}
> +
> +	while (waitpid(-1, &status, 0) > 0)
> +		if (WEXITSTATUS(status) != 0)
> +			tst_res(TFAIL, "child exit status is %d",
> +					WEXITSTATUS(status));

Does the test fork a child? I do not see any place in the code that
would do so, did I miss something?

> +}
> +
> +static void verify_ksm(void)
> +{
> +	create_memory();

This is useless indirection.

> +}
> +
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.options = (struct tst_option[]) {
> +		{}
> +	},
> +	.save_restore = (const struct tst_path_val[]) {
> +		{"/sys/kernel/mm/ksm/run", NULL, TST_SR_TCONF},
> +		{"/sys/kernel/mm/ksm/sleep_millisecs", NULL, TST_SR_TCONF},
> +		{"/sys/kernel/mm/ksm/smart_scan", "1",
> +			TST_SR_SKIP_MISSING | TST_SR_TCONF},
> +		{}
> +	},
> +	.needs_kconfigs = (const char *const[]){
> +		"CONFIG_KSM=y",
> +		NULL
> +	},
> +	.test_all = verify_ksm,
> +};
> -- 
> 2.39.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp