Messages in this thread | | | Date | Fri, 8 Sep 2017 11:08:38 -0400 (EDT) | From | "Ziqian SUN (Zamir)" <> | Subject | Re: [PATCH v2] tracing: Ignore mmiotrace from kernel commandline |
| |
> On Wed, 30 Aug 2017 12:19:30 +0800 > "Ziqian SUN (Zamir)" <zsun@redhat.com> wrote: > > > From: "Ziqian SUN (Zamir)" <zsun@redhat.com> > > > > The mmiotrace tracer cannot be enabled with ftrace=mmiotrace in > > kernel commandline. With this patch, a trace_noboot_tracer_list > > is implemented and will be checked during system boot. If the > > tracer declares itself as not for boot up, system will print out > > a message and continue booting. > > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=196557 > > Signed-off-by: Ziqian SUN (Zamir) <zsun@redhat.com> > > --- > > kernel/trace/trace.c | 34 +++++++++++++++++++++++++++++++++- > > kernel/trace/trace.h | 11 +++++++++++ > > kernel/trace/trace_mmiotrace.c | 4 ++++ > > 3 files changed, 48 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > > index 44004d8..0d25b1e 100644 > > --- a/kernel/trace/trace.c > > +++ b/kernel/trace/trace.c > > @@ -319,6 +319,33 @@ void trace_free_pid_list(struct trace_pid_list > > *pid_list) > > kfree(pid_list); > > } > > > > +struct mutex trace_noboot_tracer_lock; > > + > > +DEFINE_MUTEX(trace_noboot_tracer_lock); > > +LIST_HEAD(trace_noboot_tracer_list); > > + > > +void trace_noboot_tracer_put(struct tracer_noboot *noboot_tracer) > > +{ > > + mutex_lock(&trace_noboot_tracer_lock); > > + list_add(&noboot_tracer->list, &trace_noboot_tracer_list); > > + mutex_unlock(&trace_noboot_tracer_lock); > > +} > > + > > +bool trace_noboot_tracer_find(const char *str) > > +{ > > + mutex_lock(&trace_noboot_tracer_lock); > > + struct tracer_noboot *ptr = NULL; > > + > > + list_for_each_entry(ptr, &trace_noboot_tracer_list, list) { > > + if (strcmp(ptr->name, str) == 0) { > > + mutex_unlock(&trace_noboot_tracer_lock); > > + return true; > > + } > > + } > > + mutex_unlock(&trace_noboot_tracer_lock); > > + return false; > > +} > > + > > /** > > * trace_find_filtered_pid - check if a pid exists in a filtered_pid list > > * @filtered_pids: The list of pids to check > > @@ -1641,8 +1668,13 @@ int __init register_tracer(struct tracer *type) > > if (strncmp(default_bootup_tracer, type->name, MAX_TRACER_SIZE)) > > goto out_unlock; > > > > + /* If the tracer should not be enabled by kernel parameter */ > > + if (trace_noboot_tracer_find(type->name)) { > > + pr_warn("Tracer '%s' cannot be enabled during boot\n", type->name); > > + goto out_unlock; > > + } > > + > > printk(KERN_INFO "Starting tracer '%s'\n", type->name); > > - /* Do we want this tracer to start on bootup? */ > > tracing_set_tracer(&global_trace, type->name); > > default_bootup_tracer = NULL; > > > > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h > > index 490ba22..23bc668 100644 > > --- a/kernel/trace/trace.h > > +++ b/kernel/trace/trace.h > > @@ -502,6 +502,17 @@ enum { > > TRACE_IRQ_BIT, > > }; > > > > +/* > > + * For tracer that cannot be enabled during boot time. > > + */ > > + > > +struct tracer_noboot { > > + const char *name; > > + struct list_head list; > > +}; > > + > > +void trace_noboot_tracer_put(struct tracer_noboot *noboot_tracer); > > + > > #define trace_recursion_set(bit) do { (current)->trace_recursion |= > > (1<<(bit)); } while (0) > > #define trace_recursion_clear(bit) do { (current)->trace_recursion &= > > ~(1<<(bit)); } while (0) > > #define trace_recursion_test(bit) ((current)->trace_recursion & > > (1<<(bit))) > > diff --git a/kernel/trace/trace_mmiotrace.c > > b/kernel/trace/trace_mmiotrace.c > > index cd7480d..06aa0a5 100644 > > --- a/kernel/trace/trace_mmiotrace.c > > +++ b/kernel/trace/trace_mmiotrace.c > > @@ -286,6 +286,10 @@ static enum print_line_t mmio_print_line(struct > > trace_iterator *iter) > > > > __init static int init_mmio_trace(void) > > { > > + struct tracer_noboot mmio_notrace; > > + > > + mmio_notrace.name = "mmiotrace"; > > + trace_noboot_tracer_put(&mmio_notrace); > > return register_tracer(&mmio_tracer); > > } > > device_initcall(init_mmio_trace); > > This is way overkill. Just add > > bool noboot; > > to struct tracer in trace.h and then to the mmiotracer: > > struct tracer mmio_tracer = { > [..] > .noboot = true, > }; > > > And then in tracing_set_tracer() add after: > > if (t == tr->current_trace) > goto out; > > this: > > if (system_state < SYSTEM_RUNNING && t->noboot) { > WARN(1, "Tracer %s not allowed on command line", > t->name); > goto out; > } > > That's it. No need for locking or anything else. >
Thanks for the suggestions. I will send a patch v3 after testing.
> -- Steve > > >
-- Ziqian SUN
| |