All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] trace: don't print an extra separator of flags
@ 2012-02-19 11:16 Andrew Vagin
  2012-02-20 18:09 ` Steven Rostedt
  2012-02-27  9:31 ` [tip:perf/core] tracing: Don't " tip-bot for Andrey Vagin
  0 siblings, 2 replies; 7+ messages in thread
From: Andrew Vagin @ 2012-02-19 11:16 UTC (permalink / raw)
  To: linux-kernel; +Cc: avagin, Ingo Molnar, Frederic Weisbecker, Steven Rostedt

From: Andrey Vagin <avagin@openvz.org>

It occurs if __print_flags is used more than once

Signed-off-by: Andrew Vagin <avagin@openvz.org>
---
 kernel/trace/trace_output.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 0d6ff35..3efd718 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -300,7 +300,7 @@ ftrace_print_flags_seq(struct trace_seq *p, const char *delim,
 	unsigned long mask;
 	const char *str;
 	const char *ret = p->buffer + p->len;
-	int i;
+	int i, first = 1;
 
 	for (i = 0;  flag_array[i].name && flags; i++) {
 
@@ -310,8 +310,10 @@ ftrace_print_flags_seq(struct trace_seq *p, const char *delim,
 
 		str = flag_array[i].name;
 		flags &= ~mask;
-		if (p->len && delim)
+		if (!first && delim)
 			trace_seq_puts(p, delim);
+		else
+			first = 0;
 		trace_seq_puts(p, str);
 	}
 
-- 
1.7.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] trace: don't print an extra separator of flags
  2012-02-19 11:16 [PATCH] trace: don't print an extra separator of flags Andrew Vagin
@ 2012-02-20 18:09 ` Steven Rostedt
  2012-02-20 19:39   ` avagin
  2012-02-27  9:31 ` [tip:perf/core] tracing: Don't " tip-bot for Andrey Vagin
  1 sibling, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2012-02-20 18:09 UTC (permalink / raw)
  To: Andrew Vagin; +Cc: linux-kernel, Ingo Molnar, Frederic Weisbecker

On Sun, 2012-02-19 at 14:16 +0300, Andrew Vagin wrote:
> From: Andrey Vagin <avagin@openvz.org>
> 
> It occurs if __print_flags is used more than once

Hi, which tracepoint does this? I like to see what the issue is.

Thanks,

-- Steve

> 
> Signed-off-by: Andrew Vagin <avagin@openvz.org>
> ---
>  kernel/trace/trace_output.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index 0d6ff35..3efd718 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -300,7 +300,7 @@ ftrace_print_flags_seq(struct trace_seq *p, const char *delim,
>  	unsigned long mask;
>  	const char *str;
>  	const char *ret = p->buffer + p->len;
> -	int i;
> +	int i, first = 1;
>  
>  	for (i = 0;  flag_array[i].name && flags; i++) {
>  
> @@ -310,8 +310,10 @@ ftrace_print_flags_seq(struct trace_seq *p, const char *delim,
>  
>  		str = flag_array[i].name;
>  		flags &= ~mask;
> -		if (p->len && delim)
> +		if (!first && delim)
>  			trace_seq_puts(p, delim);
> +		else
> +			first = 0;
>  		trace_seq_puts(p, str);
>  	}
>  



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] trace: don't print an extra separator of flags
  2012-02-20 18:09 ` Steven Rostedt
@ 2012-02-20 19:39   ` avagin
  2012-02-20 19:47     ` Steven Rostedt
  0 siblings, 1 reply; 7+ messages in thread
From: avagin @ 2012-02-20 19:39 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Andrew Vagin, linux-kernel, Ingo Molnar, Frederic Weisbecker

On 02/20/2012 10:09 PM, Steven Rostedt wrote:
> On Sun, 2012-02-19 at 14:16 +0300, Andrew Vagin wrote:
>> From: Andrey Vagin<avagin@openvz.org>
>>
>> It occurs if __print_flags is used more than once
>
> Hi, which tracepoint does this? I like to see what the issue is.

The mainstream kernel doesn't have such trace-point, but I have.

Do you want to say that this bug should not be fixed in this case? :)

>
> Thanks,
>
> -- Steve
>
>>
>> Signed-off-by: Andrew Vagin<avagin@openvz.org>
>> ---
>>   kernel/trace/trace_output.c |    6 ++++--
>>   1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
>> index 0d6ff35..3efd718 100644
>> --- a/kernel/trace/trace_output.c
>> +++ b/kernel/trace/trace_output.c
>> @@ -300,7 +300,7 @@ ftrace_print_flags_seq(struct trace_seq *p, const char *delim,
>>   	unsigned long mask;
>>   	const char *str;
>>   	const char *ret = p->buffer + p->len;
>> -	int i;
>> +	int i, first = 1;
>>
>>   	for (i = 0;  flag_array[i].name&&  flags; i++) {
>>
>> @@ -310,8 +310,10 @@ ftrace_print_flags_seq(struct trace_seq *p, const char *delim,
>>
>>   		str = flag_array[i].name;
>>   		flags&= ~mask;
>> -		if (p->len&&  delim)
>> +		if (!first&&  delim)
>>   			trace_seq_puts(p, delim);
>> +		else
>> +			first = 0;
>>   		trace_seq_puts(p, str);
>>   	}
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] trace: don't print an extra separator of flags
  2012-02-20 19:39   ` avagin
@ 2012-02-20 19:47     ` Steven Rostedt
  2012-02-20 20:43       ` [PATCH] " Andrew Vagin
  0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2012-02-20 19:47 UTC (permalink / raw)
  To: avagin; +Cc: Andrew Vagin, linux-kernel, Ingo Molnar, Frederic Weisbecker

On Mon, 2012-02-20 at 23:39 +0400, avagin@gmail.com wrote:
> On 02/20/2012 10:09 PM, Steven Rostedt wrote:
> > On Sun, 2012-02-19 at 14:16 +0300, Andrew Vagin wrote:
> >> From: Andrey Vagin<avagin@openvz.org>
> >>
> >> It occurs if __print_flags is used more than once
> >
> > Hi, which tracepoint does this? I like to see what the issue is.
> 
> The mainstream kernel doesn't have such trace-point, but I have.

Could you send me that patch. Or a change to the kernel that shows the
problem.

> 
> Do you want to say that this bug should not be fixed in this case? :)

No, if it is a issue with the infrastructure, even though nothing
triggers the problem, it should still be fixed, especially if new code
in the future will be triggering it.

-- Steve



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] Re: [PATCH] trace: don't print an extra separator of flags
  2012-02-20 19:47     ` Steven Rostedt
@ 2012-02-20 20:43       ` Andrew Vagin
  2012-02-21  1:18         ` Steven Rostedt
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Vagin @ 2012-02-20 20:43 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, avagin, Ingo Molnar, Frederic Weisbecker

> Could you send me that patch. Or a change to the kernel that shows the
> problemi.

Without my patch:
    event-sample-1731  [000] ...1 546886.253576: foo_bar: foo hello 3 |A B
    event-sample-1731  [000] ...1 546887.253677: foo_bar: foo hello 4 |A B

With my patch:
    event-sample-1731  [000] ...1 546886.253576: foo_bar: foo hello 3 A B
    event-sample-1731  [000] ...1 546887.253677: foo_bar: foo hello 4 A B


---
 samples/trace_events/trace-events-sample.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h
index 6af3732..812be4b 100644
--- a/samples/trace_events/trace-events-sample.h
+++ b/samples/trace_events/trace-events-sample.h
@@ -90,7 +90,7 @@ TRACE_EVENT(foo_bar,
 		__entry->bar	= bar;
 	),
 
-	TP_printk("foo %s %d", __entry->foo, __entry->bar)
+	TP_printk("foo %s %d %s %s", __entry->foo, __entry->bar, __print_flags(1, "|", {1, "A"}), __print_flags(2, "&", {2, "B"}))
 );
 #endif
 
-- 
1.7.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Re: [PATCH] trace: don't print an extra separator of flags
  2012-02-20 20:43       ` [PATCH] " Andrew Vagin
@ 2012-02-21  1:18         ` Steven Rostedt
  0 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2012-02-21  1:18 UTC (permalink / raw)
  To: Andrew Vagin; +Cc: linux-kernel, Ingo Molnar, Frederic Weisbecker

On Mon, 2012-02-20 at 23:43 +0300, Andrew Vagin wrote:
> > Could you send me that patch. Or a change to the kernel that shows the
> > problemi.
> 
> Without my patch:
>     event-sample-1731  [000] ...1 546886.253576: foo_bar: foo hello 3 |A B
>     event-sample-1731  [000] ...1 546887.253677: foo_bar: foo hello 4 |A B
> 
> With my patch:
>     event-sample-1731  [000] ...1 546886.253576: foo_bar: foo hello 3 A B
>     event-sample-1731  [000] ...1 546887.253677: foo_bar: foo hello 4 A B

Ah, nevermind. This bug actually does show up in another tracepoint
somewhere (I  don't remember which). But I've been meaning to fix it.

Thanks! I'll take your patch. But since it is such a minor bug. I'll put
it into the v3.4 queue.

-- Steve



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [tip:perf/core] tracing: Don't print an extra separator of flags
  2012-02-19 11:16 [PATCH] trace: don't print an extra separator of flags Andrew Vagin
  2012-02-20 18:09 ` Steven Rostedt
@ 2012-02-27  9:31 ` tip-bot for Andrey Vagin
  1 sibling, 0 replies; 7+ messages in thread
From: tip-bot for Andrey Vagin @ 2012-02-27  9:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, fweisbec, rostedt, tglx, avagin

Commit-ID:  e404b321dbb2d6e438522b7dce9c1d0c6a8c5275
Gitweb:     http://git.kernel.org/tip/e404b321dbb2d6e438522b7dce9c1d0c6a8c5275
Author:     Andrey Vagin <avagin@openvz.org>
AuthorDate: Sun, 19 Feb 2012 14:16:07 +0300
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Mon, 20 Feb 2012 20:33:31 -0500

tracing: Don't print an extra separator of flags

If __print_flags() is used after another __print_*() function, the
temp seq_file buffer will not be empty on entry, and the delimiter will
be printed even though there's just one field. We get something like:

	|S

instead of just:

	S

This is because the length of the temp seq buffer is used to determine
if the delimiter is printed or not. But this algorithm fails when
the seq buffer is not empty on entry, and the delimiter will be printed
because it thinks that a previous field was already printed.

Link: http://lkml.kernel.org/r/1329650167-480655-1-git-send-email-avagin@openvz.org

Signed-off-by: Andrew Vagin <avagin@openvz.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_output.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 0d6ff355..3efd718 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -300,7 +300,7 @@ ftrace_print_flags_seq(struct trace_seq *p, const char *delim,
 	unsigned long mask;
 	const char *str;
 	const char *ret = p->buffer + p->len;
-	int i;
+	int i, first = 1;
 
 	for (i = 0;  flag_array[i].name && flags; i++) {
 
@@ -310,8 +310,10 @@ ftrace_print_flags_seq(struct trace_seq *p, const char *delim,
 
 		str = flag_array[i].name;
 		flags &= ~mask;
-		if (p->len && delim)
+		if (!first && delim)
 			trace_seq_puts(p, delim);
+		else
+			first = 0;
 		trace_seq_puts(p, str);
 	}
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-02-27  9:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-19 11:16 [PATCH] trace: don't print an extra separator of flags Andrew Vagin
2012-02-20 18:09 ` Steven Rostedt
2012-02-20 19:39   ` avagin
2012-02-20 19:47     ` Steven Rostedt
2012-02-20 20:43       ` [PATCH] " Andrew Vagin
2012-02-21  1:18         ` Steven Rostedt
2012-02-27  9:31 ` [tip:perf/core] tracing: Don't " tip-bot for Andrey Vagin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.