| Message ID | jnyjalhg43mdnn6su2a2kmwzqasdyjsfdvipim2i2hvqo7w6y2@st57sbo4bkxf (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] selftests: cgroup: fix test_kmem_memcg_deletion kernel mem check | expand |
On Thu, Aug 17, 2023 at 08:47:26AM -0400, Lucas Karpinski wrote: > The combination of using slab, anon, file, kernel_stack, and percpu is > not accurate for total kernel memory utilization. Checking kernel within > memory.stat provides a more accurate measurement. > > Signed-off-by: Lucas Karpinski <lkarpins@redhat.com> > --- > v1: https://lore.kernel.org/all/eex2vdlg4ow2j5bybmav73nbfzuspkk4zobnk7svua4jaypqb5@7ie6e4mci43t/ > > tools/testing/selftests/cgroup/test_kmem.c | 25 +++++----------------- > 1 file changed, 5 insertions(+), 20 deletions(-) > > diff --git a/tools/testing/selftests/cgroup/test_kmem.c b/tools/testing/selftests/cgroup/test_kmem.c > index ed2e50bb1e76..3ef979ee0edf 100644 > --- a/tools/testing/selftests/cgroup/test_kmem.c > +++ b/tools/testing/selftests/cgroup/test_kmem.c > @@ -166,7 +166,7 @@ static int cg_run_in_subcgroups(const char *parent, > */ > static int test_kmem_memcg_deletion(const char *root) > { > - long current, slab, anon, file, kernel_stack, pagetables, percpu, sock, sum; > + long current, kernel; > int ret = KSFT_FAIL; > char *parent; > > @@ -184,30 +184,15 @@ static int test_kmem_memcg_deletion(const char *root) > goto cleanup; > > current = cg_read_long(parent, "memory.current"); > - slab = cg_read_key_long(parent, "memory.stat", "slab "); > - anon = cg_read_key_long(parent, "memory.stat", "anon "); > - file = cg_read_key_long(parent, "memory.stat", "file "); > - kernel_stack = cg_read_key_long(parent, "memory.stat", "kernel_stack "); > - pagetables = cg_read_key_long(parent, "memory.stat", "pagetables "); > - percpu = cg_read_key_long(parent, "memory.stat", "percpu "); > - sock = cg_read_key_long(parent, "memory.stat", "sock "); > - if (current < 0 || slab < 0 || anon < 0 || file < 0 || > - kernel_stack < 0 || pagetables < 0 || percpu < 0 || sock < 0) > + kernel = cg_read_key_long(parent, "memory.stat", "kernel "); > + if (current < 0 || kernel < 0) > goto cleanup; > > - sum = slab + anon + file + kernel_stack + pagetables + percpu + sock; > - if (abs(sum - current) < MAX_VMSTAT_ERROR) { > + if (abs(kernel - current) < MAX_VMSTAT_ERROR) { I don't think this is what you want. Since slab, kernel_stack, pagetables and percpu are included in the kmem stats, you just want to replace those with kmem. Basically keep the anon, file and sock stats.
On Thu, Aug 17, 2023 at 05:07:02PM +0000, Shakeel Butt wrote: > On Thu, Aug 17, 2023 at 08:47:26AM -0400, Lucas Karpinski wrote: > > The combination of using slab, anon, file, kernel_stack, and percpu is > > not accurate for total kernel memory utilization. Checking kernel within > > memory.stat provides a more accurate measurement. > > > > Signed-off-by: Lucas Karpinski <lkarpins@redhat.com> > > --- > > v1: https://lore.kernel.org/all/eex2vdlg4ow2j5bybmav73nbfzuspkk4zobnk7svua4jaypqb5@7ie6e4mci43t/ > > > > tools/testing/selftests/cgroup/test_kmem.c | 25 +++++----------------- > > 1 file changed, 5 insertions(+), 20 deletions(-) > > > > diff --git a/tools/testing/selftests/cgroup/test_kmem.c b/tools/testing/selftests/cgroup/test_kmem.c > > index ed2e50bb1e76..3ef979ee0edf 100644 > > --- a/tools/testing/selftests/cgroup/test_kmem.c > > +++ b/tools/testing/selftests/cgroup/test_kmem.c > > @@ -166,7 +166,7 @@ static int cg_run_in_subcgroups(const char *parent, > > */ > > static int test_kmem_memcg_deletion(const char *root) > > { > > - long current, slab, anon, file, kernel_stack, pagetables, percpu, sock, sum; > > + long current, kernel; > > int ret = KSFT_FAIL; > > char *parent; > > > > @@ -184,30 +184,15 @@ static int test_kmem_memcg_deletion(const char *root) > > goto cleanup; > > > > current = cg_read_long(parent, "memory.current"); > > - slab = cg_read_key_long(parent, "memory.stat", "slab "); > > - anon = cg_read_key_long(parent, "memory.stat", "anon "); > > - file = cg_read_key_long(parent, "memory.stat", "file "); > > - kernel_stack = cg_read_key_long(parent, "memory.stat", "kernel_stack "); > > - pagetables = cg_read_key_long(parent, "memory.stat", "pagetables "); > > - percpu = cg_read_key_long(parent, "memory.stat", "percpu "); > > - sock = cg_read_key_long(parent, "memory.stat", "sock "); > > - if (current < 0 || slab < 0 || anon < 0 || file < 0 || > > - kernel_stack < 0 || pagetables < 0 || percpu < 0 || sock < 0) > > + kernel = cg_read_key_long(parent, "memory.stat", "kernel "); > > + if (current < 0 || kernel < 0) > > goto cleanup; > > > > - sum = slab + anon + file + kernel_stack + pagetables + percpu + sock; > > - if (abs(sum - current) < MAX_VMSTAT_ERROR) { > > + if (abs(kernel - current) < MAX_VMSTAT_ERROR) { > > I don't think this is what you want. Since slab, kernel_stack, > pagetables and percpu are included in the kmem stats, you just want to > replace those with kmem. Basically keep the anon, file and sock stats. > Will fix and send new patch version. Thanks, Lucas
diff --git a/tools/testing/selftests/cgroup/test_kmem.c b/tools/testing/selftests/cgroup/test_kmem.c index ed2e50bb1e76..3ef979ee0edf 100644 --- a/tools/testing/selftests/cgroup/test_kmem.c +++ b/tools/testing/selftests/cgroup/test_kmem.c @@ -166,7 +166,7 @@ static int cg_run_in_subcgroups(const char *parent, */ static int test_kmem_memcg_deletion(const char *root) { - long current, slab, anon, file, kernel_stack, pagetables, percpu, sock, sum; + long current, kernel; int ret = KSFT_FAIL; char *parent; @@ -184,30 +184,15 @@ static int test_kmem_memcg_deletion(const char *root) goto cleanup; current = cg_read_long(parent, "memory.current"); - slab = cg_read_key_long(parent, "memory.stat", "slab "); - anon = cg_read_key_long(parent, "memory.stat", "anon "); - file = cg_read_key_long(parent, "memory.stat", "file "); - kernel_stack = cg_read_key_long(parent, "memory.stat", "kernel_stack "); - pagetables = cg_read_key_long(parent, "memory.stat", "pagetables "); - percpu = cg_read_key_long(parent, "memory.stat", "percpu "); - sock = cg_read_key_long(parent, "memory.stat", "sock "); - if (current < 0 || slab < 0 || anon < 0 || file < 0 || - kernel_stack < 0 || pagetables < 0 || percpu < 0 || sock < 0) + kernel = cg_read_key_long(parent, "memory.stat", "kernel "); + if (current < 0 || kernel < 0) goto cleanup; - sum = slab + anon + file + kernel_stack + pagetables + percpu + sock; - if (abs(sum - current) < MAX_VMSTAT_ERROR) { + if (abs(kernel - current) < MAX_VMSTAT_ERROR) { ret = KSFT_PASS; } else { printf("memory.current = %ld\n", current); - printf("slab + anon + file + kernel_stack = %ld\n", sum); - printf("slab = %ld\n", slab); - printf("anon = %ld\n", anon); - printf("file = %ld\n", file); - printf("kernel_stack = %ld\n", kernel_stack); - printf("pagetables = %ld\n", pagetables); - printf("percpu = %ld\n", percpu); - printf("sock = %ld\n", sock); + printf("kernel = %ld\n", kernel); } cleanup:
The combination of using slab, anon, file, kernel_stack, and percpu is not accurate for total kernel memory utilization. Checking kernel within memory.stat provides a more accurate measurement. Signed-off-by: Lucas Karpinski <lkarpins@redhat.com> --- v1: https://lore.kernel.org/all/eex2vdlg4ow2j5bybmav73nbfzuspkk4zobnk7svua4jaypqb5@7ie6e4mci43t/ tools/testing/selftests/cgroup/test_kmem.c | 25 +++++----------------- 1 file changed, 5 insertions(+), 20 deletions(-)