* [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type
@ 2015-08-03 17:08 Dean Nelson
2015-08-07 10:59 ` Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Dean Nelson @ 2015-08-03 17:08 UTC (permalink / raw)
To: a.p.zijlstra, mingo, rostedt, acme; +Cc: linux-kernel, jolsa
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;
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type 2015-08-03 17:08 [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type Dean Nelson @ 2015-08-07 10:59 ` Jiri Olsa 2015-08-07 12:16 ` Namhyung Kim ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Jiri Olsa @ 2015-08-07 10:59 UTC (permalink / raw) To: Dean Nelson Cc: a.p.zijlstra, mingo, rostedt, acme, linux-kernel, jolsa, Namhyung Kim 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; > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type 2015-08-07 10:59 ` Jiri Olsa @ 2015-08-07 12:16 ` Namhyung Kim 2015-08-07 13:02 ` Dean Nelson 2015-08-07 13:01 ` Dean Nelson 2015-08-09 3:10 ` Namhyung Kim 2 siblings, 1 reply; 8+ messages in thread From: Namhyung Kim @ 2015-08-07 12:16 UTC (permalink / raw) To: Jiri Olsa Cc: Dean Nelson, a.p.zijlstra, Ingo Molnar, Steven Rostedt, Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa 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; >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type 2015-08-07 12:16 ` Namhyung Kim @ 2015-08-07 13:02 ` Dean Nelson 2015-08-09 2:38 ` Namhyung Kim 0 siblings, 1 reply; 8+ messages in thread From: Dean Nelson @ 2015-08-07 13:02 UTC (permalink / raw) To: Namhyung Kim, Jiri Olsa Cc: a.p.zijlstra, Ingo Molnar, Steven Rostedt, Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa [-- 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" ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type 2015-08-07 13:02 ` Dean Nelson @ 2015-08-09 2:38 ` Namhyung Kim 0 siblings, 0 replies; 8+ messages in thread From: Namhyung Kim @ 2015-08-09 2:38 UTC (permalink / raw) To: Dean Nelson Cc: Jiri Olsa, a.p.zijlstra, Ingo Molnar, Steven Rostedt, Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa 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" ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type 2015-08-07 10:59 ` Jiri Olsa 2015-08-07 12:16 ` Namhyung Kim @ 2015-08-07 13:01 ` Dean Nelson 2015-08-09 3:10 ` Namhyung Kim 2 siblings, 0 replies; 8+ messages in thread From: Dean Nelson @ 2015-08-07 13:01 UTC (permalink / raw) To: Jiri Olsa Cc: a.p.zijlstra, mingo, rostedt, acme, linux-kernel, jolsa, Namhyung Kim 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/ > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type 2015-08-07 10:59 ` Jiri Olsa 2015-08-07 12:16 ` Namhyung Kim 2015-08-07 13:01 ` Dean Nelson @ 2015-08-09 3:10 ` Namhyung Kim 2015-08-10 12:41 ` Dean Nelson 2 siblings, 1 reply; 8+ messages in thread From: Namhyung Kim @ 2015-08-09 3:10 UTC (permalink / raw) To: Jiri Olsa; +Cc: Dean Nelson, mingo, rostedt, acme, linux-kernel, jolsa 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; > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type 2015-08-09 3:10 ` Namhyung Kim @ 2015-08-10 12:41 ` Dean Nelson 0 siblings, 0 replies; 8+ messages in thread From: Dean Nelson @ 2015-08-10 12:41 UTC (permalink / raw) To: Namhyung Kim; +Cc: Jiri Olsa, mingo, rostedt, acme, linux-kernel 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/ > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-08-10 12:41 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-08-03 17:08 [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type Dean Nelson 2015-08-07 10:59 ` Jiri Olsa 2015-08-07 12:16 ` Namhyung Kim 2015-08-07 13:02 ` Dean Nelson 2015-08-09 2:38 ` Namhyung Kim 2015-08-07 13:01 ` Dean Nelson 2015-08-09 3:10 ` Namhyung Kim 2015-08-10 12:41 ` Dean Nelson
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.