linux-kernel - Re: [PATCH] sched: Introduce TASK_NOLOAD and TASK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.11.1505130953570.1599@ja.home.ssi.bg>
Date:	Wed, 13 May 2015 10:22:23 +0300 (EEST)
From:	Julian Anastasov <ja@....bg>
To:	Peter Zijlstra <peterz@...radead.org>
cc:	Ingo Molnar <mingo@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>, linux-kernel@...r.kernel.org,
	neilb@...e.de
Subject: Re: [PATCH] sched: Introduce TASK_NOLOAD and TASK_IDLE


	Hello,

On Tue, 12 May 2015, Peter Zijlstra wrote:

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

	Probably. But some kthreads may want to sleep,
like in the IPVS case where there is a more complex
mechanism to wake up the kthread which is a socket writer
and does not poll the socket all time.

	But I see that kthreads always need to check with
kthread_should_stop(), so if we add schedule_timeout_idle()
it should not be so simple, may be something like that
is race free on kthread_stop() event, if needed at all:

/* state: TASK_IDLE (idle) or TASK_UNINTERRUPTIBLE (busy) */
kthread_schedule_timeout(timeout, state)
{
	/* note: no underscores => set_mb */
	set_current_state(state);
	/* test_bit after memory barrier */
	if (kthread_should_stop())
		return timeout;
	return schedule_timeout(timeout);
}

Regards

--
Julian Anastasov <ja@....bg>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/