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(). The interrupt counters do not really need to be reset, and resetting them on CPU detachment is racy because the hooks are still in place, so move them out of the zero group. 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. 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 --- kernel/trace/trace_osnoise.c | 121 +++++++++++++++++------------------ 1 file changed, 60 insertions(+), 61 deletions(-) diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c index ce6d02346ea4..f07b62a5ccb8 100644 --- a/kernel/trace/trace_osnoise.c +++ b/kernel/trace/trace_osnoise.c @@ -236,15 +236,15 @@ struct osnoise_variables { struct mutex lock; /* covers kthread and fdd changes */ struct task_struct *kthread; struct fd_data *fdd; + struct osn_nmi nmi; + struct osn_irq irq; + struct osn_softirq softirq; + struct osn_thread thread; + local_t int_counter; struct_group(zero, bool sampling; pid_t pid; - struct osn_nmi nmi; - struct osn_irq irq; - struct osn_softirq softirq; - struct osn_thread thread; - local_t int_counter; ); }; @@ -317,23 +317,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. @@ -341,38 +337,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. @@ -407,6 +377,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) { @@ -2023,7 +2023,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); } @@ -2451,25 +2451,22 @@ 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); - 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); + } static int timerlat_fd_open(struct inode *inode, struct file *file) @@ -2530,8 +2527,6 @@ 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; @@ -2544,7 +2539,11 @@ static int timerlat_fd_open(struct inode *inode, struct file *file) mutex_unlock(&interface_lock); 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); @@ -2698,13 +2697,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); @@ -3018,7 +3017,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
