On Thu, Dec 08, 2016 at 11:42:50AM -0500, Thara Gopinath wrote:
Hi Leo,
Thanks for the review. It will be great if you can test this patch out and let me know of any improvements/degradation.
Yeah, will test it and let you know result.
On 12/08/2016 05:24 AM, Leo Yan wrote:
[...]
@@ -6063,10 +6094,16 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int sd_flag, int wake_f && cpumask_test_cpu(cpu, tsk_cpus_allowed(p)); }
- if (energy_aware() && !(cpu_rq(prev_cpu)->rd->overutilized))
return select_energy_cpu_brute(p, prev_cpu);
- rcu_read_lock();
- sd = rcu_dereference(cpu_rq(prev_cpu)->sd);
- if (energy_aware() &&
!is_sd_overutilized(sd,
cpu_rq(cpu)->rd)) {
new_cpu = select_energy_cpu_brute(p, prev_cpu);
goto unlock;
- }
- sd = NULL;
Is it better to place function select_energy_cpu_brute() out of rcu locking, like below?
I dont understand how this will change anything. In the above code, select_energy_cpu_brute() is rcu protected.
Your code moves function select_energy_cpu_brute() into RCU protection.
Be honest I'm not sure this change will hurt performance or not, usually it avoids to move much workload into locks.
[...]
@@ -7932,8 +7983,11 @@ static struct sched_group *find_busiest_group(struct lb_env *env) */ update_sd_lb_stats(env, &sds);
- if (energy_aware() && !env->dst_rq->rd->overutilized)
goto out_balanced;
/* Is this check really required here?? Revisit */
if (energy_aware()) {
if (!is_sd_overutilized(env->sd, env->dst_rq->rd))
goto out_balanced;
}
local = &sds.local_stat; busiest = &sds.busiest_stat;
@@ -8000,6 +8054,12 @@ static struct sched_group *find_busiest_group(struct lb_env *env) force_balance: /* Looks like there is an imbalance. Compute it */ calculate_imbalance(env, &sds);
- /* Is this the correct place to clear this flag? Should access
* to flag be locked? Revisit.
*/
- clear_sd_overutilized(env->sd, env->dst_rq->rd);
Have specific sequency to clear overutilized flag for root domain and SD level 2? Like firstly clean root domain flag and then clear SD level 2 flag.
We don't need a specific sequence. As and when each sched domain get balanced, the corresponding flag will get cleared.
Have you observed the overutilized flag is more easily to clear? If we clear overutilized flag at here, so means the overutilized flag is cleared after every time's load balance. This will let overutilized flag to be cleared very quickly and should have much less time to stay "overutilized" than before.
We could run rt-app cases for more analysis.
Thanks, Leo Yan