[lkml]   [2017]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
SubjectRe: [PATCH RFC 1/2] sched: Minimize the idle cpu selection race window.
On Fri, Nov 24, 2017 at 07:46:30PM +0100, Mike Galbraith wrote:
> On Fri, 2017-11-24 at 11:26 +0100, Uladzislau Rezki wrote:
> >
> > I guess there is misunderstanding here. The main goal is not to cover
> > pinned case, for sure. I was thinking more about below points:
> >
> > - Extend a claim_wake_up logic for making an ILB/NO_HZ decision more
> > predictable (that is good for mobile workloads). Because as it is
> > right now it simply returns a first CPU in a "nohz" mask and if we
> > know that CPU has been claimed i think it is worth to go with another
> > ILB core, since waking up on a remote CPU + doing nohz_idle_balance
> > does not improve wake-up latency and is a miss from ilb point of view.
> Using unsynchronized access of a flag set by chaotic events all over
> the box?
Well, entering NO_HZ mode and checking nohz.idle_cpus_mask
nohz_balancer_kick() -> find_new_ilb() are obviously asynchronous
events and like you say is chaotic except of accessing to the
cpumask variable (i mean atomically). So do not see any issues
with that.

> > If you have any proposal, i would be appreciated if you could
> > share your specific view.
> My view is you're barking up the wrong tree: you're making the idle
> data SIS is using more accurate, but I question the benefit.  That it
> makes an imperfect placement decision occasionally due to raciness is
> nearly meaningless compared to the cost of frequent bounce.
Before sitting down and start testing, i just illustrated how we can
apply claim_wake_up to ilb asking community a specific view on it:
drawbacks, pros/cons, proposals etc.

> Better (but still not perfect, that requires atomics) state information
> doesn't meaningfully improve decision quality.
I agree, it should be atomic, something like below:

+/* cpus with claim wake-up set bit */
+cpumask_t cpu_claim_wake_up_map;
@@ -1559,6 +1562,7 @@ int select_task_rq(struct task_struct *p, int cpu, int sd_flags, int wake_flags)
cpu = select_fallback_rq(task_cpu(p), p);

+ cpumask_test_and_set_cpu(cpu, &cpu_claim_wake_up_map);
return cpu;
+ if (cpumask_test_cpu(cpu, &cpu_claim_wake_up_map))
+ return 0;
return 1;

static inline int find_new_ilb(void)
- int ilb = cpumask_first(nohz.idle_cpus_mask);
+ cpumask_t cpumask;
+ int ilb;
+ cpumask_andnot(&cpumask, nohz.idle_cpus_mask,
+ &cpu_claim_wake_up_map);
+ ilb = cpumask_first(&cpumask);
> > Considering a core as not-idle when somebody tends to wake up a task on
> > it is a good point. If you have any specific example when it is bad,
> > please share it.
> I'm not sure how that will work out. Probably like most everything wrt
> SIS, first comes "woohoo" then "aw crap", and off to the trash it goes.

Uladzislau Rezki

 \ /
  Last update: 2017-11-28 10:36    [W:0.113 / U:1.816 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site