On Wed, 19 Oct 2016, Peter Zijlstra wrote:

quoted

Subject: sched: Better explain sleep/wakeup
From: Peter Zijlstra <peterz@infradead.org>
Date: Wed Oct 19 15:45:27 CEST 2016

There were a few questions wrt how sleep-wakeup works. Try and explain
it more.

Requested-by: Will Deacon [off-list ref]
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/sched.h |   52 ++++++++++++++++++++++++++++++++++----------------
kernel/sched/core.c   |   15 +++++++-------
2 files changed, 44 insertions(+), 23 deletions(-)

--- a/include/linux/sched.h
+++ b/include/linux/sched.h

@@ -262,20 +262,9 @@ extern char ___assert_task_state[1 - 2*!

#define set_task_state(tsk, state_value)			\
	do {							\
		(tsk)->task_state_change = _THIS_IP_;		\
-		smp_store_mb((tsk)->state, (state_value));		\
+		smp_store_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_;		\

@@ -284,11 +273,19 @@ extern char ___assert_task_state[1 - 2*!

#define set_current_state(state_value)				\
	do {							\
		current->task_state_change = _THIS_IP_;		\
-		smp_store_mb(current->state, (state_value));		\
+		smp_store_mb(current->state, (state_value));	\
	} while (0)

#else

+/*
+ * @tsk had better be current, or you get to keep the pieces.

That reminds me we were getting rid of the set_task_state() calls. Bcache was
pending, being only user in the kernel that doesn't actually use current; but
instead breaks newly (yet blocked/uninterruptible) created garbage collection
kthread. I cannot figure out why this is done (ie purposely accounting the
load avg. Furthermore gc kicks in in very specific scenarios obviously, such
as as by the allocator task, so I don't see why bcache gc should want to be
interruptible.

Kent, Jens, can we get rid of this?

diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 76f7534d1dd1..6e3c358b5759 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c

@@ -1798,7 +1798,6 @@ int bch_gc_thread_start(struct cache_set *c)
       if (IS_ERR(c->gc_thread))
               return PTR_ERR(c->gc_thread);

-       set_task_state(c->gc_thread, TASK_INTERRUPTIBLE);
       return 0;

}