From patchwork Tue Apr 16 17:51:40 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jesper Dangaard Brouer X-Patchwork-Id: 13632263 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5AE5CC4345F for ; Tue, 16 Apr 2024 17:51:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E82F56B0085; Tue, 16 Apr 2024 13:51:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E332E6B00A0; Tue, 16 Apr 2024 13:51:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CD2AC6B00A1; Tue, 16 Apr 2024 13:51:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id AD90C6B0085 for ; Tue, 16 Apr 2024 13:51:48 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 6B520140C4C for ; Tue, 16 Apr 2024 17:51:48 +0000 (UTC) X-FDA: 82016137896.17.C53A06A Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf26.hostedemail.com (Postfix) with ESMTP id BB795140003 for ; Tue, 16 Apr 2024 17:51:46 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="rBTGA/j4"; spf=pass (imf26.hostedemail.com: domain of hawk@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=hawk@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713289906; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=30qwxIg3n7d0cGAI8RQitFODbGL0D34b+7PToxEV0lQ=; b=XuO+uIyRvgpwfQ+uZ//KVTIVgyv9V74+9zp0EslntWEOQ3Rduo5CaWlmbS0vIhqPdfVEyr qPJO4vF2tNTci4eVIMvkfZOubHZ9/DkibKLIrMeYnxWTG46vzrgpyON6hQWDeOF/LF7Rr5 Gnuinp2QpE1Juj8hVLPas9I1dFIqsrY= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713289906; a=rsa-sha256; cv=none; b=ulGSq7/hi23LwTUFgqYhaEdmbc7+bgY68SeiNDXjpOxGhmBvro8y/QY4f//37EHhXsDihV XaKHH2v60XgKFpXHK/CetLNCdtuab0Z9+dX6FoS9kZ7/o3dChAT6MgCZueXFwMVeJMcAqL RR/12SHpB4rc6cATLEckUnftXtsnyaU= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="rBTGA/j4"; spf=pass (imf26.hostedemail.com: domain of hawk@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=hawk@kernel.org; dmarc=pass (policy=none) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id D055D61257; Tue, 16 Apr 2024 17:51:45 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0820FC2BD11; Tue, 16 Apr 2024 17:51:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713289905; bh=xykn9xlAKIRlCdpBDJjUFA6u/Fh4oeaFhP6Oe5BJZ9w=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=rBTGA/j4eYXaha1zep2XsWqQMo/7AuaFqKvR+9ztUt384qlMMGAN+2aCqPtF1UGaK U2ZPG0IxNZ5eBgCilfLyWL7Af8dqomP8JBPxoNHkHEQrQ9yumvgnBhvEkzyfFooiYh UfuchUwUlMiYHINfQ1r4Of3CW93zlTmy08apcPnyltNYZTekRluD60kr7Yi5rvjlco eKvF5OPmD8pQ46Zmo4DAUT/lv+uhsTwiXWRXDTCWzlfR2mmHCpoiw3Bnwu4NSLAo0f BbHg1J8OD7NoRHbsi3B6F+e5OyqeUQ1cNugA+Vu+KrvrJNxx1NXzHZ2s1+sDQcTf3h yFyl/RclpMJmg== Subject: [PATCH v1 3/3] cgroup/rstat: introduce ratelimited rstat flushing From: Jesper Dangaard Brouer To: tj@kernel.org, hannes@cmpxchg.org, lizefan.x@bytedance.com, cgroups@vger.kernel.org, yosryahmed@google.com, longman@redhat.com Cc: Jesper Dangaard Brouer , netdev@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, shakeel.butt@linux.dev, kernel-team@cloudflare.com, linux-kernel@vger.kernel.org, Arnaldo Carvalho de Melo , Sebastian Andrzej Siewior , mhocko@kernel.org Date: Tue, 16 Apr 2024 19:51:40 +0200 Message-ID: <171328990014.3930751.10674097155895405137.stgit@firesoul> In-Reply-To: <171328983017.3930751.9484082608778623495.stgit@firesoul> References: <171328983017.3930751.9484082608778623495.stgit@firesoul> User-Agent: StGit/1.5 MIME-Version: 1.0 X-Stat-Signature: 6gsdhwrwkimt7ka3xczngitsqqie7o8x X-Rspamd-Queue-Id: BB795140003 X-Rspamd-Server: rspam06 X-Rspam-User: X-HE-Tag: 1713289906-510566 X-HE-Meta: U2FsdGVkX18Lmm1sUrTkV39LbyQREyLKTeOsu7DX/SPuP80N0neZrOW2Yd83gJNg55CH45tHw71M5xu35jw1KonN1jngof4iDJq4gxlg9fYrXhAwrzEHrUJm9ClNU/urdJbJi8dFQHqo+M6K9v7rx+Kehv+BNoHwH02gxEmFb5PYqMbKbNnk5BZql8fmbxQYZPy6SFTVx0G6WgXGKL++7lGndmAemS67Xxi4UiVMNO9yLfKmqQqqcHfLgXZu1YRD2IyjhDCAoXejIKx3Kvtqmvu+zjnnAbLo2kb60H3/o8tuSOixld8/+tmRcZ8Qhf6ao7OcxUQOL8sCDkleEnPFxIwNBY5lrEw8jP55XK40LsPDwCEx7KwyuPrnfejKPTSwqtSDniBYchPqV1ZyXbUN8bNhj0xQ6cIfekWD1dGxM6P0y+fS3ibTgTooCJIah/+UpG9rLy5fmeRVh70Iugejo4xIbwAxMWsEBnZ8oysDs3mpqQLf+tTAgN3KcFoEEgu4qgCwF66Ayo61HtmNGedPd8TDRr1COZFC1LxAGWNg14KhBd64ta1foM3s66bASC7L4LW/ItjHSkwS5z667hDSJmsEjwbBL7lGWvt+L1lH/h7OSRe1pMxMNDCKcxOmDIZlN92iPl/M2kKPS2HHbjB+k6Y3RkUpwNyHQ4KvmJdHUt3o9IP4FY733yDSwkxNq45PKrx20HWyMwrJ7SKDe5B1JlB4GY9VcWOXTC9CTkiw39GxOYjNZr5Z94sEZf9Q0/PvPa8wwq98FBTl0WQL2bMJziASLdmTDEqogjL5L5Uezaj803qrCZq4jMKOz6Hg3Pf3przjXnY66JuoKPApCG8Tgp5xqMWmHJ0YXkPKS5mTzt0OvzdDINtf1+SZn6Nt0L4cOCVsf1nQWYc6sJQCuvOq5O8hNEYGF5GCJhgi0IiCknlyfX0Quax1N9F6ptxlB3kXzZgj6zLpakb+HIsc0W4 vaZVIMeQ RNuZQ6zyDxHlaUi8gakUwYlzd0kPyPNzOdK1QG0Arfi/7xAqzkbmcfoX0UDNpGT/4mL6nzd+Pslnqusdj8rKwvP/nhzMcC0xQMwvWRQIjEeK4k5wBGUzB4YqvipRLB36JxDB7fQMltOw/CmCxzmIeHMotxyotAHFKtX+0a+JeZaXcbM208BuE5qdGJrwUGhiHka+eiA8CxSITAF0jClP+hG7TxKpD1st4xPYhqxFDBhAXQu8iNxjBj6dbXg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: This patch aims to reduce userspace-triggered pressure on the global cgroup_rstat_lock by introducing a mechanism to limit how often reading stat files causes cgroup rstat flushing. In the memory cgroup subsystem, memcg_vmstats_needs_flush() combined with mem_cgroup_flush_stats_ratelimited() already limits pressure on the global lock (cgroup_rstat_lock). As a result, reading memory-related stat files (such as memory.stat, memory.numa_stat, zswap.current) is already a less userspace-triggerable issue. However, other userspace users of cgroup_rstat_flush(), such as when reading io.stat (blk-cgroup.c) and cpu.stat, lack a similar system to limit pressure on the global lock. Furthermore, userspace can easily trigger this issue by reading those stat files. Typically, normal userspace stats tools (e.g., cadvisor, nomad, systemd) spawn threads that read io.stat, cpu.stat, and memory.stat (even from the same cgroup) without realizing that on the kernel side, they share the same global lock. This limitation also helps prevent malicious userspace applications from harming the kernel by reading these stat files in a tight loop. To address this, the patch introduces cgroup_rstat_flush_ratelimited(), similar to memcg's mem_cgroup_flush_stats_ratelimited(). Flushing occurs per cgroup (even though the lock remains global) a variable named rstat_flush_last_time is introduced to track when a given cgroup was last flushed. This variable, which contains the jiffies of the flush, shares properties and a cache line with rstat_flush_next and is updated simultaneously. For cpu.stat, we need to acquire the lock (via cgroup_rstat_flush_hold) because other data is read under the lock, but we skip the expensive flushing if it occurred recently. Regarding io.stat, there is an opportunity outside the lock to skip the flush, but inside the lock, we must recheck to handle races. Signed-off-by: Jesper Dangaard Brouer --- block/blk-cgroup.c | 2 + include/linux/cgroup-defs.h | 1 + include/linux/cgroup.h | 3 +- kernel/cgroup/rstat.c | 60 ++++++++++++++++++++++++++++++++++++++++++- mm/memcontrol.c | 1 + 5 files changed, 63 insertions(+), 4 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index bdbb557feb5a..4156fedbb910 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -1162,7 +1162,7 @@ static int blkcg_print_stat(struct seq_file *sf, void *v) if (!seq_css(sf)->parent) blkcg_fill_root_iostats(); else - cgroup_rstat_flush(blkcg->css.cgroup); + cgroup_rstat_flush_ratelimited(blkcg->css.cgroup); rcu_read_lock(); hlist_for_each_entry_rcu(blkg, &blkcg->blkg_list, blkcg_node) { diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h index ea48c861cd36..366dc644e075 100644 --- a/include/linux/cgroup-defs.h +++ b/include/linux/cgroup-defs.h @@ -509,6 +509,7 @@ struct cgroup { * cgroup_rstat_flush_locked() and protected by cgroup_rstat_lock. */ struct cgroup *rstat_flush_next; + unsigned long rstat_flush_last_time; /* cgroup basic resource statistics */ struct cgroup_base_stat last_bstat; diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h index 2150ca60394b..8622b222453e 100644 --- a/include/linux/cgroup.h +++ b/include/linux/cgroup.h @@ -689,7 +689,8 @@ static inline void cgroup_path_from_kernfs_id(u64 id, char *buf, size_t buflen) */ void cgroup_rstat_updated(struct cgroup *cgrp, int cpu); void cgroup_rstat_flush(struct cgroup *cgrp); -void cgroup_rstat_flush_hold(struct cgroup *cgrp); +int cgroup_rstat_flush_hold(struct cgroup *cgrp); +int cgroup_rstat_flush_ratelimited(struct cgroup *cgrp); void cgroup_rstat_flush_release(struct cgroup *cgrp); /* diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c index a90d68a7c27f..8c71af67b197 100644 --- a/kernel/cgroup/rstat.c +++ b/kernel/cgroup/rstat.c @@ -193,6 +193,7 @@ static struct cgroup *cgroup_rstat_updated_list(struct cgroup *root, int cpu) /* Push @root to the list first before pushing the children */ head = root; root->rstat_flush_next = NULL; + root->rstat_flush_last_time = jiffies; child = rstatc->updated_children; rstatc->updated_children = root; if (child != root) @@ -261,12 +262,15 @@ static void cgroup_rstat_flush_locked(struct cgroup *cgrp) lockdep_assert_held(&cgroup_rstat_lock); + cgrp->rstat_flush_last_time = jiffies; + for_each_possible_cpu(cpu) { struct cgroup *pos = cgroup_rstat_updated_list(cgrp, cpu); for (; pos; pos = pos->rstat_flush_next) { struct cgroup_subsys_state *css; + pos->rstat_flush_last_time = jiffies; cgroup_base_stat_flush(pos, cpu); bpf_rstat_flush(pos, cgroup_parent(pos), cpu); @@ -309,6 +313,49 @@ __bpf_kfunc void cgroup_rstat_flush(struct cgroup *cgrp) __cgroup_rstat_unlock(cgrp, -1); } +#define FLUSH_TIME msecs_to_jiffies(50) + +/** + * cgroup_rstat_flush_ratelimited - limit pressure on global lock (cgroup_rstat_lock) + * @cgrp: target cgroup + * + * Trade accuracy for less pressure on global lock. Only use this when caller + * don't need 100% up-to-date stats. + * + * Userspace stats tools spawn threads reading io.stat, cpu.stat and memory.stat + * from same cgroup, but kernel side they share same global lock. This also + * limit malicious userspace from hurting kernel by reading these stat files in + * a tight loop. + * + * This function exit early if flush recently happened. + * + * Returns 1 if flush happened + */ +int cgroup_rstat_flush_ratelimited(struct cgroup *cgrp) +{ + int res = 0; + + might_sleep(); + + /* Outside lock: check if this flush and lock can be skipped */ + if (time_before(jiffies, + READ_ONCE(cgrp->rstat_flush_last_time) + FLUSH_TIME)) + return 0; + + __cgroup_rstat_lock(cgrp, -1); + + /* Recheck inside lock, do flush after enough time have passed */ + if (time_after(jiffies, + cgrp->rstat_flush_last_time + FLUSH_TIME)) { + cgroup_rstat_flush_locked(cgrp); + res = 1; + } + + __cgroup_rstat_unlock(cgrp, -1); + + return res; +} + /** * cgroup_rstat_flush_hold - flush stats in @cgrp's subtree and hold * @cgrp: target cgroup @@ -317,13 +364,22 @@ __bpf_kfunc void cgroup_rstat_flush(struct cgroup *cgrp) * paired with cgroup_rstat_flush_release(). * * This function may block. + * + * Returns 1 if flush happened */ -void cgroup_rstat_flush_hold(struct cgroup *cgrp) +int cgroup_rstat_flush_hold(struct cgroup *cgrp) __acquires(&cgroup_rstat_lock) { might_sleep(); __cgroup_rstat_lock(cgrp, -1); - cgroup_rstat_flush_locked(cgrp); + + /* Only do expensive flush after enough time have passed */ + if (time_after(jiffies, + cgrp->rstat_flush_last_time + FLUSH_TIME)) { + cgroup_rstat_flush_locked(cgrp); + return 1; + } + return 0; } /** diff --git a/mm/memcontrol.c b/mm/memcontrol.c index fabce2b50c69..771261612ec1 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -742,6 +742,7 @@ static void do_flush_stats(struct mem_cgroup *memcg) if (mem_cgroup_is_root(memcg)) WRITE_ONCE(flush_last_time, jiffies_64); + /* memcg have own ratelimit layer */ cgroup_rstat_flush(memcg->css.cgroup); }