Skip to contentSkip to conversation

selftests/cgroup: Fix false positive failures in test_percpu_basic

PATCH Applied Li Wang 2 participants 4 messages May 1 – May 6

Subsystems (4)
lore
Share & tools
lore

Li Wang

author
#0 lore raw

v2 cover selftests/cgroup: Fix false positive failures in test_percpu_basic

This patch series addresses two separate issues that cause false positive failures in the test_percpu_basic test within the cgroup kmem selftests.

The first issue stems from a hardcoded assumption about the system page size, which breaks the test on architectures with larger page sizes.

The second issue is an overly strict memory check that fails to account for the slab metadata allocated during cgroup creation.

v2:
* Replace my email with [email protected]
* Adding Waiman's Reviewed-by tag

Li Wang (2):
selftests/cgroup: Fix hardcoded page size in test_percpu_basic selftests/cgroup: include slab in test_percpu_basic memory check

 tools/testing/selftests/cgroup/test_kmem.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
Signature · 1 line
2.54.0
author
#1 lore raw

PATCH v2 1/2 selftests/cgroup: Fix hardcoded page size in test_percpu_basic

MAX_VMSTAT_ERROR uses a hardcoded page size of 4096, which assumes 4K pages. This causes test_percpu_basic to fail on systems where the kernel is configured with a larger page size, such as aarch64 systems using 16K or 64K pages, where the maximum permissible discrepancy between memory.current and percpu charges is proportionally larger.

Replace the hardcoded 4096 with sysconf(_SC_PAGESIZE) to correctly derive the page size at runtime regardless of the underlying architecture or kernel configuration.

Cc Waiman Long <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Shakeel Butt <[email protected]>
Cc Tejun Heo <[email protected]>
Cc Vlastimil Babka <[email protected]>
---
 tools/testing/selftests/cgroup/test_kmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/testing/selftests/cgroup/test_kmem.c +1 -1
index eeabd34bf08..249d7911306 100644
@@ -24,7 +24,7 @@
  * the maximum discrepancy between charge and vmstat entries is number
  * of cpus multiplied by 64 pages.
  */
-#define MAX_VMSTAT_ERROR (4096 * 64 * get_nprocs())
+#define MAX_VMSTAT_ERROR (sysconf(_SC_PAGESIZE) * 64 * get_nprocs())
 
 #define KMEM_DEAD_WAIT_RETRIES        80
 
Signature · 1 line
2.54.0
Reviewed-by on v2 1/2
#2 lore raw

Re: [PATCH v2 1/2] selftests/cgroup: Fix hardcoded page size in test_percpu_basic

Quoted 29 lines · Li Wang

MAX_VMSTAT_ERROR uses a hardcoded page size of 4096, which assumes 4K pages. This causes test_percpu_basic to fail on systems where the kernel is configured with a larger page size, such as aarch64 systems using 16K or 64K pages, where the maximum permissible discrepancy between memory.current and percpu charges is proportionally larger.

Replace the hardcoded 4096 with sysconf(_SC_PAGESIZE) to correctly derive the page size at runtime regardless of the underlying architecture or kernel configuration.

Cc Waiman Long <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Shakeel Butt <[email protected]>
Cc Tejun Heo <[email protected]>
Cc Vlastimil Babka <[email protected]>
---
  tools/testing/selftests/cgroup/test_kmem.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
tools/testing/selftests/cgroup/test_kmem.c +0 -0
index eeabd34bf08..249d7911306 100644

tools/testing/selftests/cgroup/test_kmem.c line 30

Quoted 8 lines
tools/testing/selftests/cgroup/test_kmem.c +1 -1
@@ -24,7 +24,7 @@
   * the maximum discrepancy between charge and vmstat entries is number
   * of cpus multiplied by 64 pages.
   */
-#define MAX_VMSTAT_ERROR (4096 * 64 * get_nprocs())
+#define MAX_VMSTAT_ERROR (sysconf(_SC_PAGESIZE) * 64 * get_nprocs())
 
  #define KMEM_DEAD_WAIT_RETRIES        80
 

Thanks,
Sayali

author
#3 lore raw

PATCH v2 2/2 selftests/cgroup: include slab in test_percpu_basic memory check

In reply to Li Wang

test_percpu_basic() currently compares memory.current against only memory.stat:percpu after creating 1000 child cgroups.

Observed failure:
#./test_kmem
ok 1 test_kmem_basic
ok 2 test_kmem_memcg_deletion
ok 3 test_kmem_proc_kpagecgroup
ok 4 test_kmem_kernel_stacks
ok 5 test_kmem_dead_cgroups
memory.current 11530240
percpu 8440000
not ok 6 test_percpu_basic

That assumption is too strict: child cgroup creation also allocates slab-backed metadata, so memory.current is expected to be larger than percpu alone. One visible path is:

cgroup_mkdir()

    cgroup_create()
      cgroup_addrm_file()
        cgroup_add_file()
          __kernfs_create_file()
            __kernfs_new_node()
              kmem_cache_zalloc()

These kernfs allocations are charged as slab and show up in memory.stat:slab.

Update the check to compare memory.current against (percpu + slab) within MAX_VMSTAT_ERROR, and print slab/delta in the failure message to improve diagnostics.

Cc Waiman Long <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Shakeel Butt <[email protected]>
Cc Tejun Heo <[email protected]>
Cc Vlastimil Babka <[email protected]>
---
 tools/testing/selftests/cgroup/test_kmem.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
tools/testing/selftests/cgroup/test_kmem.c +6 -5
index 249d7911306..4b579969889 100644
@@ -353,7 +353,7 @@ static int test_percpu_basic(const char *root)
 {
 	int ret = KSFT_FAIL;
 	char *parent, *child;
-	long current, percpu;
+	long current, percpu, slab;
 	int i;
 
 	parent = cg_name(root, "percpu_basic_test");
@@ -379,13 +379,14 @@ static int test_percpu_basic(const char *root)
 
 	current = cg_read_long(parent, "memory.current");
 	percpu = cg_read_key_long(parent, "memory.stat", "percpu ");
+	slab = cg_read_key_long(parent, "memory.stat", "slab ");
 
-	if (current > 0 && percpu > 0 && labs(current - percpu) <
-	    MAX_VMSTAT_ERROR)
+	if (current > 0 && percpu > 0 && slab >= 0 &&
+			labs(current - (percpu + slab)) < MAX_VMSTAT_ERROR)
 		ret = KSFT_PASS;
 	else
-		printf("memory.current %ld\npercpu %ld\n",
-		       current, percpu);
+		printf("memory.current %ld\npercpu %ld\nslab %ld\ndelta %ld\n",
+			current, percpu, slab, current - (percpu + slab));
 
 cleanup_children:
 	for (i = 0; i < 1000; i++) {
Signature · 1 line
2.54.0