On 09/07, chengchao wrote:
>
> Oleg, thank you very much.
>
> on 09/06/2016 11:22 PM, Oleg Nesterov wrote:
> > On 09/06, chengchao wrote:
> >>
> >> the key point is for CONFIG_PREEMPT_NONE=y,
> >> ...
> >> it is too much overhead for one task(fork()+exec()), isn't it?
> >
> > Yes, yes, I see, this is suboptimal. Not sure we actually do care,
> > but yes, perhaps another helper which migrates the current task makes
> > sense, I dunno.
>
> for CONFIG_PREEMPT_NONE=y, this patch wants the stopper thread can migrate
> the current
> successfully instead of doing nothing.
I understand the intent. But I am not sure this optimization makes
sense.
> > So you need something like
> >
> > void stop_one_cpu_sync(cpu_stop_fn_t fn, void *arg)
> > {
> > struct cpu_stop_work work = { .fn = fn, .arg = arg, .done =
> > NULL };
> >
> > preempt_disable();
> > cpu_stop_queue_work(raw_smp_processor_id(), &work);
> > preempt_enable_no_resched();
> > schedule();
> > }
> >
>
> > or I am totally confused. Note that it doesn't (and shouldn't) have
> > the "int cpu" argument.
> >
>
>
> if preempt happens after preempt_enable_no_resched(),
This doesn't differ from explicit schedule() call. Either way the
stopper thread will preempt us on the same CPU.
> there is still risky that the
> stop_one_cpu_sync() returns before the stopper thread can use cpu_stop_work
> safely.
> as you said previously.
No.
However, there is another problem. It can race with another
stop_one_cpu(migration_cpu_stop) which comes between preempt_disable()
and cpu_stop_queue_work(). So the caller still can migrate to another
CPU right after after preempt_enable_no_resched() and run before the
stopper thread completes the cpu_stop_work queued by us.
> int stop_one_cpu(unsigned int cpu, cpu_stop_fn_t fn, void *arg)
> {
> struct cpu_stop_done done;
> struct cpu_stop_work work = { .fn = fn, .arg = arg, .done = &done };
>
> cpu_stop_init_done(&done, 1);
> if (!cpu_stop_queue_work(cpu, &work))
> return -ENOENT;
>
> #if defined(CONFIG_PREEMPT_NONE)
> /*
> * let the stopper thread runs as soon as possible,
> * and keep current TASK_RUNNING.
> */
> scheudle();
> #endif
> wait_for_completion(&done.completion);
> return done.ret;
> }
Agreed this looks better, although I'd suggest _cond_resche().
Again, I am not sure this makes sense, I leave this to maintainers.
Oleg.
