| 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
| ||
|
Date: Mon, 28 Feb 2011 11:29:44 +0800
From: Lai Jiangshan <laijs@...fujitsu.com>
To: paulmck@...ux.vnet.ibm.com
CC: linux-kernel@...r.kernel.org, mingo@...e.hu, dipankar@...ibm.com,
akpm@...ux-foundation.org, mathieu.desnoyers@...ymtl.ca,
josh@...htriplett.org, niv@...ibm.com, tglx@...utronix.de,
peterz@...radead.org, rostedt@...dmis.org, Valdis.Kletnieks@...edu,
dhowells@...hat.com, eric.dumazet@...il.com, darren@...art.com,
"Paul E. McKenney" <paul.mckenney@...aro.org>
Subject: Re: [PATCH RFC tip/core/rcu 11/11] rcu: move TREE_RCU from softirq
to kthread
On 02/26/2011 04:32 AM, Paul E. McKenney wrote:
>>> +/*
>>> + * Handle cases where the rcu_cpu_kthread() ends up on the wrong CPU.
>>> + * This can happen while the corresponding CPU is either coming online
>>> + * or going offline. We cannot wait until the CPU is fully online
>>> + * before starting the kthread, because the various notifier functions
>>> + * can wait for RCU grace periods. So we park rcu_cpu_kthread() until
>>> + * the corresponding CPU is online.
>>> + *
>>> + * Return 1 if the kthread needs to stop, 0 otherwise.
>>> + *
>>> + * Caller must disable bh. This function can momentarily enable it.
>>> + */
>>> +static int rcu_cpu_kthread_should_stop(int cpu)
>>> +{
>>> + while (cpu_is_offline(cpu) || smp_processor_id() != cpu) {
>>> + if (kthread_should_stop())
>>> + return 1;
>>> + local_bh_enable();
>>> + schedule_timeout_uninterruptible(1);
>>> + if (smp_processor_id() != cpu)
>>> + set_cpus_allowed_ptr(current, cpumask_of(cpu));
>>
>> The current task is PF_THREAD_BOUND,
>> Why do "set_cpus_allowed_ptr(current, cpumask_of(cpu));" ?
>
> Because I have seen CPU hotplug operations unbind PF_THREAD_BOUND threads.
> In addition, I end up having to spawn the kthread at CPU_UP_PREPARE time,
> at which point the thread must run unbound because its CPU isn't online
> yet. I cannot invoke kthread_create() within the stop-machine handler
> (right?). I cannot wait until CPU_ONLINE time because that results in
> hangs when other CPU notifiers wait for grace periods.
>
> Yes, I did find out about the hangs the hard way. Why do you ask? ;-)
The current task is PF_THREAD_BOUND, "set_cpus_allowed_ptr(current, cpumask_of(cpu))"
will do nothing even it runs on the wrong CPU.
If the task runs on the wrong CPU. We have no API to force/migrate the task
to the bound CPU when the cpu becomes online. But wake_up_process() has
a side affect that it will move a slept task to the correct online CPU.
"schedule_timeout_uninterruptible(1);" will call
wake_up_process() when timeout, so it will do all thing you need.
But "set_cpus_allowed_ptr(current, cpumask_of(cpu));" will do nothing.
The code is a little nasty I think. The proper solution I like:
set the rcu_cpu_notify a proper priority, and wake up the kthread
in the notifier.
Steven, any suggestion? I just known very little about scheduler.
>
> Please feel free to suggest improvements in the header comment above
> for rcu_cpu_kthread_should_stop(), which is my apparently insufficient
> attempt to explain this.
>
>>> + local_bh_disable();
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +/*
>>> + * Per-CPU kernel thread that invokes RCU callbacks. This replaces the
>>> + * earlier RCU softirq.
>>> + */
>>> +static int rcu_cpu_kthread(void *arg)
>>> +{
>>> + int cpu = (int)(long)arg;
>>> + unsigned long flags;
>>> + int spincnt = 0;
>>> + wait_queue_head_t *wqp = &per_cpu(rcu_cpu_wq, cpu);
>>> + char work;
>>> + char *workp = &per_cpu(rcu_cpu_has_work, cpu);
>>> +
>>> + for (;;) {
>>> + wait_event_interruptible(*wqp,
>>> + *workp != 0 || kthread_should_stop());
>>> + local_bh_disable();
>>> + if (rcu_cpu_kthread_should_stop(cpu)) {
>>> + local_bh_enable();
>>> + break;
>>> + }
>>> + local_irq_save(flags);
>>> + work = *workp;
>>> + *workp = 0;
>>> + local_irq_restore(flags);
>>> + if (work)
>>> + rcu_process_callbacks();
>>> + local_bh_enable();
>>> + if (*workp != 0)
>>> + spincnt++;
>>> + else
>>> + spincnt = 0;
>>> + if (spincnt > 10) {
>>
>> "10" is a magic number here.
>
> It is indeed. Suggestions for a cpp macro name to hide it behind?
>
>>> + rcu_yield(cpu);
>>> + spincnt = 0;
>>> + }
>>> + }
>>> + return 0;
>>> +}
>>> +
>>
>>
>>> +/*
>>> + * Per-rcu_node kthread, which is in charge of waking up the per-CPU
>>> + * kthreads when needed.
>>> + */
>>> +static int rcu_node_kthread(void *arg)
>>> +{
>>> + int cpu;
>>> + unsigned long flags;
>>> + unsigned long mask;
>>> + struct rcu_node *rnp = (struct rcu_node *)arg;
>>> + struct sched_param sp;
>>> + struct task_struct *t;
>>> +
>>> + for (;;) {
>>> + wait_event_interruptible(rnp->node_wq, rnp->wakemask != 0 ||
>>> + kthread_should_stop());
>>> + if (kthread_should_stop())
>>> + break;
>>> + raw_spin_lock_irqsave(&rnp->lock, flags);
>>> + mask = rnp->wakemask;
>>> + rnp->wakemask = 0;
>>> + raw_spin_unlock_irqrestore(&rnp->lock, flags);
>>> + for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
>>> + if ((mask & 0x1) == 0)
>>> + continue;
>>> + preempt_disable();
>>> + per_cpu(rcu_cpu_has_work, cpu) = 1;
>>> + t = per_cpu(rcu_cpu_kthread_task, cpu);
>>> + if (t == NULL) {
>>> + preempt_enable();
>>> + continue;
>>> + }
>>
>> Obviously preempt_disable() is not for protecting remote percpu data.
>> Is it for disabling cpu hotplug? I am afraid the @t may leave
>> and become invalid.
>
> Indeed, acquiring the rnp->lock is safer, except that I don't trust
> calling sched_setscheduler_nocheck() in that state. So I need to check
> for the CPU being online after the preempt_disable(). This means that
> I ignore requests to do work after CPU_DYING time, but that is OK because
> force_quiescent_state() will figure out that the CPU is in fact offline.
>
> Make sense?
>
Yes.
Another:
#if CONFIG_HOTPLUG_CPU
get_task_struct() when set bit in wakemask
put_task_struct() when clear bit in wakemask
#endif
> --
> 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/
>
--
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/
Powered by blists - more mailing lists