Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 3 Jul 2023 10:19:17 -0400
From:   Phil Auld <pauld@redhat.com>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     linux-kernel@vger.kernel.org, Juri Lelli <juri.lelli@redhat.com>,
        Ingo Molnar <mingo@redhat.com>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Valentin Schneider <vschneid@redhat.com>,
        Ben Segall <bsegall@google.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Mel Gorman <mgorman@suse.de>,
        Frederic Weisbecker <frederic@kernel.org>
Subject: Re: [PATCH v4] Sched/fair: Block nohz tick_stop when cfs bandwidth
 in use
Message-ID: <20230703141917.GB67396@lorien.usersys.redhat.com>
References: <20230630135714.1018434-1-pauld@redhat.com>
 <20230630150641.GH2533791@hirez.programming.kicks-ass.net>
 <20230630152824.GC43299@lorien.usersys.redhat.com>
 <20230630160534.GA2534344@hirez.programming.kicks-ass.net>
 <20230630162910.GD43299@lorien.usersys.redhat.com>
 <20230703121009.GH4253@hirez.programming.kicks-ass.net>
 <20230703141056.GA67396@lorien.usersys.redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230703141056.GA67396@lorien.usersys.redhat.com>
Precedence: bulk

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.
> 
> 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).
>

And, for clarity, the "bit" in that case is TICK_DEP_BIT_SCHED not the new
task::bw_constrained field.  I.e. we'll endup allowing the tick to stop
for a bw limited task and thus not solve the problem.


> 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.
> 
> 
> Cheers,
> Phil
> -- 
> 

--