Reset per-cpu 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(). Signed-off-by: Crystal Wood <[email protected]> --- 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. --- kernel/trace/trace_osnoise.c | 104 +++++++++++++++++------------------ 1 file changed, 51 insertions(+), 53 deletions(-) diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c index eaecd56492e3..ccd717db9a51 100644 --- a/kernel/trace/trace_osnoise.c +++ b/kernel/trace/trace_osnoise.c @@ -319,23 +319,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. @@ -343,38 +339,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(osn_var->kthread); - WARN_ON(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. @@ -409,6 +379,36 @@ static struct osnoise_data { #endif }; +static inline void osn_reset_cpu(int cpu) +{ + struct osnoise_variables *osn = cpu_osn_var(cpu); + + WARN_ON(osn->kthread); + WARN_ON(osn->fdd); + memset(&osn->zero, 0, sizeof(osn->zero)); + 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(osn->kthread); + WARN_ON(osn->fdd); + WARN_ON(osn->sampling); + } + + tlat_var_reset(); + osnoise_data.tainted = false; +} + #ifdef CONFIG_TIMERLAT_TRACER static inline bool timerlat_enabled(void) { @@ -2036,6 +2036,7 @@ static void stop_kthread(unsigned int cpu) kthread_stop(osn_var->kthread); put_task_struct(osn_var->kthread); osn_var->kthread = NULL; + osn_reset_cpu(cpu); } mutex_unlock(&osn_var->lock); @@ -2468,18 +2469,12 @@ static void timerlat_fd_detach(struct fd_data *fdd) if (WARN_ON(fdd != osn->fdd)) return; - WARN_ON(osn->kthread); WARN_ON(fdd->thread != tlat->thread); tlat->detach = true; wake_up_process(tlat->thread); mutex_lock(&tlat->detach_lock); - memset(tlat, 0, sizeof(*tlat)); - - osn->sampling = 0; - osn->pid = 0; - WARN_ON_ONCE(hrtimer_active(&tlat->timer)); hrtimer_cancel(&tlat->timer); put_task_struct(fdd->thread); @@ -2489,6 +2484,8 @@ static void timerlat_fd_detach(struct fd_data *fdd) osn->fdd = NULL; mutex_unlock(&tlat->detach_lock); + + osn_reset_cpu(fdd->cpu); } static int timerlat_fd_open(struct inode *inode, struct file *file) @@ -2545,7 +2542,11 @@ static int timerlat_fd_open(struct inode *inode, struct file *file) osn_var->fdd = fdd; tlat = this_cpu_tmr_var(); - tlat->count = 0; + WARN_ON(tlat->thread); + WARN_ON(tlat->count != 0); + WARN_ON(tlat->tracing_thread); + WARN_ON(osn_var->sampling); + tlat->thread = current; hrtimer_setup(&tlat->timer, timerlat_irq, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_PINNED_HARD); return 0; @@ -2637,9 +2638,6 @@ timerlat_fd_read(struct file *file, char __user *ubuf, size_t count, } } } else { - tlat->tracing_thread = false; - tlat->thread = current; - /* Annotate now to drift new period */ tlat->abs_period = hrtimer_cb_get_time(&tlat->timer); @@ -3014,7 +3012,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.54.0
