Re: [LTP] [PATCH v2] lib: Introduce tst_path.h to consolidate system paths

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) Linux Test Project, 2026
> + * Copyright (c) 2026 Li Wang <li.wang@linux.dev>
> + */
> +
> +#ifndef TST_PATH__
> +#define TST_PATH__
> +
> +/* PROC */
> +#define PROC_SYS_VM		"/proc/sys/vm/"
> +#define PROC_SYS_FS		"/proc/sys/fs/"
> +#define PROC_SYS_NET		"/proc/sys/net/"
> +#define PROC_SYS_USER		"/proc/sys/user/"
> +#define PROC_SYS_KERNEL		"/proc/sys/kernel/"
> +/* SYS */
> +#define SYS_KERNEL_MM		"/sys/kernel/mm/"

I wonder if this is really better. The macro name is not shorter and the
kernel path is not going to change since it's part of API.

> +
> +/* VM */
> +#define PATH_VM_NR_HPAGES       PROC_SYS_VM "nr_hugepages"
> +#define PATH_VM_OC_HPAGES       PROC_SYS_VM "nr_overcommit_hugepages"
> +#define PATH_VM_DROP_CACHES     PROC_SYS_VM "drop_caches"
> +#define PATH_VM_COMPACT_MEMORY  PROC_SYS_VM "compact_memory"
> +
> +/* HUGETLB */
> +#define PATH_MM_HUGEPAGES	SYS_KERNEL_MM "hugepages/"
> +#define PATH_MM_THP		SYS_KERNEL_MM "transparent_hugepage/"

These make more sense since the path is long and the macro makes it
shorter.

> +/* KSM */
> +#define PATH_MM_KSM		SYS_KERNEL_MM "ksm/"
> +#define MM_KSM_FP(s)		(PATH_MM_KSM s)
> +
> +/* NETWORK */
> +#define PATH_NET_IPV4		PROC_SYS_NET "ipv4/"
> +#define NET_IPV4_FP(s)		(PATH_NET_IPV4 s)
> +
> +/* MEMINFO */
> +#define MEMINFO_HPAGE_TOTAL	"HugePages_Total:"
> +#define MEMINFO_HPAGE_FREE	"HugePages_Free:"
> +#define MEMINFO_HPAGE_RSVD	"HugePages_Rsvd:"
> +#define MEMINFO_HPAGE_SURP	"HugePages_Surp:"
> +#define MEMINFO_HPAGE_SIZE	"Hugepagesize:"
> +
> +#endif /* TST_PATH_H */
> diff --git a/include/tst_test.h b/include/tst_test.h
> index f12c59f39..4898cfe4c 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -26,6 +26,7 @@
>  #include "tst_mkfs.h"
>  #include "tst_fs.h"
>  #include "tst_pid.h"
> +#include "tst_path.h"
>  #include "tst_cmd.h"
>  #include "tst_cpu.h"
>  #include "tst_process_state.h"
> diff --git a/lib/newlib_tests/test19.c b/lib/newlib_tests/test19.c
> index a5683eaa4..0a7766192 100644
> --- a/lib/newlib_tests/test19.c
> +++ b/lib/newlib_tests/test19.c
> @@ -10,7 +10,7 @@
>  
>  static void setup(void)
>  {
> -	SAFE_FILE_PRINTF("/proc/sys/kernel/core_pattern", "changed");
> +	SAFE_FILE_PRINTF(PROC_SYS_KERNEL "core_pattern", "changed");
>  	tst_sys_conf_dump();
>  }
>  
> @@ -25,8 +25,8 @@ static struct tst_test test = {
>  	.setup = setup,
>  	.save_restore = (const struct tst_path_val[]) {
>  		{"/proc/nonexistent", NULL, TST_SR_SKIP},
> -		{"/proc/sys/kernel/numa_balancing", NULL, TST_SR_TBROK},
> -		{"/proc/sys/kernel/core_pattern", NULL, TST_SR_TCONF},
> +		{PROC_SYS_KERNEL "numa_balancing", NULL, TST_SR_TBROK},
> +		{PROC_SYS_KERNEL "core_pattern", NULL, TST_SR_TCONF},

This is exactly what I'm unsure about, it's not shorter, nor more
readable.

Maybe if this was PATH_NUMA_BALANCING and PATH_CORE_PATTERN it would be
shorter and would make much more sense.

Shortening this to SYS_KERNEL("numa_balancing") would be confusing since
we have both /proc/sys/ and /sys/

...

> diff --git a/testcases/kernel/mem/ksm/ksm01.c b/testcases/kernel/mem/ksm/ksm01.c
> index be03f943f..d61ac77b5 100644
> --- a/testcases/kernel/mem/ksm/ksm01.c
> +++ b/testcases/kernel/mem/ksm/ksm01.c
> @@ -74,13 +74,13 @@ static struct tst_test test = {
>  	},
>  	.setup = setup,
>  	.save_restore = (const struct tst_path_val[]) {
> -		{"/sys/kernel/mm/ksm/run", NULL, TST_SR_TBROK},
> -		{"/sys/kernel/mm/ksm/sleep_millisecs", NULL, TST_SR_TBROK},
> -		{"/sys/kernel/mm/ksm/max_page_sharing", NULL,
> +		{MM_KSM_FP("run"), NULL, TST_SR_TBROK},
> +		{MM_KSM_FP("sleep_millisecs"), NULL, TST_SR_TBROK},
> +		{MM_KSM_FP("max_page_sharing"), NULL,
>  			TST_SR_SKIP_MISSING | TST_SR_TCONF_RO},
> -		{"/sys/kernel/mm/ksm/merge_across_nodes", "1",
> +		{MM_KSM_FP("merge_across_nodes"), "1",
>  			TST_SR_SKIP_MISSING | TST_SR_TCONF_RO},
> -		{"/sys/kernel/mm/ksm/smart_scan", "0",
> +		{MM_KSM_FP("smart_scan"), "0",
>  			TST_SR_SKIP_MISSING | TST_SR_TBROK_RO},
>  		{}
>  	},

These looks reasonable.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp