From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 375843AA9F8 for ; Tue, 16 Jun 2026 09:30:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.92.199 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781602216; cv=none; b=uWFFf1UgQPwlQQ6Sinti3Bra3WSSkiniJUuxLlHUgEWrVKtTxg4gtUximDiXuwG+5oK9LprWAttLMPcuT3alc2kqABudFrcIaq5Tb3Fjd/ZmL5qb6Qp7BiDi7ZFtv9Ri8TE+t418XOW3LwPqWDY/WyUVi/VfIjWpTEeOmZNBLdY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781602216; c=relaxed/simple; bh=qOl/hxqrXyJwbNVlKiGYkf/BFZRp9A8PafoY69AWWtc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=C25fj3ounMZpFf5mlBDVfjuFNcT+uRD5JXIvTnoqYUPj5xPSrP0n0gbDphWGlTbuabYkhwDGnGcngaMyBS5VPjnqR85FWF6tygcSqq45J2z09LU/wDkfLeazyU2z0agaw5XJQlHDKP4KtcXsQcqu251OSFaNWdlJ76wTrrjAAh8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=pass smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=lIl3a40p; arc=none smtp.client-ip=90.155.92.199 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="lIl3a40p" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=g9k6tGde+gEsJtLFnsXm5otieF1bdVD+ZYmWNYlv3HA=; b=lIl3a40pLmS5mPrsV9K9kvkD8d r6Fdxq+RvsPHB5mpN3UeaFQHs/MXrGm/TMFDWK7n0fAAW/lH9i2Tuf91jQ1s88g8BxBBVwM+Pz52V PnuzNcJKWA8Weh41XUbYt3Jtn+jTm42d6dVWUGs7XiKZHAKbL7Tv2A/VaFrNFLF/AjMFcuqthwYcd /hvcKUShf3Vsx0Ab2/plMmIczEi4F3iuoBmYEkewifij7s2iI9f6I+UmFB5QBaOdsrTq4f+IAeYjp irbCWduaaMwlu9SaOfMfvvS90ksdQlYIuBDtJ1iogA9Ssqegf9+Aaln/R+tXxges62PBtDZ4cpikd X0BvVGhQ==; Received: from 77-249-17-252.cable.dynamic.v4.ziggo.nl ([77.249.17.252] helo=noisy.programming.kicks-ass.net) by desiato.infradead.org with esmtpsa (Exim 4.99.2 #2 (Red Hat Linux)) id 1wZQ7N-0000000AffJ-15iB; Tue, 16 Jun 2026 09:30:01 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id B90AE30031E; Tue, 16 Jun 2026 11:29:59 +0200 (CEST) Date: Tue, 16 Jun 2026 11:29:59 +0200 From: Peter Zijlstra To: Vincent Guittot Cc: mingo@redhat.com, juri.lelli@redhat.com, dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de, vschneid@redhat.com, kprateek.nayak@amd.com, linux-kernel@vger.kernel.org, qyousef@layalina.io Subject: Re: [PATCH 2/6 v2] sched/eevdf: Take into account current's lag when updating slice protection Message-ID: <20260616092959.GH42921@noisy.programming.kicks-ass.net> References: <20260615162420.420957-1-vincent.guittot@linaro.org> <20260615162420.420957-3-vincent.guittot@linaro.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260615162420.420957-3-vincent.guittot@linaro.org> On Mon, Jun 15, 2026 at 06:24:16PM +0200, Vincent Guittot wrote: > Take into account the lag of current task when setting the slice protection > in order to ensure that the absolute value of lags will remain in the > range [0 : slice+tick] > A task that already has a negative lag will see its protection reduced > whereas a task with positive lag will keep a full slice protection. > > Signed-off-by: Vincent Guittot > --- > kernel/sched/fair.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 83bce5a04f3d..b8d5d9bcc014 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -1083,6 +1083,7 @@ struct sched_entity *__pick_first_entity(struct cfs_rq *cfs_rq) > */ > static inline void set_protect_slice(struct cfs_rq *cfs_rq, struct sched_entity *se) > { > + u64 vruntime = min_vruntime(se->vruntime, avg_vruntime(cfs_rq)); > u64 slice = normalized_sysctl_sched_base_slice; > u64 vprot = se->deadline; > > @@ -1090,8 +1091,8 @@ static inline void set_protect_slice(struct cfs_rq *cfs_rq, struct sched_entity > slice = cfs_rq_min_slice(cfs_rq); > > slice = min(slice, se->slice); > - if (slice != se->slice) > - vprot = min_vruntime(vprot, se->vruntime + calc_delta_fair(slice, se)); > + if (vruntime != se->vruntime || slice != se->slice) > + vprot = min_vruntime(vprot, vruntime + calc_delta_fair(slice, se)); > > se->vprot = vprot; > } As already noted by Prateek, this doesn't seem to make much sense, since we just got selected by schedule(), we *must* be left of avg_vruntime(), otherwise we'd not be eligible and all that. > @@ -1099,8 +1100,9 @@ static inline void set_protect_slice(struct cfs_rq *cfs_rq, struct sched_entity > static inline void update_protect_slice(struct cfs_rq *cfs_rq, struct sched_entity *se) > { > u64 slice = cfs_rq_min_slice(cfs_rq); > + u64 vruntime = min_vruntime(se->vruntime, avg_vruntime(cfs_rq)); > > - se->vprot = min_vruntime(se->vprot, se->vruntime + calc_delta_fair(slice, se)); > + se->vprot = min_vruntime(se->vprot, vruntime + calc_delta_fair(slice, se)); > } > > static inline bool protect_slice(struct sched_entity *se) So: - set_protect_slice() is called at: set_next_task(.first = true), eg, only when the task gets scheduled(). - set_protect_slice() takes se->deadline as the baseline, and (when RUN_TO_PARITY) computes a shorter vprot [ min_slice vs slice ]. - update_protect_slice() is called upon (failed) wakeup preemption, new tasks have been added and as such the goal is to re-compute the min_slice and possibly reduce vprot. Right? And while update_protect_slice() would ideally use the original se->vruntime (as per set_next_task(.first = true) to compute any new (shorter) vprot, per its use of min_vruntime() it can not in fact end up with a vprot that is longer than the initial. Now, your change is to use min(se->vruntime, avg_vruntime()) to increase the chance of actually computing a shorter vprot. Still very much wrong, but possibly less wrong. Rather than taking avg_vruntime(), would it make sense to do something like: slice = cfs_rq_min_slice(cfs_rq); slice -= se->sum_exec_runtime - se->prev_sum_exec_runtime; vprot = se->vruntime; if (slice < 0) vprot -= calc_delta_fair(-slice, se); else vprot += calc_delta_fair(slice, se); se->vprot = min_vruntime(se->vprot, vprot); That is, reduce the slice with the time already ran. Now, this will go sideways in the unlikely case of renice, and possibly sched_change pattern (it seems we update prev_sum_exec_runtime for !first), but overall it might be a better approximation, no?