<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 26, 2020 at 8:16 PM Richard Palethorpe <<a href="mailto:rpalethorpe@suse.com" target="_blank">rpalethorpe@suse.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">When memcg.limit_in_bytes is set to PASS_THRESHOLD it's unlikely<br>
swapcached will increase by more than PASS_THRESHOLD unless processes<br>
in other memcgs are also increasing it. Additionally MADV_WILLNEED<br>
must remove pages from memory as it adds more so that the first page<br>
may not be in memory by the time the last page is faulted if the<br>
amount exceeds the memory limit (which it does because CHUNK_SZ ><br>
PASS_THRESSHOLD). Worse if pages are faulted in a non-linear way, or<br>
the process must access some other pages, then there is no guarantee<br>
which parts of the range will be resident in memory. This results in<br>
spurious test failures.<br>
<br>
To solve this we can set PASS_THRESHOLD to 1/4 of CHUNK_SZ and<br>
memcg.limit_in_bytes to 1/2 of CHUNK_SZ (MEM_LIMIT), then mark<br>
MEM_LIMIT bytes as needed. That way the amount in the SwapCache will<br>
easily be more than the threshold. Secondly we can run madvise again<br>
on PASS_THRESHOLD bytes and check that dirtying all of these does not<br>
result in too many page faults. We also run the second test on every<br>
occasion to ensure the test code itself is still valid. If the<br>
original bug is present then both tests fail.<br>
<br>
Finally this prints more diagnostic information to help with debugging<br>
the test.<br>
<br>
While debugging the test a kernel bug was found in 5.9 which effects<br>
CGroupV1 when use_hierarchy=0. This is unlikely to effect many users,<br>
but a fix is pending and will be referenced in the test when<br>
available. It is recommended that you set use_hierarchy=1.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Great, we could add the commit info as well after patch merging in the mainline kernel. </div></div><div class="gmail_default" style="font-size:small"><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Signed-off-by: Richard Palethorpe <<a href="mailto:rpalethorpe@suse.com" target="_blank">rpalethorpe@suse.com</a>><br></blockquote><div><span class="gmail_default" style="font-size:small">Reviewed-by: Li Wang <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>></span></div><div><span class="gmail_default" style="font-size:small"></span> </div><div><div class="gmail_default" style="font-size:small">This improvement makes sense to me apart from a tiny syntax error below.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">One additional comment, I found this test now only run with CGroupV1, </div><div class="gmail_default" style="font-size:small">and maybe we could make use of the LTP-cgroup new library after we </div><div class="gmail_default" style="font-size:small">updating that(tst_cgroup.c) to make it works well with CGroupV2.</div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
---<br>
testcases/kernel/syscalls/madvise/madvise06.c | 107 ++++++++++++++----<br>
1 file changed, 84 insertions(+), 23 deletions(-)<br>
<br>
diff --git a/testcases/kernel/syscalls/madvise/madvise06.c b/testcases/kernel/syscalls/madvise/madvise06.c<br>
index f76f3f6aa..3e70da37e 100644<br>
--- a/testcases/kernel/syscalls/madvise/madvise06.c<br>
+++ b/testcases/kernel/syscalls/madvise/madvise06.c<br>
@@ -19,6 +19,23 @@<br>
* Date: Thu May 22 11:54:17 2014 -0700<br>
*<br>
* mm: madvise: fix MADV_WILLNEED on shmem swapouts<br>
+ *<br>
+ * Two checks are performed, the first looks at how SwapCache<br>
+ * changes during madvise. When the pages are dirtied, about half<br>
+ * will be accounted for under Cached and the other half will be<br>
+ * moved into Swap. When madvise is run it will cause the pages<br>
+ * under Cached to also be moved to Swap while rotating the pages<br>
+ * already in Swap into SwapCached. So we expect that SwapCached has<br>
+ * roughly MEM_LIMIT bytes added to it, but for reliability the<br>
+ * PASS_THRESHOLD is much lower than that.<br>
+ *<br>
+ * Secondly we run madvise again, but only on the first<br>
+ * PASS_THRESHOLD bytes to ensure these are entirely in RAM. Then we<br>
+ * dirty these pages and check there were (almost) no page<br>
+ * faults. Two faults are allowed incase some tasklet or something<br>
+ * else unexpected, but irrelevant procedure, registers a fault to<br>
+ * our process.<br>
+ *<br>
*/<br>
<br>
#include <errno.h><br>
@@ -28,8 +45,10 @@<br>
#include "tst_test.h"<br>
<br>
#define CHUNK_SZ (400*1024*1024L)<br>
-#define CHUNK_PAGES (CHUNK_SZ / pg_sz)<br>
+#define MEM_LIMIT (CHUNK_SZ / 2)<br>
+#define MEMSW_LIMIT (2 * CHUNK_SZ)<br>
#define PASS_THRESHOLD (CHUNK_SZ / 4)<br>
+#define PASS_THRESHOLD_KB (PASS_THRESHOLD / 1024)<br>
<br>
#define MNT_NAME "memory"<br>
#define GROUP_NAME "madvise06"<br>
@@ -37,12 +56,39 @@<br>
static const char drop_caches_fname[] = "/proc/sys/vm/drop_caches";<br>
static int pg_sz;<br>
<br>
+static long init_swap, init_swap_cached, init_cached;<br>
+<br>
static void check_path(const char *path)<br>
{<br>
if (access(path, R_OK | W_OK))<br>
tst_brk(TCONF, "file needed: %s\n", path);<br>
}<br>
<br>
+#define READ_CGMEM(item) \<br>
+ ({long tst_rval; \<br>
+ SAFE_FILE_LINES_SCANF(MNT_NAME"/"GROUP_NAME"/memory."item, \<br>
+ "%ld", \<br>
+ &tst_rval); \<br>
+ tst_rval;})<br>
+<br>
+static void meminfo_diag(const char *point)<br>
+{<br>
+ FILE_PRINTF("/proc/sys/vm/stat_refresh", "1");<br>
+ tst_res(TINFO, point);<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Here is a syntax error, to fix it as:</div><div class="gmail_default" style="font-size:small"> tst_res(TINFO, "%s", point);</div></div><div> </div></div>-- <br><div dir="ltr"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>