2021-05-12 12:12:08

by Leo Yan

[permalink] [raw]
Subject: [PATCH] locking/lockdep: Correct calling tracepoints

The commit eb1f00237aca ("lockdep,trace: Expose tracepoints") reverses
tracepoints for lock_contended() and lock_acquired(), thus the ftrace
log shows the wrong locking sequence that "acquired" event is prior to
"contended" event:

<idle>-0 [001] d.s3 20803.501685: lock_acquire: 0000000008b91ab4 &sg_policy->update_lock
<idle>-0 [001] d.s3 20803.501686: lock_acquired: 0000000008b91ab4 &sg_policy->update_lock
<idle>-0 [001] d.s3 20803.501689: lock_contended: 0000000008b91ab4 &sg_policy->update_lock
<idle>-0 [001] d.s3 20803.501690: lock_release: 0000000008b91ab4 &sg_policy->update_lock

This patch fixes calling tracepoints for lock_contended() and
lock_acquired().

Fixes: eb1f00237aca ("lockdep,trace: Expose tracepoints")
Signed-off-by: Leo Yan <[email protected]>
---
kernel/locking/lockdep.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 48d736aa03b2..7641bd407239 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -5736,7 +5736,7 @@ void lock_contended(struct lockdep_map *lock, unsigned long ip)
{
unsigned long flags;

- trace_lock_acquired(lock, ip);
+ trace_lock_contended(lock, ip);

if (unlikely(!lock_stat || !lockdep_enabled()))
return;
@@ -5754,7 +5754,7 @@ void lock_acquired(struct lockdep_map *lock, unsigned long ip)
{
unsigned long flags;

- trace_lock_contended(lock, ip);
+ trace_lock_acquired(lock, ip);

if (unlikely(!lock_stat || !lockdep_enabled()))
return;
--
2.25.1


2021-05-12 12:52:58

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] locking/lockdep: Correct calling tracepoints

On Wed, May 12, 2021 at 08:09:37PM +0800, Leo Yan wrote:
> The commit eb1f00237aca ("lockdep,trace: Expose tracepoints") reverses
> tracepoints for lock_contended() and lock_acquired(), thus the ftrace
> log shows the wrong locking sequence that "acquired" event is prior to
> "contended" event:
>
> <idle>-0 [001] d.s3 20803.501685: lock_acquire: 0000000008b91ab4 &sg_policy->update_lock
> <idle>-0 [001] d.s3 20803.501686: lock_acquired: 0000000008b91ab4 &sg_policy->update_lock
> <idle>-0 [001] d.s3 20803.501689: lock_contended: 0000000008b91ab4 &sg_policy->update_lock
> <idle>-0 [001] d.s3 20803.501690: lock_release: 0000000008b91ab4 &sg_policy->update_lock
>
> This patch fixes calling tracepoints for lock_contended() and
> lock_acquired().
>
> Fixes: eb1f00237aca ("lockdep,trace: Expose tracepoints")
> Signed-off-by: Leo Yan <[email protected]>

No idea how that happened, curious. Thanks for fixing though!

2021-05-12 13:35:55

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] locking/lockdep: Correct calling tracepoints

On Wed, 12 May 2021 14:37:12 +0200
Peter Zijlstra <[email protected]> wrote:

> On Wed, May 12, 2021 at 08:09:37PM +0800, Leo Yan wrote:
> > The commit eb1f00237aca ("lockdep,trace: Expose tracepoints") reverses
> > tracepoints for lock_contended() and lock_acquired(), thus the ftrace
> > log shows the wrong locking sequence that "acquired" event is prior to
> > "contended" event:
> >
> > <idle>-0 [001] d.s3 20803.501685: lock_acquire: 0000000008b91ab4 &sg_policy->update_lock
> > <idle>-0 [001] d.s3 20803.501686: lock_acquired: 0000000008b91ab4 &sg_policy->update_lock
> > <idle>-0 [001] d.s3 20803.501689: lock_contended: 0000000008b91ab4 &sg_policy->update_lock
> > <idle>-0 [001] d.s3 20803.501690: lock_release: 0000000008b91ab4 &sg_policy->update_lock
> >
> > This patch fixes calling tracepoints for lock_contended() and
> > lock_acquired().
> >
> > Fixes: eb1f00237aca ("lockdep,trace: Expose tracepoints")
> > Signed-off-by: Leo Yan <[email protected]>
>
> No idea how that happened, curious. Thanks for fixing though!

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Steven Rostedt (VMware) <[email protected]>
Reviewed-by: Thomas Gleixner <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
Tested-by: Marco Elver <[email protected]>

That's embarrassing :-p

-- Steve

2021-05-12 14:14:12

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] locking/lockdep: Correct calling tracepoints

On Wed, May 12, 2021 at 09:32:02AM -0400, Steven Rostedt wrote:
> On Wed, 12 May 2021 14:37:12 +0200
> Peter Zijlstra <[email protected]> wrote:
>
> > On Wed, May 12, 2021 at 08:09:37PM +0800, Leo Yan wrote:
> > > The commit eb1f00237aca ("lockdep,trace: Expose tracepoints") reverses
> > > tracepoints for lock_contended() and lock_acquired(), thus the ftrace
> > > log shows the wrong locking sequence that "acquired" event is prior to
> > > "contended" event:
> > >
> > > <idle>-0 [001] d.s3 20803.501685: lock_acquire: 0000000008b91ab4 &sg_policy->update_lock
> > > <idle>-0 [001] d.s3 20803.501686: lock_acquired: 0000000008b91ab4 &sg_policy->update_lock
> > > <idle>-0 [001] d.s3 20803.501689: lock_contended: 0000000008b91ab4 &sg_policy->update_lock
> > > <idle>-0 [001] d.s3 20803.501690: lock_release: 0000000008b91ab4 &sg_policy->update_lock
> > >
> > > This patch fixes calling tracepoints for lock_contended() and
> > > lock_acquired().
> > >
> > > Fixes: eb1f00237aca ("lockdep,trace: Expose tracepoints")
> > > Signed-off-by: Leo Yan <[email protected]>
> >
> > No idea how that happened, curious. Thanks for fixing though!
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> Reviewed-by: Steven Rostedt (VMware) <[email protected]>
> Reviewed-by: Thomas Gleixner <[email protected]>
> Acked-by: Rafael J. Wysocki <[email protected]>
> Tested-by: Marco Elver <[email protected]>
>
> That's embarrassing :-p

Thing is, if you look at the list copy:

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

things were okay. I suspect there was a conflict at some point and
resolution got it backwards because the hunks are so similar and I never
noticed :/

2021-05-12 16:08:39

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] locking/lockdep: Correct calling tracepoints

On Wed, 12 May 2021 16:05:33 +0200
Peter Zijlstra <[email protected]> wrote:

> Thing is, if you look at the list copy:
>
> https://lore.kernel.org/lkml/[email protected]/
>
> things were okay. I suspect there was a conflict at some point and
> resolution got it backwards because the hunks are so similar and I never
> noticed :/

Was this a git merge issue or a quilt merge issue ?

-- Steve

2021-05-12 19:49:49

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] locking/lockdep: Correct calling tracepoints

On Wed, May 12, 2021 at 11:14:39AM -0400, Steven Rostedt wrote:
> On Wed, 12 May 2021 16:05:33 +0200
> Peter Zijlstra <[email protected]> wrote:
>
> > Thing is, if you look at the list copy:
> >
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > things were okay. I suspect there was a conflict at some point and
> > resolution got it backwards because the hunks are so similar and I never
> > noticed :/
>
> Was this a git merge issue or a quilt merge issue ?

Probably quilt, I never do anything like that with git.

2021-05-19 19:24:08

by tip-bot2 for Haifeng Xu

[permalink] [raw]
Subject: [tip: locking/urgent] locking/lockdep: Correct calling tracepoints

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

Commit-ID: 89e70d5c583c55088faa2201d397ee30a15704aa
Gitweb: https://git.kernel.org/tip/89e70d5c583c55088faa2201d397ee30a15704aa
Author: Leo Yan <[email protected]>
AuthorDate: Wed, 12 May 2021 20:09:37 +08:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Tue, 18 May 2021 12:53:50 +02:00

locking/lockdep: Correct calling tracepoints

The commit eb1f00237aca ("lockdep,trace: Expose tracepoints") reverses
tracepoints for lock_contended() and lock_acquired(), thus the ftrace
log shows the wrong locking sequence that "acquired" event is prior to
"contended" event:

<idle>-0 [001] d.s3 20803.501685: lock_acquire: 0000000008b91ab4 &sg_policy->update_lock
<idle>-0 [001] d.s3 20803.501686: lock_acquired: 0000000008b91ab4 &sg_policy->update_lock
<idle>-0 [001] d.s3 20803.501689: lock_contended: 0000000008b91ab4 &sg_policy->update_lock
<idle>-0 [001] d.s3 20803.501690: lock_release: 0000000008b91ab4 &sg_policy->update_lock

This patch fixes calling tracepoints for lock_contended() and
lock_acquired().

Fixes: eb1f00237aca ("lockdep,trace: Expose tracepoints")
Signed-off-by: Leo Yan <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
kernel/locking/lockdep.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 48d736a..7641bd4 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -5736,7 +5736,7 @@ void lock_contended(struct lockdep_map *lock, unsigned long ip)
{
unsigned long flags;

- trace_lock_acquired(lock, ip);
+ trace_lock_contended(lock, ip);

if (unlikely(!lock_stat || !lockdep_enabled()))
return;
@@ -5754,7 +5754,7 @@ void lock_acquired(struct lockdep_map *lock, unsigned long ip)
{
unsigned long flags;

- trace_lock_contended(lock, ip);
+ trace_lock_acquired(lock, ip);

if (unlikely(!lock_stat || !lockdep_enabled()))
return;