0001-runtime_context-factor-out-RCU-usage-using-a-rw-lock.patch

From fdb1494b7ad44047f7ae01f28ee9d9427e432247 Mon Sep 17 00:00:00 2001
From: Sultan Alsawaf <sultan@openresty.com>
Date: Tue, 24 Nov 2020 10:50:10 -0800
Subject: [PATCH] runtime_context: factor out RCU usage using a rw lock

We can factor out the RCU insanity in here by just adding in a rw lock
and using that to synchronize _stp_runtime_contexts_free() with any code
that has the runtime context held.
---
 runtime/linux/runtime_context.h | 97 +++++++++++++--------------------
 1 file changed, 38 insertions(+), 59 deletions(-)

diff --git a/runtime/linux/runtime_context.h b/runtime/linux/runtime_context.h
index 41fecba81..18566957a 100644
--- a/runtime/linux/runtime_context.h
+++ b/runtime/linux/runtime_context.h
@@ -11,15 +11,14 @@
 #ifndef _LINUX_RUNTIME_CONTEXT_H_
 #define _LINUX_RUNTIME_CONTEXT_H_
 
-#ifndef __rcu
-#define __rcu
-#endif
-
-static struct context __rcu *contexts[NR_CPUS] = { NULL };
+/* Can't use STP_DEFINE_RWLOCK() or this might be replaced with a spin lock */
+static DEFINE_RWLOCK(_stp_context_lock);
+static DEFINE_PER_CPU(struct context *, contexts);
+static atomic_t _stp_context_stop = ATOMIC_INIT(0);
 
 static int _stp_runtime_contexts_alloc(void)
 {
-	int cpu;
+	unsigned int cpu;
 
 	for_each_possible_cpu(cpu) {
 		/* Module init, so in user context, safe to use
@@ -31,91 +30,67 @@ static int _stp_runtime_contexts_alloc(void)
 				    (unsigned long) sizeof (struct context));
 			return -ENOMEM;
 		}
-		rcu_assign_pointer(contexts[cpu], c);
+		per_cpu(contexts, cpu) = c;
 	}
 	return 0;
 }
 
 /* We should be free of all probes by this time, but for example the timer for
  * _stp_ctl_work_callback may still be running and looking for contexts.  We
- * use RCU-sched synchronization to be sure its safe to free them.  */
+ * use _stp_context_stop and a write lock to be sure its safe to free them.  */
 static void _stp_runtime_contexts_free(void)
 {
-	// Note that 'free_contexts' is static because it is
-	// (probably) too big to fit on a kernel function's stack.
-	static struct context *free_contexts[NR_CPUS] = { NULL };
-	int cpu;
+	unsigned long flags;
+	unsigned int cpu;
 
-	/* First, save all the pointers.  */
-	rcu_read_lock_sched();
-	for_each_possible_cpu(cpu) {
-		free_contexts[cpu] = rcu_dereference_sched(contexts[cpu]);
-	}
-	rcu_read_unlock_sched();
+	/* Sync to make sure existing readers are done */
+	atomic_set(&_stp_context_stop, 1);
+	write_lock_irqsave(&_stp_context_lock, flags);
+	write_unlock_irqrestore(&_stp_context_lock, flags);
 
-	/* Now clear all pointers to prevent new readers.  */
-	for_each_possible_cpu(cpu) {
-		rcu_assign_pointer(contexts[cpu], NULL);
-	}
-
-	/* Sync to make sure existing readers are done.  */
-	stp_synchronize_sched();
-
-	/* Now we can actually free the contexts.  */
-	for_each_possible_cpu(cpu) {
-		struct context *c = free_contexts[cpu];
-		if (c != NULL) {
-			free_contexts[cpu] = NULL;
-			_stp_vfree(c);
-		}
-	}
+	/* Now we can actually free the contexts */
+	for_each_possible_cpu(cpu)
+		_stp_vfree(per_cpu(contexts, cpu));
 }
 
 static inline struct context * _stp_runtime_get_context(void)
 {
-	// RHBZ1788662 rcu operations are rejected in idle-cpu contexts
-	// in effect: skip probe if it's in rcu-idle state
-#if defined(STAPCONF_RCU_IS_WATCHING) || LINUX_VERSION_CODE >= KERNEL_VERSION(3,13,0) // linux commit #5c173eb8
-        if (! rcu_is_watching())
-		return 0;
-#elif LINUX_VERSION_CODE >= KERNEL_VERSION(3,3,0) // linux commit #9b2e4f18
-        if (! rcu_is_cpu_idle())
-		return 0;
-#else
-	; // XXX older kernels didn't put tracepoints in idle-cpu
-#endif
-	return rcu_dereference_sched(contexts[smp_processor_id()]);
+	if (atomic_read(&_stp_context_stop))
+		return NULL;
+
+	return per_cpu(contexts, smp_processor_id());
 }
 
 static struct context * _stp_runtime_entryfn_get_context(void)
+	__acquires(&_stp_context_lock)
 {
 	struct context* __restrict__ c = NULL;
-	preempt_disable ();
+
+	if (!read_trylock(&_stp_context_lock))
+		return NULL;
+
 	c = _stp_runtime_get_context();
 	if (c != NULL) {
-		if (atomic_inc_return(&c->busy) == 1) {
-			// NB: Notice we're not re-enabling preemption
+		if (!atomic_cmpxchg(&c->busy, 0, 1)) {
+			// NB: Notice we're not releasing _stp_context_lock
 			// here. We exepect the calling code to call
 			// _stp_runtime_entryfn_get_context() and
 			// _stp_runtime_entryfn_put_context() as a
 			// pair.
 			return c;
 		}
-		atomic_dec(&c->busy);
 	}
-	preempt_enable_no_resched();
+	read_unlock(&_stp_context_lock);
 	return NULL;
 }
 
 static inline void _stp_runtime_entryfn_put_context(struct context *c)
+	__releases(&_stp_context_lock)
 {
 	if (c) {
-		if (c == _stp_runtime_get_context())
-			atomic_dec(&c->busy);
-		/* else, warn about bad state? */
-		preempt_enable_no_resched();
+		atomic_set(&c->busy, 0);
+		read_unlock(&_stp_context_lock);
 	}
-	return;
 }
 
 static void _stp_runtime_context_wait(void)
@@ -130,9 +105,13 @@ static void _stp_runtime_context_wait(void)
 		int i;
 
 		holdon = 0;
-		rcu_read_lock_sched();
+		read_lock(&_stp_context_lock);
+		if (atomic_read(&_stp_context_stop)) {
+			read_unlock(&_stp_context_lock);
+			break;
+		}
 		for_each_possible_cpu(i) {
-			struct context *c = rcu_dereference_sched(contexts[i]);
+			struct context *c = per_cpu(contexts, i);
 			if (c != NULL
 			    && atomic_read (& c->busy)) {
 				holdon = 1;
@@ -146,7 +125,7 @@ static void _stp_runtime_context_wait(void)
 				}
 			}
 		}
-		rcu_read_unlock_sched();
+		read_unlock(&_stp_context_lock);
 
 		/*
 		 * Just in case things are really really stuck, a
-- 
2.29.2

systemtap-sum.diff

--- systemtap-before.sum	2020-11-18 08:54:39.017487512 -0800
+++ systemtap-after.sum	2020-11-24 16:48:16.405913154 -0800
@@ -1,4 +1,4 @@
-Test Run By root on Tue Nov 17 18:37:19 2020
+Test Run By root on Tue Nov 24 12:07:09 2020
 Native configuration is x86_64-unknown-linux-gnu
 
 		=== systemtap tests ===
@@ -9,7 +9,7 @@
 Running target unix
 
 Host: Linux centos7-bb 3.10.0-1127.19.1.el7.x86_64 #1 SMP Tue Aug 25 17:23:54 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
-Snapshot: version 4.4/0.177, commit release-4.4-10-g532eb9a15020
+Snapshot: version 4.4/0.177, commit release-4.4-11-gc2a88ed387fd
 GCC: 4.8.5 [gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-39)]
 Distro: CentOS Linux release 7.8.2003 (Core)
 SElinux: Enforcing
@@ -253,7 +253,7 @@
 PASS: auto_path2
 PASS: auto_path3
 Running /home/sultan/systemtap/testsuite/systemtap.base/backtrace.exp ...
-PASS: backtrace (3 38)
+PASS: backtrace (3 35)
 PASS: backtrace-unwindsyms (3 32)
 PASS: self-unwind-ensure-exact (0)
 Running /home/sultan/systemtap/testsuite/systemtap.base/bad-code.exp ...
@@ -642,7 +642,7 @@
 Running /home/sultan/systemtap/testsuite/systemtap.base/environment_sanity.exp ...
 
 Host: Linux centos7-bb 3.10.0-1127.19.1.el7.x86_64 #1 SMP Tue Aug 25 17:23:54 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
-Snapshot: version 4.4/0.177, commit release-4.4-10-g532eb9a15020
+Snapshot: version 4.4/0.177, commit release-4.4-11-gc2a88ed387fd
 GCC: 4.8.5 [gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-39)]
 Distro: CentOS Linux release 7.8.2003 (Core)
 SElinux: Enforcing
@@ -2786,8 +2786,7 @@
 PASS: onoffprobe function entry probed
 PASS: onoffprobe timer probed
 PASS: onoffprobe profile probed
-PASS: onoffprobe alias.one.a and alias.one and alias.* probed
-alias.one.a and alias.one and alias.* probed
+PASS: onoffprobe alias.one.a and alias.one and alias.* probed
 PASS: onoffprobe alias.one.b and alias.one and alias.* probed
 PASS: onoffprobe alias.two and alias.* probed
 Running /home/sultan/systemtap/testsuite/systemtap.base/openmp-stmt.exp ...
@@ -2797,10 +2796,10 @@
 Running /home/sultan/systemtap/testsuite/systemtap.base/optim_arridx.exp ...
 FAIL: optim_arridx
 Running /home/sultan/systemtap/testsuite/systemtap.base/optim_stats.exp ...
-PASS: TEST1  (5, 28)
-PASS: TEST2  (20, 78)
-FAIL: TEST3  (5, 0)
-PASS: TEST4  (20, 53)
+PASS: TEST1  (5, 10)
+PASS: TEST2  (20, 85)
+PASS: TEST3  (5, 8)
+PASS: TEST4  (20, 76)
 Running /home/sultan/systemtap/testsuite/systemtap.base/optim_voidstmt.exp ...
 PASS: optim_voidstmt startup
 PASS: optim_voidstmt load generation
@@ -2834,7 +2833,7 @@
 FAIL: perf process (0 - 0)
 PASS: perf counter
 FAIL: perf global (0 - 0)
-UNRESOLVED: counter order  700000 1000000
+UNRESOLVED: counter order  500000 600000
 Running /home/sultan/systemtap/testsuite/systemtap.base/plt.exp ...
 PASS: plt
 PASS: plt library
@@ -10367,8 +10366,8 @@
 
 		=== systemtap Summary ===
 
-# of expected passes		8186
-# of unexpected failures	286
+# of expected passes		8187
+# of unexpected failures	285
 # of unexpected successes	8
 # of expected failures		347
 # of known failures		82