2009-07-09 08:20:28

by Xiao Guangrong

Subject: [PATCH 1/2] tracing/filter: remove preds from struct event_subsystem

No need to save preds to event_subsystem, because it's not used.

Signed-off-by: Xiao Guangrong <[email protected]>
---
kernel/trace/trace_events_filter.c | 39 +++++------------------------------
1 files changed, 6 insertions(+), 33 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 936c621..b9aae72 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -420,17 +420,7 @@ EXPORT_SYMBOL_GPL(init_preds);

static void filter_free_subsystem_preds(struct event_subsystem *system)
{
- struct event_filter *filter = system->filter;
struct ftrace_event_call *call;
- int i;
-
- if (filter->n_preds) {
- for (i = 0; i < filter->n_preds; i++)
- filter_free_pred(filter->preds[i]);
- kfree(filter->preds);
- filter->preds = NULL;
- filter->n_preds = 0;
- }

list_for_each_entry(call, &ftrace_events, list) {
if (!call->define_fields)
@@ -607,26 +597,9 @@ static int filter_add_subsystem_pred(struct filter_parse_state *ps,
struct filter_pred *pred,
char *filter_string)
{
- struct event_filter *filter = system->filter;
struct ftrace_event_call *call;
int err = 0;

- if (!filter->preds) {
- filter->preds = kzalloc(MAX_FILTER_PRED * sizeof(pred),
- GFP_KERNEL);
-
- if (!filter->preds)
- return -ENOMEM;
- }
-
- if (filter->n_preds == MAX_FILTER_PRED) {
- parse_error(ps, FILT_ERR_TOO_MANY_PREDS, 0);
- return -ENOSPC;
- }
-
- filter->preds[filter->n_preds] = pred;
- filter->n_preds++;
-
list_for_each_entry(call, &ftrace_events, list) {

if (!call->define_fields)
@@ -1029,12 +1002,12 @@ static int replace_preds(struct event_subsystem *system,

if (elt->op == OP_AND || elt->op == OP_OR) {
pred = create_logical_pred(elt->op);
- if (call) {
+ if (call)
err = filter_add_pred(ps, call, pred);
- filter_free_pred(pred);
- } else
+ else
err = filter_add_subsystem_pred(ps, system,
pred, filter_string);
+ filter_free_pred(pred);
if (err)
return err;

@@ -1048,12 +1021,12 @@ static int replace_preds(struct event_subsystem *system,
}

pred = create_pred(elt->op, operand1, operand2);
- if (call) {
+ if (call)
err = filter_add_pred(ps, call, pred);
- filter_free_pred(pred);
- } else
+ else
err = filter_add_subsystem_pred(ps, system, pred,
filter_string);
+ filter_free_pred(pred);
if (err)
return err;

--
1.6.1.2

2009-07-09 08:22:30

by Xiao Guangrong

Subject: [PATCH 2/2] tracing/filter: remove empty subsystem and it's directory

Remove empty subsystem and it's directory when module unload.

Before patch:
# rmmod trace-events-sample.ko
# ls sample
enable filter

After patch:
# rmmod trace-events-sample.ko
# ls sample
ls: cannot access sample: No such file or directory

Signed-off-by: Xiao Guangrong <[email protected]>
---
kernel/trace/trace.h | 1 +
kernel/trace/trace_events.c | 32 +++++++++++++++++++++++++++++++-
2 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 61b4e94..0e7de4b 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -780,6 +780,7 @@ struct event_subsystem {
const char *name;
struct dentry *entry;
void *filter;
+ int nr_events;
};

struct filter_pred;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index fecac13..90cf936 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -851,8 +851,10 @@ event_subsystem_dir(const char *name, struct dentry *d_events)

/* First see if we did not already create this dir */
list_for_each_entry(system, &event_subsystems, list) {
- if (strcmp(system->name, name) == 0)
+ if (strcmp(system->name, name) == 0) {
+ system->nr_events++;
return system->entry;
+ }
}

/* need to create new entry */
@@ -871,6 +873,7 @@ event_subsystem_dir(const char *name, struct dentry *d_events)
return d_events;
}

+ system->nr_events = 1;
system->name = kstrdup(name, GFP_KERNEL);
if (!system->name) {
debugfs_remove(system->entry);
@@ -905,6 +908,32 @@ event_subsystem_dir(const char *name, struct dentry *d_events)
return system->entry;
}

+static void remove_subsystem_dir(const char *name)
+{
+ struct event_subsystem *system;
+
+ if (strcmp(name, TRACE_SYSTEM) == 0)
+ return;
+
+ list_for_each_entry(system, &event_subsystems, list) {
+ if (strcmp(system->name, name) == 0) {
+ if (!--system->nr_events) {
+ struct event_filter *filter = system->filter;
+
+ debugfs_remove_recursive(system->entry);
+ list_del(&system->list);
+ if (filter) {
+ kfree(filter->filter_string);
+ kfree(filter);
+ }
+ kfree(system->name);
+ kfree(system);
+ }
+ break;
+ }
+ }
+}
+
static int
event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
const struct file_operations *id,
@@ -1079,6 +1108,7 @@ static void trace_module_remove_events(struct module *mod)
list_del(&call->list);
trace_destroy_fields(call);
destroy_preds(call);
+ remove_subsystem_dir(call->system);
}
}

--
1.6.1.2

2009-07-10 04:04:47

by Tom Zanussi

Subject: Re: [PATCH 1/2] tracing/filter: remove preds from struct event_subsystem

On Thu, 2009-07-09 at 16:20 +0800, Xiao Guangrong wrote:
> No need to save preds to event_subsystem, because it's not used.
>
> Signed-off-by: Xiao Guangrong <[email protected]>

Makes sense to me.

Acked-by: Tom Zanussi <[email protected]>

> ---
> kernel/trace/trace_events_filter.c | 39 +++++------------------------------
> 1 files changed, 6 insertions(+), 33 deletions(-)
>
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 936c621..b9aae72 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -420,17 +420,7 @@ EXPORT_SYMBOL_GPL(init_preds);
>
> static void filter_free_subsystem_preds(struct event_subsystem *system)
> {
> - struct event_filter *filter = system->filter;
> struct ftrace_event_call *call;
> - int i;
> -
> - if (filter->n_preds) {
> - for (i = 0; i < filter->n_preds; i++)
> - filter_free_pred(filter->preds[i]);
> - kfree(filter->preds);
> - filter->preds = NULL;
> - filter->n_preds = 0;
> - }
>
> list_for_each_entry(call, &ftrace_events, list) {
> if (!call->define_fields)
> @@ -607,26 +597,9 @@ static int filter_add_subsystem_pred(struct filter_parse_state *ps,
> struct filter_pred *pred,
> char *filter_string)
> {
> - struct event_filter *filter = system->filter;
> struct ftrace_event_call *call;
> int err = 0;
>
> - if (!filter->preds) {
> - filter->preds = kzalloc(MAX_FILTER_PRED * sizeof(pred),
> - GFP_KERNEL);
> -
> - if (!filter->preds)
> - return -ENOMEM;
> - }
> -
> - if (filter->n_preds == MAX_FILTER_PRED) {
> - parse_error(ps, FILT_ERR_TOO_MANY_PREDS, 0);
> - return -ENOSPC;
> - }
> -
> - filter->preds[filter->n_preds] = pred;
> - filter->n_preds++;
> -
> list_for_each_entry(call, &ftrace_events, list) {
>
> if (!call->define_fields)
> @@ -1029,12 +1002,12 @@ static int replace_preds(struct event_subsystem *system,
>
> if (elt->op == OP_AND || elt->op == OP_OR) {
> pred = create_logical_pred(elt->op);
> - if (call) {
> + if (call)
> err = filter_add_pred(ps, call, pred);
> - filter_free_pred(pred);
> - } else
> + else
> err = filter_add_subsystem_pred(ps, system,
> pred, filter_string);
> + filter_free_pred(pred);
> if (err)
> return err;
>
> @@ -1048,12 +1021,12 @@ static int replace_preds(struct event_subsystem *system,
> }
>
> pred = create_pred(elt->op, operand1, operand2);
> - if (call) {
> + if (call)
> err = filter_add_pred(ps, call, pred);
> - filter_free_pred(pred);
> - } else
> + else
> err = filter_add_subsystem_pred(ps, system, pred,
> filter_string);
> + filter_free_pred(pred);
> if (err)
> return err;
>

2009-07-10 04:07:09

by Tom Zanussi

Subject: Re: [PATCH 2/2] tracing/filter: remove empty subsystem and it's directory

On Thu, 2009-07-09 at 16:22 +0800, Xiao Guangrong wrote:
> Remove empty subsystem and it's directory when module unload.
>
> Before patch:
> # rmmod trace-events-sample.ko
> # ls sample
> enable filter
>
> After patch:
> # rmmod trace-events-sample.ko
> # ls sample
> ls: cannot access sample: No such file or directory
>
> Signed-off-by: Xiao Guangrong <[email protected]>

Looks like it does the trick.

Acked-by: Tom Zanussi <[email protected]>

> ---
> kernel/trace/trace.h | 1 +
> kernel/trace/trace_events.c | 32 +++++++++++++++++++++++++++++++-
> 2 files changed, 32 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 61b4e94..0e7de4b 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -780,6 +780,7 @@ struct event_subsystem {
> const char *name;
> struct dentry *entry;
> void *filter;
> + int nr_events;
> };
>
> struct filter_pred;
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index fecac13..90cf936 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -851,8 +851,10 @@ event_subsystem_dir(const char *name, struct dentry *d_events)
>
> /* First see if we did not already create this dir */
> list_for_each_entry(system, &event_subsystems, list) {
> - if (strcmp(system->name, name) == 0)
> + if (strcmp(system->name, name) == 0) {
> + system->nr_events++;
> return system->entry;
> + }
> }
>
> /* need to create new entry */
> @@ -871,6 +873,7 @@ event_subsystem_dir(const char *name, struct dentry *d_events)
> return d_events;
> }
>
> + system->nr_events = 1;
> system->name = kstrdup(name, GFP_KERNEL);
> if (!system->name) {
> debugfs_remove(system->entry);
> @@ -905,6 +908,32 @@ event_subsystem_dir(const char *name, struct dentry *d_events)
> return system->entry;
> }
>
> +static void remove_subsystem_dir(const char *name)
> +{
> + struct event_subsystem *system;
> +
> + if (strcmp(name, TRACE_SYSTEM) == 0)
> + return;
> +
> + list_for_each_entry(system, &event_subsystems, list) {
> + if (strcmp(system->name, name) == 0) {
> + if (!--system->nr_events) {
> + struct event_filter *filter = system->filter;
> +
> + debugfs_remove_recursive(system->entry);
> + list_del(&system->list);
> + if (filter) {
> + kfree(filter->filter_string);
> + kfree(filter);
> + }
> + kfree(system->name);
> + kfree(system);
> + }
> + break;
> + }
> + }
> +}
> +
> static int
> event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
> const struct file_operations *id,
> @@ -1079,6 +1108,7 @@ static void trace_module_remove_events(struct module *mod)
> list_del(&call->list);
> trace_destroy_fields(call);
> destroy_preds(call);
> + remove_subsystem_dir(call->system);
> }
> }
>

2009-07-10 05:23:46

by Li Zefan

Subject: Re: [PATCH 1/2] tracing/filter: remove preds from struct event_subsystem

Xiao Guangrong wrote:
> No need to save preds to event_subsystem, because it's not used.
>
> Signed-off-by: Xiao Guangrong <[email protected]>

Nice cleanup.

Reviewed-by: Li Zefan <[email protected]>

> ---
> kernel/trace/trace_events_filter.c | 39 +++++------------------------------
> 1 files changed, 6 insertions(+), 33 deletions(-)

2009-07-10 05:28:23

by Li Zefan

Subject: Re: [PATCH 2/2] tracing/filter: remove empty subsystem and it's directory

Tom Zanussi wrote:
> On Thu, 2009-07-09 at 16:22 +0800, Xiao Guangrong wrote:
>> Remove empty subsystem and it's directory when module unload.
>>
>> Before patch:
>> # rmmod trace-events-sample.ko
>> # ls sample
>> enable filter
>>
>> After patch:
>> # rmmod trace-events-sample.ko
>> # ls sample
>> ls: cannot access sample: No such file or directory
>>
>> Signed-off-by: Xiao Guangrong <[email protected]>
>
> Looks like it does the trick.
>
> Acked-by: Tom Zanussi <[email protected]>
>

It sure does. :)

Reviewed-by: Li Zefan <[email protected]>

>> ---
>> kernel/trace/trace.h | 1 +
>> kernel/trace/trace_events.c | 32 +++++++++++++++++++++++++++++++-
>> 2 files changed, 32 insertions(+), 1 deletions(-)

2009-07-10 10:42:52

by Xiao Guangrong

Subject: [tip:tracing/core] tracing/filter: Remove preds from struct event_subsystem

Commit-ID: c5cb183608167c744cb28bbd85884be5a4ce875d
Gitweb: http://git.kernel.org/tip/c5cb183608167c744cb28bbd85884be5a4ce875d
Author: Xiao Guangrong <[email protected]>
AuthorDate: Thu, 9 Jul 2009 16:20:12 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 10 Jul 2009 11:55:27 +0200

tracing/filter: Remove preds from struct event_subsystem

No need to save preds to event_subsystem, because it's not used.

Signed-off-by: Xiao Guangrong <[email protected]>
Acked-by: Tom Zanussi <[email protected]>
Reviewed-by: Li Zefan <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>


---
kernel/trace/trace_events_filter.c | 39 +++++------------------------------
1 files changed, 6 insertions(+), 33 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 936c621..b9aae72 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -420,17 +420,7 @@ EXPORT_SYMBOL_GPL(init_preds);

static void filter_free_subsystem_preds(struct event_subsystem *system)
{
- struct event_filter *filter = system->filter;
struct ftrace_event_call *call;
- int i;
-
- if (filter->n_preds) {
- for (i = 0; i < filter->n_preds; i++)
- filter_free_pred(filter->preds[i]);
- kfree(filter->preds);
- filter->preds = NULL;
- filter->n_preds = 0;
- }

list_for_each_entry(call, &ftrace_events, list) {
if (!call->define_fields)
@@ -607,26 +597,9 @@ static int filter_add_subsystem_pred(struct filter_parse_state *ps,
struct filter_pred *pred,
char *filter_string)
{
- struct event_filter *filter = system->filter;
struct ftrace_event_call *call;
int err = 0;

- if (!filter->preds) {
- filter->preds = kzalloc(MAX_FILTER_PRED * sizeof(pred),
- GFP_KERNEL);
-
- if (!filter->preds)
- return -ENOMEM;
- }
-
- if (filter->n_preds == MAX_FILTER_PRED) {
- parse_error(ps, FILT_ERR_TOO_MANY_PREDS, 0);
- return -ENOSPC;
- }
-
- filter->preds[filter->n_preds] = pred;
- filter->n_preds++;
-
list_for_each_entry(call, &ftrace_events, list) {

if (!call->define_fields)
@@ -1029,12 +1002,12 @@ static int replace_preds(struct event_subsystem *system,

if (elt->op == OP_AND || elt->op == OP_OR) {
pred = create_logical_pred(elt->op);
- if (call) {
+ if (call)
err = filter_add_pred(ps, call, pred);
- filter_free_pred(pred);
- } else
+ else
err = filter_add_subsystem_pred(ps, system,
pred, filter_string);
+ filter_free_pred(pred);
if (err)
return err;

@@ -1048,12 +1021,12 @@ static int replace_preds(struct event_subsystem *system,
}

pred = create_pred(elt->op, operand1, operand2);
- if (call) {
+ if (call)
err = filter_add_pred(ps, call, pred);
- filter_free_pred(pred);
- } else
+ else
err = filter_add_subsystem_pred(ps, system, pred,
filter_string);
+ filter_free_pred(pred);
if (err)
return err;

2009-07-10 10:43:08

by Xiao Guangrong

Subject: [tip:tracing/core] tracing/filter: Remove empty subsystem and its directory

Commit-ID: dc82ec98a4727fd51b77e92d05fe7d2db3dcc11c
Gitweb: http://git.kernel.org/tip/dc82ec98a4727fd51b77e92d05fe7d2db3dcc11c
Author: Xiao Guangrong <[email protected]>
AuthorDate: Thu, 9 Jul 2009 16:22:22 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 10 Jul 2009 11:55:28 +0200

tracing/filter: Remove empty subsystem and its directory

Remove empty subsystem and its directory when module unload.

Before patch:
# rmmod trace-events-sample.ko
# ls sample
enable filter

After patch:
# rmmod trace-events-sample.ko
# ls sample
ls: cannot access sample: No such file or directory

Signed-off-by: Xiao Guangrong <[email protected]>
Acked-by: Tom Zanussi <[email protected]>
Reviewed-by: Li Zefan <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>


---
kernel/trace/trace.h | 1 +
kernel/trace/trace_events.c | 32 +++++++++++++++++++++++++++++++-
2 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 52eb0d8..94305c7 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -757,6 +757,7 @@ struct event_subsystem {
const char *name;
struct dentry *entry;
void *filter;
+ int nr_events;
};

struct filter_pred;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index fecac13..90cf936 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -851,8 +851,10 @@ event_subsystem_dir(const char *name, struct dentry *d_events)

/* First see if we did not already create this dir */
list_for_each_entry(system, &event_subsystems, list) {
- if (strcmp(system->name, name) == 0)
+ if (strcmp(system->name, name) == 0) {
+ system->nr_events++;
return system->entry;
+ }
}

/* need to create new entry */
@@ -871,6 +873,7 @@ event_subsystem_dir(const char *name, struct dentry *d_events)
return d_events;
}

+ system->nr_events = 1;
system->name = kstrdup(name, GFP_KERNEL);
if (!system->name) {
debugfs_remove(system->entry);
@@ -905,6 +908,32 @@ event_subsystem_dir(const char *name, struct dentry *d_events)
return system->entry;
}

+static void remove_subsystem_dir(const char *name)
+{
+ struct event_subsystem *system;
+
+ if (strcmp(name, TRACE_SYSTEM) == 0)
+ return;
+
+ list_for_each_entry(system, &event_subsystems, list) {
+ if (strcmp(system->name, name) == 0) {
+ if (!--system->nr_events) {
+ struct event_filter *filter = system->filter;
+
+ debugfs_remove_recursive(system->entry);
+ list_del(&system->list);
+ if (filter) {
+ kfree(filter->filter_string);
+ kfree(filter);
+ }
+ kfree(system->name);
+ kfree(system);
+ }
+ break;
+ }
+ }
+}
+
static int
event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
const struct file_operations *id,
@@ -1079,6 +1108,7 @@ static void trace_module_remove_events(struct module *mod)
list_del(&call->list);
trace_destroy_fields(call);
destroy_preds(call);
+ remove_subsystem_dir(call->system);
}
}

2009-07-14 17:00:43

by Steven Rostedt

Subject: Re: [tip:tracing/core] tracing/filter: Remove empty subsystem and its directory

On Fri, 10 Jul 2009, tip-bot for Xiao Guangrong wrote:

> Commit-ID: dc82ec98a4727fd51b77e92d05fe7d2db3dcc11c
> Gitweb: http://git.kernel.org/tip/dc82ec98a4727fd51b77e92d05fe7d2db3dcc11c
> Author: Xiao Guangrong <[email protected]>
> AuthorDate: Thu, 9 Jul 2009 16:22:22 +0800
> Committer: Ingo Molnar <[email protected]>
> CommitDate: Fri, 10 Jul 2009 11:55:28 +0200
>
> tracing/filter: Remove empty subsystem and its directory
>
> Remove empty subsystem and its directory when module unload.
>
> Before patch:
> # rmmod trace-events-sample.ko
> # ls sample
> enable filter
>
> After patch:
> # rmmod trace-events-sample.ko
> # ls sample
> ls: cannot access sample: No such file or directory
>
> Signed-off-by: Xiao Guangrong <[email protected]>
> Acked-by: Tom Zanussi <[email protected]>
> Reviewed-by: Li Zefan <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> LKML-Reference: <[email protected]>
> Signed-off-by: Ingo Molnar <[email protected]>

Thanks Xiao!

I've just came back from vacation so I missed acking it. But it looks
good, and Ingo pulled it in.

-- Steve