Re: [LTP] [PATCH v1] mem: disable KSM smart scan for ksm tests

[Petr Vorel <pvorel@suse.cz>]

Hi Stefan, Li,

> Hi Stefan, Petr,

> On Tue, Nov 28, 2023 at 3:46 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Stefan,

> > > This disables the "smart scan" KSM feature to make sure that the volatile
> > > count remains at 0.

> > > Signed-off-by: Stefan Roesch <devkernel.io>
> > nit: you forgot 'shr@'
> > Signed-off-by: Stefan Roesch <shr@devkernel.io>

> > > Reported-by: kernel test robot <oliver.sang@intel.com>
> > > Closes:
> > https://lore.kernel.org/oe-lkp/202311161132.13d8ce5a-oliver.sang@intel.com
> > > ---
> > >  testcases/kernel/mem/lib/mem.c | 4 ++++
> > >  1 file changed, 4 insertions(+)

> > > diff --git a/testcases/kernel/mem/lib/mem.c
> > b/testcases/kernel/mem/lib/mem.c
> > > index fbfeef026..ef274a3ac 100644
> > > --- a/testcases/kernel/mem/lib/mem.c
> > > +++ b/testcases/kernel/mem/lib/mem.c
> > > @@ -454,6 +454,9 @@ void create_same_memory(int size, int num, int unit)
> > >              {'a', size*MB}, {'a', size*MB}, {'d', size*MB}, {'d',
> > size*MB},
> > >       };

> > > +  /* Disable smart scan for correct volatile counts. */
> > > +  SAFE_FILE_PRINTF(PATH_KSM "smart_scan", "0");
> > NOTE, this fails on the systems without /sys/kernel/mm/ksm/smart_scan:

> > mem.c:458: TBROK: Failed to open FILE '/sys/kernel/mm/ksm/smart_scan' for
> > writing: EACCES (13)

> > NOTE, we normally handle the setup like this in test setup function.

> > But new API has .save_restore which is more robust for tasks  like this.
> > It's already used in ksm01.c, you need just to add this line:
> >         {"/sys/kernel/mm/ksm/smart_scan", "0", TST_SR_SKIP},


> I guess we need to set 'TST_SR_SKIP_MISSING | TST_SR_TBROK_RO'
> as the last field. Because TST_SR_SKIP will continue the test without
> writing '0' to the smart_scan file, that's not correct if the file exists.
> It will
> ignore a kernel bug (smart_scan can't be written) by that config.

> Per the Doc Petr pointed below:
>   TST_SR_SKIP_MISSING – Continue without saving the file if it does not
> exist
>   TST_SR_TBROK_RO – End test with TBROK if the file is read-only
>   TST_SR_SKIP_RO – Continue without saving the file if it is read-only
>   TST_SR_SKIP – Equivalent to 'TST_SR_SKIP_MISSING | TST_SR_SKIP_RO'



> > (instead of both SAFE_FILE_PRINTF)

> > See:

> > https://github.com/linux-test-project/ltp/wiki/C-Test-API#127-saving--restoring-procsys-values

> > I wonder if ksm01.c is the only ksm test which needs to disable this.


> I think all of the ksm0*.c tests should disable it by the config. The
> smart_scan
> will impact all the tests with invoke key function create_same_memory().

ksm05.c and ksm06.c does not use create_same_memory(). Or did I overlook
something?

> > also nit: there is a wrong indent (spaces instead of tabs), please be
> > consistent
> > with the file content.

> > NOTE: while this fixes problem on 6.7.0-rc1-2.g86e46c2-default (openSUSE),
> > it does not fixes other problem on 6.5.10 on Debian (16 errors like these
> > below):

> > mem.c:252: TFAIL: pages_shared is not 2 but 5038.
> > mem.c:252: TFAIL: pages_sharing is not 98302 but 593629.
> > mem.c:252: TFAIL: pages_volatile is not 0 but 391.
> > mem.c:252: TFAIL: pages_unshared is not 0 but 149157.

> > I have no idea if this is a real bug which needs to be fixed or test false
> > positive to be fixed, or whether the problem has already been fixed in
> > newer
> > kernels.


> It is more like a real bug, the Debain kernel-6.5.10 does not contain
> this smart_scan feature. Or you may try to build the latest kernel
> on your platform to see if it can be reproduced as well.


I'll try to reproduce this on mainline kernel 6.5, 6.6 and 6.7.


> > > +
> > >       ps = sysconf(_SC_PAGE_SIZE);
> > >       pages = MB / ps;

> > > @@ -526,6 +529,7 @@ void create_same_memory(int size, int num, int unit)

> > >       tst_res(TINFO, "stop KSM.");
> > >       SAFE_FILE_PRINTF(PATH_KSM "run", "0");
> > > +  SAFE_FILE_PRINTF(PATH_KSM "smart_scan", "1");
> > nit: Again, wrong indent. You could have seen it also in the generated
> > patch.

> > Kind regards,
> > Petr

> > >       final_group_check(0, 0, 0, 0, 0, 0, size * pages * num);

> > >       while (waitpid(-1, &status, 0) > 0)

> > > base-commit: 8c89ef3d451087ed6e18750bd5eedd10e5ab3d2e



> One more comment not related to this patch:

> @Stefan, do you have a test (or plan to) verify the 'smart_scan' feture
> works?
> As we do disables it for all ksm* tests in LTP, so, it would be great to
> have one
> for testing in enable mode. What do you think?

This makes perfect sense even if I'm not that ksm05.c and ksm06.c also needs to
disable smart_scan.

Kind regards,
Petr