linux-kernel - Re: [PATCH 2/2] sched/core: Avoid unnecessary update in tg_set_cfs

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <xm26plr5ysij.fsf@google.com>
Date: Mon, 22 Jul 2024 15:24:36 -0700
From: Benjamin Segall <bsegall@...gle.com>
To: Chuyi Zhou <zhouchuyi@...edance.com>
Cc: mingo@...hat.com,  peterz@...radead.org,  juri.lelli@...hat.com,
  vincent.guittot@...aro.org,  dietmar.eggemann@....com,
  rostedt@...dmis.org,  mgorman@...e.de,  vschneid@...hat.com,
  chengming.zhou@...ux.dev,  linux-kernel@...r.kernel.org,
  joshdon@...gle.com
Subject: Re: [PATCH 2/2] sched/core: Avoid unnecessary update in
 tg_set_cfs_bandwidth

Chuyi Zhou <zhouchuyi@...edance.com> writes:

> In the kubernetes production environment, we have observed a high
> frequency of writes to cpu.max, approximately every 2~4 seconds for each
> cgroup, with the same value being written each time. This can result in
> unnecessary overhead, especially on machines with a large number of CPUs
> and cgroups.
>
> This is because kubelet and runc attempt to persist resource
> configurations through frequent updates with same value in this manner.
> While optimizations can be made to kubelet and runc to avoid such
> overhead(e.g. check the current value of cpu request/limit before writing
> to cpu.max), it is still worth to bail out from tg_set_cfs_bandwidth() if
> we attempt to update with the same value.

Yeah, this is silly of userspace, but we also do the same sort of check
for shares, so sure.

We do need it to be inside of a lock though, multiple threads could
in theory write to the file at once. Just move it down inside of
cfs_constraints_mutex and that's fine.

>
> Signed-off-by: Chuyi Zhou <zhouchuyi@...edance.com>
> ---
>  kernel/sched/core.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 6d35c48239be..4db3ef2a703b 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -9081,6 +9081,8 @@ static int tg_set_cfs_bandwidth(struct task_group *tg, u64 period, u64 quota,
>  				     burst + quota > max_cfs_runtime))
>  		return -EINVAL;
>  
> +	if (cfs_b->period == ns_to_ktime(period) && cfs_b->quota == quota && cfs_b->burst == burst)
> +		return 0;
>  	/*
>  	 * Prevent race between setting of cfs_rq->runtime_enabled and
>  	 * unthrottle_offline_cfs_rqs().