Re: [PATCH v3 tip/core/rcu 1/9] rcu: Add call_rcu_tasks()

Peter Zijlstra
2014-08-09T06:16:05Z

[-- Attachment #1: Type: text/plain, Size: 2413 bytes --]

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.

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]