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

[Petr Vorel <pvorel@suse.cz>]

Hi Stefan,

...
> +++ b/testcases/kernel/mem/ksm/ksm07.c
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright (C) 2010-2017  Red Hat, Inc.
> + *

> + * This program is free software;  you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY;  without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> + * the GNU General Public License for more details.
NOTE: we use SPDX instead of verbose GPL (see ksm06.c).
> + *

NOTE: we have special doc format which starts like this (see ksm06.c):
/*\
 * [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.
Hm, we don't have to any helper in LTP API to detect this, so at least we
document this.
> + *
> + */

The result is then uploaded:
https://github.com/linux-test-project/ltp/releases/download/20230929/metadata.20230929.html

Therefore please use:

// SPDX-License-Identifier: GPL-2.0-or-later
/*
 * Copyright (C) 2010-2017  Red Hat, Inc.
 */
/*\
 * [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.
 */

> +
> +#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"
> +
> +static void verify_ksm(void)
> +{
> +	create_memory_for_smartscan();
I wonder, if we reusable create_memory_for_smartscan() later. Maybe it should be
put into ksm07 for the start.

Also, the test needs to run on older kernel - exit with TCONF (which is not
considered as a failure) instead of TBROK which does now:
mem.c:488: TBROK: Failed to open FILE '/sys/kernel/mm/ksm/pages_skipped' for reading: ENOENT (2)

If the testing code stays in testcases/kernel/mem/lib/mem.c, you will have to
stat() it. But if it's really just this test specific and you move it to
ksm07.c, then you can simply add the handling via .save_restore.

> +}
> +
> +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_TBROK},
> +		{"/sys/kernel/mm/ksm/sleep_millisecs", NULL, TST_SR_TBROK},
> +		{"/sys/kernel/mm/ksm/smart_scan", "1",
> +			TST_SR_SKIP_MISSING | TST_SR_TBROK_RO},
> +		{}
> +	},
> +	.needs_kconfigs = (const char *const[]){
> +		"CONFIG_KSM=y",
> +		NULL
> +	},
> +	.test_all = verify_ksm,
> +};

Also, there are missing static:
$ cd testcases/kernel/mem/ksm/; make check-ksm07
CHECK testcases/kernel/mem/ksm/ksm07.c
ksm07.c:1: WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
make: [../../../../include/mk/rules.mk:56: check-ksm07] Error 1 (ignored)
ksm07.c: note: in included file:
ksm_common.h:14:5: warning: Symbol 'size' has no prototype or library ('tst_') prefix. Should it be static?
ksm_common.h:14:29: warning: Symbol 'num' has no prototype or library ('tst_') prefix. Should it be static?
ksm_common.h:14:38: warning: Symbol 'unit' has no prototype or library ('tst_') prefix. Should it be static?
ksm_common.h:15:6: warning: Symbol 'opt_sizestr' has no prototype or library ('tst_') prefix. Should it be static?
ksm_common.h:15:20: warning: Symbol 'opt_numstr' has no prototype or library ('tst_') prefix. Should it be static?
ksm_common.h:15:33: warning: Symbol 'opt_unitstr' has no prototype or library ('tst_') prefix. Should it be static?

Kind regards,
Petr

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