Messages in this thread | | | Date | Tue, 24 Aug 2021 10:11:46 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v3 1/2] sched/fair: Add NOHZ balancer flag for nohz.next_balance updates |
| |
On Mon, Aug 23, 2021 at 03:53:16PM +0200, Dietmar Eggemann wrote: > >> I'm a bit puzzled by this; that function has: > >> > >> SCHED_WARN_ON((flags & NOHZ_KICK_MASK) == NOHZ_BALANCE_KICK); > >> > >> Which: > >> > >> - isn't updated > >> - implies STATS must be set when BALANCE > > > > Yup > > > >> > >> the latter gives rise to my confusion; why add that gate on STATS? It > >> just doesn't make sense to do a BALANCE and not update STATS. > > > > AFAIA that warning was only there to catch BALANCE && !STATS, so I didn't > > tweak it. > > > > Now, you could still end up with > > > > flags == NOHZ_NEXT_KICK > > > > (e.g. nohz.next_balance is in the future, but a new CPU entered NOHZ-idle > > and needs its own rq.next_balance collated into the nohz struct) > > > > in which case you don't do any blocked load update, hence the > > gate. In v1 I had that piggyback on NOHZ_STATS_KICK, but Vincent noted > > that might not be the best given blocked load updates can be time > > consuming - hence the separate flag. > > Maybe the confusion stems from the fact that the NOHZ_NEXT_KICK-set > changes are only introduced in 2/2? > > @@ -10417,6 +10418,9 @@ static void nohz_balancer_kick(struct rq *rq) > unlock: > rcu_read_unlock(); > out: > + if (READ_ONCE(nohz.needs_update)) > + flags |= NOHZ_NEXT_KICK; > +
The confusion was about how we'd ever get there and not have STATS set, but i guess having it all nicely gated does make it saner.
Thanks!
| |