linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -tip 0/2] fix lock inversion in lockless sys_times()
@ 2014-09-12 13:12 riel
  2014-09-12 13:12 ` [PATCH -tip 1/2] seqlock: add irqsave variant of read_seqbegin_or_lock riel
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: riel @ 2014-09-12 13:12 UTC (permalink / raw)
  To: peterz; +Cc: linux-kernel, mingo, prarit, oleg, sgruszka

The sig->stats_lock nests inside the tasklist_lock and the
sighand->siglock in __exit_signal and wait_task_zombie.

However, both of those locks can be taken from irq context,
which means we need to use the interrupt safe variant of
read_seqbegin_or_lock. This blocks interrupts when the "lock"
branch is taken (seq is odd), preventing the lock inversion.

On the first (lockless) pass through the loop, irqs are not
blocked.

This fixes the lockdep complaints that Stanislaw reported.


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

* [PATCH -tip 1/2] seqlock: add irqsave variant of read_seqbegin_or_lock
  2014-09-12 13:12 [PATCH -tip 0/2] fix lock inversion in lockless sys_times() riel
@ 2014-09-12 13:12 ` riel
  2014-09-19 11:44   ` [tip:sched/core] seqlock: Add irqsave variant of read_seqbegin_or_lock() tip-bot for Rik van Riel
  2014-09-12 13:12 ` [PATCH -tip 2/2] sched,time: fix lock inversion in thread_group_cputime riel
  2014-09-14  9:22 ` [PATCH -tip 0/2] fix lock inversion in lockless sys_times() Peter Zijlstra
  2 siblings, 1 reply; 6+ messages in thread
From: riel @ 2014-09-12 13:12 UTC (permalink / raw)
  To: peterz; +Cc: linux-kernel, mingo, prarit, oleg, sgruszka

From: Rik van Riel <riel@redhat.com>

There are cases where read_seqbegin_or_lock needs to block irqs,
because the seqlock in question nests inside a lock that is also
be taken from irq context.

Add read_seqbegin_or_lock_irqsave and done_seqretry_irqrestore, which
are almost identical to read_seqbegin_or_lock and done_seqretry.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 include/linux/seqlock.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
index cc35963..4364cd3 100644
--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -456,4 +456,21 @@ read_sequnlock_excl_irqrestore(seqlock_t *sl, unsigned long flags)
 	spin_unlock_irqrestore(&sl->lock, flags);
 }
 
+static inline unsigned long read_seqbegin_or_lock_irqsave(seqlock_t *lock,
+							  int *seq)
+{
+	unsigned long flags = 0;
+	if (!(*seq & 1))	/* Even */
+		*seq = read_seqbegin(lock);
+	else			/* Odd */
+		read_seqlock_excl_irqsave(lock, flags);
+	return flags;
+}
+
+static inline void done_seqretry_irqrestore(seqlock_t *lock, int seq,
+					    unsigned long flags)
+{
+	if (seq & 1)
+		read_sequnlock_excl_irqrestore(lock, flags);
+}
 #endif /* __LINUX_SEQLOCK_H */
-- 
1.9.3


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

* [PATCH -tip 2/2] sched,time: fix lock inversion in thread_group_cputime
  2014-09-12 13:12 [PATCH -tip 0/2] fix lock inversion in lockless sys_times() riel
  2014-09-12 13:12 ` [PATCH -tip 1/2] seqlock: add irqsave variant of read_seqbegin_or_lock riel
@ 2014-09-12 13:12 ` riel
  2014-09-19 11:44   ` [tip:sched/core] sched, time: Fix lock inversion in thread_group_cputime() tip-bot for Rik van Riel
  2014-09-14  9:22 ` [PATCH -tip 0/2] fix lock inversion in lockless sys_times() Peter Zijlstra
  2 siblings, 1 reply; 6+ messages in thread
From: riel @ 2014-09-12 13:12 UTC (permalink / raw)
  To: peterz; +Cc: linux-kernel, mingo, prarit, oleg, sgruszka

From: Rik van Riel <riel@redhat.com>

The sig->stats_lock nests inside the tasklist_lock and the
sighand->siglock in __exit_signal and wait_task_zombie.

However, both of those locks can be taken from irq context,
which means we need to use the interrupt safe variant of
read_seqbegin_or_lock. This blocks interrupts when the "lock"
branch is taken (seq is odd), preventing the lock inversion.

On the first (lockless) pass through the loop, irqs are not
blocked.

Signed-off-by: Rik van Riel <riel@redhat.com>
Reported-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 kernel/sched/cputime.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 2b57031..64492df 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -289,13 +289,14 @@ void thread_group_cputime(struct task_struct *tsk, struct task_cputime *times)
 	cputime_t utime, stime;
 	struct task_struct *t;
 	unsigned int seq, nextseq;
+	unsigned long flags;
 
 	rcu_read_lock();
 	/* Attempt a lockless read on the first round. */
 	nextseq = 0;
 	do {
 		seq = nextseq;
-		read_seqbegin_or_lock(&sig->stats_lock, &seq);
+		flags = read_seqbegin_or_lock_irqsave(&sig->stats_lock, &seq);
 		times->utime = sig->utime;
 		times->stime = sig->stime;
 		times->sum_exec_runtime = sig->sum_sched_runtime;
@@ -309,7 +310,7 @@ void thread_group_cputime(struct task_struct *tsk, struct task_cputime *times)
 		/* If lockless access failed, take the lock. */
 		nextseq = 1;
 	} while (need_seqretry(&sig->stats_lock, seq));
-	done_seqretry(&sig->stats_lock, seq);
+	done_seqretry_irqrestore(&sig->stats_lock, seq, flags);
 	rcu_read_unlock();
 }
 
-- 
1.9.3


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

* Re: [PATCH -tip 0/2] fix lock inversion in lockless sys_times()
  2014-09-12 13:12 [PATCH -tip 0/2] fix lock inversion in lockless sys_times() riel
  2014-09-12 13:12 ` [PATCH -tip 1/2] seqlock: add irqsave variant of read_seqbegin_or_lock riel
  2014-09-12 13:12 ` [PATCH -tip 2/2] sched,time: fix lock inversion in thread_group_cputime riel
@ 2014-09-14  9:22 ` Peter Zijlstra
  2 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2014-09-14  9:22 UTC (permalink / raw)
  To: riel; +Cc: linux-kernel, mingo, prarit, oleg, sgruszka

On Fri, Sep 12, 2014 at 09:12:13AM -0400, riel@redhat.com wrote:
> The sig->stats_lock nests inside the tasklist_lock and the
> sighand->siglock in __exit_signal and wait_task_zombie.
> 
> However, both of those locks can be taken from irq context,
> which means we need to use the interrupt safe variant of
> read_seqbegin_or_lock. This blocks interrupts when the "lock"
> branch is taken (seq is odd), preventing the lock inversion.
> 
> On the first (lockless) pass through the loop, irqs are not
> blocked.
> 
> This fixes the lockdep complaints that Stanislaw reported.
> 

Thanks Rik

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

* [tip:sched/core] seqlock: Add irqsave variant of read_seqbegin_or_lock()
  2014-09-12 13:12 ` [PATCH -tip 1/2] seqlock: add irqsave variant of read_seqbegin_or_lock riel
@ 2014-09-19 11:44   ` tip-bot for Rik van Riel
  0 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Rik van Riel @ 2014-09-19 11:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, mathieu.desnoyers, trond.myklebust,
	torvalds, peterz, sboyd, viro, riel, john.stultz, tglx

Commit-ID:  ef8ac06359ddf95431cf6bb04ad2b36fff562328
Gitweb:     http://git.kernel.org/tip/ef8ac06359ddf95431cf6bb04ad2b36fff562328
Author:     Rik van Riel <riel@redhat.com>
AuthorDate: Fri, 12 Sep 2014 09:12:14 -0400
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 19 Sep 2014 12:35:16 +0200

seqlock: Add irqsave variant of read_seqbegin_or_lock()

There are cases where read_seqbegin_or_lock() needs to block irqs,
because the seqlock in question nests inside a lock that is also
be taken from irq context.

Add read_seqbegin_or_lock_irqsave() and done_seqretry_irqrestore(), which
are almost identical to read_seqbegin_or_lock() and done_seqretry().

Signed-off-by: Rik van Riel <riel@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: prarit@redhat.com
Cc: oleg@redhat.com
Cc: sgruszka@redhat.com
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Link: http://lkml.kernel.org/r/1410527535-9814-2-git-send-email-riel@redhat.com
[ Improved the readability of the code a bit. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/seqlock.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
index cc35963..f5df8f6 100644
--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -456,4 +456,23 @@ read_sequnlock_excl_irqrestore(seqlock_t *sl, unsigned long flags)
 	spin_unlock_irqrestore(&sl->lock, flags);
 }
 
+static inline unsigned long
+read_seqbegin_or_lock_irqsave(seqlock_t *lock, int *seq)
+{
+	unsigned long flags = 0;
+
+	if (!(*seq & 1))	/* Even */
+		*seq = read_seqbegin(lock);
+	else			/* Odd */
+		read_seqlock_excl_irqsave(lock, flags);
+
+	return flags;
+}
+
+static inline void
+done_seqretry_irqrestore(seqlock_t *lock, int seq, unsigned long flags)
+{
+	if (seq & 1)
+		read_sequnlock_excl_irqrestore(lock, flags);
+}
 #endif /* __LINUX_SEQLOCK_H */

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

* [tip:sched/core] sched, time: Fix lock inversion in thread_group_cputime()
  2014-09-12 13:12 ` [PATCH -tip 2/2] sched,time: fix lock inversion in thread_group_cputime riel
@ 2014-09-19 11:44   ` tip-bot for Rik van Riel
  0 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Rik van Riel @ 2014-09-19 11:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, torvalds, peterz, riel, tglx, sgruszka

Commit-ID:  9c368b5b6eccce1cbd7f68142106b3b4ddb1c5b5
Gitweb:     http://git.kernel.org/tip/9c368b5b6eccce1cbd7f68142106b3b4ddb1c5b5
Author:     Rik van Riel <riel@redhat.com>
AuthorDate: Fri, 12 Sep 2014 09:12:15 -0400
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 19 Sep 2014 12:35:17 +0200

sched, time: Fix lock inversion in thread_group_cputime()

The sig->stats_lock nests inside the tasklist_lock and the
sighand->siglock in __exit_signal and wait_task_zombie.

However, both of those locks can be taken from irq context,
which means we need to use the interrupt safe variant of
read_seqbegin_or_lock. This blocks interrupts when the "lock"
branch is taken (seq is odd), preventing the lock inversion.

On the first (lockless) pass through the loop, irqs are not
blocked.

Reported-by: Stanislaw Gruszka <sgruszka@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: prarit@redhat.com
Cc: oleg@redhat.com
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1410527535-9814-3-git-send-email-riel@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/cputime.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 2b57031..64492df 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -289,13 +289,14 @@ void thread_group_cputime(struct task_struct *tsk, struct task_cputime *times)
 	cputime_t utime, stime;
 	struct task_struct *t;
 	unsigned int seq, nextseq;
+	unsigned long flags;
 
 	rcu_read_lock();
 	/* Attempt a lockless read on the first round. */
 	nextseq = 0;
 	do {
 		seq = nextseq;
-		read_seqbegin_or_lock(&sig->stats_lock, &seq);
+		flags = read_seqbegin_or_lock_irqsave(&sig->stats_lock, &seq);
 		times->utime = sig->utime;
 		times->stime = sig->stime;
 		times->sum_exec_runtime = sig->sum_sched_runtime;
@@ -309,7 +310,7 @@ void thread_group_cputime(struct task_struct *tsk, struct task_cputime *times)
 		/* If lockless access failed, take the lock. */
 		nextseq = 1;
 	} while (need_seqretry(&sig->stats_lock, seq));
-	done_seqretry(&sig->stats_lock, seq);
+	done_seqretry_irqrestore(&sig->stats_lock, seq, flags);
 	rcu_read_unlock();
 }
 

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

end of thread, other threads:[~2014-09-19 11:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-12 13:12 [PATCH -tip 0/2] fix lock inversion in lockless sys_times() riel
2014-09-12 13:12 ` [PATCH -tip 1/2] seqlock: add irqsave variant of read_seqbegin_or_lock riel
2014-09-19 11:44   ` [tip:sched/core] seqlock: Add irqsave variant of read_seqbegin_or_lock() tip-bot for Rik van Riel
2014-09-12 13:12 ` [PATCH -tip 2/2] sched,time: fix lock inversion in thread_group_cputime riel
2014-09-19 11:44   ` [tip:sched/core] sched, time: Fix lock inversion in thread_group_cputime() tip-bot for Rik van Riel
2014-09-14  9:22 ` [PATCH -tip 0/2] fix lock inversion in lockless sys_times() Peter Zijlstra

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