From: Frederic Weisbecker <fweisbec@gmail.com>
To: Tom Zanussi <tzanussi@gmail.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>,
LKML <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@elte.hu>,
Steven Rostedt <rostedt@goodmis.org>,
paulmck@linux.vnet.ibm.com
Subject: Re: [PATCH] tracing/filters: add filter_mutex to protect filter predicates
Date: Thu, 16 Apr 2009 18:17:37 +0200 [thread overview]
Message-ID: <20090416161736.GH6004@nowhere> (raw)
In-Reply-To: <1239863904.6903.12.camel@tropicana>
On Thu, Apr 16, 2009 at 01:38:24AM -0500, Tom Zanussi wrote:
> This patch adds a filter_mutex to prevent the filter predicates from
> being accessed concurrently by various external functions.
>
> It's based on a previous patch by Li Zefan:
> "[PATCH 7/7] tracing/filters: make filter preds RCU safe"
>
> but any problems with it were added by me. ;-)
>
> Signed-off-by: Tom Zanussi <tzanussi@gmail.com>
>
> ---
> kernel/trace/trace.h | 4 +-
> kernel/trace/trace_events.c | 4 +-
> kernel/trace/trace_events_filter.c | 91 +++++++++++++++++++++++++++--------
> 3 files changed, 75 insertions(+), 24 deletions(-)
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index eb92307..8750e83 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -759,13 +759,15 @@ struct filter_pred {
> };
>
> extern void filter_free_pred(struct filter_pred *pred);
> -extern void filter_print_preds(struct filter_pred **preds, int n_preds,
> +extern void filter_print_preds(struct ftrace_event_call *call,
> struct trace_seq *s);
> extern int filter_parse(char **pbuf, struct filter_pred *pred);
> extern int filter_add_pred(struct ftrace_event_call *call,
> struct filter_pred *pred);
> extern void filter_disable_preds(struct ftrace_event_call *call);
> extern void filter_free_subsystem_preds(struct event_subsystem *system);
> +extern void filter_print_subsystem_preds(struct event_subsystem *system,
> + struct trace_seq *s);
> extern int filter_add_subsystem_pred(struct event_subsystem *system,
> struct filter_pred *pred);
>
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 6591d83..3ec7bf1 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -484,7 +484,7 @@ event_filter_read(struct file *filp, char __user *ubuf, size_t cnt,
>
> trace_seq_init(s);
>
> - filter_print_preds(call->preds, call->n_preds, s);
> + filter_print_preds(call, s);
> r = simple_read_from_buffer(ubuf, cnt, ppos, s->buffer, s->len);
>
> kfree(s);
> @@ -554,7 +554,7 @@ subsystem_filter_read(struct file *filp, char __user *ubuf, size_t cnt,
>
> trace_seq_init(s);
>
> - filter_print_preds(system->preds, system->n_preds, s);
> + filter_print_subsystem_preds(system, s);
> r = simple_read_from_buffer(ubuf, cnt, ppos, s->buffer, s->len);
>
> kfree(s);
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index f8e5eab..970201c 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -22,10 +22,13 @@
> #include <linux/uaccess.h>
> #include <linux/module.h>
> #include <linux/ctype.h>
> +#include <linux/mutex.h>
>
> #include "trace.h"
> #include "trace_output.h"
>
> +static DEFINE_MUTEX(filter_mutex);
> +
> static int filter_pred_64(struct filter_pred *pred, void *event)
> {
> u64 *addr = (u64 *)(event + pred->offset);
> @@ -112,8 +115,8 @@ int filter_match_preds(struct ftrace_event_call *call, void *rec)
> }
> EXPORT_SYMBOL_GPL(filter_match_preds);
>
> -void filter_print_preds(struct filter_pred **preds, int n_preds,
> - struct trace_seq *s)
> +static void __filter_print_preds(struct filter_pred **preds, int n_preds,
> + struct trace_seq *s)
> {
> char *field_name;
> struct filter_pred *pred;
> @@ -138,6 +141,21 @@ void filter_print_preds(struct filter_pred **preds, int n_preds,
> }
> }
>
> +void filter_print_preds(struct ftrace_event_call *call, struct trace_seq *s)
> +{
> + mutex_lock(&filter_mutex);
> + __filter_print_preds(call->preds, call->n_preds, s);
> + mutex_unlock(&filter_mutex);
> +}
> +
> +void filter_print_subsystem_preds(struct event_subsystem *system,
> + struct trace_seq *s)
> +{
> + mutex_lock(&filter_mutex);
> + __filter_print_preds(system->preds, system->n_preds, s);
> + mutex_unlock(&filter_mutex);
> +}
> +
> static struct ftrace_event_field *
> find_event_field(struct ftrace_event_call *call, char *name)
> {
> @@ -180,7 +198,7 @@ static int filter_set_pred(struct filter_pred *dest,
> return 0;
> }
>
> -void filter_disable_preds(struct ftrace_event_call *call)
> +static void __filter_disable_preds(struct ftrace_event_call *call)
> {
> int i;
>
> @@ -190,6 +208,13 @@ void filter_disable_preds(struct ftrace_event_call *call)
> call->preds[i]->fn = filter_pred_none;
> }
>
> +void filter_disable_preds(struct ftrace_event_call *call)
> +{
> + mutex_lock(&filter_mutex);
> + __filter_disable_preds(call);
> + mutex_unlock(&filter_mutex);
> +}
> +
> int init_preds(struct ftrace_event_call *call)
> {
> struct filter_pred *pred;
> @@ -223,7 +248,7 @@ oom:
> }
> EXPORT_SYMBOL_GPL(init_preds);
>
> -void filter_free_subsystem_preds(struct event_subsystem *system)
> +static void __filter_free_subsystem_preds(struct event_subsystem *system)
> {
> struct ftrace_event_call *call;
> int i;
> @@ -241,18 +266,25 @@ void filter_free_subsystem_preds(struct event_subsystem *system)
> continue;
>
> if (!strcmp(call->system, system->name))
> - filter_disable_preds(call);
> + __filter_disable_preds(call);
> }
> }
>
> -static int __filter_add_pred(struct ftrace_event_call *call,
> - struct filter_pred *pred,
> - filter_pred_fn_t fn)
> +void filter_free_subsystem_preds(struct event_subsystem *system)
> +{
> + mutex_lock(&filter_mutex);
> + __filter_free_subsystem_preds(system);
> + mutex_unlock(&filter_mutex);
> +}
> +
> +static int filter_add_pred_fn(struct ftrace_event_call *call,
> + struct filter_pred *pred,
> + filter_pred_fn_t fn)
> {
> int idx, err;
>
> if (call->n_preds && !pred->compound)
> - filter_disable_preds(call);
> + __filter_disable_preds(call);
>
> if (call->n_preds == MAX_FILTER_PRED)
> return -ENOSPC;
> @@ -276,7 +308,8 @@ static int is_string_field(const char *type)
> return 0;
> }
>
> -int filter_add_pred(struct ftrace_event_call *call, struct filter_pred *pred)
> +static int __filter_add_pred(struct ftrace_event_call *call,
> + struct filter_pred *pred)
> {
> struct ftrace_event_field *field;
> filter_pred_fn_t fn;
> @@ -293,7 +326,7 @@ int filter_add_pred(struct ftrace_event_call *call, struct filter_pred *pred)
> return -EINVAL;
> fn = filter_pred_string;
> pred->str_len = field->size;
> - return __filter_add_pred(call, pred, fn);
> + return filter_add_pred_fn(call, pred, fn);
> } else {
> if (pred->str_len)
> return -EINVAL;
> @@ -316,7 +349,18 @@ int filter_add_pred(struct ftrace_event_call *call, struct filter_pred *pred)
> return -EINVAL;
> }
>
> - return __filter_add_pred(call, pred, fn);
> + return filter_add_pred_fn(call, pred, fn);
> +}
> +
> +int filter_add_pred(struct ftrace_event_call *call, struct filter_pred *pred)
> +{
> + int err;
> +
> + mutex_lock(&filter_mutex);
> + err = __filter_add_pred(call, pred);
> + mutex_unlock(&filter_mutex);
> +
> + return err;
> }
>
> int filter_add_subsystem_pred(struct event_subsystem *system,
> @@ -324,20 +368,27 @@ int filter_add_subsystem_pred(struct event_subsystem *system,
> {
> struct ftrace_event_call *call;
>
> + mutex_lock(&filter_mutex);
> +
> if (system->n_preds && !pred->compound)
> - filter_free_subsystem_preds(system);
> + __filter_free_subsystem_preds(system);
>
> if (!system->n_preds) {
> system->preds = kzalloc(MAX_FILTER_PRED * sizeof(pred),
> GFP_KERNEL);
> - if (!system->preds)
> + if (!system->preds) {
> + mutex_unlock(&filter_mutex);
> return -ENOMEM;
> + }
> }
>
> - if (system->n_preds == MAX_FILTER_PRED)
> + if (system->n_preds == MAX_FILTER_PRED) {
> + mutex_unlock(&filter_mutex);
> return -ENOSPC;
> + }
>
> system->preds[system->n_preds] = pred;
> + system->n_preds++;
>
> list_for_each_entry(call, &ftrace_events, list) {
> int err;
> @@ -348,17 +399,15 @@ int filter_add_subsystem_pred(struct event_subsystem *system,
> if (strcmp(call->system, system->name))
> continue;
>
> - if (!find_event_field(call, pred->field_name))
> - continue;
> -
> - err = filter_add_pred(call, pred);
> + err = __filter_add_pred(call, pred);
> if (err == -ENOMEM) {
> system->preds[system->n_preds] = NULL;
> - return err;
> + system->n_preds--;
> + break;
> }
> }
>
> - system->n_preds++;
> + mutex_unlock(&filter_mutex);
>
> return 0;
> }
Looks good!
Thanks.
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
prev parent reply other threads:[~2009-04-16 16:18 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-04-16 6:38 [PATCH] tracing/filters: add filter_mutex to protect filter predicates Tom Zanussi
2009-04-16 8:33 ` Li Zefan
2009-04-17 0:31 ` Ingo Molnar
2009-04-16 16:17 ` Frederic Weisbecker [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090416161736.GH6004@nowhere \
--to=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lizf@cn.fujitsu.com \
--cc=mingo@elte.hu \
--cc=paulmck@linux.vnet.ibm.com \
--cc=rostedt@goodmis.org \
--cc=tzanussi@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox