2021-03-18 17:31:04

by Waiman Long

[permalink] [raw]
Subject: [PATCH-tip 0/5] locking/locktorture: Fix locktorture ww_mutex test problems

This is a follow-up patch series for the previous patchset on fixing
locktorture ww_mutex test problem [1]. The first 3 patches of that
series were merged into tip. It turns out that the last one of the
three wasn't quite right. So this patch series revert the last patch.

The rests of the patch series fix the ww_mutex testing problem in
locktorture as well as removing the DEFINE_WW_MUTEX() macro from
include/linux/ww_mutex.h.

[1] https://lore.kernel.org/lkml/[email protected]/

Waiman Long (5):
locking/ww_mutex: Revert "Treat ww_mutex_lock() like a trylock"
locking/locktorture: Fix false positive circular locking splat in
ww_mutex test
locking/ww_mutex: Remove DEFINE_WW_MUTEX() macro
locking/locktorture: Pass thread id to lock/unlock functions
locking/locktorture: locking/locktorture: Fix incorrect use of
ww_acquire_ctx in ww_mutex test

include/linux/ww_mutex.h | 17 +---
kernel/locking/locktorture.c | 146 +++++++++++++++++++++++------------
kernel/locking/mutex.c | 5 +-
3 files changed, 100 insertions(+), 68 deletions(-)

--
2.18.1


2021-03-18 17:32:26

by Waiman Long

[permalink] [raw]
Subject: [PATCH-tip 2/5] locking/locktorture: Fix false positive circular locking splat in ww_mutex test

In order to avoid false positive circular locking lockdep splat
when runnng the ww_mutex torture test, we need to make sure that
the ww_mutexes have the same lock class as the acquire_ctx. This
means the ww_mutexes must have the same lockdep key as the
acquire_ctx. Unfortunately the current DEFINE_WW_MUTEX() macro fails
to do that. As a result, we add an init method for the ww_mutex test
to do explicit ww_mutex_init()'s of the ww_mutexes to avoid the false
positive warning.

Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Waiman Long <[email protected]>
---
kernel/locking/locktorture.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 0ab94e1f1276..3c27f43ab3e8 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -357,10 +357,20 @@ static struct lock_torture_ops mutex_lock_ops = {
};

#include <linux/ww_mutex.h>
+/*
+ * The torture ww_mutexes should belong to the same lock class as
+ * torture_ww_class to avoid lockdep problem. The ww_mutex_init()
+ * function is called for initialization to ensure that.
+ */
static DEFINE_WD_CLASS(torture_ww_class);
-static DEFINE_WW_MUTEX(torture_ww_mutex_0, &torture_ww_class);
-static DEFINE_WW_MUTEX(torture_ww_mutex_1, &torture_ww_class);
-static DEFINE_WW_MUTEX(torture_ww_mutex_2, &torture_ww_class);
+static struct ww_mutex torture_ww_mutex_0, torture_ww_mutex_1, torture_ww_mutex_2;
+
+static void torture_ww_mutex_init(void)
+{
+ ww_mutex_init(&torture_ww_mutex_0, &torture_ww_class);
+ ww_mutex_init(&torture_ww_mutex_1, &torture_ww_class);
+ ww_mutex_init(&torture_ww_mutex_2, &torture_ww_class);
+}

static int torture_ww_mutex_lock(void)
__acquires(torture_ww_mutex_0)
@@ -418,6 +428,7 @@ __releases(torture_ww_mutex_2)
}

static struct lock_torture_ops ww_mutex_lock_ops = {
+ .init = torture_ww_mutex_init,
.writelock = torture_ww_mutex_lock,
.write_delay = torture_mutex_delay,
.task_boost = torture_boost_dummy,
--
2.18.1

2021-03-19 11:11:50

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH-tip 0/5] locking/locktorture: Fix locktorture ww_mutex test problems


* Waiman Long <[email protected]> wrote:

> This is a follow-up patch series for the previous patchset on fixing
> locktorture ww_mutex test problem [1]. The first 3 patches of that
> series were merged into tip. It turns out that the last one of the
> three wasn't quite right. So this patch series revert the last patch.

I zapped that 3rd commit from locking/urgent yesterday already, so we
can cleanly apply the 4 patches from this series.

Thanks,

Ingo

2021-03-19 11:18:20

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH-tip 0/5] locking/locktorture: Fix locktorture ww_mutex test problems


* Waiman Long <[email protected]> wrote:

> This is a follow-up patch series for the previous patchset on fixing
> locktorture ww_mutex test problem [1]. The first 3 patches of that
> series were merged into tip. It turns out that the last one of the
> three wasn't quite right. So this patch series revert the last patch.
>
> The rests of the patch series fix the ww_mutex testing problem in
> locktorture as well as removing the DEFINE_WW_MUTEX() macro from
> include/linux/ww_mutex.h.
>
> [1] https://lore.kernel.org/lkml/[email protected]/
>
> Waiman Long (5):
> locking/ww_mutex: Revert "Treat ww_mutex_lock() like a trylock"
> locking/locktorture: Fix false positive circular locking splat in
> ww_mutex test
> locking/ww_mutex: Remove DEFINE_WW_MUTEX() macro
> locking/locktorture: Pass thread id to lock/unlock functions
> locking/locktorture: locking/locktorture: Fix incorrect use of
> ww_acquire_ctx in ww_mutex test

Applied, thanks Waiman.

I kept these two fixes in locking/urgent, for a v5.12 merge:

bee645788e07: ("locking/ww_mutex: Fix acquire/release imbalance in ww_acquire_init()/ww_acquire_fini()")
5de2055d31ea: ("locking/ww_mutex: Simplify use_ww_ctx & ww_ctx handling")

As this bug could affect actual ww_mutex users.

And queued up these four in locking/core, for a v5.13 merge:

8c52cca04f97: ("locking/locktorture: Fix incorrect use of ww_acquire_ctx in ww_mutex test")
aa3a5f31877e: ("locking/locktorture: Pass thread id to lock/unlock functions")
5261ced47f8e: ("locking/ww_mutex: Remove DEFINE_WW_MUTEX() macro")
2ea55bbba23e: ("locking/locktorture: Fix false positive circular locking splat in ww_mutex test")

As these bugs are basically limited to a debugging facility.

( But we could also merge them into v5.12, if you think it's
justified. No strong opinions either way. )

Thanks,

Ingo

Subject: [tip: locking/core] locking/locktorture: Fix false positive circular locking splat in ww_mutex test

The following commit has been merged into the locking/core branch of tip:

Commit-ID: 2ea55bbba23e9d36996299664d618393c8602646
Gitweb: https://git.kernel.org/tip/2ea55bbba23e9d36996299664d618393c8602646
Author: Waiman Long <[email protected]>
AuthorDate: Thu, 18 Mar 2021 13:28:11 -04:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Fri, 19 Mar 2021 12:13:09 +01:00

locking/locktorture: Fix false positive circular locking splat in ww_mutex test

In order to avoid false positive circular locking lockdep splat
when runnng the ww_mutex torture test, we need to make sure that
the ww_mutexes have the same lock class as the acquire_ctx. This
means the ww_mutexes must have the same lockdep key as the
acquire_ctx. Unfortunately the current DEFINE_WW_MUTEX() macro fails
to do that. As a result, we add an init method for the ww_mutex test
to do explicit ww_mutex_init()'s of the ww_mutexes to avoid the false
positive warning.

Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Waiman Long <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/locking/locktorture.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 0ab94e1..3c27f43 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -357,10 +357,20 @@ static struct lock_torture_ops mutex_lock_ops = {
};

#include <linux/ww_mutex.h>
+/*
+ * The torture ww_mutexes should belong to the same lock class as
+ * torture_ww_class to avoid lockdep problem. The ww_mutex_init()
+ * function is called for initialization to ensure that.
+ */
static DEFINE_WD_CLASS(torture_ww_class);
-static DEFINE_WW_MUTEX(torture_ww_mutex_0, &torture_ww_class);
-static DEFINE_WW_MUTEX(torture_ww_mutex_1, &torture_ww_class);
-static DEFINE_WW_MUTEX(torture_ww_mutex_2, &torture_ww_class);
+static struct ww_mutex torture_ww_mutex_0, torture_ww_mutex_1, torture_ww_mutex_2;
+
+static void torture_ww_mutex_init(void)
+{
+ ww_mutex_init(&torture_ww_mutex_0, &torture_ww_class);
+ ww_mutex_init(&torture_ww_mutex_1, &torture_ww_class);
+ ww_mutex_init(&torture_ww_mutex_2, &torture_ww_class);
+}

static int torture_ww_mutex_lock(void)
__acquires(torture_ww_mutex_0)
@@ -418,6 +428,7 @@ __releases(torture_ww_mutex_2)
}

static struct lock_torture_ops ww_mutex_lock_ops = {
+ .init = torture_ww_mutex_init,
.writelock = torture_ww_mutex_lock,
.write_delay = torture_mutex_delay,
.task_boost = torture_boost_dummy,

2021-03-19 15:32:08

by Waiman Long

[permalink] [raw]
Subject: Re: [PATCH-tip 0/5] locking/locktorture: Fix locktorture ww_mutex test problems

On 3/19/21 7:16 AM, Ingo Molnar wrote:
> * Waiman Long <[email protected]> wrote:
>
>> This is a follow-up patch series for the previous patchset on fixing
>> locktorture ww_mutex test problem [1]. The first 3 patches of that
>> series were merged into tip. It turns out that the last one of the
>> three wasn't quite right. So this patch series revert the last patch.
>>
>> The rests of the patch series fix the ww_mutex testing problem in
>> locktorture as well as removing the DEFINE_WW_MUTEX() macro from
>> include/linux/ww_mutex.h.
>>
>> [1] https://lore.kernel.org/lkml/[email protected]/
>>
>> Waiman Long (5):
>> locking/ww_mutex: Revert "Treat ww_mutex_lock() like a trylock"
>> locking/locktorture: Fix false positive circular locking splat in
>> ww_mutex test
>> locking/ww_mutex: Remove DEFINE_WW_MUTEX() macro
>> locking/locktorture: Pass thread id to lock/unlock functions
>> locking/locktorture: locking/locktorture: Fix incorrect use of
>> ww_acquire_ctx in ww_mutex test
> Applied, thanks Waiman.
>
> I kept these two fixes in locking/urgent, for a v5.12 merge:
>
> bee645788e07: ("locking/ww_mutex: Fix acquire/release imbalance in ww_acquire_init()/ww_acquire_fini()")
> 5de2055d31ea: ("locking/ww_mutex: Simplify use_ww_ctx & ww_ctx handling")
>
> As this bug could affect actual ww_mutex users.
>
> And queued up these four in locking/core, for a v5.13 merge:
>
> 8c52cca04f97: ("locking/locktorture: Fix incorrect use of ww_acquire_ctx in ww_mutex test")
> aa3a5f31877e: ("locking/locktorture: Pass thread id to lock/unlock functions")
> 5261ced47f8e: ("locking/ww_mutex: Remove DEFINE_WW_MUTEX() macro")
> 2ea55bbba23e: ("locking/locktorture: Fix false positive circular locking splat in ww_mutex test")
>
> As these bugs are basically limited to a debugging facility.
>
> ( But we could also merge them into v5.12, if you think it's
> justified. No strong opinions either way. )

I think v5.13 merge is fine. This problem exists for quite a while and
no one notices it. So it is not really that urgent.

Thank a lot for the quick action.

Cheers,
Longman