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, 12 Jun 2023 17:38:46 -0400
From:   Phil Auld <pauld@redhat.com>
To:     Benjamin Segall <bsegall@google.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>,
        Peter Zijlstra <peterz@infradead.org>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Valentin Schneider <vschneid@redhat.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Mel Gorman <mgorman@suse.de>
Subject: Re: [PATCH v2] sched/nohz: Add HRTICK_BW for using cfs bandwidth
 with nohz_full
Message-ID: <20230612213846.GA524810@lorien.usersys.redhat.com>
References: <20230608125228.56097-1-pauld@redhat.com>
 <xm26fs6w9zt4.fsf@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <xm26fs6w9zt4.fsf@google.com>
Precedence: bulk

On Mon, Jun 12, 2023 at 01:43:35PM -0700 Benjamin Segall wrote:
> Phil Auld <pauld@redhat.com> writes:
> 
> > CFS bandwidth limits and NOHZ full don't play well together.  Tasks
> > can easily run well past their quotas before a remote tick does
> > accounting.  This leads to long, multi-period stalls before such
> > tasks can run again.  Use the hrtick mechanism to set a sched
> > tick to fire at remaining_runtime in the future if we are on
> > a nohz full cpu, if the task has quota and if we are likely to
> > disable the tick (nr_running == 1).  This allows for bandwidth
> > accounting before tasks go too far over quota.
> >
> > A number of container workloads use a dynamic number of real
> > nohz tasks but also have other work that is limited which ends
> > up running on the "spare" nohz cpus.  This is an artifact of
> > having to specify nohz_full cpus at boot. Adding this hrtick
> > resolves the issue of long stalls on these tasks. Currently
> > the scheduler, when faced with these conflicting requirements
> > choosed to favor nohz_full even though that is already best
> > effort. Here we make it favor respecting the bandwidth
> > limitations which are not supposed to be best effort.
> >
> > Add the sched_feat HRTICK_BW off by default to allow users to
> > enable this only when needed.
> >
> > Signed-off-by: Phil Auld <pauld@redhat.com>
> > Suggested-by: Juri Lelli <jlelli@redhat.com>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Vincent Guittot <vincent.guittot@linaro.org>
> > Cc: Juri Lelli <juri.lelli@redhat.com>
> > Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> > Cc: Valentin Schneider <vschneid@redhat.com>
> > Cc: Ben Segall <bsegall@google.com>
> > ---
> >
> > v2: Clean up building issues with various related CONFIG changes. Add a
> > check to start the hrtick in __account_cfs_rq_runtime() for when the
> > task gets more runtime.
> 
> This starts the hrtick before/without going through schedule, but the
> fact that it's still the standard hrtick()->task_tick(queued=1) means it
> will still set need_resched even if it succeeds at getting runtime.
> 

Ah, I see what you are saying. I was hoping to leverage the existing hrtick.
I certainly don't want to add a new one for this.  As it is it seems to be
doing what is expected (at some expense of extra schedules). If that expense
is too high then this may not be helpful.

I guess I will try the don't-stop-the-tick approach. I'm more concerned with getting
accurate quota support in this case than removing the tick.

Cheers,
Phil


--