From: Dean Nelson <dnelson@redhat.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: a.p.zijlstra@chello.org, mingo@redhat.com, rostedt@goodmis.org,
acme@kernel.org, linux-kernel@vger.kernel.org, jolsa@kernel.org,
Namhyung Kim <namhyung@kernel.org>
Subject: Re: [PATCH] tools lib traceevent: add checks for returned EVENT_ERROR type
Date: Fri, 7 Aug 2015 08:01:02 -0500 [thread overview]
Message-ID: <55C4AC0E.6060808@redhat.com> (raw)
In-Reply-To: <20150807105910.GE8624@krava.brq.redhat.com>
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/
>
next prev parent reply other threads:[~2015-08-07 13:01 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2015-08-09 3:10 ` Namhyung Kim
2015-08-10 12:41 ` Dean Nelson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55C4AC0E.6060808@redhat.com \
--to=dnelson@redhat.com \
--cc=a.p.zijlstra@chello.org \
--cc=acme@kernel.org \
--cc=jolsa@kernel.org \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.