Messages in this thread | | | Subject | Re: [PATCH] sched/core: simpler function for sched_exec migration | From | chengchao <> | Date | Thu, 8 Sep 2016 10:17:48 +0800 |
| |
Oled, thank you for moving this patch on.
on 09/07/2016 08:35 PM, Oleg Nesterov wrote: > 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. >
For CONFIG_PREEMPT_NONE=y, when sched_exec() needs migration, sched_exec() calls stop_one_cpu(task_cpu(p), migration_cpu_stop, &arg).
If stopper thread can not migrate for us,why should we call stop_one_cpu() here? It just makes the task TASK_UNINTERRUPTIBLE, wakes up the stopper thread, executes the migration_cpu_stop, and the stopper thread wakes up the task.
But in fact, all above works are almost unuseful, the reason is that the migration_cpu_stop doesn't migrate for us. why? the migration_cpu_stop() needs the task is TASK_ON_RQ_QUEUED before it calls __migrate_task().
This patch can make the task TASK_RUNNING instead of TASK_UNINTERRUPTIBLE, so the migration_cpu_stop() can migrate happily.
Does this optimization make sense now?
Any different opinions are always welcome.
>> 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. >
You have done much works for this patch. Thanks again.
> Oleg. > >
| |