Re: [LTP] [PATCH 5/7] mem/ksm06: Move ksm restoration into the tst_test struct

[Li Wang <liwang@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3358 bytes --]

On Mon, Mar 7, 2022 at 4:56 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > > > But for the two knobs(run, sleep_millisecs) that should exist unless
> > > > the kernel disables KSM. So here we'd better start with prefix '!'
> and
> > > > add .needs_kconfg for ???CONFIG_KSM=y' check.
> > > > (This also fit for other ksm tests)
> > >
> > > I guess that if we put ! before the merge_across_nodes that would cause
> > > TBROK on systems without CONFIG_NUMA or kernels without that feature.
> > >
> > > So what about just removing the question marks there and adding
> > > .need_kconfigs for KSM and NUMA?
> > >
> >
> > Er, that's exactly what I meant in the last email, maybe you overlooked
> > the last sentence:).
> >
> > i.e.
> >
> > "
> >   prefix ! for 'run' and 'sleep_milisecs'
> >   no prefix for 'merge_across_nodes'
> >   .need_kconfigs for KSM and NUMA
> > "
>
> Ah, right, sorry. What about this:
>
> diff --git a/testcases/kernel/mem/ksm/ksm06.c
> b/testcases/kernel/mem/ksm/ksm06.c
> index 61507b2aa..e734786c1 100644
> --- a/testcases/kernel/mem/ksm/ksm06.c
> +++ b/testcases/kernel/mem/ksm/ksm06.c
> @@ -39,9 +39,6 @@
>  #ifdef HAVE_NUMA_V2
>  #include <numaif.h>
>
> -static int run = -1;
> -static int sleep_millisecs = -1;
> -static int merge_across_nodes = -1;
>  static unsigned long nr_pages = 100;
>
>  static char *n_opt;
> @@ -133,35 +130,11 @@ static void test_ksm(void)
>
>  static void setup(void)
>  {
> -       if (access(PATH_KSM "merge_across_nodes", F_OK) == -1)
> -               tst_brk(TCONF, "no merge_across_nodes sysfs knob");
> -
>         if (!is_numa(NULL, NH_MEMS, 2))
>                 tst_brk(TCONF, "The case needs a NUMA system.");
>
>         if (n_opt)
>                 nr_pages = SAFE_STRTOUL(n_opt, 0, ULONG_MAX);
> -
> -       /* save the current value */
> -       SAFE_FILE_SCANF(PATH_KSM "run", "%d", &run);
> -       SAFE_FILE_SCANF(PATH_KSM "merge_across_nodes",
> -                       "%d", &merge_across_nodes);
> -       SAFE_FILE_SCANF(PATH_KSM "sleep_millisecs",
> -                       "%d", &sleep_millisecs);
> -}
> -
> -static void cleanup(void)
> -{
> -       if (merge_across_nodes != -1) {
> -               FILE_PRINTF(PATH_KSM "merge_across_nodes",
> -                           "%d", merge_across_nodes);
> -       }
> -
> -       if (sleep_millisecs != -1)
> -               FILE_PRINTF(PATH_KSM "sleep_millisecs", "%d",
> sleep_millisecs);
> -
> -       if (run != -1)
> -               FILE_PRINTF(PATH_KSM "run", "%d", run);
>  }
>
>  static struct tst_test test = {
> @@ -171,11 +144,18 @@ static struct tst_test test = {
>                 {}
>         },
>         .setup = setup,
> -       .cleanup = cleanup,
>         .save_restore = (const char * const[]) {
>                 "?/sys/kernel/mm/ksm/max_page_sharing",
> +               "!/sys/kernel/mm/ksm/run",
> +               "!/sys/kernel/mm/ksm/sleep_millisecs",
> +               "/sys/kernel/mm/ksm/merge_across_nodes",
>                 NULL,
>         },
> +       .needs_kconfigs = (const char *const[]){
> +               "CONFIG_KSM=y",
> +               "CONFIG_NUMA=y",
> +               NULL
> +       },
>         .test_all = test_ksm,
>  };
>
>
> If we add merge_across_nodes without any prefix we can as well remove
> the check for the file existence in the test setup.
>

ACK.

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 4839 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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