sched: Introduce TASK_NOLOAD and TASK_IDLE

Subject: sched: Introduce TASK_NOLOAD and TASK_IDLE
From: Peter Zijlstra <[email protected]>
Date: Fri May 8 14:23:45 CEST 2015

Currently people use TASK_INTERRUPTIBLE to idle kthreads and wait for 'work' because TASK_UNINTERRUPTIBLE contributes to the loadavg. Having all idle kthreads contribute to the loadavg is somewhat silly.

Now mostly this works OK, because kthreads have all their signals masked. However there's a few sites where this is causing problems and TASK_UNINTERRUPTIBLE should be used, except for that loadavg issue.

This patch adds TASK_NOLOAD which, when combined with TASK_UNINTERRUPTIBLE avoids the loadavg accounting.

As most of imagined usage sites are loops where a thread wants to idle, waiting for work, a helper TASK_IDLE is introduced.

Hello,

Quoted 12 lines · Peter Zijlstra

Subject: sched: Introduce TASK_NOLOAD and TASK_IDLE
From: Peter Zijlstra <[email protected]>
Date: Fri May 8 14:23:45 CEST 2015

Currently people use TASK_INTERRUPTIBLE to idle kthreads and wait for 'work' because TASK_UNINTERRUPTIBLE contributes to the loadavg. Having all idle kthreads contribute to the loadavg is somewhat silly.

Now mostly this works OK, because kthreads have all their signals masked. However there's a few sites where this is causing problems and TASK_UNINTERRUPTIBLE should be used, except for that loadavg issue.

Quoted 5 lines

This patch adds TASK_NOLOAD which, when combined with TASK_UNINTERRUPTIBLE avoids the loadavg accounting.

As most of imagined usage sites are loops where a thread wants to idle, waiting for work, a helper TASK_IDLE is introduced.

After checking our code in net/netfilter/ipvs/ip_vs_sync.c,
sync_thread_master(), we may also need some wrappers:

- schedule_timeout_idle (instead of schedule_timeout call):

	__set_current_state(TASK_IDLE);
	return schedule_timeout(timeout);

	- we here are really idle, so "N" looks ok

- pair of __wait_event_idle(wq, condition) and
wait_event_idle(wq, condition) macros

- we here are write-blocked for socket, not sure if this blocked vs idle difference is useful to represent, in new bit for this blocked state "B"=2048, with 2 TASK_NOLOAD variants: N(idle) and B(blocked, 2|1024|2048, eg. for read-blocked or write-blocked). It will need additional argument 'state'/'blocked' for *wait_event_idle().

Regards

--
Julian Anastasov <[email protected]>

After checking our code in net/netfilter/ipvs/ip_vs_sync.c,
sync_thread_master(), we may also need some wrappers:

- schedule_timeout_idle (instead of schedule_timeout call):

	__set_current_state(TASK_IDLE);
	return schedule_timeout(timeout);

	- we here are really idle, so "N" looks ok

- pair of __wait_event_idle(wq, condition) and
wait_event_idle(wq, condition) macros

- we here are write-blocked for socket, not sure if this blocked vs idle difference is useful to represent, in new bit for this blocked state "B"=2048, with 2 TASK_NOLOAD variants: N(idle) and B(blocked, 2|1024|2048, eg. for read-blocked or write-blocked). It will need additional argument 'state'/'blocked' for *wait_event_idle().

Quoted 5 lines

- schedule_timeout_idle (instead of schedule_timeout call):

	__set_current_state(TASK_IDLE);
	return schedule_timeout(timeout);

	- we here are really idle, so "N" looks ok

So I don't get the point of the schedule_timeout_*() stubs. What are they for? Why would one use an unconditional schedule_timeout() call? Isn't that what msleep() is for?

Quoted 9 lines · Peter Zijlstra

Quoted 5 lines

- schedule_timeout_idle (instead of schedule_timeout call):

	__set_current_state(TASK_IDLE);
	return schedule_timeout(timeout);

	- we here are really idle, so "N" looks ok

So I don't get the point of the schedule_timeout_*() stubs. What are they for? Why would one use an unconditional schedule_timeout() call? Isn't that what msleep() is for?

msleep will not return until timeout has expired.
Instead, we want to notice the kthread_should_stop() event immediately. Additionally, TASK_UNINTERRUPTIBLE will increase the load average. We can do it with extra wait queue and the new __wait_event_idle_timeout but I guess schedule_timeout_idle will be a good replacement for schedule_timeout_interruptible calls when used for kthreads.

include/linux/wait.h do { \ · line 394

Quoted 8 lines

+ * like wait_event_timeout() -- except it uses TASK_IDLE to avoid loadavg
+ */

include/linux/wait.h +6 -0

+#define wait_event_idle_timeout(wq, condition, timeout)			\
+({									\
+	long __ret = timeout;						\
+	might_sleep();							\
+	if (!___wait_cond_timeout(condition))				\
+		ret = __wait_event_idle_timeout(wq, condition, timeout);\

ret may need underscores here...

Quoted 7 lines

msleep will not return until timeout has expired.
Instead, we want to notice the kthread_should_stop() event immediately. Additionally, TASK_UNINTERRUPTIBLE will increase the load average. We can do it with extra wait queue and the new __wait_event_idle_timeout but I guess schedule_timeout_idle will be a good replacement for schedule_timeout_interruptible calls when used for kthreads.

Fair enough I suppose, but then calling it schedule*() is just plain wrong, it does not behave/act like a normal schedule() call.

Lemme go look at how widely abused that is.

*sigh*, its all over the place :/

$ git grep "schedule_timeout_\(interruptible\|killable\|uninterruptible\)" | wc -l
392

That said; I still don't see the point of schedule_timeout_idle(), we should not sleep poll for state like that. We should only use TASK_IDLE when we are in fact _IDLE_ and do not have work to do, at which point one should use an wait_event() like construct to wait for new work.