Re: [PATCH v6 3/3] sched/rt: Check to push the task when changing its affinity

2015-04-20 Thread Steven Rostedt
On Mon, 20 Apr 2015 16:22:48 +0800
Xunlei Pang  wrote:


> + rq = task_rq(p);
> +
> + if (new_weight > 1 &&
> + rt_task(rq->curr) &&
> + rq->rt.rt_nr_total > 1 &&
> + !test_tsk_need_resched(rq->curr)) {
> + /*
> +  * We own p->pi_lock and rq->lock. rq->lock might
> +  * get released when doing direct pushing, however
> +  * p->pi_lock is always held, so it's safe to assign
> +  * new_mask and new_weight to p below.
> +  */
> + if (!task_running(rq, p)) {
> + cpumask_copy(>cpus_allowed, new_mask);
> + p->nr_cpus_allowed = new_weight;
> + direct_push = 1;
> + } else if (cpumask_test_cpu(task_cpu(p), new_mask)) {
> + struct task_struct *next;
> +
> + cpumask_copy(>cpus_allowed, new_mask);
> + p->nr_cpus_allowed = new_weight;
> + if (!cpupri_find(>rd->cpupri, p, NULL))
> + goto update;
> +
> + /*
> +  * At this point, current task gets migratable most
> +  * likely due to the change of its affinity, let's
> +  * figure out if we can migrate it.
> +  *
> +  * Can we find any task with the same priority as
> +  * current? To accomplish this, firstly we requeue
> +  * current to the tail and peek next, then restore
> +  * current to the head.

"current"? Don't you mean 'p'?


> +  */
> + requeue_task_rt(rq, p, 0);
> + next = peek_next_task_rt(rq);
> + requeue_task_rt(rq, p, 1);

Actually, I'm totally confused by all this. Why are you looking at
next? The running task (current) should not be in the next_task_rt()
list to begin with. If p can not preempt current, and cpupri_find()
finds something for p to run on, then move it there, and be done with
it.

I also looked at set_cpus_allowed_ptr() and think we should probably
add a p->sched_class->pick_cpu() and do:

if (p->sched_class->pick_cpu)
dest_cpu = p->sched_class->pick_cpu(p, cpu_active_mask,
new_mask);
else
dest_cpu = cpumask_any_and(cpu_active_mask, new_mask);

Hmm, maybe all this work should be done there, or have
do_set_cpus_allowed() return a value that states that the
sched_class->set_cpus_allowed() did all the work for us.

-- Steve

> + if (next != p && next->prio == p->prio) {
> + /*
> +  * Target found, so let's reschedule to try
> +  * and push current away.
> +  */
> + requeue_task_rt(rq, next, 1);
> + preempt_push = 1;
> + }
> + }
> + }
> +
> +update:
>   /*
>* Only update if the process changes its state from whether it
>* can migrate or not.
>*/
> - if ((p->nr_cpus_allowed > 1) == (weight > 1))
> - return;
> -
> - rq = task_rq(p);
> + if ((old_weight > 1) == (new_weight > 1))
> + goto out;
>  
>   /*
>* The process used to be able to migrate OR it can now migrate
>*/
> - if (weight <= 1) {
> + if (new_weight <= 1) {
>   if (!task_current(rq, p))
>   dequeue_pushable_task(rq, p);
>   BUG_ON(!rq->rt.rt_nr_migratory);
> @@ -2129,6 +2184,12 @@ static void set_cpus_allowed_rt(struct task_struct *p,
>   }
>  
>   update_rt_migration(>rt);
> +
> +out:
> + if (direct_push)
> + push_rt_tasks(rq);
> + else if (preempt_push)
> + resched_curr(rq);
>  }
>  
>  /* Assumes rq->lock is held */

--
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/


Re: [PATCH v6 3/3] sched/rt: Check to push the task when changing its affinity

2015-04-20 Thread Steven Rostedt
On Mon, 20 Apr 2015 16:22:48 +0800
Xunlei Pang xlp...@126.com wrote:


 + rq = task_rq(p);
 +
 + if (new_weight  1 
 + rt_task(rq-curr) 
 + rq-rt.rt_nr_total  1 
 + !test_tsk_need_resched(rq-curr)) {
 + /*
 +  * We own p-pi_lock and rq-lock. rq-lock might
 +  * get released when doing direct pushing, however
 +  * p-pi_lock is always held, so it's safe to assign
 +  * new_mask and new_weight to p below.
 +  */
 + if (!task_running(rq, p)) {
 + cpumask_copy(p-cpus_allowed, new_mask);
 + p-nr_cpus_allowed = new_weight;
 + direct_push = 1;
 + } else if (cpumask_test_cpu(task_cpu(p), new_mask)) {
 + struct task_struct *next;
 +
 + cpumask_copy(p-cpus_allowed, new_mask);
 + p-nr_cpus_allowed = new_weight;
 + if (!cpupri_find(rq-rd-cpupri, p, NULL))
 + goto update;
 +
 + /*
 +  * At this point, current task gets migratable most
 +  * likely due to the change of its affinity, let's
 +  * figure out if we can migrate it.
 +  *
 +  * Can we find any task with the same priority as
 +  * current? To accomplish this, firstly we requeue
 +  * current to the tail and peek next, then restore
 +  * current to the head.

current? Don't you mean 'p'?


 +  */
 + requeue_task_rt(rq, p, 0);
 + next = peek_next_task_rt(rq);
 + requeue_task_rt(rq, p, 1);

Actually, I'm totally confused by all this. Why are you looking at
next? The running task (current) should not be in the next_task_rt()
list to begin with. If p can not preempt current, and cpupri_find()
finds something for p to run on, then move it there, and be done with
it.

I also looked at set_cpus_allowed_ptr() and think we should probably
add a p-sched_class-pick_cpu() and do:

if (p-sched_class-pick_cpu)
dest_cpu = p-sched_class-pick_cpu(p, cpu_active_mask,
new_mask);
else
dest_cpu = cpumask_any_and(cpu_active_mask, new_mask);

Hmm, maybe all this work should be done there, or have
do_set_cpus_allowed() return a value that states that the
sched_class-set_cpus_allowed() did all the work for us.

-- Steve

 + if (next != p  next-prio == p-prio) {
 + /*
 +  * Target found, so let's reschedule to try
 +  * and push current away.
 +  */
 + requeue_task_rt(rq, next, 1);
 + preempt_push = 1;
 + }
 + }
 + }
 +
 +update:
   /*
* Only update if the process changes its state from whether it
* can migrate or not.
*/
 - if ((p-nr_cpus_allowed  1) == (weight  1))
 - return;
 -
 - rq = task_rq(p);
 + if ((old_weight  1) == (new_weight  1))
 + goto out;
  
   /*
* The process used to be able to migrate OR it can now migrate
*/
 - if (weight = 1) {
 + if (new_weight = 1) {
   if (!task_current(rq, p))
   dequeue_pushable_task(rq, p);
   BUG_ON(!rq-rt.rt_nr_migratory);
 @@ -2129,6 +2184,12 @@ static void set_cpus_allowed_rt(struct task_struct *p,
   }
  
   update_rt_migration(rq-rt);
 +
 +out:
 + if (direct_push)
 + push_rt_tasks(rq);
 + else if (preempt_push)
 + resched_curr(rq);
  }
  
  /* Assumes rq-lock is held */

--
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/