| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20210215162705.GA45713@pc638.lan>
Date: Mon, 15 Feb 2021 17:27:05 +0100
From: Uladzislau Rezki <urezki@...il.com>
To: "Paul E. McKenney" <paulmck@...nel.org>
Cc: Uladzislau Rezki <urezki@...il.com>,
LKML <linux-kernel@...r.kernel.org>, RCU <rcu@...r.kernel.org>,
Michael Ellerman <mpe@...erman.id.au>,
Michal Hocko <mhocko@...e.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Daniel Axtens <dja@...ens.net>,
Frederic Weisbecker <frederic@...nel.org>,
Neeraj Upadhyay <neeraju@...eaurora.org>,
Joel Fernandes <joel@...lfernandes.org>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
"Theodore Y . Ts'o" <tytso@....edu>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Oleksiy Avramchenko <oleksiy.avramchenko@...ymobile.com>
Subject: Re: [PATCH 1/2] rcuscale: add kfree_rcu() single-argument scale test
On Tue, Feb 09, 2021 at 05:00:52PM -0800, Paul E. McKenney wrote:
> On Tue, Feb 09, 2021 at 09:13:43PM +0100, Uladzislau Rezki wrote:
> > On Thu, Feb 04, 2021 at 01:46:48PM -0800, Paul E. McKenney wrote:
> > > On Fri, Jan 29, 2021 at 09:05:04PM +0100, Uladzislau Rezki (Sony) wrote:
> > > > To stress and test a single argument of kfree_rcu() call, we
> > > > should to have a special coverage for it. We used to have it
> > > > in the test-suite related to vmalloc stressing. The reason is
> > > > the rcuscale is a correct place for RCU related things.
> > > >
> > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@...il.com>
> > >
> > > This is a great addition, but it would be even better if there was
> > > a way to say "test both in one run". One way to do this is to have
> > > torture_param() variables for both kfree_rcu_test_single and (say)
> > > kfree_rcu_test_double, both bool and both initialized to false. If both
> > > have the same value (false or true) both are tested, otherwise only
> > > the one with value true is tested. The value of this is that it allows
> > > testing of both options with one test.
> > >
> > Make sense to me :)
> >
> > >From ba083a543a123455455c81230b7b5a9aa2a9cb7f Mon Sep 17 00:00:00 2001
> > From: "Uladzislau Rezki (Sony)" <urezki@...il.com>
> > Date: Fri, 29 Jan 2021 19:51:27 +0100
> > Subject: [PATCH v2 1/1] rcuscale: add kfree_rcu() single-argument scale test
> >
> > To stress and test a single argument of kfree_rcu() call, we
> > should to have a special coverage for it. We used to have it
> > in the test-suite related to vmalloc stressing. The reason is
> > the rcuscale is a correct place for RCU related things.
> >
> > Therefore introduce two torture_param() variables, one is for
> > single-argument scale test and another one for double-argument
> > scale test.
> >
> > By default kfree_rcu_test_single and kfree_rcu_test_double are
> > initialized to false. If both have the same value (false or true)
> > both are tested in one run, otherwise only the one with value
> > true is tested. The value of this is that it allows testing of
> > both options with one test.
> >
> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@...il.com>
> > ---
> > kernel/rcu/rcuscale.c | 33 ++++++++++++++++++++++++++++-----
> > 1 file changed, 28 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
> > index 06491d5530db..0cde5c17f06c 100644
> > --- a/kernel/rcu/rcuscale.c
> > +++ b/kernel/rcu/rcuscale.c
> > @@ -625,6 +625,8 @@ rcu_scale_shutdown(void *arg)
> > torture_param(int, kfree_nthreads, -1, "Number of threads running loops of kfree_rcu().");
> > torture_param(int, kfree_alloc_num, 8000, "Number of allocations and frees done in an iteration.");
> > torture_param(int, kfree_loops, 10, "Number of loops doing kfree_alloc_num allocations and frees.");
> > +torture_param(int, kfree_rcu_test_single, 0, "Do we run a kfree_rcu() single-argument scale test?");
> > +torture_param(int, kfree_rcu_test_double, 0, "Do we run a kfree_rcu() double-argument scale test?");
>
> Good! But why int instead of bool?
>
> > static struct task_struct **kfree_reader_tasks;
> > static int kfree_nrealthreads;
> > @@ -641,7 +643,7 @@ kfree_scale_thread(void *arg)
> > {
> > int i, loop = 0;
> > long me = (long)arg;
> > - struct kfree_obj *alloc_ptr;
> > + struct kfree_obj *alloc_ptr[2];
>
> You lost me on this one...
>
> > u64 start_time, end_time;
> > long long mem_begin, mem_during = 0;
> >
> > @@ -665,12 +667,33 @@ kfree_scale_thread(void *arg)
> > mem_during = (mem_during + si_mem_available()) / 2;
> > }
> >
> > + // By default kfree_rcu_test_single and kfree_rcu_test_double are
> > + // initialized to false. If both have the same value (false or true)
> > + // both are tested in one run, otherwise only the one with value
> > + // true is tested.
> > for (i = 0; i < kfree_alloc_num; i++) {
> > - alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> > - if (!alloc_ptr)
> > - return -ENOMEM;
> > + alloc_ptr[0] = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> > + alloc_ptr[1] = (kfree_rcu_test_single == kfree_rcu_test_double) ?
> > + kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL) : NULL;
> > +
> > + // 0 ptr. is freed either over single or double argument.
> > + if (alloc_ptr[0]) {
> > + if (kfree_rcu_test_single == kfree_rcu_test_double ||
> > + kfree_rcu_test_single) {
> > + kfree_rcu(alloc_ptr[0]);
> > + } else {
> > + kfree_rcu(alloc_ptr[0], rh);
> > + }
> > + }
> > +
> > + // 1 ptr. is always freed over double argument.
> > + if (alloc_ptr[1])
> > + kfree_rcu(alloc_ptr[1], rh);
> >
> > - kfree_rcu(alloc_ptr, rh);
> > + if (!alloc_ptr[0] ||
> > + (kfree_rcu_test_single == kfree_rcu_test_double &&
> > + !alloc_ptr[1]))
> > + return -ENOMEM;
>
> How about something like this?
>
> bool krts = kfree_rcu_test_single || kfree_rcu_test_single == kfree_rcu_test_double;
> bool krtd = kfree_rcu_test_double || kfree_rcu_test_single == kfree_rcu_test_double;
> bool krtb = kfree_rcu_test_single && kfree_rcu_test_double;
> DEFINE_TORTURE_RANDOM(tr);
>
> ...
>
> alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> if (!alloc_ptr)
> return -ENOMEM;
> if (krtd || (krtb && (torture_random(&tr) & 0x800)))
> kfree_rcu(alloc_ptr, rh);
> else
> kfree_rcu(alloc_ptr);
>
> > }
> >
> > cond_resched();
>
Sorry for my late answer. I got it differently as we discussed offline.
Please see below the v3. Hope we are on the same page now :)
>From e7181d01c5f7fab3418d6df155ccf06aff189328 Mon Sep 17 00:00:00 2001
From: "Uladzislau Rezki (Sony)" <urezki@...il.com>
Date: Fri, 29 Jan 2021 19:51:27 +0100
Subject: [PATCH v3 1/1] rcuscale: add kfree_rcu() single-argument scale test
To stress and test a single argument of kfree_rcu() call, we
should to have a special coverage for it. We used to have it
in the test-suite related to vmalloc stressing. The reason is
the rcuscale is a correct place for RCU related things.
Therefore introduce two torture_param() variables, one is for
single-argument scale test and another one for double-argument
scale test.
By default kfree_rcu_test_single and kfree_rcu_test_double are
initialized to false. If both have the same value (false or true)
both are randomly tested, otherwise only the one with value true
is tested. The value of this is that it allows testing of both
options with one test.
Signed-off-by: Uladzislau Rezki (Sony) <urezki@...il.com>
---
kernel/rcu/rcuscale.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
index 06491d5530db..0fb540e2b22b 100644
--- a/kernel/rcu/rcuscale.c
+++ b/kernel/rcu/rcuscale.c
@@ -625,6 +625,8 @@ rcu_scale_shutdown(void *arg)
torture_param(int, kfree_nthreads, -1, "Number of threads running loops of kfree_rcu().");
torture_param(int, kfree_alloc_num, 8000, "Number of allocations and frees done in an iteration.");
torture_param(int, kfree_loops, 10, "Number of loops doing kfree_alloc_num allocations and frees.");
+torture_param(bool, kfree_rcu_test_single, false, "Do we run a kfree_rcu() single-argument scale test?");
+torture_param(bool, kfree_rcu_test_double, false, "Do we run a kfree_rcu() double-argument scale test?");
static struct task_struct **kfree_reader_tasks;
static int kfree_nrealthreads;
@@ -644,10 +646,13 @@ kfree_scale_thread(void *arg)
struct kfree_obj *alloc_ptr;
u64 start_time, end_time;
long long mem_begin, mem_during = 0;
+ bool kfree_rcu_test_both;
+ DEFINE_TORTURE_RANDOM(tr);
VERBOSE_SCALEOUT_STRING("kfree_scale_thread task started");
set_cpus_allowed_ptr(current, cpumask_of(me % nr_cpu_ids));
set_user_nice(current, MAX_NICE);
+ kfree_rcu_test_both = (kfree_rcu_test_single == kfree_rcu_test_double);
start_time = ktime_get_mono_fast_ns();
@@ -670,7 +675,15 @@ kfree_scale_thread(void *arg)
if (!alloc_ptr)
return -ENOMEM;
- kfree_rcu(alloc_ptr, rh);
+ // By default kfree_rcu_test_single and kfree_rcu_test_double are
+ // initialized to false. If both have the same value (false or true)
+ // both are randomly tested, otherwise only the one with value true
+ // is tested.
+ if ((kfree_rcu_test_single && !kfree_rcu_test_double) ||
+ (kfree_rcu_test_both && torture_random(&tr) & 0x800))
+ kfree_rcu(alloc_ptr);
+ else
+ kfree_rcu(alloc_ptr, rh);
}
cond_resched();
--
2.20.1
>
> And this is why I was so confused about the earlier OOMs. We need
> something stronger, and not here, but rather inside the above loop.
> The function rcu_torture_fwd_prog_cond_resched() does what is needed,
> which needs to be moved to kernel/torture.c or to be a static inline in
> include/linux/torture.h so that it can be invoked here.
>
> The flooding we are looking to emulate has to have frequent trips into
> userspace, and rcu_torture_fwd_prog_cond_resched() is the way that we
> emulate those trips.
>
> But please make this change be a separate patch.
>
OK. I will have a look at it!
--
Vlad Rezki
Powered by blists - more mailing lists