lkml.org 
[lkml]   [2008]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/7] work_on_cpu: helper for doing task on a CPU.
On 10/23, Gautham R Shenoy wrote:
>
> On Thu, Oct 23, 2008 at 11:40:36AM +0200, Oleg Nesterov wrote:
> >
> > IOW, I'd suggest
> >
> > long work_on_cpu(unsigned int cpu, long (*fn)(void *), void *arg)
> > {
> > struct work_for_cpu wfc;
> >
> > INIT_WORK(&wfc.work, do_work_for_cpu);
> > wfc.fn = fn;
> > wfc.arg = arg;
> > wfc.ret = -EINVAL;
> >
> > get_online_cpus();
> > if (likely(cpu_online(cpu))) {
> > schedule_work_on(cpu, &wfc.work);
> > flush_work(&wfc.work);
> > }
>
> OK, how about doing the following? That will solve the problem
> of deadlock you pointed out in patch 6.
>
> get_online_cpus();
> if (likely(per_cpu(cpu_state, cpuid) == CPU_ONLINE)) {
> schedule_work_on(cpu, &wfc.work);
> flush_work(&wfc.work);
> } else if (per_cpu(cpu_state, cpuid) != CPU_DEAD)) {
> /*
> * We're the CPU-Hotplug thread. Call the
> * function synchronously so that we don't
> * deadlock with any pending work-item blocked
> * on get_online_cpus()
> */
> cpumask_t orignal_mask = current->cpus_allowed;
> set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu);
> wfc.ret = fn(arg);
> set_cpus_allowed_ptr(current, &original_mask);

Not sure I understand...

_cpu_up() does raw_notifier_call_chain(CPU_ONLINE) after __cpu_up(),
at this point per_cpu(cpu_state) == CPU_ONLINE.

(OK, this is not exactly true, start_secondary() updates cpu_online_map
and only then cpu_state = CPU_ONLINE, but __cpu_up() waits for
cpu_online(cpu) == T).


Anyway, personally I dislike this special case. We must not use work_on_cpu()
if we hold the lock which can be used by some work_struct, cpu_hotplug is not
special at all.

Oleg.



\
 
 \ /
  Last update: 2008-10-23 18:37    [from the cache]
©2003-2011 Jasper Spaans