[PATCH v7 06/10] tracing/osnoise: Reset cleanup

[Crystal Wood <crwood@redhat.com>]

Reset per-cpu timerlat state every time we stop a kthread or detach an
fd, rather than assume everything gets cleaned up properly, and avoiding
the need to explicitly zero individual fields.

Also, stop initializing tlat->thread lazily, add some debug checks to
detect an unclean state, and clear osnoise_data.tainted in
osn_var_reset().

Combine the per-cpu tlat cleanup and clearing osn->sampling and osn->pid into
osn_reset_cpu(), which is called consistently regardless of the tracer mode.

Don't clear the rest of the osnoise per-cpu state except when the full
tracer has been reset, to avoid racing with interrupt trace hooks.

Signed-off-by: Crystal Wood <crwood@redhat.com>
---
v2: New patch.  Without this, I was seeing sampling = 1 leaking from a
kthread context to an fd context, which confused the latter into
operating with tlat->thread set to the old kernel thread, causing the fd
thread to hang.

v3: Reset the per-cpu context unconditionally in stop_kthread(), so sampling
gets cleared as noted in patch 3/9

v4: Only reset the cpu data if the fdd is a match

v6: Revive patch, but remove the interrupt counters from the zeroed area

v7:
 - Go back to clearing interrupt counters on full tracer reset, explicily clearing
   osn->sampling and osn->pid only during cpu detachment.  The issue
   pointed out by Sashiko in v6, where we could have stale data after a
   detach, is still there but it is not a new issue and is out of scope
   for this patchset, and unlike v6 it doesn't persist across full tracer
   resets.
 - Use WARN_ON_ONCE consistently
---
 kernel/trace/trace_osnoise.c | 117 ++++++++++++++++++-----------------
 1 file changed, 61 insertions(+), 56 deletions(-)

diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
index e61f57230eef..8db0697b69e4 100644
--- a/kernel/trace/trace_osnoise.c
+++ b/kernel/trace/trace_osnoise.c
@@ -237,9 +237,10 @@ struct osnoise_variables {
 	struct mutex		lock; /* covers kthread and fdd changes */
 	struct task_struct	*kthread;
 	struct fd_data		*fdd;
+	bool			sampling;
 
+	/* reset only when the entire tracer is restarted */
 	struct_group(zero,
-		bool			sampling;
 		pid_t			pid;
 		struct osn_nmi		nmi;
 		struct osn_irq		irq;
@@ -287,6 +288,7 @@ struct timerlat_variables {
 	 */
 	struct mutex		detach_lock;
 
+	/* reset on per-cpu detachment */
 	struct_group(zero,
 		struct task_struct	*thread;
 		struct hrtimer		timer;
@@ -318,23 +320,19 @@ static inline struct timerlat_variables *cpu_tmr_var(int cpu)
 	return per_cpu_ptr(&per_cpu_timerlat_var, cpu);
 }
 
+static inline void tlat_reset_cpu(int cpu)
+{
+	struct timerlat_variables *tlat = cpu_tmr_var(cpu);
+
+	WARN_ON_ONCE(tlat->thread);
+	memset(&tlat->zero, 0, sizeof(tlat->zero));
+}
+
 /*
- * tlat_var_reset - Reset the values of the given timerlat_variables
+ * tlat_var_reset - Reset non-per-cpu state
  */
 static inline void tlat_var_reset(void)
 {
-	struct timerlat_variables *tlat_var;
-	int cpu;
-
-	/*
-	 * So far, all the values are initialized as 0, so
-	 * zeroing the structure is perfect.
-	 */
-	for_each_online_cpu(cpu) {
-		tlat_var = cpu_tmr_var(cpu);
-		WARN_ON_ONCE(tlat_var->thread);
-		memset(&tlat_var->zero, 0, sizeof(tlat_var->zero));
-	}
 	/*
 	 * Reset also align_next, to be filled by a new offset by the first timerlat
 	 * thread that wakes up, if TIMERLAT_ALIGN is set.
@@ -342,38 +340,12 @@ static inline void tlat_var_reset(void)
 	atomic64_set(&align_next, 0);
 }
 #else /* CONFIG_TIMERLAT_TRACER */
-#define tlat_var_reset()	do {} while (0)
-#endif /* CONFIG_TIMERLAT_TRACER */
-
-/*
- * osn_var_reset - Reset the values of the given osnoise_variables
- */
-static inline void osn_var_reset(void)
+static inline void tlat_reset_cpu(int cpu)
 {
-	struct osnoise_variables *osn_var;
-	int cpu;
-
-	/*
-	 * So far, all the values are initialized as 0, so
-	 * zeroing the structure is perfect.
-	 */
-	for_each_online_cpu(cpu) {
-		osn_var = cpu_osn_var(cpu);
-
-		WARN_ON_ONCE(osn_var->kthread);
-		WARN_ON_ONCE(osn_var->fdd);
-		memset(&osn_var->zero, 0, sizeof(osn_var->zero));
-	}
 }
 
-/*
- * osn_var_reset_all - Reset the value of all per-cpu osnoise_variables
- */
-static inline void osn_var_reset_all(void)
-{
-	osn_var_reset();
-	tlat_var_reset();
-}
+#define tlat_var_reset()	do {} while (0)
+#endif /* CONFIG_TIMERLAT_TRACER */
 
 /*
  * Tells NMIs to call back to the osnoise tracer to record timestamps.
@@ -408,6 +380,39 @@ static struct osnoise_data {
 #endif
 };
 
+static inline void osn_reset_cpu(int cpu)
+{
+	struct osnoise_variables *osn = cpu_osn_var(cpu);
+
+	WARN_ON_ONCE(osn->kthread);
+	WARN_ON_ONCE(osn->fdd);
+
+	osn->sampling = false;
+	osn->pid = 0;
+	tlat_reset_cpu(cpu);
+}
+
+/*
+ * osn_var_reset - Reset non-per-cpu state, and check for stale data
+ */
+static inline void osn_var_reset(void)
+{
+	struct osnoise_variables *osn;
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		osn = cpu_osn_var(cpu);
+
+		WARN_ON_ONCE(osn->kthread);
+		WARN_ON_ONCE(osn->fdd);
+		WARN_ON_ONCE(osn->sampling);
+		memset(&osn->zero, 0, sizeof(osn->zero));
+	}
+
+	tlat_var_reset();
+	osnoise_data.tainted = false;
+}
+
 #ifdef CONFIG_TIMERLAT_TRACER
 static inline bool timerlat_enabled(void)
 {
@@ -2024,7 +2029,7 @@ static void stop_kthread(unsigned int cpu)
 		osn_var->kthread = NULL;
 	}
 
-	osn_var->sampling = false;
+	osn_reset_cpu(cpu);
 	mutex_unlock(&osn_var->lock);
 }
 
@@ -2452,23 +2457,19 @@ static void timerlat_fd_detach(struct fd_data *fdd)
 	if (WARN_ON_ONCE(fdd != osn->fdd))
 		return;
 
-	WARN_ON_ONCE(osn->kthread);
 	WARN_ON_ONCE(fdd->thread != tlat->thread);
 	tlat->detach = true;
 	wake_up_process(tlat->thread);
 
 	mutex_lock(&tlat->detach_lock);
 
-	osn->sampling = 0;
-	osn->pid = 0;
-
 	WARN_ON_ONCE(hrtimer_active(&tlat->timer));
 	hrtimer_cancel(&tlat->timer);
 	put_task_struct(fdd->thread);
 
 	fdd->thread = NULL;
+	tlat->thread = NULL;
 	osn->fdd = NULL;
-	memset(&tlat->zero, 0, sizeof(tlat->zero));
 
 	mutex_unlock(&tlat->detach_lock);
 }
@@ -2531,13 +2532,17 @@ static int timerlat_fd_open(struct inode *inode, struct file *file)
 	fdd->cpu = cpu;
 	file->private_data = fdd;
 
-	tlat->tracing_thread = false;
-	tlat->thread = current;
 	get_task_struct(current);
 
 	osn_var->pid = current->pid;
 	osn_var->fdd = fdd;
 
+	WARN_ON_ONCE(tlat->thread);
+	WARN_ON_ONCE(tlat->count != 0);
+	WARN_ON_ONCE(tlat->tracing_thread);
+	WARN_ON_ONCE(osn_var->sampling);
+	tlat->thread = current;
+
 	hrtimer_setup(&tlat->timer, timerlat_irq, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_PINNED_HARD);
 
 	/*
@@ -2682,13 +2687,13 @@ timerlat_fd_read(struct file *file, char __user *ubuf, size_t count,
 static int timerlat_fd_release(struct inode *inode, struct file *file)
 {
 	struct fd_data *fdd = file->private_data;
-	struct osnoise_variables *osn_var;
-
-	osn_var = cpu_osn_var(fdd->cpu);
+	struct osnoise_variables *osn_var = cpu_osn_var(fdd->cpu);
 
 	mutex_lock(&osn_var->lock);
-	if (fdd == osn_var->fdd)
+	if (fdd == osn_var->fdd) {
 		timerlat_fd_detach(fdd);
+		osn_reset_cpu(fdd->cpu);
+	}
 	mutex_unlock(&osn_var->lock);
 
 	kfree(fdd);
@@ -3002,7 +3007,7 @@ static int osnoise_workload_start(void)
 	if (osnoise_has_registered_instances())
 		return 0;
 
-	osn_var_reset_all();
+	osn_var_reset();
 
 	retval = osnoise_hook_events();
 	if (retval)
-- 
2.56.0