lkml.org 
[lkml]   [2010]   [Dec]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH 08/17] sched: Drop the rq argument to sched_class::select_task_rq()
On Fri, Dec 24, 2010 at 01:23:46PM +0100, Peter Zijlstra wrote:
> In preparation of calling select_task_rq() without rq->lock held, drop
> the dependency on the rq argument.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> ---
> @@ -3416,27 +3409,22 @@ void sched_exec(void)
> {
> struct task_struct *p = current;
> unsigned long flags;
> - struct rq *rq;
> int dest_cpu;
>
> - rq = task_rq_lock(p, &flags);
> - dest_cpu = p->sched_class->select_task_rq(rq, p, SD_BALANCE_EXEC, 0);
> + raw_spin_lock_irqsave(&p->pi_lock, flags);

Seems this should go to patch 07/17 ;)

> + dest_cpu = p->sched_class->select_task_rq(p, SD_BALANCE_EXEC, 0);
> if (dest_cpu == smp_processor_id())
> goto unlock;
>
> - /*
> - * select_task_rq() can race against ->cpus_allowed
> - */
> - if (cpumask_test_cpu(dest_cpu, &p->cpus_allowed) &&
> - likely(cpu_active(dest_cpu)) && migrate_task(p, rq)) {
> + if (likely(cpu_active(dest_cpu)) && need_migrate_task(p)) {

If we drop rq_lock, need_migrate_task() maybe return true but
p is already running on other cpu. Thus we do a wrong migration
call.

Thanks,
Yong
> struct migration_arg arg = { p, dest_cpu };
>
> - task_rq_unlock(rq, &flags);
> - stop_one_cpu(cpu_of(rq), migration_cpu_stop, &arg);
> + raw_spin_unlock_irqrestore(&p->pi_lock, flags);
> + stop_one_cpu(task_cpu(p), migration_cpu_stop, &arg);
> return;
> }
> unlock:
> - task_rq_unlock(rq, &flags);
> + raw_spin_unlock_irqrestore(&p->pi_lock, flags);
> }
>
> #endif
> @@ -5681,7 +5669,7 @@ int set_cpus_allowed_ptr(struct task_str
> goto out;
>
> dest_cpu = cpumask_any_and(cpu_active_mask, new_mask);
> - if (migrate_task(p, rq)) {
> + if (need_migrate_task(p)) {
> struct migration_arg arg = { p, dest_cpu };
> /* Need help from migration thread: drop lock and wait. */
> __task_rq_unlock(rq);
> Index: linux-2.6/kernel/sched_fair.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched_fair.c
> +++ linux-2.6/kernel/sched_fair.c
> @@ -1623,7 +1623,7 @@ static int select_idle_sibling(struct ta
> * preempt must be disabled.
> */
> static int
> -select_task_rq_fair(struct rq *rq, struct task_struct *p, int sd_flag, int wake_flags)
> +select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
> {
> struct sched_domain *tmp, *affine_sd = NULL, *sd = NULL;
> int cpu = smp_processor_id();
> Index: linux-2.6/kernel/sched_idletask.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched_idletask.c
> +++ linux-2.6/kernel/sched_idletask.c
> @@ -7,7 +7,7 @@
>
> #ifdef CONFIG_SMP
> static int
> -select_task_rq_idle(struct rq *rq, struct task_struct *p, int sd_flag, int flags)
> +select_task_rq_idle(struct task_struct *p, int sd_flag, int flags)
> {
> return task_cpu(p); /* IDLE tasks as never migrated */
> }
> Index: linux-2.6/kernel/sched_rt.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched_rt.c
> +++ linux-2.6/kernel/sched_rt.c
> @@ -973,11 +973,18 @@ static void yield_task_rt(struct rq *rq)
> static int find_lowest_rq(struct task_struct *task);
>
> static int
> -select_task_rq_rt(struct rq *rq, struct task_struct *p, int sd_flag, int flags)
> +select_task_rq_rt(struct task_struct *p, int sd_flag, int flags)
> {
> if (sd_flag != SD_BALANCE_WAKE)
> return smp_processor_id();
>
> +#if 0
> + /*
> + * XXX without holding rq->lock the below is racy, need to
> + * rewrite it in a racy but non-dangerous way so that we mostly
> + * get the benefit of the heuristic but don't crash the kernel
> + * if we get it wrong ;-)
> + */
> /*
> * If the current task is an RT task, then
> * try to see if we can wake this RT task up on another
> @@ -1002,6 +1009,7 @@ select_task_rq_rt(struct rq *rq, struct
>
> return (cpu == -1) ? task_cpu(p) : cpu;
> }
> +#endif
>
> /*
> * Otherwise, just let it ride on the affined RQ and the
> Index: linux-2.6/kernel/sched_stoptask.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched_stoptask.c
> +++ linux-2.6/kernel/sched_stoptask.c
> @@ -9,8 +9,7 @@
>
> #ifdef CONFIG_SMP
> static int
> -select_task_rq_stop(struct rq *rq, struct task_struct *p,
> - int sd_flag, int flags)
> +select_task_rq_stop(struct task_struct *p, int sd_flag, int flags)
> {
> return task_cpu(p); /* stop tasks as never migrate */
> }


\
 
 \ /
  Last update: 2010-12-29 15:35    [W:0.229 / U:0.076 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site