mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH REPOST] seqlock: Do the lockdep annotation before locking in do_write_seqcount_begin_nested()
@ 2023-09-20 10:46 Sebastian Andrzej Siewior
  2023-09-21  6:41 ` [tip: locking/urgent] locking/seqlock: " tip-bot2 for Sebastian Andrzej Siewior
  0 siblings, 1 reply; 2+ messages in thread
From: Sebastian Andrzej Siewior @ 2023-09-20 10:46 UTC (permalink / raw)
  To: linux-kernel, Peter Zijlstra
  Cc: Ingo Molnar, Will Deacon, Waiman Long, Boqun Feng,
	Thomas Gleixner, Tetsuo Handa

It was brought up by Tetsuo that the following sequence
   write_seqlock_irqsave()
   printk_deferred_enter()

could lead to a deadlock if the lockdep annotation within
write_seqlock_irqsave() triggers. The problem is that the sequence
counter is incremented before the lockdep annotation is performed. The
lockdep splat would then attempt to invoke printk() but the reader side,
of the same seqcount, could have a tty_port::lock acquired waiting for
the sequence number to become even again.

The other lockdep annotations come before the actual locking because "we
want to see the locking error before it happens". There is no reason why
seqcount should be different here.

Do the lockdep annotation first then perform the locking operation (the
sequence increment).

Fixes: 1ca7d67cf5d5a ("seqcount: Add lockdep functionality to seqcount/seqlock structures")
Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Closes: https://lore.kernel.org/20230621130641.-5iueY1I@linutronix.de
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---

The previous submission was in
    https://lore.kernel.org/r/20230623171232.892937-2-bigeasy@linutronix.de

The mm bits in the referenced thread have been merged as of -rc1.

 include/linux/seqlock.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -512,8 +512,8 @@ do {									\
 
 static inline void do_write_seqcount_begin_nested(seqcount_t *s, int subclass)
 {
-	do_raw_write_seqcount_begin(s);
 	seqcount_acquire(&s->dep_map, subclass, 0, _RET_IP_);
+	do_raw_write_seqcount_begin(s);
 }
 
 /**

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

* [tip: locking/urgent] locking/seqlock: Do the lockdep annotation before locking in do_write_seqcount_begin_nested()
  2023-09-20 10:46 [PATCH REPOST] seqlock: Do the lockdep annotation before locking in do_write_seqcount_begin_nested() Sebastian Andrzej Siewior
@ 2023-09-21  6:41 ` tip-bot2 for Sebastian Andrzej Siewior
  0 siblings, 0 replies; 2+ messages in thread
From: tip-bot2 for Sebastian Andrzej Siewior @ 2023-09-21  6:41 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Tetsuo Handa, Sebastian Andrzej Siewior, Ingo Molnar, x86, linux-kernel

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

Commit-ID:     41b43b6c6e30a832c790b010a06772e793bca193
Gitweb:        https://git.kernel.org/tip/41b43b6c6e30a832c790b010a06772e793bca193
Author:        Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate:    Wed, 20 Sep 2023 12:46:27 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Thu, 21 Sep 2023 08:37:44 +02:00

locking/seqlock: Do the lockdep annotation before locking in do_write_seqcount_begin_nested()

It was brought up by Tetsuo that the following sequence:

   write_seqlock_irqsave()
   printk_deferred_enter()

could lead to a deadlock if the lockdep annotation within
write_seqlock_irqsave() triggers.

The problem is that the sequence counter is incremented before the lockdep
annotation is performed. The lockdep splat would then attempt to invoke
printk() but the reader side, of the same seqcount, could have a
tty_port::lock acquired waiting for the sequence number to become even again.

The other lockdep annotations come before the actual locking because "we
want to see the locking error before it happens". There is no reason why
seqcount should be different here.

Do the lockdep annotation first then perform the locking operation (the
sequence increment).

Fixes: 1ca7d67cf5d5a ("seqcount: Add lockdep functionality to seqcount/seqlock structures")
Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20230920104627._DTHgPyA@linutronix.de

Closes: https://lore.kernel.org/20230621130641.-5iueY1I@linutronix.de
---
 include/linux/seqlock.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
index 987a59d..e9bd2f6 100644
--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -512,8 +512,8 @@ do {									\
 
 static inline void do_write_seqcount_begin_nested(seqcount_t *s, int subclass)
 {
-	do_raw_write_seqcount_begin(s);
 	seqcount_acquire(&s->dep_map, subclass, 0, _RET_IP_);
+	do_raw_write_seqcount_begin(s);
 }
 
 /**

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

end of thread, other threads:[~2023-09-21 21:13 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-20 10:46 [PATCH REPOST] seqlock: Do the lockdep annotation before locking in do_write_seqcount_begin_nested() Sebastian Andrzej Siewior
2023-09-21  6:41 ` [tip: locking/urgent] locking/seqlock: " tip-bot2 for Sebastian Andrzej Siewior

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome