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:   Fri, 30 Jun 2023 18:05:34 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Phil Auld <pauld@redhat.com>
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: <20230630160534.GA2534344@hirez.programming.kicks-ass.net>
References: <20230630135714.1018434-1-pauld@redhat.com>
 <20230630150641.GH2533791@hirez.programming.kicks-ass.net>
 <20230630152824.GC43299@lorien.usersys.redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230630152824.GC43299@lorien.usersys.redhat.com>
Precedence: bulk

On Fri, Jun 30, 2023 at 11:28:24AM -0400, Phil Auld wrote:

> No. Or at least not without plumbing the enqueued/dequeued task all the way
> through.  I can do it that way if you prefer but that seemed a lot more
> intrusive. When we are in sched_can_stop_tick() we don't have access to the
> cfs task which will end up running. Curr is idle in that case.  We'd have to
> essential run pick_next_task_fair() to find the task to check which seemed
> wrong. Maybe there is a better way?

Ah, you worry about where we have two runnable tasks, one is bandwidth
constrained the other is not. One task goes away, how can we tell what
the remaining task is?

This is never a concern for add_nr_running(), the only case there is
0->1 and then only the hierarchy you just walked for enqueue is
relevant.

But if you remove the unconstrained task, sub_nr_running() can't tell
what the remaining task is.

Unless, of course, you have enqueue() set a bit somewhere in
task_struct::sched_bw_constrained:1.

Then pick and your should_stop thing can look at that, no?