Changelog: v3: * More cleanup (patch 3) (suggested by Yosry Ahmed). * Check swap peak in swapin test v2: * Make the swapin test also checks for zswap usage (patch 3) (suggested by Yosry Ahmed) * Some test simplifications/cleanups (patch 3) (suggested by Yosry Ahmed).
Fix a broken zswap kselftest due to cgroup zswap writeback counter renaming, and add 2 zswap kselftests, one to cover the (z)swapin case, and another to check that no zswapping happens when the cgroup limit is 0.
Also, add the zswap kselftest file to zswap maintainer entry so that get_maintainers script can find zswap maintainers.
Nhat Pham (3): selftests: zswap: add zswap selftest file to zswap maintainer entry selftests: fix the zswap invasive shrink test selftests: add zswapin and no zswap tests
MAINTAINERS | 1 + tools/testing/selftests/cgroup/test_zswap.c | 122 +++++++++++++++++++- 2 files changed, 121 insertions(+), 2 deletions(-)
base-commit: 91f3daa1765ee4e0c89987dc25f72c40f07af34d
Make it easier for contributors to find the zswap maintainers when they update the zswap tests.
Signed-off-by: Nhat Pham nphamcs@gmail.com Acked-by: Yosry Ahmed yosryahmed@google.com --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS index 6527850f24d1..126090f0e418 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -24398,6 +24398,7 @@ F: include/linux/zpool.h F: include/linux/zswap.h F: mm/zpool.c F: mm/zswap.c +F: tools/testing/selftests/cgroup/test_zswap.c
THE REST M: Linus Torvalds torvalds@linux-foundation.org
The zswap no invasive shrink selftest breaks because we rename the zswap writeback counter (see [1]). Fix the test.
[1]: https://patchwork.kernel.org/project/linux-kselftest/patch/20231205193307.24...
Fixes: a697dc2be925 ("selftests: cgroup: update per-memcg zswap writeback selftest") Signed-off-by: Nhat Pham nphamcs@gmail.com Acked-by: Yosry Ahmed yosryahmed@google.com --- tools/testing/selftests/cgroup/test_zswap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c index 47fdaa146443..32ce975b21d1 100644 --- a/tools/testing/selftests/cgroup/test_zswap.c +++ b/tools/testing/selftests/cgroup/test_zswap.c @@ -52,7 +52,7 @@ static int get_zswap_stored_pages(size_t *value)
static int get_cg_wb_count(const char *cg) { - return cg_read_key_long(cg, "memory.stat", "zswp_wb"); + return cg_read_key_long(cg, "memory.stat", "zswpwb"); }
static long get_zswpout(const char *cgroup)
Add a selftest to cover the zswapin code path, allocating more memory than the cgroup limit to trigger swapout/zswapout, then reading the pages back in memory several times. This is inspired by a recently encountered kernel crash on the zswapin path in our internal kernel, which went undetected because of a lack of test coverage for this path.
Add a selftest to verify that when memory.zswap.max = 0, no pages can go to the zswap pool for the cgroup.
Suggested-by: Rik van Riel riel@surriel.com Suggested-by: Yosry Ahmed yosryahmed@google.com Signed-off-by: Nhat Pham nphamcs@gmail.com --- tools/testing/selftests/cgroup/test_zswap.c | 120 +++++++++++++++++++- 1 file changed, 119 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c index 32ce975b21d1..c263610a4a60 100644 --- a/tools/testing/selftests/cgroup/test_zswap.c +++ b/tools/testing/selftests/cgroup/test_zswap.c @@ -60,6 +60,27 @@ static long get_zswpout(const char *cgroup) return cg_read_key_long(cgroup, "memory.stat", "zswpout "); }
+static int allocate_and_read_bytes(const char *cgroup, void *arg) +{ + size_t size = (size_t)arg; + char *mem = (char *)malloc(size); + int ret = 0; + + if (!mem) + return -1; + for (int i = 0; i < size; i += 4095) + mem[i] = 'a'; + + /* go through the allocated memory to (z)swap in and out pages */ + for (int i = 0; i < size; i += 4095) { + if (mem[i] != 'a') + ret = -1; + } + + free(mem); + return ret; +} + static int allocate_bytes(const char *cgroup, void *arg) { size_t size = (size_t)arg; @@ -100,7 +121,6 @@ static int test_zswap_usage(const char *root) int ret = KSFT_FAIL; char *test_group;
- /* Set up */ test_group = cg_name(root, "no_shrink_test"); if (!test_group) goto out; @@ -133,6 +153,102 @@ static int test_zswap_usage(const char *root) return ret; }
+/* + * Check that when memory.zswap.max = 0, no pages can go to the zswap pool for + * the cgroup. + */ +static int test_swapin_nozswap(const char *root) +{ + int ret = KSFT_FAIL; + char *test_group; + long swap_peak, zswpout; + + test_group = cg_name(root, "no_zswap_test"); + if (!test_group) + goto out; + if (cg_create(test_group)) + goto out; + if (cg_write(test_group, "memory.max", "8M")) + goto out; + if (cg_write(test_group, "memory.zswap.max", "0")) + goto out; + + /* Allocate and read more than memory.max to trigger swapin */ + if (cg_run(test_group, allocate_and_read_bytes, (void *)MB(32))) + goto out; + + /* Verify that pages are swapped out, but no zswap happened */ + swap_peak = cg_read_long(test_group, "memory.swap.peak"); + if (swap_peak < 0) { + ksft_print_msg("failed to get cgroup's swap_peak\n"); + goto out; + } + + if (swap_peak == 0) { + ksft_print_msg("pages should be swapped out\n"); + goto out; + } + + zswpout = get_zswpout(test_group); + if (zswpout < 0) { + ksft_print_msg("failed to get zswpout\n"); + goto out; + } + + if (zswpout > 0) { + ksft_print_msg("zswapout > 0 when memory.zswap.max = 0\n"); + goto out; + } + + ret = KSFT_PASS; + +out: + cg_destroy(test_group); + free(test_group); + return ret; +} + +/* Simple test to verify the (z)swapin code paths */ +static int test_zswapin(const char *root) +{ + int ret = KSFT_FAIL; + char *test_group; + long zswpin; + + /* Set up */ + test_group = cg_name(root, "zswapin_test"); + if (!test_group) + goto out; + if (cg_create(test_group)) + goto out; + if (cg_write(test_group, "memory.max", "8M")) + goto out; + if (cg_write(test_group, "memory.zswap.max", "max")) + goto out; + + /* Allocate and read more than memory.max to trigger (z)swap in */ + if (cg_run(test_group, allocate_and_read_bytes, (void *)MB(32))) + goto out; + + zswpin = cg_read_key_long(test_group, "memory.stat", "zswpin "); + if (zswpin < 0) { + ksft_print_msg("failed to get zswpin\n"); + goto out; + } + + if (zswpin == 0) { + ksft_print_msg("zswpin should not be 0\n"); + goto out; + } + + ret = KSFT_PASS; + +out: + cg_destroy(test_group); + free(test_group); + return ret; +} + /* * When trying to store a memcg page in zswap, if the memcg hits its memory * limit in zswap, writeback should affect only the zswapped pages of that @@ -309,6 +425,8 @@ struct zswap_test { const char *name; } tests[] = { T(test_zswap_usage), + T(test_swapin_nozswap), + T(test_zswapin), T(test_no_kmem_bypass), T(test_no_invasive_cgroup_shrink), };
On Mon, Feb 05, 2024 at 02:56:08PM -0800, Nhat Pham wrote:
Add a selftest to cover the zswapin code path, allocating more memory than the cgroup limit to trigger swapout/zswapout, then reading the pages back in memory several times. This is inspired by a recently encountered kernel crash on the zswapin path in our internal kernel, which went undetected because of a lack of test coverage for this path.
Add a selftest to verify that when memory.zswap.max = 0, no pages can go to the zswap pool for the cgroup.
Suggested-by: Rik van Riel riel@surriel.com Suggested-by: Yosry Ahmed yosryahmed@google.com Signed-off-by: Nhat Pham nphamcs@gmail.com
LGTM with a few nits below: Acked-by: Yosry Ahmed yosryahmed@google.com
Thanks!
tools/testing/selftests/cgroup/test_zswap.c | 120 +++++++++++++++++++- 1 file changed, 119 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c index 32ce975b21d1..c263610a4a60 100644 --- a/tools/testing/selftests/cgroup/test_zswap.c +++ b/tools/testing/selftests/cgroup/test_zswap.c @@ -60,6 +60,27 @@ static long get_zswpout(const char *cgroup) return cg_read_key_long(cgroup, "memory.stat", "zswpout "); } +static int allocate_and_read_bytes(const char *cgroup, void *arg) +{
- size_t size = (size_t)arg;
- char *mem = (char *)malloc(size);
- int ret = 0;
- if (!mem)
return -1;- for (int i = 0; i < size; i += 4095)
mem[i] = 'a';- /* go through the allocated memory to (z)swap in and out pages */
nit: s/go/Go
- for (int i = 0; i < size; i += 4095) {
if (mem[i] != 'a')ret = -1;- }
- free(mem);
- return ret;
+}
static int allocate_bytes(const char *cgroup, void *arg) { size_t size = (size_t)arg; @@ -100,7 +121,6 @@ static int test_zswap_usage(const char *root) int ret = KSFT_FAIL; char *test_group;
- /* Set up */
We removed this comment here.
test_group = cg_name(root, "no_shrink_test"); if (!test_group) goto out; @@ -133,6 +153,102 @@ static int test_zswap_usage(const char *root) return ret; } +/*
- Check that when memory.zswap.max = 0, no pages can go to the zswap pool for
- the cgroup.
- */
+static int test_swapin_nozswap(const char *root) +{
- int ret = KSFT_FAIL;
- char *test_group;
- long swap_peak, zswpout;
- test_group = cg_name(root, "no_zswap_test");
- if (!test_group)
goto out;- if (cg_create(test_group))
goto out;- if (cg_write(test_group, "memory.max", "8M"))
goto out;- if (cg_write(test_group, "memory.zswap.max", "0"))
goto out;- /* Allocate and read more than memory.max to trigger swapin */
- if (cg_run(test_group, allocate_and_read_bytes, (void *)MB(32)))
goto out;- /* Verify that pages are swapped out, but no zswap happened */
- swap_peak = cg_read_long(test_group, "memory.swap.peak");
- if (swap_peak < 0) {
ksft_print_msg("failed to get cgroup's swap_peak\n");goto out;- }
- if (swap_peak == 0) {
ksft_print_msg("pages should be swapped out\n");goto out;- }
We can actually check that this number is >= 24M instead. Not a big deal, but might as well.
- zswpout = get_zswpout(test_group);
- if (zswpout < 0) {
ksft_print_msg("failed to get zswpout\n");goto out;- }
- if (zswpout > 0) {
ksft_print_msg("zswapout > 0 when memory.zswap.max = 0\n");goto out;- }
- ret = KSFT_PASS;
+out:
- cg_destroy(test_group);
- free(test_group);
- return ret;
+}
+/* Simple test to verify the (z)swapin code paths */ +static int test_zswapin(const char *root) +{
- int ret = KSFT_FAIL;
- char *test_group;
- long zswpin;
- /* Set up */
Yet we added a similar one here :)
- test_group = cg_name(root, "zswapin_test");
- if (!test_group)
goto out;- if (cg_create(test_group))
goto out;- if (cg_write(test_group, "memory.max", "8M"))
goto out;- if (cg_write(test_group, "memory.zswap.max", "max"))
goto out;- /* Allocate and read more than memory.max to trigger (z)swap in */
- if (cg_run(test_group, allocate_and_read_bytes, (void *)MB(32)))
goto out;- zswpin = cg_read_key_long(test_group, "memory.stat", "zswpin ");
- if (zswpin < 0) {
ksft_print_msg("failed to get zswpin\n");goto out;- }
- if (zswpin == 0) {
ksft_print_msg("zswpin should not be 0\n");goto out;- }
Same here, we can check that zswpin is at least 24M worth of events. Again, not a big deal, but might as well.
On Mon, Feb 5, 2024 at 3:05 PM Yosry Ahmed yosryahmed@google.com wrote:
On Mon, Feb 05, 2024 at 02:56:08PM -0800, Nhat Pham wrote:
Add a selftest to cover the zswapin code path, allocating more memory than the cgroup limit to trigger swapout/zswapout, then reading the pages back in memory several times. This is inspired by a recently encountered kernel crash on the zswapin path in our internal kernel, which went undetected because of a lack of test coverage for this path.
Add a selftest to verify that when memory.zswap.max = 0, no pages can go to the zswap pool for the cgroup.
Suggested-by: Rik van Riel riel@surriel.com Suggested-by: Yosry Ahmed yosryahmed@google.com Signed-off-by: Nhat Pham nphamcs@gmail.com
LGTM with a few nits below: Acked-by: Yosry Ahmed yosryahmed@google.com
Thanks!
tools/testing/selftests/cgroup/test_zswap.c | 120 +++++++++++++++++++- 1 file changed, 119 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c index 32ce975b21d1..c263610a4a60 100644 --- a/tools/testing/selftests/cgroup/test_zswap.c +++ b/tools/testing/selftests/cgroup/test_zswap.c @@ -60,6 +60,27 @@ static long get_zswpout(const char *cgroup) return cg_read_key_long(cgroup, "memory.stat", "zswpout "); }
+static int allocate_and_read_bytes(const char *cgroup, void *arg) +{
size_t size = (size_t)arg;char *mem = (char *)malloc(size);int ret = 0;if (!mem)return -1;for (int i = 0; i < size; i += 4095)mem[i] = 'a';/* go through the allocated memory to (z)swap in and out pages */nit: s/go/Go
for (int i = 0; i < size; i += 4095) {if (mem[i] != 'a')ret = -1;}free(mem);return ret;+}
static int allocate_bytes(const char *cgroup, void *arg) { size_t size = (size_t)arg; @@ -100,7 +121,6 @@ static int test_zswap_usage(const char *root) int ret = KSFT_FAIL; char *test_group;
/* Set up */We removed this comment here.
test_group = cg_name(root, "no_shrink_test"); if (!test_group) goto out;@@ -133,6 +153,102 @@ static int test_zswap_usage(const char *root) return ret; }
+/*
- Check that when memory.zswap.max = 0, no pages can go to the zswap pool for
- the cgroup.
- */
+static int test_swapin_nozswap(const char *root) +{
int ret = KSFT_FAIL;char *test_group;long swap_peak, zswpout;test_group = cg_name(root, "no_zswap_test");if (!test_group)goto out;if (cg_create(test_group))goto out;if (cg_write(test_group, "memory.max", "8M"))goto out;if (cg_write(test_group, "memory.zswap.max", "0"))goto out;/* Allocate and read more than memory.max to trigger swapin */if (cg_run(test_group, allocate_and_read_bytes, (void *)MB(32)))goto out;/* Verify that pages are swapped out, but no zswap happened */swap_peak = cg_read_long(test_group, "memory.swap.peak");if (swap_peak < 0) {ksft_print_msg("failed to get cgroup's swap_peak\n");goto out;}if (swap_peak == 0) {ksft_print_msg("pages should be swapped out\n");goto out;}We can actually check that this number is >= 24M instead. Not a big deal, but might as well.
zswpout = get_zswpout(test_group);if (zswpout < 0) {ksft_print_msg("failed to get zswpout\n");goto out;}if (zswpout > 0) {ksft_print_msg("zswapout > 0 when memory.zswap.max = 0\n");goto out;}ret = KSFT_PASS;+out:
cg_destroy(test_group);free(test_group);return ret;+}
+/* Simple test to verify the (z)swapin code paths */ +static int test_zswapin(const char *root) +{
int ret = KSFT_FAIL;char *test_group;long zswpin;/* Set up */Yet we added a similar one here :)
Urgh I am stupid :) I meant to remove this, but accidentally removed it from the old test instead :)
test_group = cg_name(root, "zswapin_test");if (!test_group)goto out;if (cg_create(test_group))goto out;if (cg_write(test_group, "memory.max", "8M"))goto out;if (cg_write(test_group, "memory.zswap.max", "max"))goto out;/* Allocate and read more than memory.max to trigger (z)swap in */if (cg_run(test_group, allocate_and_read_bytes, (void *)MB(32)))goto out;zswpin = cg_read_key_long(test_group, "memory.stat", "zswpin ");if (zswpin < 0) {ksft_print_msg("failed to get zswpin\n");goto out;}if (zswpin == 0) {ksft_print_msg("zswpin should not be 0\n");goto out;}Same here, we can check that zswpin is at least 24M worth of events. Again, not a big deal, but might as well.
Remove redundant "set up" comment and add check to ensure enough data is swapped out (in swapin test) and zswapped-in.
Suggested-by: Yosry Ahmed yosryahmed@google.com Signed-off-by: Nhat Pham nphamcs@gmail.com --- tools/testing/selftests/cgroup/test_zswap.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c index c263610a4a60..f0e488ed90d8 100644 --- a/tools/testing/selftests/cgroup/test_zswap.c +++ b/tools/testing/selftests/cgroup/test_zswap.c @@ -71,7 +71,7 @@ static int allocate_and_read_bytes(const char *cgroup, void *arg) for (int i = 0; i < size; i += 4095) mem[i] = 'a';
- /* go through the allocated memory to (z)swap in and out pages */ + /* Go through the allocated memory to (z)swap in and out pages */ for (int i = 0; i < size; i += 4095) { if (mem[i] != 'a') ret = -1; @@ -184,8 +184,8 @@ static int test_swapin_nozswap(const char *root) goto out; }
- if (swap_peak == 0) { - ksft_print_msg("pages should be swapped out\n"); + if (swap_peak < MB(24)) { + ksft_print_msg("at least 24MB of memory should be swapped out\n"); goto out; }
@@ -215,7 +215,6 @@ static int test_zswapin(const char *root) char *test_group; long zswpin;
- /* Set up */ test_group = cg_name(root, "zswapin_test"); if (!test_group) goto out; @@ -236,8 +235,8 @@ static int test_zswapin(const char *root) goto out; }
- if (zswpin == 0) { - ksft_print_msg("zswpin should not be 0\n"); + if (zswpin < MB(24) / PAGE_SIZE) { + ksft_print_msg("at least 24MB should be brought back from zswap\n"); goto out; }
@@ -260,7 +259,6 @@ static int test_no_invasive_cgroup_shrink(const char *root) size_t control_allocation_size = MB(10); char *control_allocation, *wb_group = NULL, *control_group = NULL;
- /* Set up */ wb_group = setup_test_group_1M(root, "per_memcg_wb_test1"); if (!wb_group) return KSFT_FAIL;
base-commit: 9d193b36872d153e02e80c26203de4ee15127b58 -- 2.40.1
linux-kselftest-mirror@lists.linaro.org
-
Nhat Pham -
Yosry Ahmed