[PATCH-next 1/5] lib/test_vmalloc.c: remove two kvfree_rcu() tests

[PATCH-next 1/5] lib/test_vmalloc.c: remove two kvfree_rcu() tests

[Uladzislau Rezki (Sony)]

Remove two test cases related to kvfree_rcu() and SLAB. Those are
considered as redundant now, because similar test functionality
has recently been introduced in the "rcuscale" RCU test-suite.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 lib/test_vmalloc.c | 40 ----------------------------------------
 1 file changed, 40 deletions(-)

diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c
index 5cf2fe9aab9e..4eb6abdaa74e 100644
--- a/lib/test_vmalloc.c
+++ b/lib/test_vmalloc.c
@@ -47,8 +47,6 @@ __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: kvfree_rcu_1_arg_slab_test\n"
-		"\t\tid: 2048, name: kvfree_rcu_2_arg_slab_test\n"
 		/* Add a new test case description here. */
 );
 
@@ -363,42 +361,6 @@ kvfree_rcu_2_arg_vmalloc_test(void)
 	return 0;
 }
 
-static int
-kvfree_rcu_1_arg_slab_test(void)
-{
-	struct test_kvfree_rcu *p;
-	int i;
-
-	for (i = 0; i < test_loop_count; i++) {
-		p = kmalloc(sizeof(*p), GFP_KERNEL);
-		if (!p)
-			return -1;
-
-		p->array[0] = 'a';
-		kvfree_rcu(p);
-	}
-
-	return 0;
-}
-
-static int
-kvfree_rcu_2_arg_slab_test(void)
-{
-	struct test_kvfree_rcu *p;
-	int i;
-
-	for (i = 0; i < test_loop_count; i++) {
-		p = kmalloc(sizeof(*p), GFP_KERNEL);
-		if (!p)
-			return -1;
-
-		p->array[0] = 'a';
-		kvfree_rcu(p, rcu);
-	}
-
-	return 0;
-}
-
 struct test_case_desc {
 	const char *test_name;
 	int (*test_func)(void);
@@ -415,8 +377,6 @@ 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 },
-	{ "kvfree_rcu_1_arg_slab_test", kvfree_rcu_1_arg_slab_test },
-	{ "kvfree_rcu_2_arg_slab_test", kvfree_rcu_2_arg_slab_test },
 	/* Add a new test case here. */
 };
 
-- 
2.20.1

[PATCH-next 2/5] lib/test_vmalloc.c: add a new 'nr_threads' parameter

[Uladzislau Rezki (Sony)]

By using this parameter we can specify how many workers are
created to perform vmalloc tests. By default it is one CPU.
The maximum value is set to 1024.

As a result of this change a 'single_cpu_test' one becomes
obsolete, therefore it is no longer needed.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 lib/test_vmalloc.c | 88 +++++++++++++++++++++-------------------------
 1 file changed, 40 insertions(+), 48 deletions(-)

diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c
index 4eb6abdaa74e..d337985e4c5e 100644
--- a/lib/test_vmalloc.c
+++ b/lib/test_vmalloc.c
@@ -23,8 +23,8 @@
 	module_param(name, type, 0444);			\
 	MODULE_PARM_DESC(name, msg)				\
 
-__param(bool, single_cpu_test, false,
-	"Use single first online CPU to run tests");
+__param(int, nr_threads, 0,
+	"Number of workers to perform tests(min: 1 max: 1024)");
 
 __param(bool, sequential_test_order, false,
 	"Use sequential stress tests order");
@@ -50,13 +50,6 @@ __param(int, run_test_mask, INT_MAX,
 		/* Add a new test case description here. */
 );
 
-/*
- * Depends on single_cpu_test parameter. If it is true, then
- * use first online CPU to trigger a test on, otherwise go with
- * all online CPUs.
- */
-static cpumask_t cpus_run_test_mask = CPU_MASK_NONE;
-
 /*
  * Read write semaphore for synchronization of setup
  * phase that is done in main thread and workers.
@@ -386,16 +379,13 @@ struct test_case_data {
 	u64 time;
 };
 
-/* Split it to get rid of: WARNING: line over 80 characters */
-static struct test_case_data
-	per_cpu_test_data[NR_CPUS][ARRAY_SIZE(test_case_array)];
-
 static struct test_driver {
 	struct task_struct *task;
+	struct test_case_data data[ARRAY_SIZE(test_case_array)];
+
 	unsigned long start;
 	unsigned long stop;
-	int cpu;
-} per_cpu_test_driver[NR_CPUS];
+} *tdriver;
 
 static void shuffle_array(int *arr, int n)
 {
@@ -423,9 +413,6 @@ static int test_func(void *private)
 	ktime_t kt;
 	u64 delta;
 
-	if (set_cpus_allowed_ptr(current, cpumask_of(t->cpu)) < 0)
-		pr_err("Failed to set affinity to %d CPU\n", t->cpu);
-
 	for (i = 0; i < ARRAY_SIZE(test_case_array); i++)
 		random_array[i] = i;
 
@@ -450,9 +437,9 @@ static int test_func(void *private)
 		kt = ktime_get();
 		for (j = 0; j < test_repeat_count; j++) {
 			if (!test_case_array[index].test_func())
-				per_cpu_test_data[t->cpu][index].test_passed++;
+				t->data[index].test_passed++;
 			else
-				per_cpu_test_data[t->cpu][index].test_failed++;
+				t->data[index].test_failed++;
 		}
 
 		/*
@@ -461,7 +448,7 @@ static int test_func(void *private)
 		delta = (u64) ktime_us_delta(ktime_get(), kt);
 		do_div(delta, (u32) test_repeat_count);
 
-		per_cpu_test_data[t->cpu][index].time = delta;
+		t->data[index].time = delta;
 	}
 	t->stop = get_cycles();
 
@@ -477,53 +464,56 @@ static int test_func(void *private)
 	return 0;
 }
 
-static void
+static int
 init_test_configurtion(void)
 {
 	/*
-	 * Reset all data of all CPUs.
+	 * A maximum number of workers is defined as hard-coded
+	 * value and set to 1024. We add such gap just in case
+	 * and for potential heavy stressing.
 	 */
-	memset(per_cpu_test_data, 0, sizeof(per_cpu_test_data));
+	nr_threads = clamp(nr_threads, 1, 1024);
 
-	if (single_cpu_test)
-		cpumask_set_cpu(cpumask_first(cpu_online_mask),
-			&cpus_run_test_mask);
-	else
-		cpumask_and(&cpus_run_test_mask, cpu_online_mask,
-			cpu_online_mask);
+	/* Allocate the space for test instances. */
+	tdriver = kcalloc(nr_threads, sizeof(*tdriver), GFP_KERNEL);
+	if (tdriver == NULL)
+		return -1;
 
 	if (test_repeat_count <= 0)
 		test_repeat_count = 1;
 
 	if (test_loop_count <= 0)
 		test_loop_count = 1;
+
+	return 0;
 }
 
 static void do_concurrent_test(void)
 {
-	int cpu, ret;
+	int i, ret;
 
 	/*
 	 * Set some basic configurations plus sanity check.
 	 */
-	init_test_configurtion();
+	ret = init_test_configurtion();
+	if (ret < 0)
+		return;
 
 	/*
 	 * Put on hold all workers.
 	 */
 	down_write(&prepare_for_test_rwsem);
 
-	for_each_cpu(cpu, &cpus_run_test_mask) {
-		struct test_driver *t = &per_cpu_test_driver[cpu];
+	for (i = 0; i < nr_threads; i++) {
+		struct test_driver *t = &tdriver[i];
 
-		t->cpu = cpu;
-		t->task = kthread_run(test_func, t, "vmalloc_test/%d", cpu);
+		t->task = kthread_run(test_func, t, "vmalloc_test/%d", i);
 
 		if (!IS_ERR(t->task))
 			/* Success. */
 			atomic_inc(&test_n_undone);
 		else
-			pr_err("Failed to start kthread for %d CPU\n", cpu);
+			pr_err("Failed to start %d kthread\n", i);
 	}
 
 	/*
@@ -541,29 +531,31 @@ static void do_concurrent_test(void)
 		ret = wait_for_completion_timeout(&test_all_done_comp, HZ);
 	} while (!ret);
 
-	for_each_cpu(cpu, &cpus_run_test_mask) {
-		struct test_driver *t = &per_cpu_test_driver[cpu];
-		int i;
+	for (i = 0; i < nr_threads; i++) {
+		struct test_driver *t = &tdriver[i];
+		int j;
 
 		if (!IS_ERR(t->task))
 			kthread_stop(t->task);
 
-		for (i = 0; i < ARRAY_SIZE(test_case_array); i++) {
-			if (!((run_test_mask & (1 << i)) >> i))
+		for (j = 0; j < ARRAY_SIZE(test_case_array); j++) {
+			if (!((run_test_mask & (1 << j)) >> j))
 				continue;
 
 			pr_info(
 				"Summary: %s passed: %d failed: %d repeat: %d loops: %d avg: %llu usec\n",
-				test_case_array[i].test_name,
-				per_cpu_test_data[cpu][i].test_passed,
-				per_cpu_test_data[cpu][i].test_failed,
+				test_case_array[j].test_name,
+				t->data[j].test_passed,
+				t->data[j].test_failed,
 				test_repeat_count, test_loop_count,
-				per_cpu_test_data[cpu][i].time);
+				t->data[j].time);
 		}
 
-		pr_info("All test took CPU%d=%lu cycles\n",
-			cpu, t->stop - t->start);
+		pr_info("All test took worker%d=%lu cycles\n",
+			i, t->stop - t->start);
 	}
+
+	kfree(tdriver);
 }
 
 static int vmalloc_test_init(void)
-- 
2.20.1

[PATCH-next 3/5] vm/test_vmalloc.sh: adapt for updated driver interface

[Uladzislau Rezki (Sony)]

A 'single_cpu_test' parameter is odd and it does not exist
anymore. Instead there was introduced a 'nr_threads' one.
If it is not set it behaves as the former parameter.

That is why update a "stress mode" according to this change
specifying number of workers which are equal to number of CPUs.
Also update an output of help message based on a new interface.

CC: linux-kselftest@vger.kernel.org
CC: Shuah Khan <shuah@kernel.org>
Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 tools/testing/selftests/vm/test_vmalloc.sh | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/vm/test_vmalloc.sh b/tools/testing/selftests/vm/test_vmalloc.sh
index 06d2bb109f06..d73b846736f1 100755
--- a/tools/testing/selftests/vm/test_vmalloc.sh
+++ b/tools/testing/selftests/vm/test_vmalloc.sh
@@ -11,6 +11,7 @@
 
 TEST_NAME="vmalloc"
 DRIVER="test_${TEST_NAME}"
+NUM_CPUS=`grep -c ^processor /proc/cpuinfo`
 
 # 1 if fails
 exitcode=1
@@ -22,9 +23,9 @@ ksft_skip=4
 # Static templates for performance, stressing and smoke tests.
 # Also it is possible to pass any supported parameters manualy.
 #
-PERF_PARAM="single_cpu_test=1 sequential_test_order=1 test_repeat_count=3"
-SMOKE_PARAM="single_cpu_test=1 test_loop_count=10000 test_repeat_count=10"
-STRESS_PARAM="test_repeat_count=20"
+PERF_PARAM="sequential_test_order=1 test_repeat_count=3"
+SMOKE_PARAM="test_loop_count=10000 test_repeat_count=10"
+STRESS_PARAM="nr_threads=$NUM_CPUS test_repeat_count=20"
 
 check_test_requirements()
 {
@@ -58,8 +59,8 @@ run_perfformance_check()
 
 run_stability_check()
 {
-	echo "Run stability tests. In order to stress vmalloc subsystem we run"
-	echo "all available test cases on all available CPUs simultaneously."
+	echo "Run stability tests. In order to stress vmalloc subsystem all"
+	echo "available test cases are run by NUM_CPUS workers simultaneously."
 	echo "It will take time, so be patient."
 
 	modprobe $DRIVER $STRESS_PARAM > /dev/null 2>&1
@@ -92,17 +93,17 @@ usage()
 	echo "# Shows help message"
 	echo "./${DRIVER}.sh"
 	echo
-	echo "# Runs 1 test(id_1), repeats it 5 times on all online CPUs"
-	echo "./${DRIVER}.sh run_test_mask=1 test_repeat_count=5"
+	echo "# Runs 1 test(id_1), repeats it 5 times by NUM_CPUS workers"
+	echo "./${DRIVER}.sh nr_threads=$NUM_CPUS run_test_mask=1 test_repeat_count=5"
 	echo
 	echo -n "# Runs 4 tests(id_1|id_2|id_4|id_16) on one CPU with "
 	echo "sequential order"
-	echo -n "./${DRIVER}.sh single_cpu_test=1 sequential_test_order=1 "
+	echo -n "./${DRIVER}.sh sequential_test_order=1 "
 	echo "run_test_mask=23"
 	echo
-	echo -n "# Runs all tests on all online CPUs, shuffled order, repeats "
+	echo -n "# Runs all tests by NUM_CPUS workers, shuffled order, repeats "
 	echo "20 times"
-	echo "./${DRIVER}.sh test_repeat_count=20"
+	echo "./${DRIVER}.sh nr_threads=$NUM_CPUS test_repeat_count=20"
 	echo
 	echo "# Performance analysis"
 	echo "./${DRIVER}.sh performance"
-- 
2.20.1

[PATCH-next 4/5] mm/vmalloc: refactor the preloading loagic

[Uladzislau Rezki (Sony)]

Instead of keeping open-coded style, move the code related to
preloading into a separate function. Therefore introduce the
preload_this_cpu_lock() routine that prelaods a current CPU
with one extra vmap_area object.

There is no functional change as a result of this patch.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 mm/vmalloc.c | 60 +++++++++++++++++++++++-----------------------------
 1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 8b564f91a610..093c7e034ca2 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1430,6 +1430,29 @@ static void free_vmap_area(struct vmap_area *va)
 	spin_unlock(&free_vmap_area_lock);
 }
 
+static inline void
+preload_this_cpu_lock(spinlock_t *lock, gfp_t gfp_mask, int node)
+{
+	struct vmap_area *va = NULL;
+
+	/*
+	 * Preload this CPU with one extra vmap_area object. It is used
+	 * when fit type of free area is NE_FIT_TYPE. It guarantees that
+	 * a CPU that does an allocation is preloaded.
+	 *
+	 * We do it in non-atomic context, thus it allows us to use more
+	 * permissive allocation masks to be more stable under low memory
+	 * condition and high memory pressure.
+	 */
+	if (!this_cpu_read(ne_fit_preload_node))
+		va = kmem_cache_alloc_node(vmap_area_cachep, gfp_mask, node);
+
+	spin_lock(lock);
+
+	if (va && __this_cpu_cmpxchg(ne_fit_preload_node, NULL, va))
+		kmem_cache_free(vmap_area_cachep, va);
+}
+
 /*
  * Allocate a region of KVA of the specified size and alignment, within the
  * vstart and vend.
@@ -1439,7 +1462,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
 				unsigned long vstart, unsigned long vend,
 				int node, gfp_t gfp_mask)
 {
-	struct vmap_area *va, *pva;
+	struct vmap_area *va;
 	unsigned long addr;
 	int purged = 0;
 	int ret;
@@ -1465,43 +1488,14 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
 	kmemleak_scan_area(&va->rb_node, SIZE_MAX, gfp_mask);
 
 retry:
-	/*
-	 * Preload this CPU with one extra vmap_area object. It is used
-	 * when fit type of free area is NE_FIT_TYPE. Please note, it
-	 * does not guarantee that an allocation occurs on a CPU that
-	 * is preloaded, instead we minimize the case when it is not.
-	 * It can happen because of cpu migration, because there is a
-	 * race until the below spinlock is taken.
-	 *
-	 * The preload is done in non-atomic context, thus it allows us
-	 * to use more permissive allocation masks to be more stable under
-	 * low memory condition and high memory pressure. In rare case,
-	 * if not preloaded, GFP_NOWAIT is used.
-	 *
-	 * Set "pva" to NULL here, because of "retry" path.
-	 */
-	pva = NULL;
-
-	if (!this_cpu_read(ne_fit_preload_node))
-		/*
-		 * Even if it fails we do not really care about that.
-		 * Just proceed as it is. If needed "overflow" path
-		 * will refill the cache we allocate from.
-		 */
-		pva = kmem_cache_alloc_node(vmap_area_cachep, gfp_mask, node);
-
-	spin_lock(&free_vmap_area_lock);
-
-	if (pva && __this_cpu_cmpxchg(ne_fit_preload_node, NULL, pva))
-		kmem_cache_free(vmap_area_cachep, pva);
+	preload_this_cpu_lock(&free_vmap_area_lock, gfp_mask, node);
+	addr = __alloc_vmap_area(size, align, vstart, vend);
+	spin_unlock(&free_vmap_area_lock);
 
 	/*
 	 * If an allocation fails, the "vend" address is
 	 * returned. Therefore trigger the overflow path.
 	 */
-	addr = __alloc_vmap_area(size, align, vstart, vend);
-	spin_unlock(&free_vmap_area_lock);
-
 	if (unlikely(addr == vend))
 		goto overflow;
 
-- 
2.20.1

[PATCH-next 5/5] mm/vmalloc: remove an empty line

[Uladzislau Rezki (Sony)]

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 mm/vmalloc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 093c7e034ca2..1e643280cbcf 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1503,7 +1503,6 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
 	va->va_end = addr + size;
 	va->vm = NULL;
 
-
 	spin_lock(&vmap_area_lock);
 	insert_vmap_area(va, &vmap_area_root, &vmap_area_list);
 	spin_unlock(&vmap_area_lock);
-- 
2.20.1

Re: [PATCH-next 5/5] mm/vmalloc: remove an empty line

[Souptick Joarder]

On Sat, Apr 3, 2021 at 1:53 AM Uladzislau Rezki (Sony) <urezki@gmail.com> wrote:
>
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>

How about merging it with patch [4/5] ?

> ---
>  mm/vmalloc.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 093c7e034ca2..1e643280cbcf 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1503,7 +1503,6 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
>         va->va_end = addr + size;
>         va->vm = NULL;
>
> -
>         spin_lock(&vmap_area_lock);
>         insert_vmap_area(va, &vmap_area_root, &vmap_area_list);
>         spin_unlock(&vmap_area_lock);
> --
> 2.20.1
>
>

Re: [PATCH-next 5/5] mm/vmalloc: remove an empty line

[Uladzislau Rezki]

> On Sat, Apr 3, 2021 at 1:53 AM Uladzislau Rezki (Sony) <urezki@gmail.com> wrote:
> >
> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> 
> How about merging it with patch [4/5] ?
> 
I had in mind such concern. Yes we can do a squashing with [4/5].
If there are other comments i will rework whole series if not we
can ask Andrew to merge it.

Thank you.

--
Vlad Rezki

Re: [PATCH-next 2/5] lib/test_vmalloc.c: add a new 'nr_threads' parameter

[Andrew Morton]

On Fri,  2 Apr 2021 22:22:34 +0200 "Uladzislau Rezki (Sony)" <urezki@gmail.com> wrote:

> By using this parameter we can specify how many workers are
> created to perform vmalloc tests. By default it is one CPU.
> The maximum value is set to 1024.
> 
> As a result of this change a 'single_cpu_test' one becomes
> obsolete, therefore it is no longer needed.
> 

Why limit to 1024?  Maybe testers want more - what's the downside to
permitting that?

We may need to replaced that kcalloc() with kmvalloc() though...

Re: [PATCH-next 2/5] lib/test_vmalloc.c: add a new 'nr_threads' parameter

[Uladzislau Rezki]

> On Fri,  2 Apr 2021 22:22:34 +0200 "Uladzislau Rezki (Sony)" <urezki@gmail.com> wrote:
> 
> > By using this parameter we can specify how many workers are
> > created to perform vmalloc tests. By default it is one CPU.
> > The maximum value is set to 1024.
> > 
> > As a result of this change a 'single_cpu_test' one becomes
> > obsolete, therefore it is no longer needed.
> > 
> 
> Why limit to 1024?  Maybe testers want more - what's the downside to
> permitting that?
>
I was thinking mainly about if a tester issues enormous number of kthreads,
so a system is not able to handle it. Therefore i clamped that value to 1024.

From the other hand we can give more wide permissions, in that case a
user should think more carefully about what is passed. For example we
can limit max value by USHRT_MAX what is 65536.

> 
> We may need to replaced that kcalloc() with kmvalloc() though...
>
Yep. If we limit to USHRT_MAX, the maximum amount of memory for
internal data would be ~12MB. Something like below:

diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c
index d337985e4c5e..a5103e3461bf 100644
--- a/lib/test_vmalloc.c
+++ b/lib/test_vmalloc.c
@@ -24,7 +24,7 @@
        MODULE_PARM_DESC(name, msg)                             \

 __param(int, nr_threads, 0,
-       "Number of workers to perform tests(min: 1 max: 1024)");
+       "Number of workers to perform tests(min: 1 max: 65536)");

 __param(bool, sequential_test_order, false,
        "Use sequential stress tests order");
@@ -469,13 +469,13 @@ init_test_configurtion(void)
 {
        /*
         * A maximum number of workers is defined as hard-coded
-        * value and set to 1024. We add such gap just in case
+        * value and set to 65536. We add such gap just in case
         * and for potential heavy stressing.
         */
-       nr_threads = clamp(nr_threads, 1, 1024);
+       nr_threads = clamp(nr_threads, 1, 65536);

        /* Allocate the space for test instances. */
-       tdriver = kcalloc(nr_threads, sizeof(*tdriver), GFP_KERNEL);
+       tdriver = kvcalloc(nr_threads, sizeof(*tdriver), GFP_KERNEL);
        if (tdriver == NULL)
                return -1;

@@ -555,7 +555,7 @@ static void do_concurrent_test(void)
                        i, t->stop - t->start);
        }

-       kfree(tdriver);
+       kvfree(tdriver);
 }

 static int vmalloc_test_init(void)

Does it sound reasonable for you?

--
Vlad Rezki

Re: [PATCH-next 2/5] lib/test_vmalloc.c: add a new 'nr_threads' parameter

[Andrew Morton]

On Sat, 3 Apr 2021 14:31:43 +0200 Uladzislau Rezki <urezki@gmail.com> wrote:

> > 
> > We may need to replaced that kcalloc() with kmvalloc() though...
> >
> Yep. If we limit to USHRT_MAX, the maximum amount of memory for
> internal data would be ~12MB. Something like below:
> 
> diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c
> index d337985e4c5e..a5103e3461bf 100644
> --- a/lib/test_vmalloc.c
> +++ b/lib/test_vmalloc.c
> @@ -24,7 +24,7 @@
>         MODULE_PARM_DESC(name, msg)                             \
> 
>  __param(int, nr_threads, 0,
> -       "Number of workers to perform tests(min: 1 max: 1024)");
> +       "Number of workers to perform tests(min: 1 max: 65536)");
> 
>  __param(bool, sequential_test_order, false,
>         "Use sequential stress tests order");
> @@ -469,13 +469,13 @@ init_test_configurtion(void)
>  {
>         /*
>          * A maximum number of workers is defined as hard-coded
> -        * value and set to 1024. We add such gap just in case
> +        * value and set to 65536. We add such gap just in case
>          * and for potential heavy stressing.
>          */
> -       nr_threads = clamp(nr_threads, 1, 1024);
> +       nr_threads = clamp(nr_threads, 1, 65536);
> 
>         /* Allocate the space for test instances. */
> -       tdriver = kcalloc(nr_threads, sizeof(*tdriver), GFP_KERNEL);
> +       tdriver = kvcalloc(nr_threads, sizeof(*tdriver), GFP_KERNEL);
>         if (tdriver == NULL)
>                 return -1;
> 
> @@ -555,7 +555,7 @@ static void do_concurrent_test(void)
>                         i, t->stop - t->start);
>         }
> 
> -       kfree(tdriver);
> +       kvfree(tdriver);
>  }
> 
>  static int vmalloc_test_init(void)
> 
> Does it sound reasonable for you?

I think so.  It's a test thing so let's give testers more flexibility,
remembering that they don't need as much protection from their own
mistakes.

Re: [PATCH-next 2/5] lib/test_vmalloc.c: add a new 'nr_threads' parameter

[Uladzislau Rezki]

On Mon, Apr 05, 2021 at 07:39:20PM -0700, Andrew Morton wrote:
> On Sat, 3 Apr 2021 14:31:43 +0200 Uladzislau Rezki <urezki@gmail.com> wrote:
> 
> > > 
> > > We may need to replaced that kcalloc() with kmvalloc() though...
> > >
> > Yep. If we limit to USHRT_MAX, the maximum amount of memory for
> > internal data would be ~12MB. Something like below:
> > 
> > diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c
> > index d337985e4c5e..a5103e3461bf 100644
> > --- a/lib/test_vmalloc.c
> > +++ b/lib/test_vmalloc.c
> > @@ -24,7 +24,7 @@
> >         MODULE_PARM_DESC(name, msg)                             \
> > 
> >  __param(int, nr_threads, 0,
> > -       "Number of workers to perform tests(min: 1 max: 1024)");
> > +       "Number of workers to perform tests(min: 1 max: 65536)");
> > 
> >  __param(bool, sequential_test_order, false,
> >         "Use sequential stress tests order");
> > @@ -469,13 +469,13 @@ init_test_configurtion(void)
> >  {
> >         /*
> >          * A maximum number of workers is defined as hard-coded
> > -        * value and set to 1024. We add such gap just in case
> > +        * value and set to 65536. We add such gap just in case
> >          * and for potential heavy stressing.
> >          */
> > -       nr_threads = clamp(nr_threads, 1, 1024);
> > +       nr_threads = clamp(nr_threads, 1, 65536);
> > 
> >         /* Allocate the space for test instances. */
> > -       tdriver = kcalloc(nr_threads, sizeof(*tdriver), GFP_KERNEL);
> > +       tdriver = kvcalloc(nr_threads, sizeof(*tdriver), GFP_KERNEL);
> >         if (tdriver == NULL)
> >                 return -1;
> > 
> > @@ -555,7 +555,7 @@ static void do_concurrent_test(void)
> >                         i, t->stop - t->start);
> >         }
> > 
> > -       kfree(tdriver);
> > +       kvfree(tdriver);
> >  }
> > 
> >  static int vmalloc_test_init(void)
> > 
> > Does it sound reasonable for you?
> 
> I think so.  It's a test thing so let's give testers more flexibility,
> remembering that they don't need as much protection from their own
> mistakes.
> 
OK. I will send one more extra patch then.

--
Vlad Rezki