linux-kernel - Re: [PATCH v3 tip/core/rcu 1/9] rcu: Add call_rcu

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] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 9 Aug 2014 08:15:14 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:	linux-kernel@...r.kernel.org, mingo@...nel.org,
	laijs@...fujitsu.com, dipankar@...ibm.com,
	akpm@...ux-foundation.org, mathieu.desnoyers@...icios.com,
	josh@...htriplett.org, tglx@...utronix.de, rostedt@...dmis.org,
	dhowells@...hat.com, edumazet@...gle.com, dvhart@...ux.intel.com,
	fweisbec@...il.com, oleg@...hat.com, bobby.prani@...il.com
Subject: Re: [PATCH v3 tip/core/rcu 1/9] rcu: Add call_rcu_tasks()

On Fri, Aug 08, 2014 at 01:58:26PM -0700, Paul E. McKenney wrote:
> On Fri, Aug 08, 2014 at 09:13:26PM +0200, Peter Zijlstra wrote:
> > 
> > 
> > So I think you can make the entire thing work with
> > rcu_note_context_switch().
> > 
> > If we have the sync thing do something like:
> > 
> > 
> > 	for_each_task(t) {
> > 		atomic_inc(&rcu_tasks);
> > 		atomic_or(&t->rcu_attention, RCU_TASK);
> > 		smp_mb__after_atomic();
> > 		if (!t->on_rq) {
> > 			if (atomic_test_and_clear(&t->rcu_attention, RCU_TASK))
> > 				atomic_dec(&rcu_tasks);
> > 		}
> > 	}
> > 
> > 	wait_event(&rcu_tasks_wq, !atomic_read(&rcu_tasks));
> > 
> > 
> > And then we have rcu_task_note_context_switch() (as called from
> > rcu_note_context_switch) do:
> > 
> > 
> > 	/* we want actual context switches, ignore preemption */
> > 	if (preempt_count() & PREEMPT_ACTIVE)
> > 		return;
> > 
> > 	/* if not marked for RCU attention, bail */
> > 	if (!(atomic_read(&t->rcu_attention) & RCU_TASK))
> > 		return;
> > 
> > 	/* raced with sync_rcu_task(), all done */
> > 	if (!atomic_test_and_clear(&t->rcu_attention, RCU_TASK))
> > 		return;
> > 
> > 	/* not the last.. */
> > 	if (!atomic_dec_and_test(&rcu_tasks))
> > 		return;
> > 
> > 	wake_up(&rcu_task_rq);
> > 
> > 
> > The idea is to share rcu_attention with rcu_preempt, such that we only
> > touch a single 'extra' cacheline in case RCU doesn't need to pay
> > attention to this task.
> > 
> > Also, it would be good if we can manage to squeeze this variable in a
> > cacheline that's already touched by the schedule() so as not to incur
> > undue overhead.
> 
> This approach does not get me the idle tasks and the NO_HZ_FULL usermode
> tasks.  I am pretty sure that I am stuck polling in those cases, so I
> might as well poll.

That's so wrong its not funny. If you need some abortion to deal with
NOHZ_FULL then put it under CONFIG_NOHZ_FULL, don't burden the entire
world with it.

Also, I thought RCU already knew which CPUs were in nohz_full userspace,
so we can insta check that in the sync, together with the !->on_rq test,
if the task is running on a nohz_full cpu in rcu quiescent state, also
clear the task.

As for idle tasks, I'm not sure about those, I think that we should say
NO to anything that would require waking idle CPUs, push the pain to
ftrace/kprobes, we should _not_ be waking idle cpus.

Content of type "application/pgp-signature" skipped