Re: [PATCH v4] Sched/fair: Block nohz tick_stop when cfs bandwidth in use

[Peter Zijlstra <peterz@infradead.org>]

On Mon, Jul 03, 2023 at 10:10:56AM -0400, Phil Auld wrote:
> On Mon, Jul 03, 2023 at 02:10:09PM +0200 Peter Zijlstra wrote:
> > On Fri, Jun 30, 2023 at 12:29:10PM -0400, Phil Auld wrote:
> > 
> > > I think you are agreeing that I need the pick next code but need to remove
> > > the hierarchy walks, right?
> > 
> > Yeah, the dequeue case makes we have to care about pick, not sure we
> > then also need to care about sched_update_tick_dependency() though.
> > There is indeed a window where these two will 'race', but afaict it is
> > benign.
> > 
> 
> Hm, that's confusing.
> 
> As I see it it's the enqueue case (0->1 mostly) where we need the check
> in pick.  At that point in enqueue we only have a handle on ->curr which
> is the idle thread.

Well, the 0->1 case is trivial, we'll run the task that's enqueued, and
as such everything can DTRT and be simple.

> For the dequeue case (2->1) we need the check in the
> sched_update_tick_dependency() path because if the 1 is the task on the
> cpu (and is staying there) then we'd otherwise clear the bit when we
> shouldn't (since we aren't going to go back through pick).

The 2->1 case OTOH is tricky, because then we'll end up running a task
we've not recently seen. sub_nr_running() will hit the ==1 case and
clear TICK_DEP_BIT_SCHED.

But then pick will come and set it again, no harm done, right?

.oO Ah!, You're worried about the case where a task is already running,
a second task comes in, (1->2) and then quickly leaves again (2->1)
without passing through schedule(). And you don't want to disable the
tick if that running task needs it.

Mooo :-(

> I'm thinking that I'll try to set the bit in pick since we only care about
> it when it's the task on the cpu.  That, I think, will simplify the
> code needed to update the bit when the quota is changed (to or from
> RUNTIME_INF).
> 
> Setting the bit in enqueue/dequeue means updating it on all the queued
> task if it changes. Although I may clear it in dequeue just to not leave
> it around stale.

Hmm, no you have to set on enqueue (1->2), otherwise the running task
doesn't get preempted when it runs out of slice.

And I don't suppose you want to delay clearing to the first tick after,
because NOHZ_FULL doesn't want spurious ticks :/

What a mess.

Please document all these stupid cases in a comment, otherwise we'll go
bananas trying to make sense of the code later on.