* [PATCH 0/2] two tiny fixes for locktorture
@ 2020-09-17 13:59 Hou Tao
2020-09-17 13:59 ` [PATCH 1/2] locktorture: doesn't check nreaders_stress when no readlock support Hou Tao
2020-09-17 13:59 ` [PATCH 2/2] locktorture: call percpu_free_rwsem() to do percpu-rwsem cleanup Hou Tao
0 siblings, 2 replies; 14+ messages in thread
From: Hou Tao @ 2020-09-17 13:59 UTC (permalink / raw)
To: Paul E. McKenney, Davidlohr Bueso, Josh Triplett
Cc: linux-kernel, houtao1, rcu
Hou Tao (2):
locktorture: doesn't check nreaders_stress when no readlock support
locktorture: call percpu_free_rwsem() to do percpu-rwsem cleanup
kernel/locking/locktorture.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
--
2.25.0.4.g0ad7144999
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] locktorture: doesn't check nreaders_stress when no readlock support
2020-09-17 13:59 [PATCH 0/2] two tiny fixes for locktorture Hou Tao
@ 2020-09-17 13:59 ` Hou Tao
2020-09-17 16:58 ` Paul E. McKenney
2020-09-17 13:59 ` [PATCH 2/2] locktorture: call percpu_free_rwsem() to do percpu-rwsem cleanup Hou Tao
1 sibling, 1 reply; 14+ messages in thread
From: Hou Tao @ 2020-09-17 13:59 UTC (permalink / raw)
To: Paul E. McKenney, Davidlohr Bueso, Josh Triplett
Cc: linux-kernel, houtao1, rcu
To ensure there is always at least one locking thread.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
kernel/locking/locktorture.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 9cfa5e89cff7f..bebdf98e6cd78 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -868,7 +868,8 @@ static int __init lock_torture_init(void)
goto unwind;
}
- if (nwriters_stress == 0 && nreaders_stress == 0) {
+ if (nwriters_stress == 0 &&
+ (!cxt.cur_ops->readlock || nreaders_stress == 0)) {
pr_alert("lock-torture: must run at least one locking thread\n");
firsterr = -EINVAL;
goto unwind;
--
2.25.0.4.g0ad7144999
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] locktorture: call percpu_free_rwsem() to do percpu-rwsem cleanup
2020-09-17 13:59 [PATCH 0/2] two tiny fixes for locktorture Hou Tao
2020-09-17 13:59 ` [PATCH 1/2] locktorture: doesn't check nreaders_stress when no readlock support Hou Tao
@ 2020-09-17 13:59 ` Hou Tao
2020-09-22 23:24 ` Paul E. McKenney
1 sibling, 1 reply; 14+ messages in thread
From: Hou Tao @ 2020-09-17 13:59 UTC (permalink / raw)
To: Paul E. McKenney, Davidlohr Bueso, Josh Triplett
Cc: linux-kernel, houtao1, rcu
When do percpu-rwsem writer lock torture, the RCU callback
rcu_sync_func() may still be pending after locktorture module
is removed, and it will lead to the following Oops:
BUG: unable to handle page fault for address: ffffffffc00eb920
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 6500a067 P4D 6500a067 PUD 6500c067 PMD 13a36c067 PTE 800000013691c163
Oops: 0000 [#1] PREEMPT SMP
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.9.0-rc5+ #4
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
RIP: 0010:rcu_cblist_dequeue+0x12/0x30
Call Trace:
<IRQ>
rcu_core+0x1b1/0x860
__do_softirq+0xfe/0x326
asm_call_on_stack+0x12/0x20
</IRQ>
do_softirq_own_stack+0x5f/0x80
irq_exit_rcu+0xaf/0xc0
sysvec_apic_timer_interrupt+0x2e/0xb0
asm_sysvec_apic_timer_interrupt+0x12/0x20
Fix it by adding an exit hook in lock_torture_ops and
use it to call percpu_free_rwsem() for percpu rwsem torture
before the module is removed, so we can ensure rcu_sync_func()
completes before module exits.
Also needs to call exit hook if lock_torture_init() fails half-way,
so use ctx->cur_ops != NULL to signal that init hook has been called.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
kernel/locking/locktorture.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index bebdf98e6cd78..e91033e9b6f95 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -74,6 +74,7 @@ static void lock_torture_cleanup(void);
*/
struct lock_torture_ops {
void (*init)(void);
+ void (*exit)(void);
int (*writelock)(void);
void (*write_delay)(struct torture_random_state *trsp);
void (*task_boost)(struct torture_random_state *trsp);
@@ -571,6 +572,11 @@ void torture_percpu_rwsem_init(void)
BUG_ON(percpu_init_rwsem(&pcpu_rwsem));
}
+static void torture_percpu_rwsem_exit(void)
+{
+ percpu_free_rwsem(&pcpu_rwsem);
+}
+
static int torture_percpu_rwsem_down_write(void) __acquires(pcpu_rwsem)
{
percpu_down_write(&pcpu_rwsem);
@@ -595,6 +601,7 @@ static void torture_percpu_rwsem_up_read(void) __releases(pcpu_rwsem)
static struct lock_torture_ops percpu_rwsem_lock_ops = {
.init = torture_percpu_rwsem_init,
+ .exit = torture_percpu_rwsem_exit,
.writelock = torture_percpu_rwsem_down_write,
.write_delay = torture_rwsem_write_delay,
.task_boost = torture_boost_dummy,
@@ -786,9 +793,10 @@ static void lock_torture_cleanup(void)
/*
* Indicates early cleanup, meaning that the test has not run,
- * such as when passing bogus args when loading the module. As
- * such, only perform the underlying torture-specific cleanups,
- * and avoid anything related to locktorture.
+ * such as when passing bogus args when loading the module.
+ * However cxt->cur_ops.init() may have been invoked, so beside
+ * perform the underlying torture-specific cleanups, cur_ops.exit()
+ * will be invoked if needed.
*/
if (!cxt.lwsa && !cxt.lrsa)
goto end;
@@ -828,6 +836,12 @@ static void lock_torture_cleanup(void)
cxt.lrsa = NULL;
end:
+ /* If init() has been called, then do exit() accordingly */
+ if (cxt.cur_ops) {
+ if (cxt.cur_ops->exit)
+ cxt.cur_ops->exit();
+ cxt.cur_ops = NULL;
+ }
torture_cleanup_end();
}
@@ -835,6 +849,7 @@ static int __init lock_torture_init(void)
{
int i, j;
int firsterr = 0;
+ struct lock_torture_ops *cur_ops;
static struct lock_torture_ops *torture_ops[] = {
&lock_busted_ops,
&spin_lock_ops, &spin_lock_irq_ops,
@@ -853,8 +868,8 @@ static int __init lock_torture_init(void)
/* Process args and tell the world that the torturer is on the job. */
for (i = 0; i < ARRAY_SIZE(torture_ops); i++) {
- cxt.cur_ops = torture_ops[i];
- if (strcmp(torture_type, cxt.cur_ops->name) == 0)
+ cur_ops = torture_ops[i];
+ if (strcmp(torture_type, cur_ops->name) == 0)
break;
}
if (i == ARRAY_SIZE(torture_ops)) {
@@ -869,12 +884,13 @@ static int __init lock_torture_init(void)
}
if (nwriters_stress == 0 &&
- (!cxt.cur_ops->readlock || nreaders_stress == 0)) {
+ (!cur_ops->readlock || nreaders_stress == 0)) {
pr_alert("lock-torture: must run at least one locking thread\n");
firsterr = -EINVAL;
goto unwind;
}
+ cxt.cur_ops = cur_ops;
if (cxt.cur_ops->init)
cxt.cur_ops->init();
--
2.25.0.4.g0ad7144999
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] locktorture: doesn't check nreaders_stress when no readlock support
2020-09-17 13:59 ` [PATCH 1/2] locktorture: doesn't check nreaders_stress when no readlock support Hou Tao
@ 2020-09-17 16:58 ` Paul E. McKenney
2020-09-18 1:13 ` Hou Tao
0 siblings, 1 reply; 14+ messages in thread
From: Paul E. McKenney @ 2020-09-17 16:58 UTC (permalink / raw)
To: Hou Tao; +Cc: Davidlohr Bueso, Josh Triplett, linux-kernel, rcu
On Thu, Sep 17, 2020 at 09:59:09PM +0800, Hou Tao wrote:
> To ensure there is always at least one locking thread.
>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> kernel/locking/locktorture.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
> index 9cfa5e89cff7f..bebdf98e6cd78 100644
> --- a/kernel/locking/locktorture.c
> +++ b/kernel/locking/locktorture.c
> @@ -868,7 +868,8 @@ static int __init lock_torture_init(void)
> goto unwind;
> }
>
> - if (nwriters_stress == 0 && nreaders_stress == 0) {
> + if (nwriters_stress == 0 &&
> + (!cxt.cur_ops->readlock || nreaders_stress == 0)) {
You lost me on this one. How does it help to allow tests with zero
writers on exclusive locks? Or am I missing something subtle here?
Thanx, Paul
> pr_alert("lock-torture: must run at least one locking thread\n");
> firsterr = -EINVAL;
> goto unwind;
> --
> 2.25.0.4.g0ad7144999
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] locktorture: doesn't check nreaders_stress when no readlock support
2020-09-17 16:58 ` Paul E. McKenney
@ 2020-09-18 1:13 ` Hou Tao
2020-09-18 3:37 ` Paul E. McKenney
0 siblings, 1 reply; 14+ messages in thread
From: Hou Tao @ 2020-09-18 1:13 UTC (permalink / raw)
To: paulmck; +Cc: Davidlohr Bueso, Josh Triplett, linux-kernel, rcu
Hi Paul,
On 2020/9/18 0:58, Paul E. McKenney wrote:
> On Thu, Sep 17, 2020 at 09:59:09PM +0800, Hou Tao wrote:
>> To ensure there is always at least one locking thread.
>>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>> kernel/locking/locktorture.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
>> index 9cfa5e89cff7f..bebdf98e6cd78 100644
>> --- a/kernel/locking/locktorture.c
>> +++ b/kernel/locking/locktorture.c
>> @@ -868,7 +868,8 @@ static int __init lock_torture_init(void)
>> goto unwind;
>> }
>>
>> - if (nwriters_stress == 0 && nreaders_stress == 0) {
>> + if (nwriters_stress == 0 &&
>> + (!cxt.cur_ops->readlock || nreaders_stress == 0)) {
>
> You lost me on this one. How does it help to allow tests with zero
> writers on exclusive locks? Or am I missing something subtle here?
>
The purpose is to prohibit test with only readers on exclusive locks, not allow it.
So if the module parameters are "torture_type=mutex_lock nwriters_stress=0 nreaders_stress=3",
locktorture can fail early instead of continuing but doing nothing useful.
Regards,
Tao
> Thanx, Paul
>
>> pr_alert("lock-torture: must run at least one locking thread\n");
>> firsterr = -EINVAL;
>> goto unwind;
>> --
>> 2.25.0.4.g0ad7144999
>>
> .
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] locktorture: doesn't check nreaders_stress when no readlock support
2020-09-18 1:13 ` Hou Tao
@ 2020-09-18 3:37 ` Paul E. McKenney
2020-09-18 11:44 ` [PATCH v2 " Hou Tao
0 siblings, 1 reply; 14+ messages in thread
From: Paul E. McKenney @ 2020-09-18 3:37 UTC (permalink / raw)
To: Hou Tao; +Cc: Davidlohr Bueso, Josh Triplett, linux-kernel, rcu
On Fri, Sep 18, 2020 at 09:13:14AM +0800, Hou Tao wrote:
> Hi Paul,
>
> On 2020/9/18 0:58, Paul E. McKenney wrote:
> > On Thu, Sep 17, 2020 at 09:59:09PM +0800, Hou Tao wrote:
> >> To ensure there is always at least one locking thread.
> >>
> >> Signed-off-by: Hou Tao <houtao1@huawei.com>
> >> ---
> >> kernel/locking/locktorture.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
> >> index 9cfa5e89cff7f..bebdf98e6cd78 100644
> >> --- a/kernel/locking/locktorture.c
> >> +++ b/kernel/locking/locktorture.c
> >> @@ -868,7 +868,8 @@ static int __init lock_torture_init(void)
> >> goto unwind;
> >> }
> >>
> >> - if (nwriters_stress == 0 && nreaders_stress == 0) {
> >> + if (nwriters_stress == 0 &&
> >> + (!cxt.cur_ops->readlock || nreaders_stress == 0)) {
> >
> > You lost me on this one. How does it help to allow tests with zero
> > writers on exclusive locks? Or am I missing something subtle here?
> >
> The purpose is to prohibit test with only readers on exclusive locks, not allow it.
>
> So if the module parameters are "torture_type=mutex_lock nwriters_stress=0 nreaders_stress=3",
> locktorture can fail early instead of continuing but doing nothing useful.
Very good!
Now please make that clear in the commit log. (Your English looks to
me to be more than equal to that challenge.)
In this commit log, please first state what is wrong. Then what the
change is and how it improves things.
Thanx, Paul
> Regards,
> Tao
>
> > Thanx, Paul
> >
> >> pr_alert("lock-torture: must run at least one locking thread\n");
> >> firsterr = -EINVAL;
> >> goto unwind;
> >> --
> >> 2.25.0.4.g0ad7144999
> >>
> > .
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/2] locktorture: doesn't check nreaders_stress when no readlock support
2020-09-18 3:37 ` Paul E. McKenney
@ 2020-09-18 11:44 ` Hou Tao
2020-09-18 17:59 ` Paul E. McKenney
0 siblings, 1 reply; 14+ messages in thread
From: Hou Tao @ 2020-09-18 11:44 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Josh Triplett, Davidlohr Bueso, linux-kernel, rcu, houtao1
When do locktorture for exclusive lock which doesn't have readlock
support, the following module parameters will be considered as valid:
torture_type=mutex_lock nwriters_stress=0 nreaders_stress=1
But locktorture will do nothing useful, so instead of permitting
these useless parameters, let's reject these parameters by returning
-EINVAL during module init.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
kernel/locking/locktorture.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 9cfa5e89cff7f..bebdf98e6cd78 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -868,7 +868,8 @@ static int __init lock_torture_init(void)
goto unwind;
}
- if (nwriters_stress == 0 && nreaders_stress == 0) {
+ if (nwriters_stress == 0 &&
+ (!cxt.cur_ops->readlock || nreaders_stress == 0)) {
pr_alert("lock-torture: must run at least one locking thread\n");
firsterr = -EINVAL;
goto unwind;
--
2.25.0.4.g0ad7144999
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] locktorture: doesn't check nreaders_stress when no readlock support
2020-09-18 11:44 ` [PATCH v2 " Hou Tao
@ 2020-09-18 17:59 ` Paul E. McKenney
2020-09-19 3:25 ` Hou Tao
0 siblings, 1 reply; 14+ messages in thread
From: Paul E. McKenney @ 2020-09-18 17:59 UTC (permalink / raw)
To: Hou Tao; +Cc: Josh Triplett, Davidlohr Bueso, linux-kernel, rcu
On Fri, Sep 18, 2020 at 07:44:24PM +0800, Hou Tao wrote:
> When do locktorture for exclusive lock which doesn't have readlock
> support, the following module parameters will be considered as valid:
>
> torture_type=mutex_lock nwriters_stress=0 nreaders_stress=1
>
> But locktorture will do nothing useful, so instead of permitting
> these useless parameters, let's reject these parameters by returning
> -EINVAL during module init.
>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
Much better, much easier for people a year from now to understand.
Queued for v5.11, thank you!
I did edit the commit log a bit as shown below, so please let me
know if I messed anything up.
Thanx, Paul
commit 4985c52e3b5237666265e59f56856f485ee36e71
Author: Hou Tao <houtao1@huawei.com>
Date: Fri Sep 18 19:44:24 2020 +0800
locktorture: Ignore nreaders_stress if no readlock support
Exclusive locks do not have readlock support, which means that a
locktorture run with the following module parameters will do nothing:
torture_type=mutex_lock nwriters_stress=0 nreaders_stress=1
This commit therefore rejects this combination for exclusive locks by
returning -EINVAL during module init.
Signed-off-by: Hou Tao <houtao1@huawei.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 316531d..046ea2d 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -870,7 +870,8 @@ static int __init lock_torture_init(void)
goto unwind;
}
- if (nwriters_stress == 0 && nreaders_stress == 0) {
+ if (nwriters_stress == 0 &&
+ (!cxt.cur_ops->readlock || nreaders_stress == 0)) {
pr_alert("lock-torture: must run at least one locking thread\n");
firsterr = -EINVAL;
goto unwind;
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] locktorture: doesn't check nreaders_stress when no readlock support
2020-09-18 17:59 ` Paul E. McKenney
@ 2020-09-19 3:25 ` Hou Tao
0 siblings, 0 replies; 14+ messages in thread
From: Hou Tao @ 2020-09-19 3:25 UTC (permalink / raw)
To: paulmck; +Cc: Josh Triplett, Davidlohr Bueso, linux-kernel, rcu
Hi Paul,
On 2020/9/19 1:59, Paul E. McKenney wrote:
> On Fri, Sep 18, 2020 at 07:44:24PM +0800, Hou Tao wrote:
>> When do locktorture for exclusive lock which doesn't have readlock
>> support, the following module parameters will be considered as valid:
>>
>> torture_type=mutex_lock nwriters_stress=0 nreaders_stress=1
>>
>> But locktorture will do nothing useful, so instead of permitting
>> these useless parameters, let's reject these parameters by returning
>> -EINVAL during module init.
>>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>
> Much better, much easier for people a year from now to understand.
> Queued for v5.11, thank you!
>
> I did edit the commit log a bit as shown below, so please let me
> know if I messed anything up.
>
Thanks for your edit, it looks more clearer.
Regards,
Tao
> Thanx, Paul
>
> commit 4985c52e3b5237666265e59f56856f485ee36e71
> Author: Hou Tao <houtao1@huawei.com>
> Date: Fri Sep 18 19:44:24 2020 +0800
>
> locktorture: Ignore nreaders_stress if no readlock support
>
> Exclusive locks do not have readlock support, which means that a
> locktorture run with the following module parameters will do nothing:
>
> torture_type=mutex_lock nwriters_stress=0 nreaders_stress=1
>
> This commit therefore rejects this combination for exclusive locks by
> returning -EINVAL during module init.
>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
>
> diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
> index 316531d..046ea2d 100644
> --- a/kernel/locking/locktorture.c
> +++ b/kernel/locking/locktorture.c
> @@ -870,7 +870,8 @@ static int __init lock_torture_init(void)
> goto unwind;
> }
>
> - if (nwriters_stress == 0 && nreaders_stress == 0) {
> + if (nwriters_stress == 0 &&
> + (!cxt.cur_ops->readlock || nreaders_stress == 0)) {
> pr_alert("lock-torture: must run at least one locking thread\n");
> firsterr = -EINVAL;
> goto unwind;
> .
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] locktorture: call percpu_free_rwsem() to do percpu-rwsem cleanup
2020-09-17 13:59 ` [PATCH 2/2] locktorture: call percpu_free_rwsem() to do percpu-rwsem cleanup Hou Tao
@ 2020-09-22 23:24 ` Paul E. McKenney
2020-09-23 2:24 ` Hou Tao
0 siblings, 1 reply; 14+ messages in thread
From: Paul E. McKenney @ 2020-09-22 23:24 UTC (permalink / raw)
To: Hou Tao; +Cc: Davidlohr Bueso, Josh Triplett, linux-kernel, rcu
On Thu, Sep 17, 2020 at 09:59:10PM +0800, Hou Tao wrote:
> When do percpu-rwsem writer lock torture, the RCU callback
> rcu_sync_func() may still be pending after locktorture module
> is removed, and it will lead to the following Oops:
>
> BUG: unable to handle page fault for address: ffffffffc00eb920
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 6500a067 P4D 6500a067 PUD 6500c067 PMD 13a36c067 PTE 800000013691c163
> Oops: 0000 [#1] PREEMPT SMP
> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.9.0-rc5+ #4
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
> RIP: 0010:rcu_cblist_dequeue+0x12/0x30
> Call Trace:
> <IRQ>
> rcu_core+0x1b1/0x860
> __do_softirq+0xfe/0x326
> asm_call_on_stack+0x12/0x20
> </IRQ>
> do_softirq_own_stack+0x5f/0x80
> irq_exit_rcu+0xaf/0xc0
> sysvec_apic_timer_interrupt+0x2e/0xb0
> asm_sysvec_apic_timer_interrupt+0x12/0x20
>
> Fix it by adding an exit hook in lock_torture_ops and
> use it to call percpu_free_rwsem() for percpu rwsem torture
> before the module is removed, so we can ensure rcu_sync_func()
> completes before module exits.
>
> Also needs to call exit hook if lock_torture_init() fails half-way,
> so use ctx->cur_ops != NULL to signal that init hook has been called.
Good catch, but please see below for comments and questions.
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> kernel/locking/locktorture.c | 28 ++++++++++++++++++++++------
> 1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
> index bebdf98e6cd78..e91033e9b6f95 100644
> --- a/kernel/locking/locktorture.c
> +++ b/kernel/locking/locktorture.c
> @@ -74,6 +74,7 @@ static void lock_torture_cleanup(void);
> */
> struct lock_torture_ops {
> void (*init)(void);
> + void (*exit)(void);
This is fine, but why not also add a flag to the lock_torture_cxt
structure that is set when the ->init() function is called? Perhaps
something like this in lock_torture_init():
if (cxt.cur_ops->init) {
cxt.cur_ops->init();
cxt.initcalled = true;
}
> int (*writelock)(void);
> void (*write_delay)(struct torture_random_state *trsp);
> void (*task_boost)(struct torture_random_state *trsp);
> @@ -571,6 +572,11 @@ void torture_percpu_rwsem_init(void)
> BUG_ON(percpu_init_rwsem(&pcpu_rwsem));
> }
>
> +static void torture_percpu_rwsem_exit(void)
> +{
> + percpu_free_rwsem(&pcpu_rwsem);
> +}
> +
> static int torture_percpu_rwsem_down_write(void) __acquires(pcpu_rwsem)
> {
> percpu_down_write(&pcpu_rwsem);
> @@ -595,6 +601,7 @@ static void torture_percpu_rwsem_up_read(void) __releases(pcpu_rwsem)
>
> static struct lock_torture_ops percpu_rwsem_lock_ops = {
> .init = torture_percpu_rwsem_init,
> + .exit = torture_percpu_rwsem_exit,
> .writelock = torture_percpu_rwsem_down_write,
> .write_delay = torture_rwsem_write_delay,
> .task_boost = torture_boost_dummy,
> @@ -786,9 +793,10 @@ static void lock_torture_cleanup(void)
>
> /*
> * Indicates early cleanup, meaning that the test has not run,
> - * such as when passing bogus args when loading the module. As
> - * such, only perform the underlying torture-specific cleanups,
> - * and avoid anything related to locktorture.
> + * such as when passing bogus args when loading the module.
> + * However cxt->cur_ops.init() may have been invoked, so beside
> + * perform the underlying torture-specific cleanups, cur_ops.exit()
> + * will be invoked if needed.
> */
> if (!cxt.lwsa && !cxt.lrsa)
> goto end;
> @@ -828,6 +836,12 @@ static void lock_torture_cleanup(void)
> cxt.lrsa = NULL;
>
> end:
> + /* If init() has been called, then do exit() accordingly */
> + if (cxt.cur_ops) {
> + if (cxt.cur_ops->exit)
> + cxt.cur_ops->exit();
> + cxt.cur_ops = NULL;
> + }
The above can then be:
if (cxt.initcalled && cxt.cur_ops->exit)
cxt.cur_ops->exit();
Maybe you also need to clear cxt.initcalled at this point, but I don't
immediately see why that would be needed.
> torture_cleanup_end();
> }
>
> @@ -835,6 +849,7 @@ static int __init lock_torture_init(void)
> {
> int i, j;
> int firsterr = 0;
> + struct lock_torture_ops *cur_ops;
And then you don't need this extra pointer. Not that this pointer is bad
in and of itself, but using (!cxt.cur_ops) to indicate that the ->init()
function has not been called is an accident waiting to happen.
And the changes below are no longer needed.
Or am I missing something subtle?
Thanx, Paul
> static struct lock_torture_ops *torture_ops[] = {
> &lock_busted_ops,
> &spin_lock_ops, &spin_lock_irq_ops,
> @@ -853,8 +868,8 @@ static int __init lock_torture_init(void)
>
> /* Process args and tell the world that the torturer is on the job. */
> for (i = 0; i < ARRAY_SIZE(torture_ops); i++) {
> - cxt.cur_ops = torture_ops[i];
> - if (strcmp(torture_type, cxt.cur_ops->name) == 0)
> + cur_ops = torture_ops[i];
> + if (strcmp(torture_type, cur_ops->name) == 0)
> break;
> }
> if (i == ARRAY_SIZE(torture_ops)) {
> @@ -869,12 +884,13 @@ static int __init lock_torture_init(void)
> }
>
> if (nwriters_stress == 0 &&
> - (!cxt.cur_ops->readlock || nreaders_stress == 0)) {
> + (!cur_ops->readlock || nreaders_stress == 0)) {
> pr_alert("lock-torture: must run at least one locking thread\n");
> firsterr = -EINVAL;
> goto unwind;
> }
>
> + cxt.cur_ops = cur_ops;
> if (cxt.cur_ops->init)
> cxt.cur_ops->init();
>
> --
> 2.25.0.4.g0ad7144999
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] locktorture: call percpu_free_rwsem() to do percpu-rwsem cleanup
2020-09-22 23:24 ` Paul E. McKenney
@ 2020-09-23 2:24 ` Hou Tao
2020-09-23 3:51 ` Paul E. McKenney
0 siblings, 1 reply; 14+ messages in thread
From: Hou Tao @ 2020-09-23 2:24 UTC (permalink / raw)
To: paulmck; +Cc: Davidlohr Bueso, Josh Triplett, linux-kernel, rcu
Hi Paul,
> On 2020/9/23 7:24, Paul E. McKenney wrote:
snip
>> Fix it by adding an exit hook in lock_torture_ops and
>> use it to call percpu_free_rwsem() for percpu rwsem torture
>> before the module is removed, so we can ensure rcu_sync_func()
>> completes before module exits.
>>
>> Also needs to call exit hook if lock_torture_init() fails half-way,
>> so use ctx->cur_ops != NULL to signal that init hook has been called.
>
> Good catch, but please see below for comments and questions.
>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>> kernel/locking/locktorture.c | 28 ++++++++++++++++++++++------
>> 1 file changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
>> index bebdf98e6cd78..e91033e9b6f95 100644
>> --- a/kernel/locking/locktorture.c
>> +++ b/kernel/locking/locktorture.c
>> @@ -74,6 +74,7 @@ static void lock_torture_cleanup(void);
>> */
>> struct lock_torture_ops {
>> void (*init)(void);
>> + void (*exit)(void);
>
> This is fine, but why not also add a flag to the lock_torture_cxt
> structure that is set when the ->init() function is called? Perhaps
> something like this in lock_torture_init():
>
> if (cxt.cur_ops->init) {
> cxt.cur_ops->init();
> cxt.initcalled = true;
> }
>
You are right. Add a new field to indicate the init hook has been
called is much better than reusing ctx->cur_ops != NULL to do that.
>> int (*writelock)(void);
>> void (*write_delay)(struct torture_random_state *trsp);
>> void (*task_boost)(struct torture_random_state *trsp);
>> @@ -571,6 +572,11 @@ void torture_percpu_rwsem_init(void)
>> BUG_ON(percpu_init_rwsem(&pcpu_rwsem));
>> }
>>
>> +static void torture_percpu_rwsem_exit(void)
>> +{
>> + percpu_free_rwsem(&pcpu_rwsem);
>> +}
>> +
snip
>> @@ -828,6 +836,12 @@ static void lock_torture_cleanup(void)
>> cxt.lrsa = NULL;
>>
>> end:
>> + /* If init() has been called, then do exit() accordingly */
>> + if (cxt.cur_ops) {
>> + if (cxt.cur_ops->exit)
>> + cxt.cur_ops->exit();
>> + cxt.cur_ops = NULL;
>> + }
>
> The above can then be:
>
> if (cxt.initcalled && cxt.cur_ops->exit)
> cxt.cur_ops->exit();
>
> Maybe you also need to clear cxt.initcalled at this point, but I don't
> immediately see why that would be needed.
>
Because we are doing cleanup, so I think reset initcalled to false is OK
after the cleanup is done.
>> torture_cleanup_end();
>> }
>>
>> @@ -835,6 +849,7 @@ static int __init lock_torture_init(void)
>> {
>> int i, j;
>> int firsterr = 0;
>> + struct lock_torture_ops *cur_ops;
>
> And then you don't need this extra pointer. Not that this pointer is bad
> in and of itself, but using (!cxt.cur_ops) to indicate that the ->init()
> function has not been called is an accident waiting to happen.
>
> And the changes below are no longer needed.
>
> Or am I missing something subtle?
>
Thanks for your suggestion. Will send v2.
Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] locktorture: call percpu_free_rwsem() to do percpu-rwsem cleanup
2020-09-23 2:24 ` Hou Tao
@ 2020-09-23 3:51 ` Paul E. McKenney
2020-09-24 14:18 ` [PATCH v2 " Hou Tao
0 siblings, 1 reply; 14+ messages in thread
From: Paul E. McKenney @ 2020-09-23 3:51 UTC (permalink / raw)
To: Hou Tao; +Cc: Davidlohr Bueso, Josh Triplett, linux-kernel, rcu
On Wed, Sep 23, 2020 at 10:24:20AM +0800, Hou Tao wrote:
> Hi Paul,
>
> > On 2020/9/23 7:24, Paul E. McKenney wrote:
> snip
>
> >> Fix it by adding an exit hook in lock_torture_ops and
> >> use it to call percpu_free_rwsem() for percpu rwsem torture
> >> before the module is removed, so we can ensure rcu_sync_func()
> >> completes before module exits.
> >>
> >> Also needs to call exit hook if lock_torture_init() fails half-way,
> >> so use ctx->cur_ops != NULL to signal that init hook has been called.
> >
> > Good catch, but please see below for comments and questions.
> >
> >> Signed-off-by: Hou Tao <houtao1@huawei.com>
> >> ---
> >> kernel/locking/locktorture.c | 28 ++++++++++++++++++++++------
> >> 1 file changed, 22 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
> >> index bebdf98e6cd78..e91033e9b6f95 100644
> >> --- a/kernel/locking/locktorture.c
> >> +++ b/kernel/locking/locktorture.c
> >> @@ -74,6 +74,7 @@ static void lock_torture_cleanup(void);
> >> */
> >> struct lock_torture_ops {
> >> void (*init)(void);
> >> + void (*exit)(void);
> >
> > This is fine, but why not also add a flag to the lock_torture_cxt
> > structure that is set when the ->init() function is called? Perhaps
> > something like this in lock_torture_init():
> >
> > if (cxt.cur_ops->init) {
> > cxt.cur_ops->init();
> > cxt.initcalled = true;
> > }
> >
>
> You are right. Add a new field to indicate the init hook has been
> called is much better than reusing ctx->cur_ops != NULL to do that.
>
> >> int (*writelock)(void);
> >> void (*write_delay)(struct torture_random_state *trsp);
> >> void (*task_boost)(struct torture_random_state *trsp);
> >> @@ -571,6 +572,11 @@ void torture_percpu_rwsem_init(void)
> >> BUG_ON(percpu_init_rwsem(&pcpu_rwsem));
> >> }
> >>
> >> +static void torture_percpu_rwsem_exit(void)
> >> +{
> >> + percpu_free_rwsem(&pcpu_rwsem);
> >> +}
> >> +
> snip
>
> >> @@ -828,6 +836,12 @@ static void lock_torture_cleanup(void)
> >> cxt.lrsa = NULL;
> >>
> >> end:
> >> + /* If init() has been called, then do exit() accordingly */
> >> + if (cxt.cur_ops) {
> >> + if (cxt.cur_ops->exit)
> >> + cxt.cur_ops->exit();
> >> + cxt.cur_ops = NULL;
> >> + }
> >
> > The above can then be:
> >
> > if (cxt.initcalled && cxt.cur_ops->exit)
> > cxt.cur_ops->exit();
> >
> > Maybe you also need to clear cxt.initcalled at this point, but I don't
> > immediately see why that would be needed.
> >
> Because we are doing cleanup, so I think reset initcalled to false is OK
> after the cleanup is done.
Maybe best to try it both ways and see how each really works?
We might each have our opinions, but the computer's opinion is the one
that really counts. ;-)
> >> torture_cleanup_end();
> >> }
> >>
> >> @@ -835,6 +849,7 @@ static int __init lock_torture_init(void)
> >> {
> >> int i, j;
> >> int firsterr = 0;
> >> + struct lock_torture_ops *cur_ops;
> >
> > And then you don't need this extra pointer. Not that this pointer is bad
> > in and of itself, but using (!cxt.cur_ops) to indicate that the ->init()
> > function has not been called is an accident waiting to happen.
> >
> > And the changes below are no longer needed.
> >
> > Or am I missing something subtle?
> >
> Thanks for your suggestion. Will send v2.
Looking forward to seeing it!
Thanx, Paul
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/2] locktorture: call percpu_free_rwsem() to do percpu-rwsem cleanup
2020-09-23 3:51 ` Paul E. McKenney
@ 2020-09-24 14:18 ` Hou Tao
2020-09-25 17:13 ` Paul E. McKenney
0 siblings, 1 reply; 14+ messages in thread
From: Hou Tao @ 2020-09-24 14:18 UTC (permalink / raw)
To: Paul E . McKenney; +Cc: Davidlohr Bueso, Josh Triplett, linux-kernel, rcu
When do percpu-rwsem writer lock torture, the RCU callback
rcu_sync_func() may still be pending after locktorture module
is removed, and it will lead to the following Oops:
BUG: unable to handle page fault for address: ffffffffc00eb920
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 6500a067 P4D 6500a067 PUD 6500c067 PMD 13a36c067 PTE 800000013691c163
Oops: 0000 [#1] PREEMPT SMP
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.9.0-rc5+ #4
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
RIP: 0010:rcu_cblist_dequeue+0x12/0x30
Call Trace:
<IRQ>
rcu_core+0x1b1/0x860
__do_softirq+0xfe/0x326
asm_call_on_stack+0x12/0x20
</IRQ>
do_softirq_own_stack+0x5f/0x80
irq_exit_rcu+0xaf/0xc0
sysvec_apic_timer_interrupt+0x2e/0xb0
asm_sysvec_apic_timer_interrupt+0x12/0x20
Fix it by adding an exit hook in lock_torture_ops and
use it to call percpu_free_rwsem() for percpu rwsem torture
before the module is removed, so we can ensure rcu_sync_func()
completes before module exits.
Also needs to call exit hook if lock_torture_init() fails half-way,
so add init_called field in lock_torture_cxt to indicate that
init hook has been called.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
v2: add init_called field in lock_torture_cxt instead of reusing
cxt->cur_ops for error handling
kernel/locking/locktorture.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index bebdf98e6cd78..1fbbcf76f495b 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -74,6 +74,7 @@ static void lock_torture_cleanup(void);
*/
struct lock_torture_ops {
void (*init)(void);
+ void (*exit)(void);
int (*writelock)(void);
void (*write_delay)(struct torture_random_state *trsp);
void (*task_boost)(struct torture_random_state *trsp);
@@ -90,12 +91,13 @@ struct lock_torture_cxt {
int nrealwriters_stress;
int nrealreaders_stress;
bool debug_lock;
+ bool init_called;
atomic_t n_lock_torture_errors;
struct lock_torture_ops *cur_ops;
struct lock_stress_stats *lwsa; /* writer statistics */
struct lock_stress_stats *lrsa; /* reader statistics */
};
-static struct lock_torture_cxt cxt = { 0, 0, false,
+static struct lock_torture_cxt cxt = { 0, 0, false, false,
ATOMIC_INIT(0),
NULL, NULL};
/*
@@ -571,6 +573,11 @@ void torture_percpu_rwsem_init(void)
BUG_ON(percpu_init_rwsem(&pcpu_rwsem));
}
+static void torture_percpu_rwsem_exit(void)
+{
+ percpu_free_rwsem(&pcpu_rwsem);
+}
+
static int torture_percpu_rwsem_down_write(void) __acquires(pcpu_rwsem)
{
percpu_down_write(&pcpu_rwsem);
@@ -595,6 +602,7 @@ static void torture_percpu_rwsem_up_read(void) __releases(pcpu_rwsem)
static struct lock_torture_ops percpu_rwsem_lock_ops = {
.init = torture_percpu_rwsem_init,
+ .exit = torture_percpu_rwsem_exit,
.writelock = torture_percpu_rwsem_down_write,
.write_delay = torture_rwsem_write_delay,
.task_boost = torture_boost_dummy,
@@ -786,9 +794,10 @@ static void lock_torture_cleanup(void)
/*
* Indicates early cleanup, meaning that the test has not run,
- * such as when passing bogus args when loading the module. As
- * such, only perform the underlying torture-specific cleanups,
- * and avoid anything related to locktorture.
+ * such as when passing bogus args when loading the module.
+ * However cxt->cur_ops.init() may have been invoked, so beside
+ * perform the underlying torture-specific cleanups, cur_ops.exit()
+ * will be invoked if needed.
*/
if (!cxt.lwsa && !cxt.lrsa)
goto end;
@@ -828,6 +837,11 @@ static void lock_torture_cleanup(void)
cxt.lrsa = NULL;
end:
+ if (cxt.init_called) {
+ if (cxt.cur_ops->exit)
+ cxt.cur_ops->exit();
+ cxt.init_called = false;
+ }
torture_cleanup_end();
}
@@ -875,8 +889,10 @@ static int __init lock_torture_init(void)
goto unwind;
}
- if (cxt.cur_ops->init)
+ if (cxt.cur_ops->init) {
cxt.cur_ops->init();
+ cxt.init_called = true;
+ }
if (nwriters_stress >= 0)
cxt.nrealwriters_stress = nwriters_stress;
--
2.25.0.4.g0ad7144999
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] locktorture: call percpu_free_rwsem() to do percpu-rwsem cleanup
2020-09-24 14:18 ` [PATCH v2 " Hou Tao
@ 2020-09-25 17:13 ` Paul E. McKenney
0 siblings, 0 replies; 14+ messages in thread
From: Paul E. McKenney @ 2020-09-25 17:13 UTC (permalink / raw)
To: Hou Tao; +Cc: Davidlohr Bueso, Josh Triplett, linux-kernel, rcu
On Thu, Sep 24, 2020 at 10:18:54PM +0800, Hou Tao wrote:
> When do percpu-rwsem writer lock torture, the RCU callback
> rcu_sync_func() may still be pending after locktorture module
> is removed, and it will lead to the following Oops:
>
> BUG: unable to handle page fault for address: ffffffffc00eb920
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 6500a067 P4D 6500a067 PUD 6500c067 PMD 13a36c067 PTE 800000013691c163
> Oops: 0000 [#1] PREEMPT SMP
> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.9.0-rc5+ #4
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
> RIP: 0010:rcu_cblist_dequeue+0x12/0x30
> Call Trace:
> <IRQ>
> rcu_core+0x1b1/0x860
> __do_softirq+0xfe/0x326
> asm_call_on_stack+0x12/0x20
> </IRQ>
> do_softirq_own_stack+0x5f/0x80
> irq_exit_rcu+0xaf/0xc0
> sysvec_apic_timer_interrupt+0x2e/0xb0
> asm_sysvec_apic_timer_interrupt+0x12/0x20
>
> Fix it by adding an exit hook in lock_torture_ops and
> use it to call percpu_free_rwsem() for percpu rwsem torture
> before the module is removed, so we can ensure rcu_sync_func()
> completes before module exits.
>
> Also needs to call exit hook if lock_torture_init() fails half-way,
> so add init_called field in lock_torture_cxt to indicate that
> init hook has been called.
>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
Nice, thank you! I have queued this for v5.11.
As always, I could not resist reworking the commit log as below, and
as always, please check to make sure that I didn't mess anything up.
Thanx, Paul
------------------------------------------------------------------------
commit f794b408207152995aefdf41ee9a3f5c974f622a
Author: Hou Tao <houtao1@huawei.com>
Date: Thu Sep 24 22:18:54 2020 +0800
locktorture: Invoke percpu_free_rwsem() to do percpu-rwsem cleanup
When executing the LOCK06 locktorture scenario featuring percpu-rwsem,
the RCU callback rcu_sync_func() may still be pending after locktorture
module is removed. This can in turn lead to the following Oops:
BUG: unable to handle page fault for address: ffffffffc00eb920
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 6500a067 P4D 6500a067 PUD 6500c067 PMD 13a36c067 PTE 800000013691c163
Oops: 0000 [#1] PREEMPT SMP
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.9.0-rc5+ #4
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
RIP: 0010:rcu_cblist_dequeue+0x12/0x30
Call Trace:
<IRQ>
rcu_core+0x1b1/0x860
__do_softirq+0xfe/0x326
asm_call_on_stack+0x12/0x20
</IRQ>
do_softirq_own_stack+0x5f/0x80
irq_exit_rcu+0xaf/0xc0
sysvec_apic_timer_interrupt+0x2e/0xb0
asm_sysvec_apic_timer_interrupt+0x12/0x20
This commit avoids tis problem by adding an exit hook in lock_torture_ops
and using it to call percpu_free_rwsem() for percpu rwsem torture during
the module-cleanup function, thus ensuring that rcu_sync_func() completes
before module exits.
It is also necessary to call the exit hook if lock_torture_init()
fails half-way, so this commit also adds an ->init_called field in
lock_torture_cxt to indicate that exit hook, if present, must be called.
Signed-off-by: Hou Tao <houtao1@huawei.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 79fbd97..fd838ce 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -76,6 +76,7 @@ static void lock_torture_cleanup(void);
*/
struct lock_torture_ops {
void (*init)(void);
+ void (*exit)(void);
int (*writelock)(void);
void (*write_delay)(struct torture_random_state *trsp);
void (*task_boost)(struct torture_random_state *trsp);
@@ -92,12 +93,13 @@ struct lock_torture_cxt {
int nrealwriters_stress;
int nrealreaders_stress;
bool debug_lock;
+ bool init_called;
atomic_t n_lock_torture_errors;
struct lock_torture_ops *cur_ops;
struct lock_stress_stats *lwsa; /* writer statistics */
struct lock_stress_stats *lrsa; /* reader statistics */
};
-static struct lock_torture_cxt cxt = { 0, 0, false,
+static struct lock_torture_cxt cxt = { 0, 0, false, false,
ATOMIC_INIT(0),
NULL, NULL};
/*
@@ -573,6 +575,11 @@ static void torture_percpu_rwsem_init(void)
BUG_ON(percpu_init_rwsem(&pcpu_rwsem));
}
+static void torture_percpu_rwsem_exit(void)
+{
+ percpu_free_rwsem(&pcpu_rwsem);
+}
+
static int torture_percpu_rwsem_down_write(void) __acquires(pcpu_rwsem)
{
percpu_down_write(&pcpu_rwsem);
@@ -597,6 +604,7 @@ static void torture_percpu_rwsem_up_read(void) __releases(pcpu_rwsem)
static struct lock_torture_ops percpu_rwsem_lock_ops = {
.init = torture_percpu_rwsem_init,
+ .exit = torture_percpu_rwsem_exit,
.writelock = torture_percpu_rwsem_down_write,
.write_delay = torture_rwsem_write_delay,
.task_boost = torture_boost_dummy,
@@ -789,9 +797,10 @@ static void lock_torture_cleanup(void)
/*
* Indicates early cleanup, meaning that the test has not run,
- * such as when passing bogus args when loading the module. As
- * such, only perform the underlying torture-specific cleanups,
- * and avoid anything related to locktorture.
+ * such as when passing bogus args when loading the module.
+ * However cxt->cur_ops.init() may have been invoked, so beside
+ * perform the underlying torture-specific cleanups, cur_ops.exit()
+ * will be invoked if needed.
*/
if (!cxt.lwsa && !cxt.lrsa)
goto end;
@@ -831,6 +840,11 @@ static void lock_torture_cleanup(void)
cxt.lrsa = NULL;
end:
+ if (cxt.init_called) {
+ if (cxt.cur_ops->exit)
+ cxt.cur_ops->exit();
+ cxt.init_called = false;
+ }
torture_cleanup_end();
}
@@ -878,8 +892,10 @@ static int __init lock_torture_init(void)
goto unwind;
}
- if (cxt.cur_ops->init)
+ if (cxt.cur_ops->init) {
cxt.cur_ops->init();
+ cxt.init_called = true;
+ }
if (nwriters_stress >= 0)
cxt.nrealwriters_stress = nwriters_stress;
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-09-25 17:13 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17 13:59 [PATCH 0/2] two tiny fixes for locktorture Hou Tao
2020-09-17 13:59 ` [PATCH 1/2] locktorture: doesn't check nreaders_stress when no readlock support Hou Tao
2020-09-17 16:58 ` Paul E. McKenney
2020-09-18 1:13 ` Hou Tao
2020-09-18 3:37 ` Paul E. McKenney
2020-09-18 11:44 ` [PATCH v2 " Hou Tao
2020-09-18 17:59 ` Paul E. McKenney
2020-09-19 3:25 ` Hou Tao
2020-09-17 13:59 ` [PATCH 2/2] locktorture: call percpu_free_rwsem() to do percpu-rwsem cleanup Hou Tao
2020-09-22 23:24 ` Paul E. McKenney
2020-09-23 2:24 ` Hou Tao
2020-09-23 3:51 ` Paul E. McKenney
2020-09-24 14:18 ` [PATCH v2 " Hou Tao
2020-09-25 17:13 ` Paul E. McKenney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).