Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type
From: Dean Nelson
Date: Fri Aug 07 2015 - 09:02:32 EST
On 08/07/2015 07:16 AM, Namhyung Kim wrote:
Hi,
On Fri, Aug 7, 2015 at 7:59 PM, Jiri Olsa <jolsa@xxxxxxxxxx> 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@xxxxxxxxxx>
---
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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"