[PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type

[PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type

[Dean Nelson]

Running the following perf-stat command on an arm64 system produces the
following result...

  [root@aarch64 ~]# perf stat -e kmem:mm_page_alloc -a sleep 1
    Warning: [kmem:mm_page_alloc] function sizeof not defined
    Warning: Error: expected type 4 but read 0
  Segmentation fault
  [root@aarch64 ~]#

The second warning message and SIGSEGV stem from the issue expressed in the
first warning message, and are the result of ignoring the EVENT_ERROR type
returned back through the call chain.

Dealing with the first warning message is beyond the scope of this patch. But
the second warning is addressed by this patch's first hunk. And the SIGSEGV is
eliminated by its second hunk.

Signed-off-by: Dean Nelson <dnelson@redhat.com>
---
 tools/lib/traceevent/event-parse.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index cc25f05..72e2933 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -1680,6 +1680,9 @@ process_cond(struct event_format *event, struct print_arg *top, char **tok)
 	type = process_arg(event, left, &token);
 
  again:
+	if (type == EVENT_ERROR)
+		goto out_free;
+
 	/* Handle other operations in the arguments */
 	if (type == EVENT_OP && strcmp(token, ":") != 0) {
 		type = process_op(event, left, &token);
@@ -1940,7 +1943,7 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
 
 		type = process_arg_token(event, right, tok, type);
 
-		if (right->type == PRINT_OP &&
+		if (type != EVENT_ERROR && right->type == PRINT_OP &&
 		    get_op_prio(arg->op.op) < get_op_prio(right->op.op)) {
 			struct print_arg tmp;
 

Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type

[Jiri Olsa]

On Mon, Aug 03, 2015 at 01:08:05PM -0400, Dean Nelson wrote:
> Running the following perf-stat command on an arm64 system produces the
> following result...
> 
>   [root@aarch64 ~]# perf stat -e kmem:mm_page_alloc -a sleep 1
>     Warning: [kmem:mm_page_alloc] function sizeof not defined
>     Warning: Error: expected type 4 but read 0
>   Segmentation fault
>   [root@aarch64 ~]#

hum, what kernel are you running on? I dont see that warning
on my system:

[jolsa@krava perf]$ sudo ./perf stat -e kmem:mm_page_alloc -a sleep 1

 Performance counter stats for 'system wide':

               227      kmem:mm_page_alloc                                          

       1.000762466 seconds time elapsed

Cc-ing Namhyung


jirka

> 
> The second warning message and SIGSEGV stem from the issue expressed in the
> first warning message, and are the result of ignoring the EVENT_ERROR type
> returned back through the call chain.
> 
> Dealing with the first warning message is beyond the scope of this patch. But
> the second warning is addressed by this patch's first hunk. And the SIGSEGV is
> eliminated by its second hunk.
> 
> Signed-off-by: Dean Nelson <dnelson@redhat.com>
> ---
>  tools/lib/traceevent/event-parse.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index cc25f05..72e2933 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -1680,6 +1680,9 @@ process_cond(struct event_format *event, struct print_arg *top, char **tok)
>  	type = process_arg(event, left, &token);
>  
>   again:
> +	if (type == EVENT_ERROR)
> +		goto out_free;
> +
>  	/* Handle other operations in the arguments */
>  	if (type == EVENT_OP && strcmp(token, ":") != 0) {
>  		type = process_op(event, left, &token);
> @@ -1940,7 +1943,7 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
>  
>  		type = process_arg_token(event, right, tok, type);
>  
> -		if (right->type == PRINT_OP &&
> +		if (type != EVENT_ERROR && right->type == PRINT_OP &&
>  		    get_op_prio(arg->op.op) < get_op_prio(right->op.op)) {
>  			struct print_arg tmp;
>  

Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type

[Namhyung Kim]

Hi,

On Fri, Aug 7, 2015 at 7:59 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Mon, Aug 03, 2015 at 01:08:05PM -0400, Dean Nelson wrote:
>> Running the following perf-stat command on an arm64 system produces the
>> following result...
>>
>>   [root@aarch64 ~]# perf stat -e kmem:mm_page_alloc -a sleep 1
>>     Warning: [kmem:mm_page_alloc] function sizeof not defined
>>     Warning: Error: expected type 4 but read 0
>>   Segmentation fault

Oops,


>>   [root@aarch64 ~]#
>
> hum, what kernel are you running on? I dont see that warning
> on my system:
>
> [jolsa@krava perf]$ sudo ./perf stat -e kmem:mm_page_alloc -a sleep 1
>
>  Performance counter stats for 'system wide':
>
>                227      kmem:mm_page_alloc
>
>        1.000762466 seconds time elapsed
>
> Cc-ing Namhyung

Yeah, it seems his kernel has sizeof() somewhere in the event format.
Anyway, it's not good to see a segfault.

Dean, could you share your event format file?

  $ sudo cat /sys/kernel/debug/tracing/events/kmem/mm_page_alloc/format

Thanks,
Namhyung


>
>>
>> The second warning message and SIGSEGV stem from the issue expressed in the
>> first warning message, and are the result of ignoring the EVENT_ERROR type
>> returned back through the call chain.
>>
>> Dealing with the first warning message is beyond the scope of this patch. But
>> the second warning is addressed by this patch's first hunk. And the SIGSEGV is
>> eliminated by its second hunk.
>>
>> Signed-off-by: Dean Nelson <dnelson@redhat.com>
>> ---
>>  tools/lib/traceevent/event-parse.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
>> index cc25f05..72e2933 100644
>> --- a/tools/lib/traceevent/event-parse.c
>> +++ b/tools/lib/traceevent/event-parse.c
>> @@ -1680,6 +1680,9 @@ process_cond(struct event_format *event, struct print_arg *top, char **tok)
>>       type = process_arg(event, left, &token);
>>
>>   again:
>> +     if (type == EVENT_ERROR)
>> +             goto out_free;
>> +
>>       /* Handle other operations in the arguments */
>>       if (type == EVENT_OP && strcmp(token, ":") != 0) {
>>               type = process_op(event, left, &token);
>> @@ -1940,7 +1943,7 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
>>
>>               type = process_arg_token(event, right, tok, type);
>>
>> -             if (right->type == PRINT_OP &&
>> +             if (type != EVENT_ERROR && right->type == PRINT_OP &&
>>                   get_op_prio(arg->op.op) < get_op_prio(right->op.op)) {
>>                       struct print_arg tmp;
>>

Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type

[Dean Nelson]

On 08/07/2015 05:59 AM, Jiri Olsa wrote:
> On Mon, Aug 03, 2015 at 01:08:05PM -0400, Dean Nelson wrote:
>> Running the following perf-stat command on an arm64 system produces the
>> following result...
>>
>>    [root@aarch64 ~]# perf stat -e kmem:mm_page_alloc -a sleep 1
>>      Warning: [kmem:mm_page_alloc] function sizeof not defined
>>      Warning: Error: expected type 4 but read 0
>>    Segmentation fault
>>    [root@aarch64 ~]#
>
> hum, what kernel are you running on? I dont see that warning

The system is based on v4.2-rc3 and my .config has...

   CONFIG_SPARSEMEM_VMEMMAP=y

which matters, as the sizeof operator is introduced through...

   #define pfn_to_page __pfn_to_page

when __pfn_to_page is defined as...

   #define __pfn_to_page(pfn)      (vmemmap + (pfn))

which only happens when '#elif defined(CONFIG_SPARSEMEM_VMEMMAP)' is
true.

Now vmemmap was redefined about a year ago by commit 08375198b01001c0e43b
to be...

   #define VMEMMAP_SIZE            ALIGN((1UL << (VA_BITS - PAGE_SHIFT)) 
* sizeof(struct page), PUD_SIZE)
   #define VMALLOC_END             (PAGE_OFFSET - PUD_SIZE - 
VMEMMAP_SIZE - SZ_64K)
   #define vmemmap                 ((struct page *)(VMALLOC_END + SZ_64K))

And VMEMMAP_SIZE has the sizeof operator. The ALIGN() macro introduces
a typeof operator which also would be an issue if the parser had gotten
that far.

This vmemmap stuff can be found in arch/arm64/include/asm/pgtable.h.

Now backing up a step or two, the pfn_to_page() macro is referenced in
include/trace/events/kmem.h as the first of the 'args' passed to
TP_prink()...

   TRACE_EVENT(mm_page_alloc,
           :
           TP_printk("page=%p pfn=%lu order=%d migratetype=%d gfp_flags=%s",
                   __entry->pfn != -1UL ? pfn_to_page(__entry->pfn) : NULL,
                   __entry->pfn != -1UL ? __entry->pfn : 0,
                   __entry->order,
                   __entry->migratetype,
                   show_gfp_flags(__entry->gfp_flags))
   );

Hope that helps.


> on my system:
>
> [jolsa@krava perf]$ sudo ./perf stat -e kmem:mm_page_alloc -a sleep 1
>
>   Performance counter stats for 'system wide':
>
>                 227      kmem:mm_page_alloc
>
>         1.000762466 seconds time elapsed
>
> Cc-ing Namhyung
>
>
> jirka
>
>>
>> The second warning message and SIGSEGV stem from the issue expressed in the
>> first warning message, and are the result of ignoring the EVENT_ERROR type
>> returned back through the call chain.
>>
>> Dealing with the first warning message is beyond the scope of this patch. But
>> the second warning is addressed by this patch's first hunk. And the SIGSEGV is
>> eliminated by its second hunk.
>>
>> Signed-off-by: Dean Nelson <dnelson@redhat.com>
>> ---
>>   tools/lib/traceevent/event-parse.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
>> index cc25f05..72e2933 100644
>> --- a/tools/lib/traceevent/event-parse.c
>> +++ b/tools/lib/traceevent/event-parse.c
>> @@ -1680,6 +1680,9 @@ process_cond(struct event_format *event, struct print_arg *top, char **tok)
>>   	type = process_arg(event, left, &token);
>>
>>    again:
>> +	if (type == EVENT_ERROR)
>> +		goto out_free;
>> +
>>   	/* Handle other operations in the arguments */
>>   	if (type == EVENT_OP && strcmp(token, ":") != 0) {
>>   		type = process_op(event, left, &token);
>> @@ -1940,7 +1943,7 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
>>
>>   		type = process_arg_token(event, right, tok, type);
>>
>> -		if (right->type == PRINT_OP &&
>> +		if (type != EVENT_ERROR && right->type == PRINT_OP &&
>>   		    get_op_prio(arg->op.op) < get_op_prio(right->op.op)) {
>>   			struct print_arg tmp;
>>
> --
> 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/
>

Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type

[Dean Nelson]

[-- Attachment #1: Type: text/plain, Size: 3226 bytes --]

On 08/07/2015 07:16 AM, Namhyung Kim wrote:
> Hi,
>
> On Fri, Aug 7, 2015 at 7:59 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>> On Mon, Aug 03, 2015 at 01:08:05PM -0400, Dean Nelson wrote:
>>> Running the following perf-stat command on an arm64 system produces the
>>> following result...
>>>
>>>    [root@aarch64 ~]# perf stat -e kmem:mm_page_alloc -a sleep 1
>>>      Warning: [kmem:mm_page_alloc] function sizeof not defined
>>>      Warning: Error: expected type 4 but read 0
>>>    Segmentation fault
>
> Oops,
>
>
>>>    [root@aarch64 ~]#
>>
>> hum, what kernel are you running on? I dont see that warning
>> on my system:
>>
>> [jolsa@krava perf]$ sudo ./perf stat -e kmem:mm_page_alloc -a sleep 1
>>
>>   Performance counter stats for 'system wide':
>>
>>                 227      kmem:mm_page_alloc
>>
>>         1.000762466 seconds time elapsed
>>
>> Cc-ing Namhyung
>
> Yeah, it seems his kernel has sizeof() somewhere in the event format.
> Anyway, it's not good to see a segfault.
>
> Dean, could you share your event format file?
>
>    $ sudo cat /sys/kernel/debug/tracing/events/kmem/mm_page_alloc/format

Sure, I've attached it. See my other email in reply to jirka's question
about what kernel I was running on, for some details about where the
sizeof operator comes from.

>
> Thanks,
> Namhyung
>
>
>>
>>>
>>> The second warning message and SIGSEGV stem from the issue expressed in the
>>> first warning message, and are the result of ignoring the EVENT_ERROR type
>>> returned back through the call chain.
>>>
>>> Dealing with the first warning message is beyond the scope of this patch. But
>>> the second warning is addressed by this patch's first hunk. And the SIGSEGV is
>>> eliminated by its second hunk.
>>>
>>> Signed-off-by: Dean Nelson <dnelson@redhat.com>
>>> ---
>>>   tools/lib/traceevent/event-parse.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
>>> index cc25f05..72e2933 100644
>>> --- a/tools/lib/traceevent/event-parse.c
>>> +++ b/tools/lib/traceevent/event-parse.c
>>> @@ -1680,6 +1680,9 @@ process_cond(struct event_format *event, struct print_arg *top, char **tok)
>>>        type = process_arg(event, left, &token);
>>>
>>>    again:
>>> +     if (type == EVENT_ERROR)
>>> +             goto out_free;
>>> +
>>>        /* Handle other operations in the arguments */
>>>        if (type == EVENT_OP && strcmp(token, ":") != 0) {
>>>                type = process_op(event, left, &token);
>>> @@ -1940,7 +1943,7 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
>>>
>>>                type = process_arg_token(event, right, tok, type);
>>>
>>> -             if (right->type == PRINT_OP &&
>>> +             if (type != EVENT_ERROR && right->type == PRINT_OP &&
>>>                    get_op_prio(arg->op.op) < get_op_prio(right->op.op)) {
>>>                        struct print_arg tmp;
>>>
> --
> 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/
>


[-- Attachment #2: kmem-mm_page_alloc-format --]
[-- Type: text/plain, Size: 3073 bytes --]

name: mm_page_alloc
ID: 360
format:
	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
	field:int common_pid;	offset:4;	size:4;	signed:1;

	field:unsigned long pfn;	offset:8;	size:8;	signed:0;
	field:unsigned int order;	offset:16;	size:4;	signed:0;
	field:gfp_t gfp_flags;	offset:20;	size:4;	signed:0;
	field:int migratetype;	offset:24;	size:4;	signed:1;

print fmt: "page=%p pfn=%lu order=%d migratetype=%d gfp_flags=%s", REC->pfn != -1UL ? (((struct page *)((((0xffffffffffffffffUL) << ((42) - 1)) - (1UL << ((16 - 3) * 2 + 3)) - (((((1UL << ((42) - 16)) * sizeof(struct page))) + ((typeof(((1UL << ((42) - 16)) * sizeof(struct page))))(((1UL << ((16 - 3) * 2 + 3)))) - 1)) & ~((typeof(((1UL << ((42) - 16)) * sizeof(struct page))))(((1UL << ((16 - 3) * 2 + 3)))) - 1)) - 0x00010000) + 0x00010000)) + (REC->pfn)) : ((void *)0), REC->pfn != -1UL ? REC->pfn : 0, REC->order, REC->migratetype, (REC->gfp_flags) ? __print_flags(REC->gfp_flags, "|", {(unsigned long)(((((( gfp_t)0x10u) | (( gfp_t)0x40u) | (( gfp_t)0x80u) | (( gfp_t)0x20000u)) | (( gfp_t)0x02u)) | (( gfp_t)0x08u)) | (( gfp_t)0x4000u) | (( gfp_t)0x10000u) | (( gfp_t)0x1000u) | (( gfp_t)0x200u) | (( gfp_t)0x400000u)), "GFP_TRANSHUGE"}, {(unsigned long)((((( gfp_t)0x10u) | (( gfp_t)0x40u) | (( gfp_t)0x80u) | (( gfp_t)0x20000u)) | (( gfp_t)0x02u)) | (( gfp_t)0x08u)), "GFP_HIGHUSER_MOVABLE"}, {(unsigned long)(((( gfp_t)0x10u) | (( gfp_t)0x40u) | (( gfp_t)0x80u) | (( gfp_t)0x20000u)) | (( gfp_t)0x02u)), "GFP_HIGHUSER"}, {(unsigned long)((( gfp_t)0x10u) | (( gfp_t)0x40u) | (( gfp_t)0x80u) | (( gfp_t)0x20000u)), "GFP_USER"}, {(unsigned long)((( gfp_t)0x10u) | (( gfp_t)0x40u) | (( gfp_t)0x80u) | (( gfp_t)0x80000u)), "GFP_TEMPORARY"}, {(unsigned long)((( gfp_t)0x10u) | (( gfp_t)0x40u) | (( gfp_t)0x80u)), "GFP_KERNEL"}, {(unsigned long)((( gfp_t)0x10u) | (( gfp_t)0x40u)), "GFP_NOFS"}, {(unsigned long)((( gfp_t)0x20u)), "GFP_ATOMIC"}, {(unsigned long)((( gfp_t)0x10u)), "GFP_NOIO"}, {(unsigned long)(( gfp_t)0x20u), "GFP_HIGH"}, {(unsigned long)(( gfp_t)0x10u), "GFP_WAIT"}, {(unsigned long)(( gfp_t)0x40u), "GFP_IO"}, {(unsigned long)(( gfp_t)0x100u), "GFP_COLD"}, {(unsigned long)(( gfp_t)0x200u), "GFP_NOWARN"}, {(unsigned long)(( gfp_t)0x400u), "GFP_REPEAT"}, {(unsigned long)(( gfp_t)0x800u), "GFP_NOFAIL"}, {(unsigned long)(( gfp_t)0x1000u), "GFP_NORETRY"}, {(unsigned long)(( gfp_t)0x4000u), "GFP_COMP"}, {(unsigned long)(( gfp_t)0x8000u), "GFP_ZERO"}, {(unsigned long)(( gfp_t)0x10000u), "GFP_NOMEMALLOC"}, {(unsigned long)(( gfp_t)0x2000u), "GFP_MEMALLOC"}, {(unsigned long)(( gfp_t)0x20000u), "GFP_HARDWALL"}, {(unsigned long)(( gfp_t)0x40000u), "GFP_THISNODE"}, {(unsigned long)(( gfp_t)0x80000u), "GFP_RECLAIMABLE"}, {(unsigned long)(( gfp_t)0x08u), "GFP_MOVABLE"}, {(unsigned long)(( gfp_t)0x200000u), "GFP_NOTRACK"}, {(unsigned long)(( gfp_t)0x400000u), "GFP_NO_KSWAPD"}, {(unsigned long)(( gfp_t)0x800000u), "GFP_OTHER_NODE"} ) : "GFP_NOWAIT"

Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type

[Namhyung Kim]

Hi Dean,

On Fri, Aug 07, 2015 at 08:02:25AM -0500, Dean Nelson wrote:
> On 08/07/2015 07:16 AM, Namhyung Kim wrote:
> >Hi,
> >
> >On Fri, Aug 7, 2015 at 7:59 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> >>On Mon, Aug 03, 2015 at 01:08:05PM -0400, Dean Nelson wrote:
> >>>Running the following perf-stat command on an arm64 system produces the
> >>>following result...
> >>>
> >>>   [root@aarch64 ~]# perf stat -e kmem:mm_page_alloc -a sleep 1
> >>>     Warning: [kmem:mm_page_alloc] function sizeof not defined
> >>>     Warning: Error: expected type 4 but read 0
> >>>   Segmentation fault
> >
> >Oops,
> >
> >
> >>>   [root@aarch64 ~]#
> >>
> >>hum, what kernel are you running on? I dont see that warning
> >>on my system:
> >>
> >>[jolsa@krava perf]$ sudo ./perf stat -e kmem:mm_page_alloc -a sleep 1
> >>
> >>  Performance counter stats for 'system wide':
> >>
> >>                227      kmem:mm_page_alloc
> >>
> >>        1.000762466 seconds time elapsed
> >>
> >>Cc-ing Namhyung
> >
> >Yeah, it seems his kernel has sizeof() somewhere in the event format.
> >Anyway, it's not good to see a segfault.
> >
> >Dean, could you share your event format file?
> >
> >   $ sudo cat /sys/kernel/debug/tracing/events/kmem/mm_page_alloc/format
> 
> Sure, I've attached it. See my other email in reply to jirka's question
> about what kernel I was running on, for some details about where the
> sizeof operator comes from.

Thanks for the explanation.  It seems there's a problem in processing
'?' operator.  I'll take a look at it.

Thanks,
Namhyung


> name: mm_page_alloc
> ID: 360
> format:
> 	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
> 	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
> 	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
> 	field:int common_pid;	offset:4;	size:4;	signed:1;
> 
> 	field:unsigned long pfn;	offset:8;	size:8;	signed:0;
> 	field:unsigned int order;	offset:16;	size:4;	signed:0;
> 	field:gfp_t gfp_flags;	offset:20;	size:4;	signed:0;
> 	field:int migratetype;	offset:24;	size:4;	signed:1;
> 
> print fmt: "page=%p pfn=%lu order=%d migratetype=%d gfp_flags=%s", REC->pfn != -1UL ? (((struct page *)((((0xffffffffffffffffUL) << ((42) - 1)) - (1UL << ((16 - 3) * 2 + 3)) - (((((1UL << ((42) - 16)) * sizeof(struct page))) + ((typeof(((1UL << ((42) - 16)) * sizeof(struct page))))(((1UL << ((16 - 3) * 2 + 3)))) - 1)) & ~((typeof(((1UL << ((42) - 16)) * sizeof(struct page))))(((1UL << ((16 - 3) * 2 + 3)))) - 1)) - 0x00010000) + 0x00010000)) + (REC->pfn)) : ((void *)0), REC->pfn != -1UL ? REC->pfn : 0, REC->order, REC->migratetype, (REC->gfp_flags) ? __print_flags(REC->gfp_flags, "|", {(unsigned long)(((((( gfp_t)0x10u) | (( gfp_t)0x40u) | (( gfp_t)0x80u) | (( gfp_t)0x20000u)) | (( gfp_t)0x02u)) | (( gfp_t)0x08u)) | (( gfp_t)0x4000u) | (( gfp_t)0x10000u) | (( gfp_t)0x1000u) | (( gfp_t)0x200u) | (( gfp_t)0x400000u)), "GFP_TRANSHUGE"}, {(unsigned long)((((( gfp_t)0x10u) | (( gfp_t)0x40u) | (( gfp_t)0x80u) | (( gfp_t)0x20000u)) | (( gfp_t)0x02u)) | (( gfp_t)0x08u)), "GFP_HIGHUSER_MOVABLE"}, {(unsigned long)(((( gfp_t)0x10u) | (( gfp_t)0x40u) | (( gfp_t)0x80u) | (( gfp_t)0x20000u)) | (( gfp_t)0x02u)), "GFP_HIGHUSER"}, {(unsigned long)((( gfp_t)0x10u) | (( gfp_t)0x40u) | (( gfp_t)0x80u) | (( gfp_t)0x20000u)), "GFP_USER"}, {(unsigned long)((( gfp_t)0x10u) | (( gfp_t)0x40u) | (( gfp_t)0x80u) | (( gfp_t)0x80000u)), "GFP_TEMPORARY"}, {(unsigned long)((( gfp_t)0x10u) | (( gfp_t)0x40u) | (( gfp_t)0x80u)), "GFP_KERNEL"}, {(unsigned long)((( gfp_t)0x10u) | (( gfp_t)0x40u)), "GFP_NOFS"}, {(unsigned long)((( gfp_t)0x20u)), "GFP_ATOMIC"}, {(unsigned long)((( gfp_t)0x10u)), "GFP_NOIO"}, {(unsigned long)(( gfp_t)0x20u), "GFP_HIGH"}, {(unsigned long)(( gfp_t)0x10u), "GFP_WAIT"}, {(unsigned long)(( gfp_t)0x40u), "GFP_IO"}, {(unsigned long)(( gfp_t)0x100u), "GFP_COLD"}, {(unsigned long)(( gfp_t)0x200u), "GFP_NOWARN"}, {(unsigned long)(( gfp_t)0x400u), "GFP_REPEAT"}, {(unsigned long)(( gfp_t)0x800u), "GFP_NOFAIL"}, {(unsigned long)(( gfp_t)0x1000u), "GFP_NORETRY"}, {(unsigned long)(( gfp_t)0x4000u), "GFP_COMP"}, {(unsigned long)(( gfp_t)0x8000u), "GFP_ZERO"}, {(unsigned long)(( gfp_t)0x10000u), "GFP_NOMEMALLOC"}, {(unsigned long)(( gfp_t)0x2000u), "GFP_MEMALLOC"}, {(unsigned long)(( gfp_t)0x20000u), "GFP_HARDWALL"}, {(unsigned long)(( gfp_t)0x40000u), "GFP_THISNODE"}, {(unsigned long)(( gfp_t)0x80000u), "GFP_RECLAIMABLE"}, {(unsigned long)(( gfp_t)0x08u), "GFP_MOVABLE"}, {(unsigned long)(( gfp_t)0x200000u), "GFP_NOTRACK"}, {(unsigned long)(( gfp_t)0x400000u), "GFP_NO_KSWAPD"}, {(unsigned long)(( gfp_t)0x800000u), "GFP_OTHER_NODE"} ) : "GFP_NOWAIT"

Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type

[Namhyung Kim]

On Fri, Aug 07, 2015 at 12:59:10PM +0200, Jiri Olsa wrote:
> On Mon, Aug 03, 2015 at 01:08:05PM -0400, Dean Nelson wrote:
> > The second warning message and SIGSEGV stem from the issue expressed in the
> > first warning message, and are the result of ignoring the EVENT_ERROR type
> > returned back through the call chain.
> > 
> > Dealing with the first warning message is beyond the scope of this patch. But
> > the second warning is addressed by this patch's first hunk. And the SIGSEGV is
> > eliminated by its second hunk.
> > 
> > Signed-off-by: Dean Nelson <dnelson@redhat.com>
> > ---
> >  tools/lib/traceevent/event-parse.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> > index cc25f05..72e2933 100644
> > --- a/tools/lib/traceevent/event-parse.c
> > +++ b/tools/lib/traceevent/event-parse.c
> > @@ -1680,6 +1680,9 @@ process_cond(struct event_format *event, struct print_arg *top, char **tok)
> >  	type = process_arg(event, left, &token);
> >  
> >   again:
> > +	if (type == EVENT_ERROR)
> > +		goto out_free;
> > +
> >  	/* Handle other operations in the arguments */
> >  	if (type == EVENT_OP && strcmp(token, ":") != 0) {
> >  		type = process_op(event, left, &token);
> > @@ -1940,7 +1943,7 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
> >  
> >  		type = process_arg_token(event, right, tok, type);
> >  
> > -		if (right->type == PRINT_OP &&
> > +		if (type != EVENT_ERROR && right->type == PRINT_OP &&

I think you'd better put the error check on separate lines.  Other
than that look good to me.

Thanks,
Namhyung


> >  		    get_op_prio(arg->op.op) < get_op_prio(right->op.op)) {
> >  			struct print_arg tmp;
> >  

Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type

[Dean Nelson]

On 08/08/2015 10:10 PM, Namhyung Kim wrote:
> On Fri, Aug 07, 2015 at 12:59:10PM +0200, Jiri Olsa wrote:
>> On Mon, Aug 03, 2015 at 01:08:05PM -0400, Dean Nelson wrote:
>>> The second warning message and SIGSEGV stem from the issue expressed in the
>>> first warning message, and are the result of ignoring the EVENT_ERROR type
>>> returned back through the call chain.
>>>
>>> Dealing with the first warning message is beyond the scope of this patch. But
>>> the second warning is addressed by this patch's first hunk. And the SIGSEGV is
>>> eliminated by its second hunk.
>>>
>>> Signed-off-by: Dean Nelson <dnelson@redhat.com>
>>> ---
>>>   tools/lib/traceevent/event-parse.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
>>> index cc25f05..72e2933 100644
>>> --- a/tools/lib/traceevent/event-parse.c
>>> +++ b/tools/lib/traceevent/event-parse.c
>>> @@ -1680,6 +1680,9 @@ process_cond(struct event_format *event, struct print_arg *top, char **tok)
>>>   	type = process_arg(event, left, &token);
>>>
>>>    again:
>>> +	if (type == EVENT_ERROR)
>>> +		goto out_free;
>>> +
>>>   	/* Handle other operations in the arguments */
>>>   	if (type == EVENT_OP && strcmp(token, ":") != 0) {
>>>   		type = process_op(event, left, &token);
>>> @@ -1940,7 +1943,7 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
>>>
>>>   		type = process_arg_token(event, right, tok, type);
>>>
>>> -		if (right->type == PRINT_OP &&
>>> +		if (type != EVENT_ERROR && right->type == PRINT_OP &&
>
> I think you'd better put the error check on separate lines.  Other
> than that look good to me.

Okay. I've posted a v2. Consider v1 NAK'd.

And thank you Namhyung for reviewing this patch!


>
> Thanks,
> Namhyung
>
>
>>>   		    get_op_prio(arg->op.op) < get_op_prio(right->op.op)) {
>>>   			struct print_arg tmp;
>>>
> --
> 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/
>