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