linux-kernel - Re: [PATCH] sched/fair: Fix CPU bandwidth limit bypass during CPU hotplug

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: <Z1PcC2h-VWOIAkaG@linux.ibm.com>
Date: Sat, 7 Dec 2024 10:54:27 +0530
From: Vishal Chourasia <vishalc@...ux.ibm.com>
To: Srikar Dronamraju <srikar@...ux.ibm.com>
Cc: linux-kernel@...r.kernel.org, mingo@...hat.com, peterz@...radead.org,
        juri.lelli@...hat.com, vincent.guittot@...aro.org,
        dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
        mgorman@...e.de, vschneid@...hat.com, sshegde@...ux.ibm.com
Subject: Re: [PATCH] sched/fair: Fix CPU bandwidth limit bypass during CPU
 hotplug

On Fri, Dec 06, 2024 at 02:37:54PM +0530, Srikar Dronamraju wrote:
> * Vishal Chourasia <vishalc@...ux.ibm.com> [2024-11-26 12:18:13]:
> 
> > CPU controller limits are not properly enforced during CPU hotplug operations,
> > particularly during CPU offline. When a CPU goes offline, throttled
> > processes are unintentionally being unthrottled across all CPUs in the system,
> > allowing them to exceed their assigned quota limits.
> > 
> <snip>
> 
> > Signed-off-by: Vishal Chourasia <vishalc@...ux.ibm.com>
> > ---
> >  kernel/sched/fair.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index fbdca89c677f..c436e2307e6f 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -6684,7 +6684,8 @@ static void __maybe_unused unthrottle_offline_cfs_rqs(struct rq *rq)
> >  	list_for_each_entry_rcu(tg, &task_groups, list) {
> >  		struct cfs_rq *cfs_rq = tg->cfs_rq[cpu_of(rq)];
> >  
> > -		if (!cfs_rq->runtime_enabled)
> > +		/* Don't unthrottle an active cfs_rq unnecessarily */
> 
> Per the patch description, its not just unnecessary but unthrottling is
> buggy. Unnecessary would mean its just an overhead. Here we dont want to
> unthrottle. Other than that this seems to be fine to me.
sure.


Also, I missed running the patch against checkpatch.pl 

Will be sending out v2 with the updates.

vishalc
> 
> > +		if (!cfs_rq->runtime_enabled || cpumask_test_cpu(cpu_of(rq), cpu_active_mask))
> >  			continue;
> 
> -- 
> Thanks and Regards
> Srikar Dronamraju