From: Peter Zijlstra <peterz@infradead.org>
To: mingo@kernel.org, oleg@redhat.com, torvalds@linux-foundation.org
Cc: tglx@linutronix.de, ilya.dryomov@inktank.com,
umgwanakikbuti@gmail.com, linux-kernel@vger.kernel.org,
Peter Zijlstra <peterz@infradead.org>
Subject: [RFC][PATCH 7/7] sched: Debug nested sleeps
Date: Mon, 04 Aug 2014 12:30:32 +0200 [thread overview]
Message-ID: <20140804103537.758217281@infradead.org> (raw)
In-Reply-To: 20140804103025.478913141@infradead.org
[-- Attachment #1: peterz-might_sleep_nesting.patch --]
[-- Type: text/plain, Size: 3670 bytes --]
Validate we call might_sleep() with TASK_RUNNING, which catches places
where we nest blocking primitives, eg. mutex usage in a wait loop.
Since all blocking is arranged through task_struct::state, nesting
this is going to cause two distinct issues:
- the inner primitive will set TASK_RUNNING and the outer will not
block
- the outer sets !TASK_RUNNING and the inner expects to be called
with TASK_RUNNING and blocks forever (mutex_lock).
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
---
include/linux/sched.h | 46 ++++++++++++++++++++++++++++++++++++++++++++--
kernel/sched/core.c | 13 +++++++++++++
2 files changed, 57 insertions(+), 2 deletions(-)
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -241,6 +241,43 @@ extern char ___assert_task_state[1 - 2*!
((task->state & TASK_UNINTERRUPTIBLE) != 0 && \
(task->flags & PF_FROZEN) == 0)
+#ifdef CONFIG_DEBUG_ATOMIC_SLEEP
+
+#define __set_task_state(tsk, state_value) \
+ do { \
+ (tsk)->task_state_change = _THIS_IP_; \
+ (tsk)->state = (state_value); \
+ } while (0)
+#define set_task_state(tsk, state_value) \
+ do { \
+ (tsk)->task_state_change = _THIS_IP_; \
+ set_mb((tsk)->state, (state_value)); \
+ } while (0)
+
+/*
+ * set_current_state() includes a barrier so that the write of current->state
+ * is correctly serialised wrt the caller's subsequent test of whether to
+ * actually sleep:
+ *
+ * set_current_state(TASK_UNINTERRUPTIBLE);
+ * if (do_i_need_to_sleep())
+ * schedule();
+ *
+ * If the caller does not need such serialisation then use __set_current_state()
+ */
+#define __set_current_state(state_value) \
+ do { \
+ current->task_state_change = _THIS_IP_; \
+ current->state = (state_value); \
+ } while (0)
+#define set_current_state(state_value) \
+ do { \
+ current->task_state_change = _THIS_IP_; \
+ set_mb(current->state, (state_value)); \
+ } while (0)
+
+#else
+
#define __set_task_state(tsk, state_value) \
do { (tsk)->state = (state_value); } while (0)
#define set_task_state(tsk, state_value) \
@@ -257,11 +294,13 @@ extern char ___assert_task_state[1 - 2*!
*
* If the caller does not need such serialisation then use __set_current_state()
*/
-#define __set_current_state(state_value) \
+#define __set_current_state(state_value) \
do { current->state = (state_value); } while (0)
-#define set_current_state(state_value) \
+#define set_current_state(state_value) \
set_mb(current->state, (state_value))
+#endif
+
/* Task command name length */
#define TASK_COMM_LEN 16
@@ -1650,6 +1689,9 @@ struct task_struct {
unsigned int sequential_io;
unsigned int sequential_io_avg;
#endif
+#ifdef CONFIG_DEBUG_ATOMIC_SLEEP
+ unsigned long task_state_change;
+#endif
};
/* Future-safe accessor for struct task_struct's cpus_allowed. */
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -7077,6 +7077,19 @@ void __might_sleep(const char *file, int
{
static unsigned long prev_jiffy; /* ratelimiting */
+ /*
+ * Blocking primitives will set (and therefore destroy) current->state,
+ * since we will exit with TASK_RUNNING make sure we enter with it,
+ * otherwise we will destroy state.
+ */
+ if (WARN(current->state != TASK_RUNNING,
+ "do not call blocking ops when !TASK_RUNNING; "
+ "state=%lx set at [<%p>] %pS\n",
+ current->state,
+ (void *)current->task_state_change,
+ (void *)current->task_state_change))
+ __set_current_state(TASK_RUNNING);
+
rcu_sleep_check(); /* WARN_ON_ONCE() by default, no rate limit reqd. */
if ((preempt_count_equals(preempt_offset) && !irqs_disabled() &&
!is_idle_task(current)) ||
next prev parent reply other threads:[~2014-08-04 10:40 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-04 10:30 [RFC][PATCH 0/7] nested sleeps, fixes and debug infra Peter Zijlstra
2014-08-04 10:30 ` [RFC][PATCH 1/7] wait: Provide infrastructure to deal with nested blocking Peter Zijlstra
2014-08-04 13:44 ` Peter Zijlstra
2014-08-04 18:35 ` Oleg Nesterov
2014-08-04 10:30 ` [RFC][PATCH 2/7] wait: Provide Peter Zijlstra
2014-08-04 10:30 ` [RFC][PATCH 3/7] exit: Desl with nested sleeps Peter Zijlstra
2014-08-04 18:53 ` Oleg Nesterov
2014-08-04 10:30 ` [RFC][PATCH 4/7] inotify: Deal " Peter Zijlstra
2014-08-04 19:23 ` Oleg Nesterov
2014-08-04 21:02 ` Peter Zijlstra
2014-08-05 2:22 ` Lai Jiangshan
2014-08-05 7:28 ` Peter Zijlstra
2014-08-04 10:30 ` [RFC][PATCH 5/7] tty: " Peter Zijlstra
2014-08-05 23:29 ` Greg Kroah-Hartman
2014-08-04 10:30 ` [RFC][PATCH 6/7] smp: Correctly deal " Peter Zijlstra
2014-08-04 10:30 ` Peter Zijlstra [this message]
2014-08-05 8:33 ` [RFC][PATCH 0/7] nested sleeps, fixes and debug infra Ilya Dryomov
2014-08-05 13:06 ` Peter Zijlstra
2014-08-06 7:51 ` Ilya Dryomov
2014-08-06 8:31 ` Peter Zijlstra
2014-08-06 21:16 ` David Miller
2014-08-07 8:10 ` Peter Zijlstra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140804103537.758217281@infradead.org \
--to=peterz@infradead.org \
--cc=ilya.dryomov@inktank.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=oleg@redhat.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=umgwanakikbuti@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).