linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: JP Kobryn <inwardvessel@gmail.com>
To: shakeel.butt@linux.dev, tj@kernel.org, mhocko@kernel.org,
	hannes@cmpxchg.org, yosryahmed@google.com,
	akpm@linux-foundation.org
Cc: linux-mm@kvack.org, cgroups@vger.kernel.org, kernel-team@meta.com
Subject: [PATCH 08/11] cgroup: rstat cpu lock indirection
Date: Mon, 17 Feb 2025 19:14:45 -0800	[thread overview]
Message-ID: <20250218031448.46951-9-inwardvessel@gmail.com> (raw)
In-Reply-To: <20250218031448.46951-1-inwardvessel@gmail.com>

Where functions access the global per-cpu lock, change their signature
to accept the lock instead as a paremeter. Change the code within these
functions to only access the parameter. This indirection allows for
future code to accept different locks, increasing extensibity. For
example, a new lock could be added specifically for the bpf cgroups and
it would not contend with the existing lock.

Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
---
 kernel/cgroup/rstat.c | 74 +++++++++++++++++++++++++------------------
 1 file changed, 43 insertions(+), 31 deletions(-)

diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c
index 4cb0f3ffc1db..9f6da3ea3c8c 100644
--- a/kernel/cgroup/rstat.c
+++ b/kernel/cgroup/rstat.c
@@ -177,7 +177,7 @@ void _cgroup_rstat_cpu_unlock(raw_spinlock_t *lock, int cpu,
 }
 
 static void __cgroup_rstat_updated(struct cgroup_rstat *rstat, int cpu,
-		struct cgroup_rstat_ops *ops)
+		struct cgroup_rstat_ops *ops, raw_spinlock_t *cpu_lock)
 {
 	struct cgroup *cgrp;
 	unsigned long flags;
@@ -194,7 +194,7 @@ static void __cgroup_rstat_updated(struct cgroup_rstat *rstat, int cpu,
 		return;
 
 	cgrp = ops->cgroup_fn(rstat);
-	flags = _cgroup_rstat_cpu_lock(&cgroup_rstat_cpu_lock, cpu, cgrp, true);
+	flags = _cgroup_rstat_cpu_lock(cpu_lock, cpu, cgrp, true);
 
 	/* put @rstat and all ancestors on the corresponding updated lists */
 	while (true) {
@@ -222,7 +222,7 @@ static void __cgroup_rstat_updated(struct cgroup_rstat *rstat, int cpu,
 		rstat = parent;
 	}
 
-	_cgroup_rstat_cpu_unlock(&cgroup_rstat_cpu_lock, cpu, cgrp, flags, true);
+	_cgroup_rstat_cpu_unlock(cpu_lock, cpu, cgrp, flags, true);
 }
 
 /**
@@ -236,13 +236,15 @@ static void __cgroup_rstat_updated(struct cgroup_rstat *rstat, int cpu,
  */
 void cgroup_rstat_updated(struct cgroup_subsys_state *css, int cpu)
 {
-	__cgroup_rstat_updated(&css->rstat, cpu, &rstat_css_ops);
+	__cgroup_rstat_updated(&css->rstat, cpu, &rstat_css_ops,
+			&cgroup_rstat_cpu_lock);
 }
 
 #ifdef CONFIG_CGROUP_BPF
 __bpf_kfunc void bpf_cgroup_rstat_updated(struct cgroup *cgroup, int cpu)
 {
-	__cgroup_rstat_updated(&(cgroup->bpf.rstat), cpu, &rstat_bpf_ops);
+	__cgroup_rstat_updated(&(cgroup->bpf.rstat), cpu, &rstat_bpf_ops,
+			&cgroup_rstat_cpu_lock);
 }
 #endif /* CONFIG_CGROUP_BPF */
 
@@ -319,7 +321,8 @@ static struct cgroup_rstat *cgroup_rstat_push_children(
  * here is the cgroup root whose updated_next can be self terminated.
  */
 static struct cgroup_rstat *cgroup_rstat_updated_list(
-		struct cgroup_rstat *root, int cpu, struct cgroup_rstat_ops *ops)
+		struct cgroup_rstat *root, int cpu, struct cgroup_rstat_ops *ops,
+		raw_spinlock_t *cpu_lock)
 {
 	struct cgroup_rstat_cpu *rstatc = rstat_cpu(root, cpu);
 	struct cgroup_rstat *head = NULL, *parent, *child;
@@ -327,7 +330,7 @@ static struct cgroup_rstat *cgroup_rstat_updated_list(
 	unsigned long flags;
 
 	cgrp = ops->cgroup_fn(root);
-	flags = _cgroup_rstat_cpu_lock(&cgroup_rstat_cpu_lock, cpu, cgrp, false);
+	flags = _cgroup_rstat_cpu_lock(cpu_lock, cpu, cgrp, false);
 
 	/* Return NULL if this subtree is not on-list */
 	if (!rstatc->updated_next)
@@ -364,7 +367,7 @@ static struct cgroup_rstat *cgroup_rstat_updated_list(
 	if (child != root)
 		head = cgroup_rstat_push_children(head, child, cpu, ops);
 unlock_ret:
-	_cgroup_rstat_cpu_unlock(&cgroup_rstat_cpu_lock, cpu, cgrp, flags, false);
+	_cgroup_rstat_cpu_unlock(cpu_lock, cpu, cgrp, flags, false);
 	return head;
 }
 
@@ -422,43 +425,46 @@ static inline void __cgroup_rstat_unlock(spinlock_t *lock,
 
 /* see cgroup_rstat_flush() */
 static void cgroup_rstat_flush_locked(struct cgroup_rstat *rstat,
-		struct cgroup_rstat_ops *ops)
-	__releases(&cgroup_rstat_lock) __acquires(&cgroup_rstat_lock)
+		struct cgroup_rstat_ops *ops, spinlock_t *lock,
+		raw_spinlock_t *cpu_lock)
+	__releases(lock) __acquires(lock)
 {
 	int cpu;
 
-	lockdep_assert_held(&cgroup_rstat_lock);
+	lockdep_assert_held(lock);
 
 	for_each_possible_cpu(cpu) {
 		struct cgroup_rstat *pos = cgroup_rstat_updated_list(
-				rstat, cpu, ops);
+				rstat, cpu, ops, cpu_lock);
 
 		for (; pos; pos = pos->rstat_flush_next)
 			ops->flush_fn(pos, cpu);
 
 		/* play nice and yield if necessary */
-		if (need_resched() || spin_needbreak(&cgroup_rstat_lock)) {
+		if (need_resched() || spin_needbreak(lock)) {
 			struct cgroup *cgrp;
 
 			cgrp = ops->cgroup_fn(rstat);
-			__cgroup_rstat_unlock(&cgroup_rstat_lock, cgrp, cpu);
+			__cgroup_rstat_unlock(lock, cgrp, cpu);
 			if (!cond_resched())
 				cpu_relax();
-			__cgroup_rstat_lock(&cgroup_rstat_lock, cgrp, cpu);
+			__cgroup_rstat_lock(lock, cgrp, cpu);
 		}
 	}
 }
 
 static void __cgroup_rstat_flush(struct cgroup_rstat *rstat,
-		struct cgroup_rstat_ops *ops)
+		struct cgroup_rstat_ops *ops, spinlock_t *lock,
+		raw_spinlock_t *cpu_lock)
+	__acquires(lock) __releases(lock)
 {
 	struct cgroup *cgrp;
 
 	might_sleep();
 	cgrp = ops->cgroup_fn(rstat);
-	__cgroup_rstat_lock(&cgroup_rstat_lock, cgrp, -1);
-	cgroup_rstat_flush_locked(rstat, ops);
-	__cgroup_rstat_unlock(&cgroup_rstat_lock, cgrp, -1);
+	__cgroup_rstat_lock(lock, cgrp, -1);
+	cgroup_rstat_flush_locked(rstat, ops, lock, cpu_lock);
+	__cgroup_rstat_unlock(lock, cgrp, -1);
 }
 
 /**
@@ -476,26 +482,29 @@ static void __cgroup_rstat_flush(struct cgroup_rstat *rstat,
  */
 void cgroup_rstat_flush(struct cgroup_subsys_state *css)
 {
-	__cgroup_rstat_flush(&css->rstat, &rstat_css_ops);
+	__cgroup_rstat_flush(&css->rstat, &rstat_css_ops,
+			&cgroup_rstat_lock, &cgroup_rstat_cpu_lock);
 }
 
 #ifdef CONFIG_CGROUP_BPF
 __bpf_kfunc void bpf_cgroup_rstat_flush(struct cgroup *cgroup)
 {
-	__cgroup_rstat_flush(&(cgroup->bpf.rstat), &rstat_bpf_ops);
+	__cgroup_rstat_flush(&(cgroup->bpf.rstat), &rstat_bpf_ops,
+			&cgroup_rstat_lock, &cgroup_rstat_cpu_lock);
 }
 #endif /* CONFIG_CGROUP_BPF */
 
 static void __cgroup_rstat_flush_hold(struct cgroup_rstat *rstat,
-		struct cgroup_rstat_ops *ops)
-	__acquires(&cgroup_rstat_lock)
+		struct cgroup_rstat_ops *ops, spinlock_t *lock,
+		raw_spinlock_t *cpu_lock)
+	__acquires(lock)
 {
 	struct cgroup *cgrp;
 
 	might_sleep();
 	cgrp = ops->cgroup_fn(rstat);
-	__cgroup_rstat_lock(&cgroup_rstat_lock, cgrp, -1);
-	cgroup_rstat_flush_locked(rstat, ops);
+	__cgroup_rstat_lock(lock, cgrp, -1);
+	cgroup_rstat_flush_locked(rstat, ops, lock, cpu_lock);
 }
 
 /**
@@ -509,7 +518,8 @@ static void __cgroup_rstat_flush_hold(struct cgroup_rstat *rstat,
  */
 void cgroup_rstat_flush_hold(struct cgroup_subsys_state *css)
 {
-	__cgroup_rstat_flush_hold(&css->rstat, &rstat_css_ops);
+	__cgroup_rstat_flush_hold(&css->rstat, &rstat_css_ops,
+			&cgroup_rstat_lock, &cgroup_rstat_cpu_lock);
 }
 
 /**
@@ -517,13 +527,13 @@ void cgroup_rstat_flush_hold(struct cgroup_subsys_state *css)
  * @rstat: rstat node used to find associated cgroup used by tracepoint
  */
 static void __cgroup_rstat_flush_release(struct cgroup_rstat *rstat,
-		struct cgroup_rstat_ops *ops)
-	__releases(&cgroup_rstat_lock)
+		struct cgroup_rstat_ops *ops, spinlock_t *lock)
+	__releases(lock)
 {
 	struct cgroup *cgrp;
 
 	cgrp = ops->cgroup_fn(rstat);
-	__cgroup_rstat_unlock(&cgroup_rstat_lock, cgrp, -1);
+	__cgroup_rstat_unlock(lock, cgrp, -1);
 }
 
 /**
@@ -532,7 +542,8 @@ static void __cgroup_rstat_flush_release(struct cgroup_rstat *rstat,
  */
 void cgroup_rstat_flush_release(struct cgroup_subsys_state *css)
 {
-	__cgroup_rstat_flush_release(&css->rstat, &rstat_css_ops);
+	__cgroup_rstat_flush_release(&css->rstat, &rstat_css_ops,
+			&cgroup_rstat_lock);
 }
 
 static void __cgroup_rstat_init(struct cgroup_rstat *rstat)
@@ -605,7 +616,8 @@ int bpf_cgroup_rstat_init(struct cgroup_bpf *bpf)
 
 void bpf_cgroup_rstat_exit(struct cgroup_bpf *bpf)
 {
-	__cgroup_rstat_flush(&bpf->rstat, &rstat_bpf_ops);
+	__cgroup_rstat_flush(&bpf->rstat, &rstat_bpf_ops,
+			&cgroup_rstat_lock, &cgroup_rstat_cpu_lock);
 	__cgroup_rstat_exit(&bpf->rstat);
 }
 #endif /* CONFIG_CGROUP_BPF */
-- 
2.48.1



  parent reply	other threads:[~2025-02-18  3:15 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-18  3:14 [PATCH 00/11] cgroup: separate rstat trees JP Kobryn
2025-02-18  3:14 ` [PATCH 01/11] cgroup: move rstat pointers into struct of their own JP Kobryn
2025-02-19  1:05   ` Shakeel Butt
2025-02-19  1:23     ` Shakeel Butt
2025-02-20 16:53   ` Yosry Ahmed
2025-02-24 17:06     ` JP Kobryn
2025-02-24 18:36       ` Yosry Ahmed
2025-02-18  3:14 ` [PATCH 02/11] cgroup: add level of indirection for cgroup_rstat struct JP Kobryn
2025-02-19  2:26   ` Shakeel Butt
2025-02-20 17:08     ` Yosry Ahmed
2025-02-19  5:57   ` kernel test robot
2025-02-18  3:14 ` [PATCH 03/11] cgroup: move cgroup_rstat from cgroup to cgroup_subsys_state JP Kobryn
2025-02-20 17:06   ` Shakeel Butt
2025-02-20 17:22     ` Yosry Ahmed
2025-02-25 19:20       ` JP Kobryn
2025-02-18  3:14 ` [PATCH 04/11] cgroup: introduce cgroup_rstat_ops JP Kobryn
2025-02-19  7:21   ` kernel test robot
2025-02-20 17:50   ` Shakeel Butt
2025-02-18  3:14 ` [PATCH 05/11] cgroup: separate rstat for bpf cgroups JP Kobryn
2025-02-21 18:14   ` Shakeel Butt
2025-02-18  3:14 ` [PATCH 06/11] cgroup: rstat lock indirection JP Kobryn
2025-02-21 22:09   ` Shakeel Butt
2025-02-18  3:14 ` [PATCH 07/11] cgroup: fetch cpu-specific lock in rstat cpu lock helpers JP Kobryn
2025-02-21 22:35   ` Shakeel Butt
2025-02-18  3:14 ` JP Kobryn [this message]
2025-02-19  8:48   ` [PATCH 08/11] cgroup: rstat cpu lock indirection kernel test robot
2025-02-22  0:18   ` Shakeel Butt
2025-02-18  3:14 ` [PATCH 09/11] cgroup: separate rstat locks for bpf cgroups JP Kobryn
2025-02-18  3:14 ` [PATCH 10/11] cgroup: separate rstat locks for subsystems JP Kobryn
2025-02-22  0:23   ` Shakeel Butt
2025-02-18  3:14 ` [PATCH 11/11] cgroup: separate rstat list pointers from base stats JP Kobryn
2025-02-22  0:28   ` Shakeel Butt
2025-02-20 15:51 ` [PATCH 00/11] cgroup: separate rstat trees Tejun Heo
2025-02-27 23:44   ` JP Kobryn
2025-02-20 17:26 ` Yosry Ahmed
2025-02-20 17:53   ` Shakeel Butt
2025-02-20 17:59     ` Yosry Ahmed
2025-02-20 18:14       ` JP Kobryn
2025-02-20 20:04         ` Yosry Ahmed
2025-02-20 20:22           ` Yosry Ahmed
2025-02-24 21:13           ` Shakeel Butt
2025-02-24 21:54             ` Yosry Ahmed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250218031448.46951-9-inwardvessel@gmail.com \
    --to=inwardvessel@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=kernel-team@meta.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=shakeel.butt@linux.dev \
    --cc=tj@kernel.org \
    --cc=yosryahmed@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).