diff mbox series

[bpf-next,v5,3/6] selftests/vm: extend test_vmalloc to test execmem_* APIs

Message ID 20221128190245.2337461-4-song@kernel.org (mailing list archive)
State New
Headers show
Series execmem_alloc for BPF programs | expand

Commit Message

Song Liu Nov. 28, 2022, 7:02 p.m. UTC
Add logic to test execmem_[alloc|fill|free] in test_vmalloc.c.
No need to change tools/testing/selftests/vm/test_vmalloc.sh.

Gate the export of execmem_* with DEBUG_TEST_VMALLOC_EXEMEM_ALLOC so
they are only exported when the developers are running tests.

Signed-off-by: Song Liu <song@kernel.org>
---
 lib/test_vmalloc.c | 35 +++++++++++++++++++++++++++++++++++
 mm/vmalloc.c       | 12 ++++++++++++
 2 files changed, 47 insertions(+)

Comments

Christoph Hellwig Nov. 29, 2022, 8:35 a.m. UTC | #1
> +#if DEBUG_TEST_VMALLOC_EXEMEM_ALLOC
> +EXPORT_SYMBOL_GPL(execmem_alloc);
> +#endif

> +#if DEBUG_TEST_VMALLOC_EXEMEM_ALLOC
> +EXPORT_SYMBOL_GPL(execmem_fill);
> +#endif

> +#if DEBUG_TEST_VMALLOC_EXEMEM_ALLOC
> +EXPORT_SYMBOL_GPL(execmem_free);
> +#endif

Still NAK.  These symbols never have any business being exported
ever.  Just force the test to be built-in if you want to test this
functionality.
Song Liu Nov. 29, 2022, 5:31 p.m. UTC | #2
On Tue, Nov 29, 2022 at 12:35 AM Christoph Hellwig <hch@lst.de> wrote:
>
> > +#if DEBUG_TEST_VMALLOC_EXEMEM_ALLOC
> > +EXPORT_SYMBOL_GPL(execmem_alloc);
> > +#endif
>
> > +#if DEBUG_TEST_VMALLOC_EXEMEM_ALLOC
> > +EXPORT_SYMBOL_GPL(execmem_fill);
> > +#endif
>
> > +#if DEBUG_TEST_VMALLOC_EXEMEM_ALLOC
> > +EXPORT_SYMBOL_GPL(execmem_free);
> > +#endif
>
> Still NAK.  These symbols never have any business being exported
> ever.  Just force the test to be built-in if you want to test this
> functionality.

OK. I knew this was really ugly. I just want to know your thoughts on it.

I guess we can just drop 3/6 of the set, and everything else would just work.

Thanks,
Song
Christoph Hellwig Nov. 29, 2022, 8:22 p.m. UTC | #3
On Tue, Nov 29, 2022 at 09:31:24AM -0800, Song Liu wrote:
> I guess we can just drop 3/6 of the set, and everything else would just work.

Yes.
diff mbox series

Patch

diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c
index cf7780572f5b..9c78f0693f59 100644
--- a/lib/test_vmalloc.c
+++ b/lib/test_vmalloc.c
@@ -50,6 +50,7 @@  __param(int, run_test_mask, INT_MAX,
 		"\t\tid: 128,  name: pcpu_alloc_test\n"
 		"\t\tid: 256,  name: kvfree_rcu_1_arg_vmalloc_test\n"
 		"\t\tid: 512,  name: kvfree_rcu_2_arg_vmalloc_test\n"
+		"\t\tid: 1024, name: execmem_alloc_test\n"
 		/* Add a new test case description here. */
 );
 
@@ -352,6 +353,39 @@  kvfree_rcu_2_arg_vmalloc_test(void)
 	return 0;
 }
 
+/* This should match the define in vmalloc.c */
+#define DEBUG_TEST_VMALLOC_EXEMEM_ALLOC 0
+
+static int
+execmem_alloc_test(void)
+{
+	int i;
+
+	for (i = 0; i < test_loop_count; i++) {
+#if DEBUG_TEST_VMALLOC_EXEMEM_ALLOC
+		/* allocate variable size, up to 64kB */
+		size_t size = (i % 1024 + 1) * 64;
+		void *p, *tmp;
+
+		p = execmem_alloc(size, 64);
+		if (!p)
+			return -1;
+
+		tmp = execmem_fill(p, "a", 1);
+		if (tmp != p)
+			return -1;
+
+		tmp = execmem_fill(p + size - 1, "b", 1);
+		if (tmp != p + size - 1)
+			return -1;
+
+		execmem_free(p);
+#endif
+	}
+
+	return 0;
+}
+
 struct test_case_desc {
 	const char *test_name;
 	int (*test_func)(void);
@@ -368,6 +402,7 @@  static struct test_case_desc test_case_array[] = {
 	{ "pcpu_alloc_test", pcpu_alloc_test },
 	{ "kvfree_rcu_1_arg_vmalloc_test", kvfree_rcu_1_arg_vmalloc_test },
 	{ "kvfree_rcu_2_arg_vmalloc_test", kvfree_rcu_2_arg_vmalloc_test },
+	{ "execmem_alloc_test", execmem_alloc_test },
 	/* Add a new test case here. */
 };
 
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 1f7f2134f9bd..95ee8282b66c 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -3387,6 +3387,9 @@  static void move_vmap_to_free_text_tree(void *addr)
 	spin_unlock(&free_text_area_lock);
 }
 
+/* This should match the define in test_vmalloc.c */
+#define DEBUG_TEST_VMALLOC_EXEMEM_ALLOC 0
+
 /**
  * execmem_alloc - allocate virtually contiguous RO+X memory
  * @size:    allocation size
@@ -3459,6 +3462,9 @@  void *execmem_alloc(unsigned long size, unsigned long align)
 	kmem_cache_free(vmap_area_cachep, va);
 	return NULL;
 }
+#if DEBUG_TEST_VMALLOC_EXEMEM_ALLOC
+EXPORT_SYMBOL_GPL(execmem_alloc);
+#endif
 
 void __weak *arch_fill_execmem(void *dst, void *src, size_t len)
 {
@@ -3510,6 +3516,9 @@  void *execmem_fill(void *dst, void *src, size_t len)
 	spin_unlock(&vmap_area_lock);
 	return ERR_PTR(-EINVAL);
 }
+#if DEBUG_TEST_VMALLOC_EXEMEM_ALLOC
+EXPORT_SYMBOL_GPL(execmem_fill);
+#endif
 
 static struct vm_struct *find_and_unlink_text_vm(unsigned long start, unsigned long end)
 {
@@ -3633,6 +3642,9 @@  void execmem_free(void *addr)
 out:
 	spin_unlock(&free_text_area_lock);
 }
+#if DEBUG_TEST_VMALLOC_EXEMEM_ALLOC
+EXPORT_SYMBOL_GPL(execmem_free);
+#endif
 
 /**
  * vmalloc_huge - allocate virtually contiguous memory, allow huge pages