diff mbox series

[06/13] testcases/kernel/mem: Move check_hugepage() + PATH_THP

Message ID 20241218184518.16190-7-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
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

Comments

Petr Vorel Dec. 27, 2024, 10:48 a.m. UTC | #1
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
Cyril Hrubis Feb. 7, 2025, 5 p.m. UTC | #2
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...
Petr Vorel Feb. 10, 2025, 9:16 a.m. UTC | #3
> 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 mbox series

Patch

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);
 }