From mboxrd@z Thu Jan 1 00:00:00 1970 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 9AD76C021B2 for ; Thu, 20 Feb 2025 16:53:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 291042802F8; Thu, 20 Feb 2025 11:53:54 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 169E12802F2; Thu, 20 Feb 2025 11:53:54 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 009F22802F8; Thu, 20 Feb 2025 11:53:53 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id D77912802F2 for ; Thu, 20 Feb 2025 11:53:53 -0500 (EST) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 6F7A182502 for ; Thu, 20 Feb 2025 16:53:53 +0000 (UTC) X-FDA: 83140919946.01.62310B5 Received: from out-173.mta1.migadu.com (out-173.mta1.migadu.com [95.215.58.173]) by imf15.hostedemail.com (Postfix) with ESMTP id 88CA2A0007 for ; Thu, 20 Feb 2025 16:53:51 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=li1GhPji; spf=pass (imf15.hostedemail.com: domain of yosry.ahmed@linux.dev designates 95.215.58.173 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740070431; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=6BN9a67ew2xkxLArhOKgNEo4qqfDq1lcFzFd75Yb27g=; b=u37RdLOjdu4SG8UYzZkE/4oYUH2Qy+rFYBW6xSmOV7fX1UEDZrqySQC6inb5Ce6qB/jnx/ tNb68PdVv+vQ/Nylga3OfMjEjkh4/zLfrQPNbgXEFFngf9XrTbk+4Lq/+Tg017hsMNNRcP aneTVunAEB/qFVipz9IbPtcTBM43wHY= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740070431; a=rsa-sha256; cv=none; b=L15Lr5IAINs4gywRgcPgfZIEcT4qUn806iVfkpQPrfcW8OaUxbhJUryWL8EEKSxAErbqbd NVtBLoOtiJs66BdP+kEK/s+R3eiskFXT3ABqrLfv9TooHv/R92xAuR0n6v18+dx7XN3kjy BsagP9gudgpMYNJ9hZiaurdaotFf8mk= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=li1GhPji; spf=pass (imf15.hostedemail.com: domain of yosry.ahmed@linux.dev designates 95.215.58.173 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev Date: Thu, 20 Feb 2025 16:53:41 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1740070429; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=6BN9a67ew2xkxLArhOKgNEo4qqfDq1lcFzFd75Yb27g=; b=li1GhPjicEESF/SK9GMY0a8fGXlA3miatnY2Ops1O8eqGvtOBBLiX1LTN9X6u5dGAnQxgv 4s6Wu+t29rKSiZWoEdJ/SxqpvfaGSyn1n8Gx2OCGtWDMNAKmY7TkVcBydIy6rNFnelkURS wTvwB9p4z/bstOIPSHz/I4Zx0ywxiOM= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Yosry Ahmed To: JP Kobryn Cc: shakeel.butt@linux.dev, tj@kernel.org, mhocko@kernel.org, hannes@cmpxchg.org, akpm@linux-foundation.org, linux-mm@kvack.org, cgroups@vger.kernel.org, kernel-team@meta.com Subject: Re: [PATCH 01/11] cgroup: move rstat pointers into struct of their own Message-ID: References: <20250218031448.46951-1-inwardvessel@gmail.com> <20250218031448.46951-2-inwardvessel@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250218031448.46951-2-inwardvessel@gmail.com> X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Queue-Id: 88CA2A0007 X-Rspamd-Server: rspam07 X-Stat-Signature: ojngkrq4p3r569yjwpcq7baad7qotnfd X-HE-Tag: 1740070431-835434 X-HE-Meta: U2FsdGVkX1/2jGqMD9Afv8SesGVgK2nxdnq54UGenFUYKmAQC1sy6XoUKdreJKMDPTlfqoPc2xrK/jChuxMe1qNUMDR3vydks8mZd/tEgcCBrv/DD7YyFxNKk6Boeoofnl3Y/nJTwRNZEE3WnYz6Fz9mXrF18kYKkXDTp5YFdk7ij2bFI65dY/HFhRJ25jlkre1wB0w/pnJrHgdKFrXI4D3mhbkQr2yjSad9Xz7iJcQg8zaMWGb5uXfjQ2TgSJpUPsnEkvTHlsdba462zoG1u3kGRFp3ELryWUkhm8TPcKDZrcdKFhIUn/sfPhULsMVUHRTZyXNGGYvR53rTK0y4QuczAolBvCe4n0DNLfe/bRGhuPxzFMPuL49aEdRBVfa86F02KHaKOwHaUE1wCq0A7704oEKbiDimj850KtEaW5FWuXh+j+8WAX9YwcAlrJUwsbvw1H4+NoqGUdAF3wa904Tmu9SxuOJQuxGnO4aaPgbkiuqgmb9XaszPqeqBxcqWWUkaDmRGYhAZBu1KM5etnkbmUl/E29ozIcGH/FiNtp5ZgB4cm0DJfdSHuWJvM2xRRtiS5i1xOrhzJeXIqNkqM+23s8v+Lpxiq/9krr9D/Wwexm9EGqNE7LquwlJDeUh8BbaKHhVDO/9IrMfZLiyuFlfU6YtbTE/T0I/Yg5KFff2KTB2T9D5+vtfAQ9Trqz7k9Ah76Qb9jLtMxysLt671C+Rb0D+Hpy/k5Nf1ZEggj3mlYxMHVLZeWsDkmneEzLb3OIG7cEYWcRmD35X6iX4fmW3/zjC+EvSSzdjga2Cd9l1pPq9vP9gZwscwzDHGuUrIKxzF2M0AF5Ql/ePYBpFjTs9lW6HZhV8x1RW8otuTq2wiw7MfOiss1drgCjk/BoKeHjvISrKBSnp0q0sjUCt2EzLZk5KVqb/7E5MtDxn2t3bnNNuNuoJWtS5BE4fg9t8sewN4JjYYFFp8IEMC2qX T2nrDTYA rChYlnsNitIfvYfyD7BSCPRxD+CtxnAOCP51IcRMG1kqj+KyyPhyQxcW2iTxBC4UkK8nPPy1NcsgAHZNoFewJ+LrHayxWTG3wJWYJFKgsby3zJlUa3+4aPFd66Nwkq6lE7nErd8VTITovYKGrsXl4NxdV40ZVGDMaIKgKNv8+hDm67u8NuICDLlq0WntFseL657WpD8Fa2fKTtlY6Eywge7X2p6YkwlozMzHc1/3/4yDqP0D7/RY7TfQts4vQQqyS8qDuL/rWaIsG2cvXulLoUgwgQGj+DTah/PiWVJlJcWNTbSOJJgJUl9WVbA== 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: On Mon, Feb 17, 2025 at 07:14:38PM -0800, JP Kobryn wrote: > The rstat infrastructure makes use of pointers for list management. > These pointers only exist as fields in the cgroup struct, so moving them > into their own struct will allow them to be used elsewhere. The base > stat entities are included with them for now. > > Signed-off-by: JP Kobryn > --- > include/linux/cgroup-defs.h | 90 +----------------- > include/linux/cgroup_rstat.h | 92 +++++++++++++++++++ > kernel/cgroup/cgroup.c | 3 +- > kernel/cgroup/rstat.c | 27 +++--- > .../selftests/bpf/progs/btf_type_tag_percpu.c | 4 +- > 5 files changed, 112 insertions(+), 104 deletions(-) > create mode 100644 include/linux/cgroup_rstat.h > > diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h > index 1b20d2d8ef7c..6b6cc027fe70 100644 > --- a/include/linux/cgroup-defs.h > +++ b/include/linux/cgroup-defs.h > @@ -17,7 +17,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > #include > @@ -321,78 +321,6 @@ struct css_set { > struct rcu_head rcu_head; > }; > > -struct cgroup_base_stat { > - struct task_cputime cputime; > - > -#ifdef CONFIG_SCHED_CORE > - u64 forceidle_sum; > -#endif > - u64 ntime; > -}; > - > -/* > - * rstat - cgroup scalable recursive statistics. Accounting is done > - * per-cpu in cgroup_rstat_cpu which is then lazily propagated up the > - * hierarchy on reads. > - * > - * When a stat gets updated, the cgroup_rstat_cpu and its ancestors are > - * linked into the updated tree. On the following read, propagation only > - * considers and consumes the updated tree. This makes reading O(the > - * number of descendants which have been active since last read) instead of > - * O(the total number of descendants). > - * > - * This is important because there can be a lot of (draining) cgroups which > - * aren't active and stat may be read frequently. The combination can > - * become very expensive. By propagating selectively, increasing reading > - * frequency decreases the cost of each read. > - * > - * This struct hosts both the fields which implement the above - > - * updated_children and updated_next - and the fields which track basic > - * resource statistics on top of it - bsync, bstat and last_bstat. > - */ > -struct cgroup_rstat_cpu { > - /* > - * ->bsync protects ->bstat. These are the only fields which get > - * updated in the hot path. > - */ > - struct u64_stats_sync bsync; > - struct cgroup_base_stat bstat; > - > - /* > - * Snapshots at the last reading. These are used to calculate the > - * deltas to propagate to the global counters. > - */ > - struct cgroup_base_stat last_bstat; > - > - /* > - * This field is used to record the cumulative per-cpu time of > - * the cgroup and its descendants. Currently it can be read via > - * eBPF/drgn etc, and we are still trying to determine how to > - * expose it in the cgroupfs interface. > - */ > - struct cgroup_base_stat subtree_bstat; > - > - /* > - * Snapshots at the last reading. These are used to calculate the > - * deltas to propagate to the per-cpu subtree_bstat. > - */ > - struct cgroup_base_stat last_subtree_bstat; > - > - /* > - * Child cgroups with stat updates on this cpu since the last read > - * are linked on the parent's ->updated_children through > - * ->updated_next. > - * > - * In addition to being more compact, singly-linked list pointing > - * to the cgroup makes it unnecessary for each per-cpu struct to > - * point back to the associated cgroup. > - * > - * Protected by per-cpu cgroup_rstat_cpu_lock. > - */ > - struct cgroup *updated_children; /* terminated by self cgroup */ > - struct cgroup *updated_next; /* NULL iff not on the list */ > -}; > - > struct cgroup_freezer_state { > /* Should the cgroup and its descendants be frozen. */ > bool freeze; > @@ -517,23 +445,9 @@ struct cgroup { > struct cgroup *old_dom_cgrp; /* used while enabling threaded */ > > /* per-cpu recursive resource statistics */ > - struct cgroup_rstat_cpu __percpu *rstat_cpu; > + struct cgroup_rstat rstat; > struct list_head rstat_css_list; > > - /* > - * Add padding to separate the read mostly rstat_cpu and > - * rstat_css_list into a different cacheline from the following > - * rstat_flush_next and *bstat fields which can have frequent updates. > - */ > - CACHELINE_PADDING(_pad_); > - > - /* > - * A singly-linked list of cgroup structures to be rstat flushed. > - * This is a scratch field to be used exclusively by > - * cgroup_rstat_flush_locked() and protected by cgroup_rstat_lock. > - */ > - struct cgroup *rstat_flush_next; > - > /* cgroup basic resource statistics */ > struct cgroup_base_stat last_bstat; > struct cgroup_base_stat bstat; > diff --git a/include/linux/cgroup_rstat.h b/include/linux/cgroup_rstat.h > new file mode 100644 > index 000000000000..f95474d6f8ab > --- /dev/null > +++ b/include/linux/cgroup_rstat.h > @@ -0,0 +1,92 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_RSTAT_H > +#define _LINUX_RSTAT_H > + > +#include > + > +struct cgroup_rstat_cpu; Why do we need the forward declaration instead of just defining struct cgroup_rstat_cpu first? Also, why do we need a new header for these definitions rather than just adding struct cgroup_rstat to cgroup-defs.h? > + > +/* > + * rstat - cgroup scalable recursive statistics. Accounting is done > + * per-cpu in cgroup_rstat_cpu which is then lazily propagated up the > + * hierarchy on reads. > + * > + * When a stat gets updated, the cgroup_rstat_cpu and its ancestors are > + * linked into the updated tree. On the following read, propagation only > + * considers and consumes the updated tree. This makes reading O(the > + * number of descendants which have been active since last read) instead of > + * O(the total number of descendants). > + * > + * This is important because there can be a lot of (draining) cgroups which > + * aren't active and stat may be read frequently. The combination can > + * become very expensive. By propagating selectively, increasing reading > + * frequency decreases the cost of each read. > + * > + * This struct hosts both the fields which implement the above - > + * updated_children and updated_next - and the fields which track basic > + * resource statistics on top of it - bsync, bstat and last_bstat. > + */ > +struct cgroup_rstat { > + struct cgroup_rstat_cpu __percpu *rstat_cpu; > + > + /* > + * Add padding to separate the read mostly rstat_cpu and > + * rstat_css_list into a different cacheline from the following > + * rstat_flush_next and containing struct fields which can have > + * frequent updates. > + */ > + CACHELINE_PADDING(_pad_); > + struct cgroup *rstat_flush_next; > +}; > + > +struct cgroup_base_stat { > + struct task_cputime cputime; > + > +#ifdef CONFIG_SCHED_CORE > + u64 forceidle_sum; > +#endif > + u64 ntime; > +}; > + > +struct cgroup_rstat_cpu { > + /* > + * Child cgroups with stat updates on this cpu since the last read > + * are linked on the parent's ->updated_children through > + * ->updated_next. > + * > + * In addition to being more compact, singly-linked list pointing > + * to the cgroup makes it unnecessary for each per-cpu struct to > + * point back to the associated cgroup. > + */ > + struct cgroup *updated_children; /* terminated by self */ > + struct cgroup *updated_next; /* NULL if not on the list */ > + > + /* > + * ->bsync protects ->bstat. These are the only fields which get > + * updated in the hot path. > + */ > + struct u64_stats_sync bsync; > + struct cgroup_base_stat bstat; > + > + /* > + * Snapshots at the last reading. These are used to calculate the > + * deltas to propagate to the global counters. > + */ > + struct cgroup_base_stat last_bstat; > + > + /* > + * This field is used to record the cumulative per-cpu time of > + * the cgroup and its descendants. Currently it can be read via > + * eBPF/drgn etc, and we are still trying to determine how to > + * expose it in the cgroupfs interface. > + */ > + struct cgroup_base_stat subtree_bstat; > + > + /* > + * Snapshots at the last reading. These are used to calculate the > + * deltas to propagate to the per-cpu subtree_bstat. > + */ > + struct cgroup_base_stat last_subtree_bstat; > +}; > + > +#endif /* _LINUX_RSTAT_H */