Message ID | 20241218184518.16190-7-chrubis@suse.cz |
---|---|
State | Changes Requested |
Headers | show |
Series | Get rid of testcases/kernel/mem/lib library | expand |
Hi Cyril, > +#define PATH_THP "/sys/kernel/mm/transparent_hugepage/" > + > +static inline void check_hugepage(void) > +{ > + if (access(PATH_HUGEPAGES, F_OK)) > + tst_brk(TCONF, "Huge page is not supported."); > +} I guess we don't want to move this into static inline function (used only in 2 tests. if (access(PATH_THP, F_OK) == -1) tst_brk(TCONF, "THP not enabled in kernel?"); I also wonder if we should add to the library struct tst_test test something like .requires_proc_sys which would check for files in /sys or /proc. There could be an optional parameter for TCONF message. Advantage would be to have this in docparse docs (or isn't it useful to see this)? We have .save_restore, but that's only for files and it reads the value. But it could share the flags (TST_SR_TCONF, TST_SR_TBROK, TST_SR_SKIP, ...). Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr
Hi! > > +#define PATH_THP "/sys/kernel/mm/transparent_hugepage/" > > + > > +static inline void check_hugepage(void) > > +{ > > + if (access(PATH_HUGEPAGES, F_OK)) > > + tst_brk(TCONF, "Huge page is not supported."); > > +} > > I guess we don't want to move this into static inline function (used only in 2 > tests. > > if (access(PATH_THP, F_OK) == -1) > tst_brk(TCONF, "THP not enabled in kernel?"); > > I also wonder if we should add to the library struct tst_test test something > like .requires_proc_sys which would check for files in /sys or /proc. There > could be an optional parameter for TCONF message. Advantage would be to have > this in docparse docs (or isn't it useful to see this)? > > We have .save_restore, but that's only for files and it reads the value. > But it could share the flags (TST_SR_TCONF, TST_SR_TBROK, TST_SR_SKIP, ...). Logically save_restore is not a good candidate since we are checking a directory existence here. So maybe we need to add .needs_paths array of strings into tst_test later on...
> Hi! > > > +#define PATH_THP "/sys/kernel/mm/transparent_hugepage/" > > > + > > > +static inline void check_hugepage(void) > > > +{ > > > + if (access(PATH_HUGEPAGES, F_OK)) > > > + tst_brk(TCONF, "Huge page is not supported."); > > > +} > > I guess we don't want to move this into static inline function (used only in 2 > > tests. > > if (access(PATH_THP, F_OK) == -1) > > tst_brk(TCONF, "THP not enabled in kernel?"); > > I also wonder if we should add to the library struct tst_test test something > > like .requires_proc_sys which would check for files in /sys or /proc. There > > could be an optional parameter for TCONF message. Advantage would be to have > > this in docparse docs (or isn't it useful to see this)? > > We have .save_restore, but that's only for files and it reads the value. > > But it could share the flags (TST_SR_TCONF, TST_SR_TBROK, TST_SR_SKIP, ...). > Logically save_restore is not a good candidate since we are checking a > directory existence here. So maybe we need to add .needs_paths array of > strings into tst_test later on... Yeah. .needs_paths sounds reasonable + use flags we already have (TST_SR_TCONF, TST_SR_TBROK, TST_SR_SKIP). Kind regards, Petr
diff --git a/testcases/kernel/mem/include/mem.h b/testcases/kernel/mem/include/mem.h index 03dbe91d7..ba5a996a7 100644 --- a/testcases/kernel/mem/include/mem.h +++ b/testcases/kernel/mem/include/mem.h @@ -52,17 +52,10 @@ void test_ksm_merge_across_nodes(unsigned long nr_pages); void ksm_group_check(int run, int pg_shared, int pg_sharing, int pg_volatile, int pg_unshared, int sleep_msecs, int pages_to_scan); -/* THP */ - -#define PATH_THP "/sys/kernel/mm/transparent_hugepage/" -#define PATH_KHPD PATH_THP "khugepaged/" - /* HUGETLB */ -#define PATH_HUGEPAGES "/sys/kernel/mm/hugepages/" #define PATH_SHMMAX "/proc/sys/kernel/shmmax" -void check_hugepage(void); void write_memcg(void); /* cpuset/memcg - include/tst_cgroup.h */ diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c index de9388a80..02199349d 100644 --- a/testcases/kernel/mem/lib/mem.c +++ b/testcases/kernel/mem/lib/mem.c @@ -324,12 +324,6 @@ static void verify(char **memory, char value, int proc, free(s); } -void check_hugepage(void) -{ - if (access(PATH_HUGEPAGES, F_OK)) - tst_brk(TCONF, "Huge page is not supported."); -} - struct ksm_merge_data { char data; unsigned int mergeable_size; diff --git a/testcases/kernel/mem/thp/Makefile b/testcases/kernel/mem/thp/Makefile index e95712eaf..d89ea1dd3 100644 --- a/testcases/kernel/mem/thp/Makefile +++ b/testcases/kernel/mem/thp/Makefile @@ -3,7 +3,7 @@ top_srcdir ?= ../../../.. thp04: LDLIBS += -lrt +thp04: CFLAGS += -pthread 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/thp/thp.h b/testcases/kernel/mem/thp/thp.h new file mode 100644 index 000000000..7723bedc2 --- /dev/null +++ b/testcases/kernel/mem/thp/thp.h @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) Linux Test Project, 2011-2021 + * Copyright (c) Cyril Hrubis 2024 + */ +#ifndef THP_H +#define THP_H + +#define PATH_THP "/sys/kernel/mm/transparent_hugepage/" + +static inline void check_hugepage(void) +{ + if (access(PATH_HUGEPAGES, F_OK)) + tst_brk(TCONF, "Huge page is not supported."); +} + +#endif /* THP_H */ diff --git a/testcases/kernel/mem/thp/thp01.c b/testcases/kernel/mem/thp/thp01.c index 69825b0f9..bdb2c54db 100644 --- a/testcases/kernel/mem/thp/thp01.c +++ b/testcases/kernel/mem/thp/thp01.c @@ -38,7 +38,6 @@ #include <stdlib.h> #include <unistd.h> #include "tst_test.h" -#include "mem.h" #include "tst_minmax.h" #define ARGS_SZ (256 * 32) diff --git a/testcases/kernel/mem/thp/thp02.c b/testcases/kernel/mem/thp/thp02.c index 56568d1d1..c6f9d2fd5 100644 --- a/testcases/kernel/mem/thp/thp02.c +++ b/testcases/kernel/mem/thp/thp02.c @@ -38,7 +38,8 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> -#include "mem.h" +#include "tst_test.h" +#include "thp.h" static int ps; static long hps, size; diff --git a/testcases/kernel/mem/thp/thp03.c b/testcases/kernel/mem/thp/thp03.c index 839efcb0e..e8d22669e 100644 --- a/testcases/kernel/mem/thp/thp03.c +++ b/testcases/kernel/mem/thp/thp03.c @@ -36,7 +36,8 @@ #include <stdlib.h> #include <string.h> #include <errno.h> -#include "mem.h" +#include "tst_test.h" +#include "thp.h" #include "lapi/mmap.h" static void thp_test(void); @@ -83,7 +84,7 @@ static void setup(void) check_hugepage(); - hugepage_size = SAFE_READ_MEMINFO("Hugepagesize:") * KB; + hugepage_size = SAFE_READ_MEMINFO("Hugepagesize:") * TST_KB; unaligned_size = hugepage_size * 4 - 1; page_size = SAFE_SYSCONF(_SC_PAGESIZE); }
These were only used in thp testcases, also after this the mem library is no longer needed in thp tests so it's removed from the Makefile. Also note that PATH_HUGEPAGES is defined in tst_hugepage.h which is included by tst_test.h so we can just remove this macro. Signed-off-by: Cyril Hrubis <chrubis@suse.cz> --- testcases/kernel/mem/include/mem.h | 7 ------- testcases/kernel/mem/lib/mem.c | 6 ------ testcases/kernel/mem/thp/Makefile | 2 +- testcases/kernel/mem/thp/thp.h | 17 +++++++++++++++++ testcases/kernel/mem/thp/thp01.c | 1 - testcases/kernel/mem/thp/thp02.c | 3 ++- testcases/kernel/mem/thp/thp03.c | 5 +++-- 7 files changed, 23 insertions(+), 18 deletions(-) create mode 100644 testcases/kernel/mem/thp/thp.h