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, Rusty Russell wrote:
>
> +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);
> + init_completion(&wfc.done);
> + wfc.fn = fn;
> + wfc.arg = arg;
> + get_online_cpus();
> + if (unlikely(!cpu_online(cpu))) {
> + wfc.ret = -EINVAL;
> + complete(&wfc.done);
> + } else
> + schedule_work_on(cpu, &wfc.work);

I do not claim this is wrong, but imho the code is a bit lisleading and
needs a comment (or the "fix", please see below).

Once we drop cpu_hotplug lock, CPU can go away and this work can migrate
to another cpu.

> + put_online_cpus();
> + wait_for_completion(&wfc.done);

Actually you don't need work_for_cpu->done, you can use flush_work().

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);
}
put_online_cpus();

return wfc.ret;
}

Oleg.



\
 
 \ /
  Last update: 2008-10-23 11:43    [W:0.144 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site