diff mbox series

[1/1] ksm: fix occasional page_volatile false positives

Message ID 20220512013830.8534-2-aarcange@redhat.com
State Accepted
Headers show
Series ksm: fix occasional page_volatile false positives | expand

Commit Message

Andrea Arcangeli May 12, 2022, 1:38 a.m. UTC 
The KSM scan keeps running while checking the KSM status in sysfs, but
during the KSM scan the the rmap_items in the stale unstable tree of
the old pass are removed from it and are later reinserted in the new
unstable tree of the current pass, leading to some page_volatile
false positives.

The fix is stop the KSM scan temporarily while the KSM status is being
read from sysfs.

For reference here's an instance of the fixed false positives:

mem.c:255: TFAIL: pages_volatile is not 0 but 1.
mem.c:255: TFAIL: pages_unshared is not 1 but 0.

Reported-by: Eirik Fuller <efuller@redhat.com>
Co-developed-by: Li Wang <liwan@redhat.com>
Tested-by: Li Wang <liwan@redhat.com>
Tested-by: Eirik Fuller <efuller@redhat.com>
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
---
 testcases/kernel/mem/lib/mem.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Li Wang May 13, 2022, 1:51 a.m. UTC | #1 
Andrea Arcangeli <aarcange@redhat.com> wrote:

The KSM scan keeps running while checking the KSM status in sysfs, but
> during the KSM scan the the rmap_items in the stale unstable tree of
> the old pass are removed from it and are later reinserted in the new
> unstable tree of the current pass, leading to some page_volatile
> false positives.
>
> The fix is stop the KSM scan temporarily while the KSM status is being
> read from sysfs.
>
> For reference here's an instance of the fixed false positives:
>
> mem.c:255: TFAIL: pages_volatile is not 0 but 1.
> mem.c:255: TFAIL: pages_unshared is not 1 but 0.
>
> Reported-by: Eirik Fuller <efuller@redhat.com>
> Co-developed-by: Li Wang <liwan@redhat.com>
> Tested-by: Li Wang <liwan@redhat.com>
>

This email address is valid but I prefer to use the my
name liwang@ for better remembering :).
(Anyway this doesn't matter, we can ignore this.)



> Tested-by: Eirik Fuller <efuller@redhat.com>
> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
>

Reviewed-by: Li Wang <liwang@redhat.com>

@Cryril, I vote for adding this patch in the new release.
Plz take this into consideration, thanks.
Cyril Hrubis May 13, 2022, 8:51 a.m. UTC | #2 
Hi!
> Reviewed-by: Li Wang <liwang@redhat.com>
> 
> @Cryril, I vote for adding this patch in the new release.
> Plz take this into consideration, thanks.

Pushed, thanks.
diff mbox series

Patch

diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index 090569ebb..8ddd7adf7 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -261,14 +261,31 @@  static void final_group_check(int run, int pages_shared, int pages_sharing,
 			  int pages_volatile, int pages_unshared,
 			  int sleep_millisecs, int pages_to_scan)
 {
+	int ksm_run_orig;
+
 	tst_res(TINFO, "check!");
 	check("run", run);
+
+	/*
+	 * Temporarily stop the KSM scan during the checks: during the
+	 * KSM scan the rmap_items in the stale unstable tree of the
+	 * old pass are removed from it and are later reinserted in
+	 * the new unstable tree of the current pass. So if the checks
+	 * run in the race window between removal and re-insertion, it
+	 * can lead to unexpected false positives where page_volatile
+	 * is elevated and page_unshared is recessed.
+	 */
+	SAFE_FILE_SCANF(PATH_KSM "run", "%d", &ksm_run_orig);
+	SAFE_FILE_PRINTF(PATH_KSM "run", "0");
+
 	check("pages_shared", pages_shared);
 	check("pages_sharing", pages_sharing);
 	check("pages_volatile", pages_volatile);
 	check("pages_unshared", pages_unshared);
 	check("sleep_millisecs", sleep_millisecs);
 	check("pages_to_scan", pages_to_scan);
+
+	SAFE_FILE_PRINTF(PATH_KSM "run", "%d", ksm_run_orig);
 }
 
 void ksm_group_check(int run, int pages_shared, int pages_sharing,