| Message ID | 20231127174517.2369593-1-shr@devkernel.io (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] mem: disable KSM smart scan for ksm tests | expand |
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}, (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. 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. > + > 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
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(). > > 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. > > > + > > 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?
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
On Wed, Nov 29, 2023 at 12:51 AM Petr Vorel <pvorel@suse.cz> wrote: > 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? > Good catch, I looked into these tests, seems only ksm05 is debatable for disabling smart_scan, as a simple regression, it suggests disabling ksm daemon to avoid disturb according to some workload. https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/mem/ksm/ksm05.c#L30 ksm06 is definitely need disable smart_scan, it tests KSM in different 'run' state for merge_accros_nodes. To be on the safe side, I would suggest applying the patch to all ksm* tests, and write a new single for smart_scan if needed.
> On Wed, Nov 29, 2023 at 12:51 AM Petr Vorel <pvorel@suse.cz> wrote: > > 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? > Good catch, I looked into these tests, seems only ksm05 is debatable > for disabling smart_scan, as a simple regression, it suggests disabling > ksm daemon to avoid disturb according to some workload. > https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/mem/ksm/ksm05.c#L30 > ksm06 is definitely need disable smart_scan, it tests KSM in different > 'run' state for merge_accros_nodes. Thanks for having a look. > To be on the safe side, I would suggest applying the patch to all ksm* > tests, > and write a new single for smart_scan if needed. Agree. I vote for new single for smart_scan related test. Kind regards, Petr
Petr Vorel <pvorel@suse.cz> writes: > 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> > Thanks, will be fixed with the next version. >> 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'll add it with the next version > (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. > Other tests will also need it. They also use the create_same_memory() function. > 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. > This is a different problem. "Smart scan was introduced with 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
Li Wang <liwang@redhat.com> writes: > 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(). > > > > 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. > > > > > + > > 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? > In the next version of the patch I'll add a new test case specifically to test smart scan.
Petr Vorel <pvorel@suse.cz> writes: >> On Wed, Nov 29, 2023 at 12:51 AM Petr Vorel <pvorel@suse.cz> wrote: > >> > 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? > > >> Good catch, I looked into these tests, seems only ksm05 is debatable >> for disabling smart_scan, as a simple regression, it suggests disabling >> ksm daemon to avoid disturb according to some workload. >> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/mem/ksm/ksm05.c#L30 > >> ksm06 is definitely need disable smart_scan, it tests KSM in different >> 'run' state for merge_accros_nodes. > > Thanks for having a look. > >> To be on the safe side, I would suggest applying the patch to all ksm* >> tests, >> and write a new single for smart_scan if needed. > > Agree. I vote for new single for smart_scan related test. > I'll add a new test. > Kind regards, > Petr
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"); + 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"); final_group_check(0, 0, 0, 0, 0, 0, size * pages * num); while (waitpid(-1, &status, 0) > 0)
This disables the "smart scan" KSM feature to make sure that the volatile count remains at 0. Signed-off-by: Stefan Roesch <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(+) base-commit: 8c89ef3d451087ed6e18750bd5eedd10e5ab3d2e