Re: [PATCH v7 4/8] selftests/cgroup: rename PAGE_SIZE to BUF_SIZE in cgroup_util

["Michal Koutný" <mkoutny@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 7357 bytes --]

On Fri, Apr 24, 2026 at 12:00:55PM +0800, Li Wang <li.wang@linux.dev> 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 <li.wang@linux.dev>
> Signed-off-by: Waiman Long <longman@redhat.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Michal Koutný <mkoutny@suse.com>
> Cc: Muchun Song <muchun.song@linux.dev>
> Cc: Nhat Pham <nphamcs@gmail.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Roman Gushchin <roman.gushchin@linux.dev>
> Cc: Shakeel Butt <shakeel.butt@linux.dev>
> Cc: Yosry Ahmed <yosryahmed@google.com>
> Acked-by: Nhat Pham <nphamcs@gmail.com>
> ---
>  .../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;
>  

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]