Re: [PATCH] sched/core: simpler function for sched_exec migration

[Oleg Nesterov]

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.