On Fri, Apr 24, 2026 at 12:00:55PM +0800, Li Wang <[email protected]> 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 <[email protected]> > Signed-off-by: Waiman Long <[email protected]> > Cc: Johannes Weiner <[email protected]> > Cc: Michal Hocko <[email protected]> > Cc: Michal Koutný <[email protected]> > Cc: Muchun Song <[email protected]> > Cc: Nhat Pham <[email protected]> > Cc: Tejun Heo <[email protected]> > Cc: Roman Gushchin <[email protected]> > Cc: Shakeel Butt <[email protected]> > Cc: Yosry Ahmed <[email protected]> > Acked-by: Nhat Pham <[email protected]> > --- > .../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 <stdbool.h> > #include <stdlib.h> > > -#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;
>
signature.asc
Description: PGP signature
