On Fri, Apr 24, 2026 at 12:00:55PM +0800, Li Wang wrote: > The cgroup utility code defines a local PAGE_SIZE macro hardcoded to > 4096, which is used primarily as a generic buffer size for reading cgroup > and proc files. This naming is misleading because the value has nothing > to do with the actual page size of the system. On architectures with larger > pages (e.g., 64K on arm64 or ppc64), the name suggests a relationship that > does not exist. Additionally, the name can shadow or conflict with PAGE_SIZE > definitions from system headers, leading to confusion or subtle bugs. > > To resolve this, rename the macro to BUF_SIZE to accurately reflect its > purpose as a general I/O buffer size. > > Furthermore, test_memcontrol currently relies on this hardcoded 4K value > to stride through memory and trigger page faults. Update this logic to > use the actual system page size dynamically. This micro-optimizes the > memory faulting process by ensuring it iterates correctly and efficiently > based on the underlying architecture's true page size. (This part from Waiman) > > Signed-off-by: Li Wang > Signed-off-by: Waiman Long > Cc: Johannes Weiner > Cc: Michal Hocko > Cc: Michal Koutný > Cc: Muchun Song > Cc: Nhat Pham > Cc: Tejun Heo > Cc: Roman Gushchin > Cc: Shakeel Butt > Cc: Yosry Ahmed > Acked-by: Nhat Pham > --- > .../selftests/cgroup/lib/cgroup_util.c | 18 +++++++++--------- > .../cgroup/lib/include/cgroup_util.h | 4 ++-- > tools/testing/selftests/cgroup/test_core.c | 2 +- > tools/testing/selftests/cgroup/test_freezer.c | 2 +- > .../selftests/cgroup/test_memcontrol.c | 19 ++++++++++++------- > 5 files changed, 25 insertions(+), 20 deletions(-) > > diff --git a/tools/testing/selftests/cgroup/lib/cgroup_util.c b/tools/testing/selftests/cgroup/lib/cgroup_util.c > index 6a7295347e9..9be8ac25657 100644 > --- a/tools/testing/selftests/cgroup/lib/cgroup_util.c > +++ b/tools/testing/selftests/cgroup/lib/cgroup_util.c > @@ -140,7 +140,7 @@ int cg_read_strcmp_wait(const char *cgroup, const char *control, > > int cg_read_strstr(const char *cgroup, const char *control, const char *needle) > { > - char buf[PAGE_SIZE]; > + char buf[BUF_SIZE]; > > if (cg_read(cgroup, control, buf, sizeof(buf))) > return -1; > @@ -170,7 +170,7 @@ long cg_read_long_fd(int fd) > > long cg_read_key_long(const char *cgroup, const char *control, const char *key) > { > - char buf[PAGE_SIZE]; > + char buf[BUF_SIZE]; > char *ptr; > > if (cg_read(cgroup, control, buf, sizeof(buf))) > @@ -206,7 +206,7 @@ long cg_read_key_long_poll(const char *cgroup, const char *control, > > long cg_read_lc(const char *cgroup, const char *control) > { > - char buf[PAGE_SIZE]; > + char buf[BUF_SIZE]; > const char delim[] = "\n"; > char *line; > long cnt = 0; > @@ -258,7 +258,7 @@ int cg_write_numeric(const char *cgroup, const char *control, long value) > static int cg_find_root(char *root, size_t len, const char *controller, > bool *nsdelegate) > { > - char buf[10 * PAGE_SIZE]; > + char buf[10 * BUF_SIZE]; > char *fs, *mount, *type, *options; > const char delim[] = "\n\t "; > > @@ -313,7 +313,7 @@ int cg_create(const char *cgroup) > > int cg_wait_for_proc_count(const char *cgroup, int count) > { > - char buf[10 * PAGE_SIZE] = {0}; > + char buf[10 * BUF_SIZE] = {0}; > int attempts; > char *ptr; > > @@ -338,7 +338,7 @@ int cg_wait_for_proc_count(const char *cgroup, int count) > > int cg_killall(const char *cgroup) > { > - char buf[PAGE_SIZE]; > + char buf[BUF_SIZE]; > char *ptr = buf; > > /* If cgroup.kill exists use it. */ > @@ -548,7 +548,7 @@ int cg_run_nowait(const char *cgroup, > > int proc_mount_contains(const char *option) > { > - char buf[4 * PAGE_SIZE]; > + char buf[4 * BUF_SIZE]; > ssize_t read; > > read = read_text("/proc/mounts", buf, sizeof(buf)); > @@ -560,7 +560,7 @@ int proc_mount_contains(const char *option) > > int cgroup_feature(const char *feature) > { > - char buf[PAGE_SIZE]; > + char buf[BUF_SIZE]; > ssize_t read; > > read = read_text("/sys/kernel/cgroup/features", buf, sizeof(buf)); > @@ -587,7 +587,7 @@ ssize_t proc_read_text(int pid, bool thread, const char *item, char *buf, size_t > > int proc_read_strstr(int pid, bool thread, const char *item, const char *needle) > { > - char buf[PAGE_SIZE]; > + char buf[BUF_SIZE]; > > if (proc_read_text(pid, thread, item, buf, sizeof(buf)) < 0) > return -1; > diff --git a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h > index 567b1082974..febc1723d09 100644 > --- a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h > +++ b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h > @@ -2,8 +2,8 @@ > #include > #include > > -#ifndef PAGE_SIZE > -#define PAGE_SIZE 4096 > +#ifndef BUF_SIZE > +#define BUF_SIZE 4096 > #endif I wouldn't preserve any previously defined BUF_SIZE here (as opposed to possible more conventional PAGE_SIZE value). I.e. -#ifndef PAGE_SIZE -#define PAGE_SIZE 4096 -#endif +#define BUF_SIZE 4096 But it's nothing substantial. > > #define MB(x) (x << 20) > diff --git a/tools/testing/selftests/cgroup/test_core.c b/tools/testing/selftests/cgroup/test_core.c > index 7b83c7e7c9d..88ca832d4fc 100644 > --- a/tools/testing/selftests/cgroup/test_core.c > +++ b/tools/testing/selftests/cgroup/test_core.c > @@ -87,7 +87,7 @@ static int test_cgcore_destroy(const char *root) > int ret = KSFT_FAIL; > char *cg_test = NULL; > int child_pid; > - char buf[PAGE_SIZE]; > + char buf[BUF_SIZE]; > > cg_test = cg_name(root, "cg_test"); > > diff --git a/tools/testing/selftests/cgroup/test_freezer.c b/tools/testing/selftests/cgroup/test_freezer.c > index 97fae92c838..160a9e6ad27 100644 > --- a/tools/testing/selftests/cgroup/test_freezer.c > +++ b/tools/testing/selftests/cgroup/test_freezer.c > @@ -642,7 +642,7 @@ static int test_cgfreezer_ptrace(const char *root) > */ > static int proc_check_stopped(int pid) > { > - char buf[PAGE_SIZE]; > + char buf[BUF_SIZE]; > int len; > > len = proc_read_text(pid, 0, "stat", buf, sizeof(buf)); > diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c > index b43da9bc20c..44338dbaee8 100644 > --- a/tools/testing/selftests/cgroup/test_memcontrol.c > +++ b/tools/testing/selftests/cgroup/test_memcontrol.c > @@ -26,6 +26,7 @@ > > static bool has_localevents; > static bool has_recursiveprot; > +static int page_size; > > int get_temp_fd(void) > { > @@ -34,7 +35,7 @@ int get_temp_fd(void) > > int alloc_pagecache(int fd, size_t size) > { > - char buf[PAGE_SIZE]; > + char buf[BUF_SIZE]; This buffer is actually used as the stride, so keeping it page-sized is more sensible. > struct stat st; > int i; >