linux-kernel - Re: [PATCH 0/4] Task based throttle follow ups

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <xm26348ntm9g.fsf@google.com>
Date: Mon, 15 Sep 2025 14:54:19 -0700
From: Benjamin Segall <bsegall@...gle.com>
To: Aaron Lu <ziqianlu@...edance.com>
Cc: Valentin Schneider <vschneid@...hat.com>,  K Prateek Nayak
 <kprateek.nayak@....com>,  Peter Zijlstra <peterz@...radead.org>,
  Chengming Zhou <chengming.zhou@...ux.dev>,  Josh Don
 <joshdon@...gle.com>,  Ingo Molnar <mingo@...hat.com>,  Vincent Guittot
 <vincent.guittot@...aro.org>,  Xi Wang <xii@...gle.com>,
  linux-kernel@...r.kernel.org,  Juri Lelli <juri.lelli@...hat.com>,
  Dietmar Eggemann <dietmar.eggemann@....com>,  Steven Rostedt
 <rostedt@...dmis.org>,  Mel Gorman <mgorman@...e.de>,  Chuyi Zhou
 <zhouchuyi@...edance.com>,  Jan Kiszka <jan.kiszka@...mens.com>,  Florian
 Bezdeka <florian.bezdeka@...mens.com>,  Songtang Liu
 <liusongtang@...edance.com>,  Chen Yu <yu.c.chen@...el.com>,  Matteo
 Martelli <matteo.martelli@...ethink.co.uk>,  Michal Koutný
 <mkoutny@...e.com>,  Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Subject: Re: [PATCH 0/4] Task based throttle follow ups

Aaron Lu <ziqianlu@...edance.com> writes:

> Peter noticed the inconsistency in load propagation for throttled cfs_rq
> and Ben pointed out several other places regarding throttled cfs_rq that
> could be no longer needed after task based throttle model.
>
> To ease discussing and reviewing, I've come up with this follow up
> series which implements the individual changes.
>
> Patch1 deals with load propagation. According to Peter and Prateek's
> discussion, previously, load propagation for throttled cfs_rq happened
> on unthrottle time but now with per-task throttle, it's no longer the
> case so load propagation should happen immediately or we could lose this
> propagated part.
>
> Patch2 made update_cfs_group() to continue function for cfs_rqs in
> throttled hierarchy so that cfs_rq's entity can get an up2date weight. I
> think this is mostly useful when a cfs_rq in throttled hierarchy still
> has tasks running and on tick/enqueue/dequeue, update_cfs_group() can
> update this cfs_rq's entity weight.
>
> Patch3 removed special treatment of tasks in throttled hierarchy,
> including: dequeue_entities(), check_preempt_wakeup_fair() and
> yield_task_to_fair().
>
> Patch4 inhibited load balancing to a throttled cfs_rq to make hackbench
> happy.
>
> I think patch1 is needed for correctness, patch2-4 is open for
> discussion as there are pros/cons doing things either way. Comments are
> welcome, thanks.
>
> BTW, I also noticed there is the task_is_throttled sched class callback
> and in fair, it is task_is_throttled_fair(). IIUC, it is used by core
> scheduling to find a matching cookie task to run on the sibling SMT CPU.
> For this reason, it doesn't seem very useful if we find it a task that
> is to be throttled so I kept the current implementation; but I guess
> this is also two folded if that to be throttled task is holding some
> kernel resources. Anyway, I didn't write a patch to change it in this
> series, but feel free to let me know if it should be changed.
>
> Aaron Lu (4):
>   sched/fair: Propagate load for throttled cfs_rq
>   sched/fair: update_cfs_group() for throttled cfs_rqs
>   sched/fair: Do not special case tasks in throttled hierarchy
>   sched/fair: Do not balance task to a throttled cfs_rq
>
>  kernel/sched/fair.c | 50 ++++++++++++++++++++++++++++-----------------
>  1 file changed, 31 insertions(+), 19 deletions(-)
>
>
> base-commit: 5b726e9bf9544a349090879a513a5e00da486c14

Yeah, these all make sense to me (with v2 for patch 4).

Reviewed-by: Ben Segall <bsegall@...gle.com>