[PATCH -tip 4/5] tracing: return accurate value for print_graph_prologue
Wenji Huang
Signed-off-by: Wenji Huang <wenji...@oracle.com>
---
kernel/trace/trace_functions_graph.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 112561d..bd0bdeb 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -806,7 +806,7 @@ print_graph_prologue(struct trace_iterator *iter, struct trace_seq *s,
return TRACE_TYPE_PARTIAL_LINE;
}
- return 0;
+ return TRACE_TYPE_HANDLED;
}
static enum print_line_t
--
1.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Wenji Huang
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index a718cd1..505c922 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -635,12 +635,12 @@ static int create_trace_probe(int argc, char **argv)
event = strchr(group, '/') + 1;
event[-1] = '\0';
if (strlen(group) == 0) {
- pr_info("Group name is not specifiled\n");
+ pr_info("Group name is not specified\n");
return -EINVAL;
}
}
if (strlen(event) == 0) {
- pr_info("Event name is not specifiled\n");
+ pr_info("Event name is not specified\n");
return -EINVAL;
Wenji Huang
Signed-off-by: Wenji Huang <wenji...@oracle.com>
---
kernel/trace/trace_functions_graph.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 616b135..112561d 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -855,7 +855,6 @@ print_graph_return(struct ftrace_graph_ret *trace, struct trace_seq *s,
int i;
if (data) {
- int cpu = iter->cpu;
int *depth = &(per_cpu_ptr(data->cpu_data, cpu)->depth);
/*
Wenji Huang
Signed-off-by: Wenji Huang <wenji...@oracle.com>
---
kernel/trace/trace_events.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c2a3077..3f972ad 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -60,10 +60,8 @@ int trace_define_field(struct ftrace_event_call *call, const char *type,
return 0;
err:
- if (field) {
+ if (field)
kfree(field->name);
- kfree(field->type);
- }
kfree(field);
return -ENOMEM;
Li Zefan
> Discard freeing field->type since it's not necessary and may be hazard.
>
It's redundant, but it's safe, because if we run into this failure path,
field->type is always NULL.
> Signed-off-by: Wenji Huang <wenji...@oracle.com>
Reviewed-by: Li Zefan <li...@cn.fujitsu.com>
> ---
> kernel/trace/trace_events.c | 4 +---
> 1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index c2a3077..3f972ad 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -60,10 +60,8 @@ int trace_define_field(struct ftrace_event_call *call, const char *type,
> return 0;
>
> err:
> - if (field) {
> + if (field)
> kfree(field->name);
> - kfree(field->type);
> - }
> kfree(field);
>
> return -ENOMEM;
--
Li Zefan
> On 02/24/2010 04:04 PM, Li Zefan wrote:
>> Wenji Huang wrote:
>>> Discard freeing field->type since it's not necessary and may be hazard.
>>>
>>
>> It's redundant, but it's safe, because if we run into this failure path,
>> field->type is always NULL.
>
> field->type == NULL. And allocating for field->name is before field->type.
>
> IMHO, field->type is not fixed after initialization, it's
> not safe if field->name==NULL goes to failure path.
>
It's still safe, because field is allocated using kzalloc(). ;)
Wenji Huang
>> Discard freeing field->type since it's not necessary and may be hazard.
>>
>
> It's redundant, but it's safe, because if we run into this failure path,
> field->type is always NULL.
There are two entries to failure path, field->name == NULL or
field->type == NULL. And allocating for field->name is before field->type.
IMHO, field->type is not fixed after initialization, it's
not safe if field->name==NULL goes to failure path.
Regards,
Wenji
Frederic Weisbecker
> Return TRACE_TYPE_HANDLED instead of zero to avoid confusion.
>
> Signed-off-by: Wenji Huang <wenji...@oracle.com>
> ---
> kernel/trace/trace_functions_graph.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index 112561d..bd0bdeb 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -806,7 +806,7 @@ print_graph_prologue(struct trace_iterator *iter, struct trace_seq *s,
> return TRACE_TYPE_PARTIAL_LINE;
> }
>
> - return 0;
> + return TRACE_TYPE_HANDLED;
Actually TRACE_TYPE_HANDLED = 1
So print_graph_prologue always returns 0.
And the check is inverted everywhere:
if (print_graph_prologue(...))
return TRACE_TYPE_PARTIAL_LINE;
Which means we never it fails.
It's not a big deal because there will always be something
else to print after the prologue, and this will fail too
and then return the error.
But still, if you fix this, you also need to do:
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 616b135..9da6487 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -819,7 +819,7 @@ print_graph_entry(struct ftrace_graph_ent_entry *field, struct trace_seq *s,
static enum print_line_t ret;
int cpu = iter->cpu;
- if (print_graph_prologue(iter, s, TRACE_GRAPH_ENT, call->func))
+ if (!print_graph_prologue(iter, s, TRACE_GRAPH_ENT, call->func))
return TRACE_TYPE_PARTIAL_LINE;
leaf_ret = get_return_for_leaf(iter, field);
@@ -866,7 +866,7 @@ print_graph_return(struct ftrace_graph_ret *trace, struct trace_seq *s,
*depth = trace->depth - 1;
}
- if (print_graph_prologue(iter, s, 0, 0))
+ if (!print_graph_prologue(iter, s, 0, 0))
return TRACE_TYPE_PARTIAL_LINE;
/* Overhead */
@@ -921,7 +921,7 @@ print_graph_comment(struct trace_seq *s, struct trace_entry *ent,
if (data)
depth = per_cpu_ptr(data->cpu_data, iter->cpu)->depth;
- if (print_graph_prologue(iter, s, 0, 0))
+ if (!print_graph_prologue(iter, s, 0, 0))
return TRACE_TYPE_PARTIAL_LINE;
/* No overhead */
Steven Rostedt
> On Wed, Feb 24, 2010 at 03:40:25PM +0800, Wenji Huang wrote:
> > Return TRACE_TYPE_HANDLED instead of zero to avoid confusion.
> >
> > Signed-off-by: Wenji Huang <wenji...@oracle.com>
> > ---
> It's not a big deal because there will always be something
> else to print after the prologue, and this will fail too
> and then return the error.
I'll apply the rest of the series and leave this patch out.
-- Steve
tip-bot for Wenji Huang
Gitweb: http://git.kernel.org/tip/a5efd925115cbc1f90195dca9a25f7b8daa10c37
Author: Wenji Huang <wenji...@oracle.com>
AuthorDate: Wed, 24 Feb 2010 15:40:23 +0800
Committer: Steven Rostedt <ros...@goodmis.org>
CommitDate: Thu, 25 Feb 2010 10:36:29 -0500
tracing: Fix typo of info text in trace_kprobe.c
Signed-off-by: Wenji Huang <wenji...@oracle.com>
LKML-Reference: <1266997226-6833-2-git-...@oracle.com>
Signed-off-by: Steven Rostedt <ros...@goodmis.org>
---
kernel/trace/trace_kprobe.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index c990299..8d4bd16 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -651,12 +651,12 @@ static int create_trace_probe(int argc, char **argv)
event = strchr(group, '/') + 1;
event[-1] = '\0';
if (strlen(group) == 0) {
- pr_info("Group name is not specifiled\n");
+ pr_info("Group name is not specified\n");
return -EINVAL;
}
}
if (strlen(event) == 0) {
- pr_info("Event name is not specifiled\n");
+ pr_info("Event name is not specified\n");
return -EINVAL;
}
}
--
tip-bot for Wenji Huang
Gitweb: http://git.kernel.org/tip/c85f3a91f84d5a85f179c2504bb7a39370c82b41
Author: Wenji Huang <wenji...@oracle.com>
AuthorDate: Wed, 24 Feb 2010 15:40:24 +0800
Committer: Steven Rostedt <ros...@goodmis.org>
CommitDate: Thu, 25 Feb 2010 10:41:24 -0500
tracing: Remove unnecessary variable in print_graph_return
The "cpu" variable is declared at the start of the function and
also within a branch, with the exact same initialization.
Remove the local variable of the same name in the branch.
Signed-off-by: Wenji Huang <wenji...@oracle.com>
LKML-Reference: <1266997226-6833-3-git-...@oracle.com>
Signed-off-by: Steven Rostedt <ros...@goodmis.org>
---
kernel/trace/trace_functions_graph.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 616b135..112561d 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -855,7 +855,6 @@ print_graph_return(struct ftrace_graph_ret *trace, struct trace_seq *s,
int i;
if (data) {
- int cpu = iter->cpu;
int *depth = &(per_cpu_ptr(data->cpu_data, cpu)->depth);
/*
--
tip-bot for Wenji Huang
Gitweb: http://git.kernel.org/tip/7b60997f73865b019e595720185c85285ca3df9a
Author: Wenji Huang <wenji...@oracle.com>
AuthorDate: Wed, 24 Feb 2010 15:40:26 +0800
Committer: Steven Rostedt <ros...@goodmis.org>
CommitDate: Thu, 25 Feb 2010 10:42:55 -0500
tracing: Simplify memory recycle of trace_define_field
Discard freeing field->type since it is not necessary.
Reviewed-by: Li Zefan <li...@cn.fujitsu.com>
Signed-off-by: Wenji Huang <wenji...@oracle.com>
LKML-Reference: <1266997226-6833-5-git-...@oracle.com>
Signed-off-by: Steven Rostedt <ros...@goodmis.org>
---
kernel/trace/trace_events.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c2a3077..3f972ad 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -60,10 +60,8 @@ int trace_define_field(struct ftrace_event_call *call, const char *type,
return 0;
err:
- if (field) {
+ if (field)
kfree(field->name);
- kfree(field->type);
- }
kfree(field);
return -ENOMEM;
--