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

Petr Vorel
2023-12-04T10:40:07Z

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