diff mbox series

[01/13] lib: tst_sys_conf: Add two functions

Message ID 20241218184518.16190-2-chrubis@suse.cz
State Changes Requested
Headers show
Series Get rid of testcases/kernel/mem/lib library | expand

Commit Message

Cyril Hrubis Dec. 18, 2024, 6:45 p.m. UTC
Add two functions to read/write integer values from/to sysfs or procfs
files.

With that we replace the get_sys_tune() and set_sys_tune() from
testcases/kernel/mem/lib/mem.c with a better implementation.

We also remove the inclusion of the mem library from tunables, since
it's no longer needed there.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/tst_sys_conf.h                        | 31 +++++++++++++++++++
 lib/tst_sys_conf.c                            | 29 +++++++++++++++++
 .../mem/hugetlb/hugeshmat/hugeshmat04.c       |  2 +-
 testcases/kernel/mem/include/mem.h            |  2 --
 testcases/kernel/mem/lib/mem.c                | 29 -----------------
 testcases/kernel/mem/oom/oom01.c              | 10 +++---
 testcases/kernel/mem/tunable/Makefile         |  1 -
 testcases/kernel/mem/tunable/max_map_count.c  | 17 +++++-----
 .../kernel/mem/tunable/min_free_kbytes.c      | 27 ++++++++--------
 .../kernel/mem/tunable/overcommit_memory.c    | 23 ++++++++------
 10 files changed, 104 insertions(+), 67 deletions(-)

Comments

Li Wang Dec. 19, 2024, 3:10 a.m. UTC | #1
On Thu, Dec 19, 2024 at 2:46 AM Cyril Hrubis <chrubis@suse.cz> wrote:

> Add two functions to read/write integer values from/to sysfs or procfs
> files.
>
> With that we replace the get_sys_tune() and set_sys_tune() from
> testcases/kernel/mem/lib/mem.c with a better implementation.
>
> We also remove the inclusion of the mem library from tunables, since
> it's no longer needed there.
>


Thanks for doing this, all touched tests pass on RHEL 9&10 from my side.

Reviewed-by: Li Wang <liwang@redhat.com>



>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  include/tst_sys_conf.h                        | 31 +++++++++++++++++++
>  lib/tst_sys_conf.c                            | 29 +++++++++++++++++
>  .../mem/hugetlb/hugeshmat/hugeshmat04.c       |  2 +-
>  testcases/kernel/mem/include/mem.h            |  2 --
>  testcases/kernel/mem/lib/mem.c                | 29 -----------------
>  testcases/kernel/mem/oom/oom01.c              | 10 +++---
>  testcases/kernel/mem/tunable/Makefile         |  1 -
>  testcases/kernel/mem/tunable/max_map_count.c  | 17 +++++-----
>  .../kernel/mem/tunable/min_free_kbytes.c      | 27 ++++++++--------
>  .../kernel/mem/tunable/overcommit_memory.c    | 23 ++++++++------
>  10 files changed, 104 insertions(+), 67 deletions(-)
>
> diff --git a/include/tst_sys_conf.h b/include/tst_sys_conf.h
> index 4c85767be..53e8c5ea8 100644
> --- a/include/tst_sys_conf.h
> +++ b/include/tst_sys_conf.h
> @@ -28,4 +28,35 @@ int tst_sys_conf_save(const struct tst_path_val *conf);
>  void tst_sys_conf_restore(int verbose);
>  void tst_sys_conf_dump(void);
>
> +/**
> + * TST_SYS_CONF_LONG_SET()
> + *
> + * Sets a sysfs or procfs file and optionally checks that it was set
> correctly.
> + *
> + * @param path A path to a sysfs or a procfs file.
> + * @param val A long int value to be written to the file.
> + * @param check If non-zero the library reads the file back and checks
> that the
> + *              value is the one we have written there. If not the
> library calls
> + *              tst_brk(TBROK, ...).
> + */
> +#define TST_SYS_CONF_LONG_SET(path, val, check) \
> +       tst_sys_conf_long_set_(__FILE__, __LINE__, path, val, check)
> +
> +void tst_sys_conf_long_set_(const char *file, const int lineno,
> +                            const char *path, long val, int check);
> +
> +
> +/**
> + * TST_SYS_CONF_LONG_GET()
> + *
> + * Gets a sysfs or procfs file value and converts it to long.
> + *
> + * @param path A path to a sysfs or a procfs file.
> + */
> +#define TST_SYS_CONF_LONG_GET(path) \
> +       tst_sys_conf_long_get_(__FILE__, __LINE__, path)
> +
> +long tst_sys_conf_long_get_(const char *file, const int lineno,
> +                            const char *path);
> +
>  #endif
> diff --git a/lib/tst_sys_conf.c b/lib/tst_sys_conf.c
> index c0981dcb1..f962fc124 100644
> --- a/lib/tst_sys_conf.c
> +++ b/lib/tst_sys_conf.c
> @@ -145,3 +145,32 @@ void tst_sys_conf_restore(int verbose)
>         }
>  }
>
> +long tst_sys_conf_long_get_(const char *file, const int lineno,
> +                            const char *path)
> +{
> +       long ret;
> +
> +       safe_file_scanf(file, lineno, NULL, path, "%ld", &ret);
> +
> +       return ret;
> +}
> +
> +void tst_sys_conf_long_set_(const char *file, const int lineno,
> +                            const char *path, long val, int check)
> +{
> +        tst_res_(file, lineno, TINFO, "Setting %s to %ld", path, val);
> +
> +        safe_file_printf(file, lineno, NULL, path, "%ld", val);
> +
> +        if (check) {
> +               long read_val;
> +
> +               safe_file_scanf(file, lineno, NULL, path, "%ld",
> &read_val);
> +
> +                if (val != read_val)
> +                        tst_brk_(file, lineno, TBROK,
> +                                "Wrote %ld to %s but read back %ld",
> +                                 val, path, read_val);
> +        }
> +
> +}
> diff --git a/testcases/kernel/mem/hugetlb/hugeshmat/hugeshmat04.c
> b/testcases/kernel/mem/hugetlb/hugeshmat/hugeshmat04.c
> index 8ad745d5b..0d7bbf64f 100644
> --- a/testcases/kernel/mem/hugetlb/hugeshmat/hugeshmat04.c
> +++ b/testcases/kernel/mem/hugetlb/hugeshmat/hugeshmat04.c
> @@ -82,7 +82,7 @@ static void setup(void)
>         long hpage_size, orig_hugepages;
>         unsigned long new_shmmax;
>
> -       orig_hugepages = get_sys_tune("nr_hugepages");
> +       orig_hugepages =
> TST_SYS_CONF_LONG_GET("/proc/sys/vm/nr_hugepages");
>         SAFE_FILE_SCANF(PATH_SHMMAX, "%lu", &new_shmmax);
>
>         if (new_shmmax < SIZE)
> diff --git a/testcases/kernel/mem/include/mem.h
> b/testcases/kernel/mem/include/mem.h
> index cdc3ca146..865d2c7e8 100644
> --- a/testcases/kernel/mem/include/mem.h
> +++ b/testcases/kernel/mem/include/mem.h
> @@ -71,8 +71,6 @@ void write_cpusets(const struct tst_cg_group *cg, long
> nd);
>  /* shared */
>  unsigned int get_a_numa_node(void);
>  int  path_exist(const char *path, ...);
> -void set_sys_tune(char *sys_file, long tune, int check);
> -long get_sys_tune(char *sys_file);
>
>  void update_shm_size(size_t *shm_size);
>
> diff --git a/testcases/kernel/mem/lib/mem.c
> b/testcases/kernel/mem/lib/mem.c
> index fbfeef026..3e0f5d1bb 100644
> --- a/testcases/kernel/mem/lib/mem.c
> +++ b/testcases/kernel/mem/lib/mem.c
> @@ -638,35 +638,6 @@ int path_exist(const char *path, ...)
>         return access(pathbuf, F_OK) == 0;
>  }
>
> -void set_sys_tune(char *sys_file, long tune, int check)
> -{
> -       long val;
> -       char path[BUFSIZ];
> -
> -       tst_res(TINFO, "set %s to %ld", sys_file, tune);
> -
> -       snprintf(path, BUFSIZ, PATH_SYSVM "%s", sys_file);
> -       SAFE_FILE_PRINTF(path, "%ld", tune);
> -
> -       if (check) {
> -               val = get_sys_tune(sys_file);
> -               if (val != tune)
> -                       tst_brk(TBROK, "%s = %ld, but expect %ld",
> -                                sys_file, val, tune);
> -       }
> -}
> -
> -long get_sys_tune(char *sys_file)
> -{
> -       char path[BUFSIZ];
> -       long tune;
> -
> -       snprintf(path, BUFSIZ, PATH_SYSVM "%s", sys_file);
> -       SAFE_FILE_SCANF(path, "%ld", &tune);
> -
> -       return tune;
> -}
> -
>  void update_shm_size(size_t * shm_size)
>  {
>         size_t shmmax;
> diff --git a/testcases/kernel/mem/oom/oom01.c
> b/testcases/kernel/mem/oom/oom01.c
> index 903785f93..157ca84b6 100644
> --- a/testcases/kernel/mem/oom/oom01.c
> +++ b/testcases/kernel/mem/oom/oom01.c
> @@ -18,18 +18,20 @@
>  #include <unistd.h>
>  #include "mem.h"
>
> +#define OVERCOMMIT_MEMORY "/proc/sys/vm/overcommit_memory"
> +
>  static void verify_oom(void)
>  {
>         /* we expect mmap to fail before OOM is hit */
> -       set_sys_tune("overcommit_memory", 2, 1);
> +       TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 2, 1);
>         oom(NORMAL, 0, ENOMEM, 0);
>
>         /* with overcommit_memory set to 0 or 1 there's no
>          * guarantee that mmap fails before OOM */
> -       set_sys_tune("overcommit_memory", 0, 1);
> +       TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 0, 1);
>         oom(NORMAL, 0, ENOMEM, 1);
>
> -       set_sys_tune("overcommit_memory", 1, 1);
> +       TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 1, 1);
>         testoom(0, 0, ENOMEM, 1);
>  }
>
> @@ -40,7 +42,7 @@ static struct tst_test test = {
>         .test_all = verify_oom,
>         .skip_in_compat = 1,
>         .save_restore = (const struct tst_path_val[]) {
> -               {"/proc/sys/vm/overcommit_memory", NULL, TST_SR_TBROK},
> +               {OVERCOMMIT_MEMORY, NULL, TST_SR_TBROK},
>                 {}
>         },
>  };
> diff --git a/testcases/kernel/mem/tunable/Makefile
> b/testcases/kernel/mem/tunable/Makefile
> index 80d64bb4a..e0014e683 100644
> --- a/testcases/kernel/mem/tunable/Makefile
> +++ b/testcases/kernel/mem/tunable/Makefile
> @@ -4,5 +4,4 @@
>  top_srcdir             ?= ../../../..
>
>  include $(top_srcdir)/include/mk/testcases.mk
> -include $(top_srcdir)/testcases/kernel/mem/include/libmem.mk
>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/mem/tunable/max_map_count.c
> b/testcases/kernel/mem/tunable/max_map_count.c
> index f33095473..71a7bbee0 100644
> --- a/testcases/kernel/mem/tunable/max_map_count.c
> +++ b/testcases/kernel/mem/tunable/max_map_count.c
> @@ -48,10 +48,13 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <sys/utsname.h>
> -#include "mem.h"
> +#include "tst_test.h"
>
>  #define MAP_COUNT_DEFAULT      1024
> -#define MAX_MAP_COUNT          65536L
> +#define MAX_MAP_COUNT_MAX      65536L
> +
> +#define OVERCOMMIT_MEMORY "/proc/sys/vm/overcommit_memory"
> +#define MAX_MAP_COUNT "/proc/sys/vm/max_map_count"
>
>  /* This is a filter to exclude map entries which aren't accounted
>   * for in the vm_area_struct's map_count.
> @@ -140,15 +143,15 @@ static void max_map_count_test(void)
>         memfree = SAFE_READ_MEMINFO("CommitLimit:") -
> SAFE_READ_MEMINFO("Committed_AS:");
>         /* 64 used as a bias to make sure no overflow happen */
>         max_iters = memfree / sysconf(_SC_PAGESIZE) * 1024 - 64;
> -       if (max_iters > MAX_MAP_COUNT)
> -               max_iters = MAX_MAP_COUNT;
> +       if (max_iters > MAX_MAP_COUNT_MAX)
> +               max_iters = MAX_MAP_COUNT_MAX;
>
>         max_maps = MAP_COUNT_DEFAULT;
>         if (max_iters < max_maps)
>                 tst_brk(TCONF, "test requires more free memory");
>
>         while (max_maps <= max_iters) {
> -               set_sys_tune("max_map_count", max_maps, 1);
> +               TST_SYS_CONF_LONG_SET(MAX_MAP_COUNT, max_maps, 1);
>
>                 switch (pid = SAFE_FORK()) {
>                 case 0:
> @@ -192,8 +195,8 @@ static struct tst_test test = {
>         .forks_child = 1,
>         .test_all = max_map_count_test,
>         .save_restore = (const struct tst_path_val[]) {
> -               {"/proc/sys/vm/overcommit_memory", "0", TST_SR_TBROK},
> -               {"/proc/sys/vm/max_map_count", NULL, TST_SR_TBROK},
> +               {OVERCOMMIT_MEMORY, "0", TST_SR_TBROK},
> +               {MAX_MAP_COUNT, NULL, TST_SR_TBROK},
>                 {}
>         },
>  };
> diff --git a/testcases/kernel/mem/tunable/min_free_kbytes.c
> b/testcases/kernel/mem/tunable/min_free_kbytes.c
> index 19da409e8..5274bb3bf 100644
> --- a/testcases/kernel/mem/tunable/min_free_kbytes.c
> +++ b/testcases/kernel/mem/tunable/min_free_kbytes.c
> @@ -35,10 +35,14 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include "lapi/abisize.h"
> -#include "mem.h"
> +#include "tst_test.h"
>
>  #define MAP_SIZE (1UL<<20)
>
> +#define OVERCOMMIT_MEMORY "/proc/sys/vm/overcommit_memory"
> +#define MIN_FREE_KBYTES "/proc/sys/vm/min_free_kbytes"
> +#define PANIC_ON_OOM "/proc/sys/vm/panic_on_oom"
> +
>  volatile int end;
>  static long default_tune = -1;
>  static unsigned long total_mem;
> @@ -88,16 +92,13 @@ static void test_tune(unsigned long overcommit_policy)
>         int ret, i;
>         unsigned long tune, memfree, memtotal;
>
> -       set_sys_tune("overcommit_memory", overcommit_policy, 1);
> +       TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, overcommit_policy, 1);
>
>         for (i = 0; i < 3; i++) {
> -               /* case1 */
>                 if (i == 0)
> -                       set_sys_tune("min_free_kbytes", default_tune, 1);
> -               /* case2 */
> +                       TST_SYS_CONF_LONG_SET(MIN_FREE_KBYTES,
> default_tune, 1);
>                 else if (i == 1) {
> -                       set_sys_tune("min_free_kbytes", 2 * default_tune,
> 1);
> -                       /* case3 */
> +                       TST_SYS_CONF_LONG_SET(MIN_FREE_KBYTES, 2 *
> default_tune, 1);
>                 } else {
>                         memfree = SAFE_READ_MEMINFO("MemFree:");
>                         memtotal = SAFE_READ_MEMINFO("MemTotal:");
> @@ -105,7 +106,7 @@ static void test_tune(unsigned long overcommit_policy)
>                         if (tune > (memtotal / 50))
>                                 tune = memtotal / 50;
>
> -                       set_sys_tune("min_free_kbytes", tune, 1);
> +                       TST_SYS_CONF_LONG_SET(MIN_FREE_KBYTES, tune, 1);
>                 }
>
>                 fflush(stdout);
> @@ -189,7 +190,7 @@ static void check_monitor(void)
>
>         while (end) {
>                 memfree = SAFE_READ_MEMINFO("MemFree:");
> -               tune = get_sys_tune("min_free_kbytes");
> +               tune = TST_SYS_CONF_LONG_GET(MIN_FREE_KBYTES);
>
>                 if (memfree < tune) {
>                         tst_res(TINFO, "MemFree is %lu kB, "
> @@ -208,14 +209,14 @@ static void sighandler(int signo
> LTP_ATTRIBUTE_UNUSED)
>
>  static void setup(void)
>  {
> -       if (get_sys_tune("panic_on_oom")) {
> +       if (TST_SYS_CONF_LONG_GET(PANIC_ON_OOM)) {
>                 tst_brk(TCONF,
>                         "panic_on_oom is set, disable it to run these
> testcases");
>         }
>
>         total_mem = SAFE_READ_MEMINFO("MemTotal:") +
> SAFE_READ_MEMINFO("SwapTotal:");
>
> -       default_tune = get_sys_tune("min_free_kbytes");
> +       default_tune = TST_SYS_CONF_LONG_GET(MIN_FREE_KBYTES);
>  }
>
>  static struct tst_test test = {
> @@ -225,8 +226,8 @@ static struct tst_test test = {
>         .setup = setup,
>         .test_all = min_free_kbytes_test,
>         .save_restore = (const struct tst_path_val[]) {
> -               {"/proc/sys/vm/overcommit_memory", NULL, TST_SR_TBROK},
> -               {"/proc/sys/vm/min_free_kbytes", NULL, TST_SR_TBROK},
> +               {OVERCOMMIT_MEMORY, NULL, TST_SR_TBROK},
> +               {MIN_FREE_KBYTES, NULL, TST_SR_TBROK},
>                 {}
>         },
>  };
> diff --git a/testcases/kernel/mem/tunable/overcommit_memory.c
> b/testcases/kernel/mem/tunable/overcommit_memory.c
> index d055dd18f..b5beebbcd 100644
> --- a/testcases/kernel/mem/tunable/overcommit_memory.c
> +++ b/testcases/kernel/mem/tunable/overcommit_memory.c
> @@ -62,12 +62,15 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <limits.h>
> -#include "mem.h"
> +#include "tst_test.h"
>
>  #define DEFAULT_OVER_RATIO     50L
>  #define EXPECT_PASS            0
>  #define EXPECT_FAIL            1
>
> +#define OVERCOMMIT_MEMORY "/proc/sys/vm/overcommit_memory"
> +#define OVERCOMMIT_RATIO "/proc/sys/vm/overcommit_ratio"
> +
>  static char *R_opt;
>  static long old_overcommit_ratio = -1;
>  static long overcommit_ratio;
> @@ -93,7 +96,7 @@ static void setup(void)
>         else
>                 overcommit_ratio = DEFAULT_OVER_RATIO;
>
> -       old_overcommit_ratio = get_sys_tune("overcommit_ratio");
> +       old_overcommit_ratio = TST_SYS_CONF_LONG_GET(OVERCOMMIT_RATIO);
>
>         mem_total = SAFE_READ_MEMINFO("MemTotal:");
>         tst_res(TINFO, "MemTotal is %ld kB", mem_total);
> @@ -115,7 +118,7 @@ static void setup(void)
>                 SAFE_SETRLIMIT(RLIMIT_AS, &lim);
>         }
>
> -       set_sys_tune("overcommit_ratio", overcommit_ratio, 1);
> +       TST_SYS_CONF_LONG_SET(OVERCOMMIT_RATIO, overcommit_ratio, 1);
>
>         calculate_total_batch_size();
>         tst_res(TINFO, "TotalBatchSize is %ld kB", total_batch_size);
> @@ -124,7 +127,7 @@ static void setup(void)
>  static void overcommit_memory_test(void)
>  {
>         /* start to test overcommit_memory=2 */
> -       set_sys_tune("overcommit_memory", 2, 1);
> +       TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 2, 1);
>
>         update_mem_commit();
>         alloc_and_check(commit_left * 2, EXPECT_FAIL);
> @@ -133,14 +136,14 @@ static void overcommit_memory_test(void)
>         alloc_and_check(commit_left / 2, EXPECT_PASS);
>
>         /* start to test overcommit_memory=0 */
> -       set_sys_tune("overcommit_memory", 0, 1);
> +       TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 0, 1);
>
>         update_mem();
>         alloc_and_check(free_total / 2, EXPECT_PASS);
>         alloc_and_check(sum_total * 2, EXPECT_FAIL);
>
>         /* start to test overcommit_memory=1 */
> -       set_sys_tune("overcommit_memory", 1, 1);
> +       TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 1, 1);
>
>         alloc_and_check(sum_total / 2, EXPECT_PASS);
>         alloc_and_check(sum_total, EXPECT_PASS);
> @@ -152,7 +155,7 @@ static int heavy_malloc(long size)
>  {
>         char *p;
>
> -       p = malloc(size * KB);
> +       p = malloc(size * TST_KB);
>         if (p != NULL) {
>                 tst_res(TINFO, "malloc %ld kB successfully", size);
>                 free(p);
> @@ -240,7 +243,7 @@ static void calculate_total_batch_size(void)
>         /* there are ncpu separate counters, that can all grow up to
>          * batch_size. So the maximum error for __vm_enough_memory is
>          * batch_size * ncpus. */
> -       total_batch_size = (batch_size * ncpus * pagesize) / KB;
> +       total_batch_size = (batch_size * ncpus * pagesize) / TST_KB;
>  }
>
>  static struct tst_test test = {
> @@ -253,8 +256,8 @@ static struct tst_test test = {
>         .test_all = overcommit_memory_test,
>         .skip_in_compat = 1,
>         .save_restore = (const struct tst_path_val[]) {
> -               {"/proc/sys/vm/overcommit_memory", NULL, TST_SR_TBROK},
> -               {"/proc/sys/vm/overcommit_ratio", NULL, TST_SR_TBROK},
> +               {OVERCOMMIT_MEMORY, NULL, TST_SR_TBROK},
> +               {OVERCOMMIT_RATIO, NULL, TST_SR_TBROK},
>                 {}
>         },
>  };
> --
> 2.45.2
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>
Petr Vorel Dec. 27, 2024, 9:04 a.m. UTC | #2
Hi Cyril,

> Add two functions to read/write integer values from/to sysfs or procfs
> files.

> With that we replace the get_sys_tune() and set_sys_tune() from
> testcases/kernel/mem/lib/mem.c with a better implementation.

> We also remove the inclusion of the mem library from tunables, since
> it's no longer needed there.

+1

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Minor formatting notes below.

> +/**
> + * TST_SYS_CONF_LONG_SET()

I wonder how to silence warning when there is no short description.

include/tst_sys_conf.h:32: [kernel-doc WARN] : missing initial short description of 'TST_SYS_CONF_LONG_SET'

> + *
> + * Sets a sysfs or procfs file and optionally checks that it was set correctly.
> + *
> + * @param path A path to a sysfs or a procfs file.
> + * @param val A long int value to be written to the file.
> + * @param check If non-zero the library reads the file back and checks that the

nit: please fix these before merge (doxygen => sphinx syntax):

include/tst_sys_conf.h:32: [kernel-doc WARN] : no description found for parameter 'path'
include/tst_sys_conf.h:32: [kernel-doc WARN] : no description found for parameter 'val'
include/tst_sys_conf.h:32: [kernel-doc WARN] : no description found for parameter 'check'

> + *              value is the one we have written there. If not the library calls
> + *              tst_brk(TBROK, ...).
> + */


> +#define TST_SYS_CONF_LONG_SET(path, val, check) \
> +	tst_sys_conf_long_set_(__FILE__, __LINE__, path, val, check)
> +
> +void tst_sys_conf_long_set_(const char *file, const int lineno,
> +                            const char *path, long val, int check);
> +
> +
> +/**
> + * TST_SYS_CONF_LONG_GET()
> + *
> + * Gets a sysfs or procfs file value and converts it to long.
> + *
> + * @param path A path to a sysfs or a procfs file.
And here:

include/tst_sys_conf.h:50: [kernel-doc WARN] : no description found for parameter 'path'

> + */
> +#define TST_SYS_CONF_LONG_GET(path) \
> +	tst_sys_conf_long_get_(__FILE__, __LINE__, path)
> +
> +long tst_sys_conf_long_get_(const char *file, const int lineno,
> +                            const char *path);
> +
>  #endif
> diff --git a/lib/tst_sys_conf.c b/lib/tst_sys_conf.c
> index c0981dcb1..f962fc124 100644
> --- a/lib/tst_sys_conf.c
> +++ b/lib/tst_sys_conf.c
> @@ -145,3 +145,32 @@ void tst_sys_conf_restore(int verbose)
>  	}
>  }

> +long tst_sys_conf_long_get_(const char *file, const int lineno,
> +                            const char *path)
nit: could you please use tabs instead of spaces?
Because it triggers warnings:

$ make check-tst_sys_conf
CHECK lib/tst_sys_conf.c
tst_sys_conf.c:149: ERROR: code indent should use tabs where possible
tst_sys_conf.c:149: WARNING: please, no spaces at the start of a line
tst_sys_conf.c:159: ERROR: code indent should use tabs where possible
tst_sys_conf.c:159: WARNING: please, no spaces at the start of a line
tst_sys_conf.c:161: ERROR: code indent should use tabs where possible
tst_sys_conf.c:161: WARNING: please, no spaces at the start of a line
tst_sys_conf.c:163: ERROR: code indent should use tabs where possible
tst_sys_conf.c:163: WARNING: please, no spaces at the start of a line
tst_sys_conf.c:165: ERROR: code indent should use tabs where possible
tst_sys_conf.c:165: WARNING: please, no spaces at the start of a line
tst_sys_conf.c:170: ERROR: code indent should use tabs where possible
tst_sys_conf.c:170: WARNING: please, no spaces at the start of a line
tst_sys_conf.c:171: ERROR: code indent should use tabs where possible
tst_sys_conf.c:171: WARNING: please, no spaces at the start of a line
tst_sys_conf.c:172: ERROR: code indent should use tabs where possible
tst_sys_conf.c:173: ERROR: code indent should use tabs where possible
tst_sys_conf.c:173: WARNING: please, no spaces at the start of a line
tst_sys_conf.c:174: ERROR: code indent should use tabs where possible
tst_sys_conf.c:174: WARNING: please, no spaces at the start of a line


> +{
> +	long ret;
> +
> +	safe_file_scanf(file, lineno, NULL, path, "%ld", &ret);
> +
> +	return ret;
> +}
> +
> +void tst_sys_conf_long_set_(const char *file, const int lineno,
> +                            const char *path, long val, int check)
> +{
> +        tst_res_(file, lineno, TINFO, "Setting %s to %ld", path, val);
> +
> +        safe_file_printf(file, lineno, NULL, path, "%ld", val);
> +
> +        if (check) {
> +		long read_val;
nit: obviously also here are tabs and spaces mixed. Minor, but worth to fix
before merge.

Maybe we would benefit to add .editorconfig [1] (see examples [2] [3]).

Also maybe to save indent?

if (!check)
	return;

Kind regards,
Petr


> +
> +		safe_file_scanf(file, lineno, NULL, path, "%ld", &read_val);
> +
> +                if (val != read_val)
> +                        tst_brk_(file, lineno, TBROK,
> +			         "Wrote %ld to %s but read back %ld",
> +                                 val, path, read_val);
> +        }
> +
> +}

[1] https://editorconfig.org/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/.editorconfig
[3] https://git.busybox.net/buildroot/tree/.editorconfig
diff mbox series

Patch

diff --git a/include/tst_sys_conf.h b/include/tst_sys_conf.h
index 4c85767be..53e8c5ea8 100644
--- a/include/tst_sys_conf.h
+++ b/include/tst_sys_conf.h
@@ -28,4 +28,35 @@  int tst_sys_conf_save(const struct tst_path_val *conf);
 void tst_sys_conf_restore(int verbose);
 void tst_sys_conf_dump(void);
 
+/**
+ * TST_SYS_CONF_LONG_SET()
+ *
+ * Sets a sysfs or procfs file and optionally checks that it was set correctly.
+ *
+ * @param path A path to a sysfs or a procfs file.
+ * @param val A long int value to be written to the file.
+ * @param check If non-zero the library reads the file back and checks that the
+ *              value is the one we have written there. If not the library calls
+ *              tst_brk(TBROK, ...).
+ */
+#define TST_SYS_CONF_LONG_SET(path, val, check) \
+	tst_sys_conf_long_set_(__FILE__, __LINE__, path, val, check)
+
+void tst_sys_conf_long_set_(const char *file, const int lineno,
+                            const char *path, long val, int check);
+
+
+/**
+ * TST_SYS_CONF_LONG_GET()
+ *
+ * Gets a sysfs or procfs file value and converts it to long.
+ *
+ * @param path A path to a sysfs or a procfs file.
+ */
+#define TST_SYS_CONF_LONG_GET(path) \
+	tst_sys_conf_long_get_(__FILE__, __LINE__, path)
+
+long tst_sys_conf_long_get_(const char *file, const int lineno,
+                            const char *path);
+
 #endif
diff --git a/lib/tst_sys_conf.c b/lib/tst_sys_conf.c
index c0981dcb1..f962fc124 100644
--- a/lib/tst_sys_conf.c
+++ b/lib/tst_sys_conf.c
@@ -145,3 +145,32 @@  void tst_sys_conf_restore(int verbose)
 	}
 }
 
+long tst_sys_conf_long_get_(const char *file, const int lineno,
+                            const char *path)
+{
+	long ret;
+
+	safe_file_scanf(file, lineno, NULL, path, "%ld", &ret);
+
+	return ret;
+}
+
+void tst_sys_conf_long_set_(const char *file, const int lineno,
+                            const char *path, long val, int check)
+{
+        tst_res_(file, lineno, TINFO, "Setting %s to %ld", path, val);
+
+        safe_file_printf(file, lineno, NULL, path, "%ld", val);
+
+        if (check) {
+		long read_val;
+
+		safe_file_scanf(file, lineno, NULL, path, "%ld", &read_val);
+
+                if (val != read_val)
+                        tst_brk_(file, lineno, TBROK,
+			         "Wrote %ld to %s but read back %ld",
+                                 val, path, read_val);
+        }
+
+}
diff --git a/testcases/kernel/mem/hugetlb/hugeshmat/hugeshmat04.c b/testcases/kernel/mem/hugetlb/hugeshmat/hugeshmat04.c
index 8ad745d5b..0d7bbf64f 100644
--- a/testcases/kernel/mem/hugetlb/hugeshmat/hugeshmat04.c
+++ b/testcases/kernel/mem/hugetlb/hugeshmat/hugeshmat04.c
@@ -82,7 +82,7 @@  static void setup(void)
 	long hpage_size, orig_hugepages;
 	unsigned long new_shmmax;
 
-	orig_hugepages = get_sys_tune("nr_hugepages");
+	orig_hugepages = TST_SYS_CONF_LONG_GET("/proc/sys/vm/nr_hugepages");
 	SAFE_FILE_SCANF(PATH_SHMMAX, "%lu", &new_shmmax);
 
 	if (new_shmmax < SIZE)
diff --git a/testcases/kernel/mem/include/mem.h b/testcases/kernel/mem/include/mem.h
index cdc3ca146..865d2c7e8 100644
--- a/testcases/kernel/mem/include/mem.h
+++ b/testcases/kernel/mem/include/mem.h
@@ -71,8 +71,6 @@  void write_cpusets(const struct tst_cg_group *cg, long nd);
 /* shared */
 unsigned int get_a_numa_node(void);
 int  path_exist(const char *path, ...);
-void set_sys_tune(char *sys_file, long tune, int check);
-long get_sys_tune(char *sys_file);
 
 void update_shm_size(size_t *shm_size);
 
diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index fbfeef026..3e0f5d1bb 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -638,35 +638,6 @@  int path_exist(const char *path, ...)
 	return access(pathbuf, F_OK) == 0;
 }
 
-void set_sys_tune(char *sys_file, long tune, int check)
-{
-	long val;
-	char path[BUFSIZ];
-
-	tst_res(TINFO, "set %s to %ld", sys_file, tune);
-
-	snprintf(path, BUFSIZ, PATH_SYSVM "%s", sys_file);
-	SAFE_FILE_PRINTF(path, "%ld", tune);
-
-	if (check) {
-		val = get_sys_tune(sys_file);
-		if (val != tune)
-			tst_brk(TBROK, "%s = %ld, but expect %ld",
-				 sys_file, val, tune);
-	}
-}
-
-long get_sys_tune(char *sys_file)
-{
-	char path[BUFSIZ];
-	long tune;
-
-	snprintf(path, BUFSIZ, PATH_SYSVM "%s", sys_file);
-	SAFE_FILE_SCANF(path, "%ld", &tune);
-
-	return tune;
-}
-
 void update_shm_size(size_t * shm_size)
 {
 	size_t shmmax;
diff --git a/testcases/kernel/mem/oom/oom01.c b/testcases/kernel/mem/oom/oom01.c
index 903785f93..157ca84b6 100644
--- a/testcases/kernel/mem/oom/oom01.c
+++ b/testcases/kernel/mem/oom/oom01.c
@@ -18,18 +18,20 @@ 
 #include <unistd.h>
 #include "mem.h"
 
+#define OVERCOMMIT_MEMORY "/proc/sys/vm/overcommit_memory"
+
 static void verify_oom(void)
 {
 	/* we expect mmap to fail before OOM is hit */
-	set_sys_tune("overcommit_memory", 2, 1);
+	TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 2, 1);
 	oom(NORMAL, 0, ENOMEM, 0);
 
 	/* with overcommit_memory set to 0 or 1 there's no
 	 * guarantee that mmap fails before OOM */
-	set_sys_tune("overcommit_memory", 0, 1);
+	TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 0, 1);
 	oom(NORMAL, 0, ENOMEM, 1);
 
-	set_sys_tune("overcommit_memory", 1, 1);
+	TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 1, 1);
 	testoom(0, 0, ENOMEM, 1);
 }
 
@@ -40,7 +42,7 @@  static struct tst_test test = {
 	.test_all = verify_oom,
 	.skip_in_compat = 1,
 	.save_restore = (const struct tst_path_val[]) {
-		{"/proc/sys/vm/overcommit_memory", NULL, TST_SR_TBROK},
+		{OVERCOMMIT_MEMORY, NULL, TST_SR_TBROK},
 		{}
 	},
 };
diff --git a/testcases/kernel/mem/tunable/Makefile b/testcases/kernel/mem/tunable/Makefile
index 80d64bb4a..e0014e683 100644
--- a/testcases/kernel/mem/tunable/Makefile
+++ b/testcases/kernel/mem/tunable/Makefile
@@ -4,5 +4,4 @@ 
 top_srcdir		?= ../../../..
 
 include $(top_srcdir)/include/mk/testcases.mk
-include $(top_srcdir)/testcases/kernel/mem/include/libmem.mk
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/mem/tunable/max_map_count.c b/testcases/kernel/mem/tunable/max_map_count.c
index f33095473..71a7bbee0 100644
--- a/testcases/kernel/mem/tunable/max_map_count.c
+++ b/testcases/kernel/mem/tunable/max_map_count.c
@@ -48,10 +48,13 @@ 
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/utsname.h>
-#include "mem.h"
+#include "tst_test.h"
 
 #define MAP_COUNT_DEFAULT	1024
-#define MAX_MAP_COUNT		65536L
+#define MAX_MAP_COUNT_MAX	65536L
+
+#define OVERCOMMIT_MEMORY "/proc/sys/vm/overcommit_memory"
+#define MAX_MAP_COUNT "/proc/sys/vm/max_map_count"
 
 /* This is a filter to exclude map entries which aren't accounted
  * for in the vm_area_struct's map_count.
@@ -140,15 +143,15 @@  static void max_map_count_test(void)
 	memfree = SAFE_READ_MEMINFO("CommitLimit:") - SAFE_READ_MEMINFO("Committed_AS:");
 	/* 64 used as a bias to make sure no overflow happen */
 	max_iters = memfree / sysconf(_SC_PAGESIZE) * 1024 - 64;
-	if (max_iters > MAX_MAP_COUNT)
-		max_iters = MAX_MAP_COUNT;
+	if (max_iters > MAX_MAP_COUNT_MAX)
+		max_iters = MAX_MAP_COUNT_MAX;
 
 	max_maps = MAP_COUNT_DEFAULT;
 	if (max_iters < max_maps)
 		tst_brk(TCONF, "test requires more free memory");
 
 	while (max_maps <= max_iters) {
-		set_sys_tune("max_map_count", max_maps, 1);
+		TST_SYS_CONF_LONG_SET(MAX_MAP_COUNT, max_maps, 1);
 
 		switch (pid = SAFE_FORK()) {
 		case 0:
@@ -192,8 +195,8 @@  static struct tst_test test = {
 	.forks_child = 1,
 	.test_all = max_map_count_test,
 	.save_restore = (const struct tst_path_val[]) {
-		{"/proc/sys/vm/overcommit_memory", "0", TST_SR_TBROK},
-		{"/proc/sys/vm/max_map_count", NULL, TST_SR_TBROK},
+		{OVERCOMMIT_MEMORY, "0", TST_SR_TBROK},
+		{MAX_MAP_COUNT, NULL, TST_SR_TBROK},
 		{}
 	},
 };
diff --git a/testcases/kernel/mem/tunable/min_free_kbytes.c b/testcases/kernel/mem/tunable/min_free_kbytes.c
index 19da409e8..5274bb3bf 100644
--- a/testcases/kernel/mem/tunable/min_free_kbytes.c
+++ b/testcases/kernel/mem/tunable/min_free_kbytes.c
@@ -35,10 +35,14 @@ 
 #include <stdio.h>
 #include <stdlib.h>
 #include "lapi/abisize.h"
-#include "mem.h"
+#include "tst_test.h"
 
 #define MAP_SIZE (1UL<<20)
 
+#define OVERCOMMIT_MEMORY "/proc/sys/vm/overcommit_memory"
+#define MIN_FREE_KBYTES "/proc/sys/vm/min_free_kbytes"
+#define PANIC_ON_OOM "/proc/sys/vm/panic_on_oom"
+
 volatile int end;
 static long default_tune = -1;
 static unsigned long total_mem;
@@ -88,16 +92,13 @@  static void test_tune(unsigned long overcommit_policy)
 	int ret, i;
 	unsigned long tune, memfree, memtotal;
 
-	set_sys_tune("overcommit_memory", overcommit_policy, 1);
+	TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, overcommit_policy, 1);
 
 	for (i = 0; i < 3; i++) {
-		/* case1 */
 		if (i == 0)
-			set_sys_tune("min_free_kbytes", default_tune, 1);
-		/* case2 */
+			TST_SYS_CONF_LONG_SET(MIN_FREE_KBYTES, default_tune, 1);
 		else if (i == 1) {
-			set_sys_tune("min_free_kbytes", 2 * default_tune, 1);
-			/* case3 */
+			TST_SYS_CONF_LONG_SET(MIN_FREE_KBYTES, 2 * default_tune, 1);
 		} else {
 			memfree = SAFE_READ_MEMINFO("MemFree:");
 			memtotal = SAFE_READ_MEMINFO("MemTotal:");
@@ -105,7 +106,7 @@  static void test_tune(unsigned long overcommit_policy)
 			if (tune > (memtotal / 50))
 				tune = memtotal / 50;
 
-			set_sys_tune("min_free_kbytes", tune, 1);
+			TST_SYS_CONF_LONG_SET(MIN_FREE_KBYTES, tune, 1);
 		}
 
 		fflush(stdout);
@@ -189,7 +190,7 @@  static void check_monitor(void)
 
 	while (end) {
 		memfree = SAFE_READ_MEMINFO("MemFree:");
-		tune = get_sys_tune("min_free_kbytes");
+		tune = TST_SYS_CONF_LONG_GET(MIN_FREE_KBYTES);
 
 		if (memfree < tune) {
 			tst_res(TINFO, "MemFree is %lu kB, "
@@ -208,14 +209,14 @@  static void sighandler(int signo LTP_ATTRIBUTE_UNUSED)
 
 static void setup(void)
 {
-	if (get_sys_tune("panic_on_oom")) {
+	if (TST_SYS_CONF_LONG_GET(PANIC_ON_OOM)) {
 		tst_brk(TCONF,
 			"panic_on_oom is set, disable it to run these testcases");
 	}
 
 	total_mem = SAFE_READ_MEMINFO("MemTotal:") + SAFE_READ_MEMINFO("SwapTotal:");
 
-	default_tune = get_sys_tune("min_free_kbytes");
+	default_tune = TST_SYS_CONF_LONG_GET(MIN_FREE_KBYTES);
 }
 
 static struct tst_test test = {
@@ -225,8 +226,8 @@  static struct tst_test test = {
 	.setup = setup,
 	.test_all = min_free_kbytes_test,
 	.save_restore = (const struct tst_path_val[]) {
-		{"/proc/sys/vm/overcommit_memory", NULL, TST_SR_TBROK},
-		{"/proc/sys/vm/min_free_kbytes", NULL, TST_SR_TBROK},
+		{OVERCOMMIT_MEMORY, NULL, TST_SR_TBROK},
+		{MIN_FREE_KBYTES, NULL, TST_SR_TBROK},
 		{}
 	},
 };
diff --git a/testcases/kernel/mem/tunable/overcommit_memory.c b/testcases/kernel/mem/tunable/overcommit_memory.c
index d055dd18f..b5beebbcd 100644
--- a/testcases/kernel/mem/tunable/overcommit_memory.c
+++ b/testcases/kernel/mem/tunable/overcommit_memory.c
@@ -62,12 +62,15 @@ 
 #include <stdio.h>
 #include <stdlib.h>
 #include <limits.h>
-#include "mem.h"
+#include "tst_test.h"
 
 #define DEFAULT_OVER_RATIO	50L
 #define EXPECT_PASS		0
 #define EXPECT_FAIL		1
 
+#define OVERCOMMIT_MEMORY "/proc/sys/vm/overcommit_memory"
+#define OVERCOMMIT_RATIO "/proc/sys/vm/overcommit_ratio"
+
 static char *R_opt;
 static long old_overcommit_ratio = -1;
 static long overcommit_ratio;
@@ -93,7 +96,7 @@  static void setup(void)
 	else
 		overcommit_ratio = DEFAULT_OVER_RATIO;
 
-	old_overcommit_ratio = get_sys_tune("overcommit_ratio");
+	old_overcommit_ratio = TST_SYS_CONF_LONG_GET(OVERCOMMIT_RATIO);
 
 	mem_total = SAFE_READ_MEMINFO("MemTotal:");
 	tst_res(TINFO, "MemTotal is %ld kB", mem_total);
@@ -115,7 +118,7 @@  static void setup(void)
 		SAFE_SETRLIMIT(RLIMIT_AS, &lim);
 	}
 
-	set_sys_tune("overcommit_ratio", overcommit_ratio, 1);
+	TST_SYS_CONF_LONG_SET(OVERCOMMIT_RATIO, overcommit_ratio, 1);
 
 	calculate_total_batch_size();
 	tst_res(TINFO, "TotalBatchSize is %ld kB", total_batch_size);
@@ -124,7 +127,7 @@  static void setup(void)
 static void overcommit_memory_test(void)
 {
 	/* start to test overcommit_memory=2 */
-	set_sys_tune("overcommit_memory", 2, 1);
+	TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 2, 1);
 
 	update_mem_commit();
 	alloc_and_check(commit_left * 2, EXPECT_FAIL);
@@ -133,14 +136,14 @@  static void overcommit_memory_test(void)
 	alloc_and_check(commit_left / 2, EXPECT_PASS);
 
 	/* start to test overcommit_memory=0 */
-	set_sys_tune("overcommit_memory", 0, 1);
+	TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 0, 1);
 
 	update_mem();
 	alloc_and_check(free_total / 2, EXPECT_PASS);
 	alloc_and_check(sum_total * 2, EXPECT_FAIL);
 
 	/* start to test overcommit_memory=1 */
-	set_sys_tune("overcommit_memory", 1, 1);
+	TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 1, 1);
 
 	alloc_and_check(sum_total / 2, EXPECT_PASS);
 	alloc_and_check(sum_total, EXPECT_PASS);
@@ -152,7 +155,7 @@  static int heavy_malloc(long size)
 {
 	char *p;
 
-	p = malloc(size * KB);
+	p = malloc(size * TST_KB);
 	if (p != NULL) {
 		tst_res(TINFO, "malloc %ld kB successfully", size);
 		free(p);
@@ -240,7 +243,7 @@  static void calculate_total_batch_size(void)
 	/* there are ncpu separate counters, that can all grow up to
 	 * batch_size. So the maximum error for __vm_enough_memory is
 	 * batch_size * ncpus. */
-	total_batch_size = (batch_size * ncpus * pagesize) / KB;
+	total_batch_size = (batch_size * ncpus * pagesize) / TST_KB;
 }
 
 static struct tst_test test = {
@@ -253,8 +256,8 @@  static struct tst_test test = {
 	.test_all = overcommit_memory_test,
 	.skip_in_compat = 1,
 	.save_restore = (const struct tst_path_val[]) {
-		{"/proc/sys/vm/overcommit_memory", NULL, TST_SR_TBROK},
-		{"/proc/sys/vm/overcommit_ratio", NULL, TST_SR_TBROK},
+		{OVERCOMMIT_MEMORY, NULL, TST_SR_TBROK},
+		{OVERCOMMIT_RATIO, NULL, TST_SR_TBROK},
 		{}
 	},
 };