On 10/14, Peter Zijlstra wrote:
>
> On Wed, Oct 14, 2015 at 09:03:56PM +0200, Oleg Nesterov wrote:
> > On 10/14, Peter Zijlstra wrote:
> > >
> > > On Thu, Oct 08, 2015 at 04:51:31PM +0200, Oleg Nesterov wrote:
> > > >
> > > > Note also that smpboot_thread_call() calls cpu_stop_unpark() which
> > > > sets enabled == T at CPU_ONLINE stage, so this CPU can't go away until
> > > > cpu_stopper_thread() is called at least once. This all means that if
> > > > cpu_stop_queue_work() succeeds, we know that work->fn() will be called.
> > >
> > > This hard relies on the fact that cpu_down uses stop machine, right?
> >
> > Not really.
> >
> > > IIRC part of the hotplug rework Thomas is doing is geared towards
> > > breaking away from stop machine. There is nothing fundamental about
> > > hot-unplug that requires stop machine.
> >
> > cpu_down() should park/kill/whatever the percpu stopper thread anyway.
> > And this path should clear ->enabled, it can also flush the pending
> > works.
>
> So the proposed patch does: ->enabled=false; park();, which can race
> with if (->enabled) wake();
Yes, so I added the comment to explain that this is fine.
> smpboot_thread_fn() will not call ->thread_fn() when should_park(), and
> thus any pending work will not get flushed.
>
> It only works now because the stopper task calls park(), which means
> cpu_stopper_thread() will flush, but that very much relies on the
> stopper thread calling park in itself.
Yes. IOW, this relies on ->selfparking == T which implies "flush before
park".
But even if we change cpu_down() to avoid stop_machine() I think we need
to keep this "selfparking" logic. In a sense that, for example, this code
void func(int cpu, cpu_stop_fn_t fn)
{
get_online_cpus();
if (cpu_online(cpu) {
int ret = stop_one_cpu(cpu, fn, NULL);
BUG_ON(ret == -ENOENT);
}
put_online_cpus();
}
should be correct. Actually this example is not very good, it would
be better to use stop_one_cpu_nowait() but currently it returns "void"
and hmm, it looks buggy ;) I'll send the fix on top of this series if
you accept it.
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
