Messages in this thread | | | Date | Wed, 7 Dec 2022 11:54:10 +0800 | From | Chen Yu <> | Subject | Re: [PATCH v3 2/2] sched/fair: Choose the CPU where short task is running during wake up |
| |
Hi Yicong, On 2022-12-06 at 21:02:11 +0800, Yicong Yang wrote: [...] > > +++ b/kernel/sched/fair.c > > @@ -6246,6 +6246,11 @@ wake_affine_idle(int this_cpu, int prev_cpu, int sync) > > if (available_idle_cpu(prev_cpu)) > > return prev_cpu; > > > > + /* The only running task is a short duration one. */ > > + if (cpu_rq(this_cpu)->nr_running == 1 && > > + is_short_task((struct task_struct *)cpu_curr(this_cpu))) > > + return this_cpu; > > + > > Is it necessary to check the ttwu pending state here and below? > My understanding is that, ttwu_pending will be set on this_cpu if 1) this_cpu is idle, or 2) waker on another LLC domain wants to wake up the wakee on this_cpu, see ttwu_queue_cond(). For 1), the nr_running is 1, so it is not idle. For 2) the chance to do a cross LLC wake up is relatively low with current patch applied. Besides, I was trying to make this proposal a dynamic version of WF_SYNC, since the latter does not check ttwu_pending, I did not add this check as well.(for now)+ > > return nr_cpumask_bits; > > } > > > > @@ -6612,6 +6617,11 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool > > time = cpu_clock(this); > > } > > > > + if (!has_idle_core && cpu_rq(target)->nr_running == 1 && > > + is_short_task((struct task_struct *)cpu_curr(target)) && > > + is_short_task(p)) > > + return target; > > + > > A short running task doesn't means a low utilization (you also mentioned in Patch 1/2). > So should we concern that we may overload the target? > The overloaded target might be expected in this case IMO. Because for a ping-pong scheduling pair, we want to saturate the target CPU to eliminate the idle time. And this strategy only takes effect when !has_idle_core. > btw, we're doing no scanning here so I may think it'll be more consistent to put this part > in select_idle_siblings(), considering we've already have some similiar judgement for the > prev_cpu, recent_used_cpu, etc. there. > Got it, I can change it in next version. > Still doing some test, will reply the results once I get them. Thanks for the test, we can tune this patch when we have the data.
thanks, Chenyu > > Thanks, > Yicong > > > if (sched_feat(SIS_UTIL)) { > > sd_share = rcu_dereference(per_cpu(sd_llc_shared, target)); > > if (sd_share) { > >
| |