linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] locking/lockdep: Correct calling tracepoints
@ 2021-05-12 12:09 Leo Yan
  2021-05-12 12:37 ` Peter Zijlstra
  2021-05-19  8:09 ` [tip: locking/urgent] " tip-bot2 for Leo Yan
  0 siblings, 2 replies; 7+ messages in thread
From: Leo Yan @ 2021-05-12 12:09 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
	Boqun Feng, Thomas Gleixner, Rafael J. Wysocki,
	Steven Rostedt (VMware),
	linux-kernel
  Cc: Leo Yan

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 <leo.yan@linaro.org>
---
 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] locking/lockdep: Correct calling tracepoints
  2021-05-12 12:09 [PATCH] locking/lockdep: Correct calling tracepoints Leo Yan
@ 2021-05-12 12:37 ` Peter Zijlstra
  2021-05-12 13:32   ` Steven Rostedt
  2021-05-19  8:09 ` [tip: locking/urgent] " tip-bot2 for Leo Yan
  1 sibling, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2021-05-12 12:37 UTC (permalink / raw)
  To: Leo Yan
  Cc: Ingo Molnar, Will Deacon, Waiman Long, Boqun Feng,
	Thomas Gleixner, Rafael J. Wysocki, Steven Rostedt (VMware),
	linux-kernel

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 <leo.yan@linaro.org>

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] locking/lockdep: Correct calling tracepoints
  2021-05-12 12:37 ` Peter Zijlstra
@ 2021-05-12 13:32   ` Steven Rostedt
  2021-05-12 14:05     ` Peter Zijlstra
  0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2021-05-12 13:32 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Leo Yan, Ingo Molnar, Will Deacon, Waiman Long, Boqun Feng,
	Thomas Gleixner, Rafael J. Wysocki, linux-kernel

On Wed, 12 May 2021 14:37:12 +0200
Peter Zijlstra <peterz@infradead.org> 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 <leo.yan@linaro.org>  
> 
> No idea how that happened, curious. Thanks for fixing though!

    Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
    Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
    Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
    Tested-by: Marco Elver <elver@google.com>

That's embarrassing :-p

-- Steve

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] locking/lockdep: Correct calling tracepoints
  2021-05-12 13:32   ` Steven Rostedt
@ 2021-05-12 14:05     ` Peter Zijlstra
  2021-05-12 15:14       ` Steven Rostedt
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2021-05-12 14:05 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Leo Yan, Ingo Molnar, Will Deacon, Waiman Long, Boqun Feng,
	Thomas Gleixner, Rafael J. Wysocki, linux-kernel

On Wed, May 12, 2021 at 09:32:02AM -0400, Steven Rostedt wrote:
> On Wed, 12 May 2021 14:37:12 +0200
> Peter Zijlstra <peterz@infradead.org> 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 <leo.yan@linaro.org>  
> > 
> > No idea how that happened, curious. Thanks for fixing though!
> 
>     Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>     Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>     Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
>     Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>     Tested-by: Marco Elver <elver@google.com>
> 
> That's embarrassing :-p

Thing is, if you look at the list copy:

  https://lore.kernel.org/lkml/20200821085348.782688941@infradead.org/

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 :/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] locking/lockdep: Correct calling tracepoints
  2021-05-12 14:05     ` Peter Zijlstra
@ 2021-05-12 15:14       ` Steven Rostedt
  2021-05-12 16:54         ` Peter Zijlstra
  0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2021-05-12 15:14 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Leo Yan, Ingo Molnar, Will Deacon, Waiman Long, Boqun Feng,
	Thomas Gleixner, Rafael J. Wysocki, linux-kernel

On Wed, 12 May 2021 16:05:33 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> Thing is, if you look at the list copy:
> 
>   https://lore.kernel.org/lkml/20200821085348.782688941@infradead.org/
> 
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] locking/lockdep: Correct calling tracepoints
  2021-05-12 15:14       ` Steven Rostedt
@ 2021-05-12 16:54         ` Peter Zijlstra
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2021-05-12 16:54 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Leo Yan, Ingo Molnar, Will Deacon, Waiman Long, Boqun Feng,
	Thomas Gleixner, Rafael J. Wysocki, linux-kernel

On Wed, May 12, 2021 at 11:14:39AM -0400, Steven Rostedt wrote:
> On Wed, 12 May 2021 16:05:33 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > Thing is, if you look at the list copy:
> > 
> >   https://lore.kernel.org/lkml/20200821085348.782688941@infradead.org/
> > 
> > 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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [tip: locking/urgent] locking/lockdep: Correct calling tracepoints
  2021-05-12 12:09 [PATCH] locking/lockdep: Correct calling tracepoints Leo Yan
  2021-05-12 12:37 ` Peter Zijlstra
@ 2021-05-19  8:09 ` tip-bot2 for Leo Yan
  1 sibling, 0 replies; 7+ messages in thread
From: tip-bot2 for Leo Yan @ 2021-05-19  8:09 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Leo Yan, Peter Zijlstra (Intel), x86, linux-kernel

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 <leo.yan@linaro.org>
AuthorDate:    Wed, 12 May 2021 20:09:37 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
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 <leo.yan@linaro.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20210512120937.90211-1-leo.yan@linaro.org
---
 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;

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-05-19  8:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-12 12:09 [PATCH] locking/lockdep: Correct calling tracepoints Leo Yan
2021-05-12 12:37 ` Peter Zijlstra
2021-05-12 13:32   ` Steven Rostedt
2021-05-12 14:05     ` Peter Zijlstra
2021-05-12 15:14       ` Steven Rostedt
2021-05-12 16:54         ` Peter Zijlstra
2021-05-19  8:09 ` [tip: locking/urgent] " tip-bot2 for Leo Yan

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).