perf: Allow leader sampling on inherited events
Jiri Olsa
reviewerv2 cover perf: Allow leader sampling on inherited events
hi,
Jen-Cheng(Tommy) Huang reported the leader sampling not working on children processes:
http://www.mail-archive.com/[email protected]/msg01644.html
The leader sampling (example below) lets the group leader event (cycles) do the sampling and reads the rest of the group (cache-misses) via PERF_FORMAT_GROUP format.
Example:
$ perf record -e '{cycles,cache-misses}:S' <workload>
$ perf report --groupThe perf report --group allows to see all events group data in single view.
The reason for leader sampling being switched off for inherited events, is that the kernel does no allow PERF_FORMAT_GROUP format on inherited events (which is used by leader sampling).
I switched on the PERF_FORMAT_GROUP format for inherited events with few other fixies in patches:
perf: Deny optimized switch for events read by PERF_SAMPLE_READ
perf: Allow PERF_FORMAT_GROUP format on inherited events
And I fixed perf tool code to be able to process data from children processes.
Anyway, I might have missed some other reason why this was never switched on in kernel, so sending this as RFC.
v2 changes:
- detection of the kernel support
- added perf script support for period column
- separate kernel change for context_equiv change
thanks for comments,
jirka
---
Jiri Olsa (9):
perf: Remove redundant parent context check from context_equiv
perf: Deny optimized switch for events read by PERF_SAMPLE_READ
perf: Allow PERF_FORMAT_GROUP format on inherited events perf tools: Add support to traverse xyarrays
perf tools: Add pr_warning_once debug macro
perf tools: Add hash of periods for struct perf_sample_id perf tools: Allow PERF_FORMAT_GROUP for inherited events perf script: Add period data column
perf script: Add period as a default output column
kernel/events/core.c | 33 ++++++++++++++------------------ tools/perf/Documentation/perf-script.txt | 2 +- tools/perf/Makefile.perf | 1 + tools/perf/builtin-script.c | 21 +++++++++++++++++---- tools/perf/perf.h | 1 + tools/perf/tests/builtin-test.c | 4 ++++ tools/perf/tests/tests.h | 1 + tools/perf/tests/xyarray.c | 33 ++++++++++++++++++++++++++++++++ tools/perf/util/debug.h | 12 ++++++++++++ tools/perf/util/evsel.c | 19 ++++++++++++++++++- tools/perf/util/evsel.h | 5 ++++- tools/perf/util/record.c | 12 ++++++++++++ tools/perf/util/session.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ tools/perf/util/xyarray.c | 4 +++- tools/perf/util/xyarray.h | 6 ++++++ 15 files changed, 214 insertions(+), 33 deletions(-) create mode 100644 tools/perf/tests/xyarray.c
Jiri Olsa
reviewerPATCH v2 1/9 perf: Remove redundant parent context check from context_equiv
As described in commit:
1f9a7268c67f perf: Do not allow optimized switch for non-cloned events
we dont allow optimized switch for non cloned contexts.
There's no need now to check if one of the context is parent of the other in context_equiv function.
--- kernel/events/core.c | 8 -------- 1 file changed, 8 deletions(-)
kernel/events/core.c +0 -8
@@ -2257,14 +2257,6 @@ static int context_equiv(struct perf_event_context *ctx1,if (ctx1->pin_count || ctx2->pin_count)return 0 ; -/* If ctx1 is the parent of ctx2 */ -if (ctx1 == ctx2->parent_ctx && ctx1->generation == ctx2->parent_gen) -return 1 ; - -/* If ctx2 is the parent of ctx1 */ -if (ctx1->parent_ctx == ctx2 && ctx1->parent_gen == ctx2->generation) -return 1 ; -/* * If ctx1 and ctx2 have the same parent; we flatten the parent * hierarchy, see perf_event_init_context().
Signature · 1 line
1.8.3.1
Peter Zijlstra
maintainerRe: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 7 lines · Jiri Olsa
As described in commit:
1f9a7268c67fperf: Do not allow optimized switch for non-cloned events
we dont allow optimized switch for non cloned contexts.There's no need now to check if one of the context is parent of the other in
context_equivfunction.
OK, so talk to me about that patch; it looks like you're slowly
reverting: 5a3126d4fe7c ("perf: Fix the perf context switch optimization").
So what exactly is the problem? I'm still jet-lagged and not seeing it.
Re: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 9 lines · Peter Zijlstra
Quoted 7 lines · Jiri Olsa
As described in commit:
1f9a7268c67fperf: Do not allow optimized switch for non-cloned events
we dont allow optimized switch for non cloned contexts.There's no need now to check if one of the context is parent of the other in
context_equivfunction.
Quoted 5 lines
OK, so talk to me about that patch; it looks like you're slowly
reverting:5a3126d4fe7c("perf: Fix the perf context switch optimization").So what exactly is the problem? I'm still jet-lagged and not seeing it.
the problem is, that we cannot allow non-cloned context to be part of the optimized switch for reasons explained in commit 1f9a7268c67f
I did not realize there was a just single commit introducing non-cloned context switch to revert ;-) attached
I haven't tested it yet.. will do with my other changes
thanks,
jirka
---
This reverts commit 5a3126d4fe7c311fe12f98fef0470f6cb582d1ef.
As explained in commit:
1f9a7268c67f perf: Do not allow optimized switch for non-cloned events
we cannot allow optimized switch for non cloned contexts. It suppressed it by forcing the condition for parent contexts.
But what we actually should do is to revert the full commit that introduced the optimized switch for non-cloned contexts. This way we get rid of unneeded (now) ctx->generation updates for non-cloned contexts.
--- kernel/events/core.c | 64 +++++++++++++++------------------------------------- 1 file changed, 18 insertions(+), 46 deletions(-)
kernel/events/core.c +18 -46
@@ -916,7 +916,6 @@ static void unclone_ctx(struct perf_event_context *ctx)put_ctx (ctx->parent_ctx); ctx->parent_ctx =NULL ; } - ctx->generation++; }static u32 perf_event_pid (struct perf_event *event,struct task_struct *p)
@@ -1154,8 +1153,6 @@ list_add_event(struct perf_event *event, struct perf_event_context *ctx) ctx->nr_events++;if (event->attr.inherit_stat) ctx->nr_stat++; - - ctx->generation++; }/*
@@ -1333,8 +1330,6 @@ list_del_event(struct perf_event *event, struct perf_event_context *ctx)*/ if (event->state >PERF_EVENT_STATE_OFF ) event->state =PERF_EVENT_STATE_OFF ; - - ctx->generation++; }static void perf_group_detach (struct perf_event *event)
@@ -2243,38 +2238,22 @@ static void ctx_sched_out(struct perf_event_context *ctx, }/* -* Test whether two contexts are equivalent, i.e. whether they have both been -* cloned from the same version of the same context. -* -* Equivalence is measured using a generation number in the context that is -* incremented on each modification to it; see unclone_ctx(), list_add_event() -* and list_del_event(). +* Test whether two contexts are equivalent, i.e. whether they +* have both been cloned from the same version of the same context +* and they both have the same number of enabled events. +* If the number of enabled events is the same, then the set +* of enabled events should be the same, because these are both +* inherited contexts, therefore we can't access individual events +* in them directly with an fd; we can only enable/disable all +* events via prctl, or enable/disable all events in a family +* via ioctl, which will have the same effect on both contexts. */ static int context_equiv (struct perf_event_context *ctx1,struct perf_event_context *ctx2) { -/* Pinning disables the swap optimization */ -if (ctx1->pin_count || ctx2->pin_count) -return 0 ; - -/* If ctx1 is the parent of ctx2 */ -if (ctx1 == ctx2->parent_ctx && ctx1->generation == ctx2->parent_gen) -return 1 ; - -/* If ctx2 is the parent of ctx1 */ -if (ctx1->parent_ctx == ctx2 && ctx1->parent_gen == ctx2->generation) -return 1 ; - -/* -* If ctx1 and ctx2 have the same parent; we flatten the parent -* hierarchy, see perf_event_init_context(). -*/ -if (ctx1->parent_ctx && ctx1->parent_ctx == ctx2->parent_ctx && - ctx1->parent_gen == ctx2->parent_gen) -return 1 ; - -/* Unmatched */ -return 0 ; +return ctx1->parent_ctx && ctx1->parent_ctx == ctx2->parent_ctx + && ctx1->parent_gen == ctx2->parent_gen + && !ctx1->pin_count && !ctx2->pin_count; } static void __perf_event_sync_stat(struct perf_event *event,
@@ -2354,7 +2333,7 @@ static void perf_event_context_sched_out(struct task_struct *task, int ctxn, {struct perf_event_context *ctx = task->perf_event_ctxp[ctxn];struct perf_event_context *next_ctx; -struct perf_event_context *parent, *next_parent; +struct perf_event_context *parent;struct perf_cpu_context *cpuctx;int do_switch =1 ;
@@ -2366,18 +2345,10 @@ static void perf_event_context_sched_out(struct task_struct *task, int ctxn,return ;rcu_read_lock (); - next_ctx = next->perf_event_ctxp[ctxn]; -if (!next_ctx) - goto unlock; - parent =rcu_dereference (ctx->parent_ctx); - next_parent =rcu_dereference (next_ctx->parent_ctx); - -/* If neither context have a parent context; they cannot be clones. */ -if (!parent || !next_parent) - goto unlock; - -if (next_parent == ctx || next_ctx == parent || next_parent == parent) { + next_ctx = next->perf_event_ctxp[ctxn]; +if (parent && next_ctx && +rcu_dereference (next_ctx->parent_ctx) == parent) {/* * Looks like the two contexts are clones, so we might be * able to optimize the context switch. We lock both
@@ -2405,7 +2376,6 @@ static void perf_event_context_sched_out(struct task_struct *task, int ctxn,raw_spin_unlock (&next_ctx->lock);raw_spin_unlock (&ctx->lock); } -unlock:rcu_read_unlock ();if (do_switch) {
@@ -7398,6 +7368,7 @@ SYSCALL_DEFINE5(perf_event_open, }perf_install_in_context (ctx, event, event->cpu); + ++ctx->generation;perf_unpin_context (ctx);mutex_unlock (&ctx->mutex);
@@ -7484,6 +7455,7 @@ perf_event_create_kernel_counter(struct perf_event_attr *attr, int cpu,WARN_ON_ONCE (ctx->parent_ctx);mutex_lock (&ctx->mutex);perf_install_in_context (ctx, event, cpu); + ++ctx->generation;perf_unpin_context (ctx);mutex_unlock (&ctx->mutex);
Signature · 1 line
1.8.3.1
Peter Zijlstra
maintainerRe: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 14 lines · Jiri Olsa
Quoted 13 lines · Peter Zijlstra
Quoted 7 lines · Jiri Olsa
As described in commit:
1f9a7268c67fperf: Do not allow optimized switch for non-cloned events
we dont allow optimized switch for non cloned contexts.There's no need now to check if one of the context is parent of the other in
context_equivfunction.OK, so talk to me about that patch; it looks like you're slowly
reverting:5a3126d4fe7c("perf: Fix the perf context switch optimization").
Quoted 5 lines
Quoted 1 line
So what exactly is the problem? I'm still jet-lagged and not seeing it.
the problem is, that we cannot allow non-cloned context to be part of the optimized switch for reasons explained in commit
1f9a7268c67f
The thing is; I don't understand those reasons. That commit log doesn't explain.
Peter Zijlstra
maintainerRe: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 15 lines · Peter Zijlstra
Quoted 14 lines · Jiri Olsa
Quoted 13 lines · Peter Zijlstra
Quoted 7 lines · Jiri Olsa
As described in commit:
1f9a7268c67fperf: Do not allow optimized switch for non-cloned events
we dont allow optimized switch for non cloned contexts.There's no need now to check if one of the context is parent of the other in
context_equivfunction.OK, so talk to me about that patch; it looks like you're slowly
reverting:5a3126d4fe7c("perf: Fix the perf context switch optimization").
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
So what exactly is the problem? I'm still jet-lagged and not seeing it.
the problem is, that we cannot allow non-cloned context to be part of the optimized switch for reasons explained in commit
1f9a7268c67fThe thing is; I don't understand those reasons. That commit log doesn't explain.
Ah wait, I finally see. I think we want to fix that exit path, not disallow the cloning.
The thing is, by not allowing this optimization simple things like eg. pipe-test say very expensive.
Peter Zijlstra
maintainerRe: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 8 lines · Peter Zijlstra
Quoted 2 lines
The thing is; I don't understand those reasons. That commit log doesn't explain.
Ah wait, I finally see. I think we want to fix that exit path, not disallow the cloning.
The thing is, by not allowing this optimization simple things like eg. pipe-test say very expensive.
So its 179033b3e064 ("perf: Add PERF_EVENT_STATE_EXIT state for events with exited task") that introduces the problem. Before that things would work correctly afaict.
The exit would remove from the context but leave the event in existence. Both the fd and the inherited events would have references to it, only once those are gone do we destroy the actual event.
Peter Zijlstra
maintainerRe: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 11 lines · Peter Zijlstra
Quoted 8 lines · Peter Zijlstra
Quoted 2 lines
The thing is; I don't understand those reasons. That commit log doesn't explain.
Ah wait, I finally see. I think we want to fix that exit path, not disallow the cloning.
The thing is, by not allowing this optimization simple things like eg. pipe-test say very expensive.
Quoted 7 lines
So its
179033b3e064("perf: AddPERF_EVENT_STATE_EXITstate for events with exited task") that introduces the problem. Before that things would work correctly afaict.The exit would remove from the context but leave the event in existence. Both the fd and the inherited events would have references to it, only once those are gone do we destroy the actual event.
I have another 'problem' with 179033b3e064. What if you 'want' to continue monitoring after the initial task died? Eg. if you want to monitor crap that unconditionally daemonizes.
So at this point I would suggest we revert both 179033b3e064 and 1f9a7268c67f so that the context switch optimization works again for simple 2 task things (pipe-test).
If we re-introduce 179033b3e064 (which I think makes sense) we need to make sure it works with the context switch optimization (we could add perf_event::task I think, it would clean up a few other things iirc) and we'd have to make it conditional so we can still monitor daemons.
Or am I totally missing things again?
Re: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 16 lines · Peter Zijlstra
Quoted 15 lines · Peter Zijlstra
Quoted 8 lines · Peter Zijlstra
Quoted 2 lines
The thing is; I don't understand those reasons. That commit log doesn't explain.
Ah wait, I finally see. I think we want to fix that exit path, not disallow the cloning.
The thing is, by not allowing this optimization simple things like eg. pipe-test say very expensive.
So its
179033b3e064("perf: AddPERF_EVENT_STATE_EXITstate for events with exited task") that introduces the problem. Before that things would work correctly afaict.
Quoted 7 lines
Quoted 3 lines
The exit would remove from the context but leave the event in existence. Both the fd and the inherited events would have references to it, only once those are gone do we destroy the actual event.
I have another 'problem' with
179033b3e064. What if you 'want' to continue monitoring after the initial task died? Eg. if you want to monitor crap that unconditionally daemonizes.
right.. did not think of that.. need to check more, but seems like just the check for children should be enough
jirka
---
kernel/events/core.c +15 -3
@@ -3568,6 +3568,19 @@ static int perf_event_read_one(struct perf_event *event,return n *sizeof (u64); } +static bool is_event_hup (struct perf_event *event) +{ +bool no_children; + +if (event->state !=PERF_EVENT_STATE_EXIT ) +return false; + +mutex_lock (&event->child_mutex); + no_children =list_empty (&event->child_list); +mutex_unlock (&event->child_mutex); +return no_children; +} +/* * Read the performance event - simple non blocking version for now */
@@ -3582,8 +3595,7 @@ perf_read_hw(struct perf_event *event, char __user *buf, size_t count)* error state (i.e. because it was pinned but it couldn't be * scheduled on to the CPU at some point). */ -if ((event->state ==PERF_EVENT_STATE_ERROR ) || - (event->state ==PERF_EVENT_STATE_EXIT )) +if ((event->state ==PERF_EVENT_STATE_ERROR ) || (is_event_hup (event)))return 0 ;if (count < event->read_size)
@@ -3614,7 +3626,7 @@ static unsigned int perf_poll(struct file *file, poll_table *wait)poll_wait (file, &event->waitq, wait); -if (event->state ==PERF_EVENT_STATE_EXIT ) +if (is_event_hup (event))return events;/*
Peter Zijlstra
maintainerRe: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 7 lines · Jiri Olsa
Quoted 3 lines
I have another 'problem' with
179033b3e064. What if you 'want' to continue monitoring after the initial task died? Eg. if you want to monitor crap that unconditionally daemonizes.right.. did not think of that.. need to check more, but seems like just the check for children should be enough
Indeed, that should work.
Quoted 26 lines
---
kernel/events/core.c +13
-0@@ -3568,6 +3568,19 @@ static int perf_event_read_one(struct perf_event *event,return n *sizeof (u64); } +static bool is_event_hup (struct perf_event *event) +{ +bool no_children; + +if (event->state !=PERF_EVENT_STATE_EXIT ) +return false; + +mutex_lock (&event->child_mutex); + no_children =list_empty (&event->child_list); +mutex_unlock (&event->child_mutex); +return no_children; +} +/* * Read the performance event - simple non blocking version for now */
kernel/events/core.c perf_read_hw(struct perf_event *event, char __user *buf, size_t count) · line 3599
Quoted 7 lines
kernel/events/core.c +1
-2@@ -3582,8 +3595,7 @@ perf_read_hw(struct perf_event *event, char __user *buf, size_t count)* error state (i.e. because it was pinned but it couldn't be * scheduled on to the CPU at some point). */ -if ((event->state ==PERF_EVENT_STATE_ERROR ) || - (event->state ==PERF_EVENT_STATE_EXIT )) +if ((event->state ==PERF_EVENT_STATE_ERROR ) || (is_event_hup (event)))return 0 ;
Do we want this? It seems like a fairly sensible thing to start a counter and wait for the thing to die, only to then read the total count. But with this on we get 0s.
I suppose Stephane's email got to you after you did this and we should be dropping this thing entirely?
Quoted 10 lines
kernel/events/core.c +1
-1if (count < event->read_size)@@ -3614,7 +3626,7 @@ static unsigned int perf_poll(struct file *file, poll_table *wait)poll_wait (file, &event->waitq, wait); -if (event->state ==PERF_EVENT_STATE_EXIT ) +if (is_event_hup (event))return events;/*
Re: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
In reply to Peter ZijlstraQuoted 28 lines · Peter Zijlstra
Quoted 11 lines · Peter Zijlstra
Quoted 8 lines · Peter Zijlstra
Quoted 2 lines
The thing is; I don't understand those reasons. That commit log doesn't explain.
Ah wait, I finally see. I think we want to fix that exit path, not disallow the cloning.
The thing is, by not allowing this optimization simple things like eg. pipe-test say very expensive.
Quoted 7 lines
So its
179033b3e064("perf: AddPERF_EVENT_STATE_EXITstate for events with exited task") that introduces the problem. Before that things would work correctly afaict.The exit would remove from the context but leave the event in existence. Both the fd and the inherited events would have references to it, only once those are gone do we destroy the actual event.
I have another 'problem' with
179033b3e064. What if you 'want' to continue monitoring after the initial task died? Eg. if you want to monitor crap that unconditionally daemonizes.So at this point I would suggest we revert both
179033b3e064and1f9a7268c67fso that the context switch optimization works again for simple 2 task things (pipe-test).
Quoted 6 lines
If we re-introduce
179033b3e064(which I think makes sense) we need to make sure it works with the context switch optimization (we could addperf_event::taskI think, it would clean up a few other things iirc) and we'd have to make it conditional so we can still monitor daemons.Or am I totally missing things again?
as I said in previous email I dont think the 179033b3e064 introduced the issue and I think we should keep 1f9a7268c67f to have correct numbers ;-)
I'll think about the fix to keep non-cloned context in optimized switch
jirka
Re: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
In reply to Peter ZijlstraQuoted 8 lines · Peter Zijlstra
Quoted 6 lines · Peter Zijlstra
Quoted 2 lines
The thing is; I don't understand those reasons. That commit log doesn't explain.
Ah wait, I finally see. I think we want to fix that exit path, not disallow the cloning.
Quoted 6 lines
Quoted 2 lines
The thing is, by not allowing this optimization simple things like eg. pipe-test say very expensive.
So its
179033b3e064("perf: AddPERF_EVENT_STATE_EXITstate for events with exited task") that introduces the problem. Before that things would work correctly afaict.
hum, I dont think so.. because the perf_remove_from_context set event to PERF_EVENT_STATE_OFF state anyway.. thus making any new cloned events disabled
the PERF_EVENT_STATE_EXIT should be just notification for poll to return HUP
Quoted 3 lines
The exit would remove from the context but leave the event in existence. Both the fd and the inherited events would have references to it, only once those are gone do we destroy the actual event.
Peter Zijlstra
maintainerRe: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 12 lines · Jiri Olsa
Quoted 11 lines · Peter Zijlstra
Quoted 8 lines · Peter Zijlstra
Quoted 2 lines
The thing is; I don't understand those reasons. That commit log doesn't explain.
Ah wait, I finally see. I think we want to fix that exit path, not disallow the cloning.
The thing is, by not allowing this optimization simple things like eg. pipe-test say very expensive.
Quoted 7 lines
Quoted 3 lines
So its
179033b3e064("perf: AddPERF_EVENT_STATE_EXITstate for events with exited task") that introduces the problem. Before that things would work correctly afaict.hum, I dont think so.. because the
perf_remove_from_contextset event toPERF_EVENT_STATE_OFFstate anyway.. thus making any new cloned events disabled
Urgh, see I knew I was missing something.
Can't we fix that? Lemme check to see what relies on this.
Peter Zijlstra
maintainerRe: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 17 lines · Peter Zijlstra
Quoted 16 lines · Jiri Olsa
Quoted 14 lines · Peter Zijlstra
Quoted 8 lines · Peter Zijlstra
Quoted 2 lines
The thing is; I don't understand those reasons. That commit log doesn't explain.
Ah wait, I finally see. I think we want to fix that exit path, not disallow the cloning.
The thing is, by not allowing this optimization simple things like eg. pipe-test say very expensive.
So its
179033b3e064("perf: AddPERF_EVENT_STATE_EXITstate for events with exited task") that introduces the problem. Before that things would work correctly afaict.
Quoted 7 lines
Quoted 3 lines
hum, I dont think so.. because the
perf_remove_from_contextset event toPERF_EVENT_STATE_OFFstate anyway.. thus making any new cloned events disabledUrgh, see I knew I was missing something.
Can't we fix that? Lemme check to see what relies on this.
2e2af50b1fab ("perf_events: Disable events when we detach them")
Seems to be about it. And I think we should solve that differently, but the best I can come up with ties into the event->ctx mess we have in that other thread.
The thing is, IOC_ENABLE/DISABLE and read() and such should act (sanely and) independent from the attached state.
Its just that the whole event->ctx migration mess is making this somewhat hard atm.
So things like perf_event_read() should not only check ctx->is_active but also worry about event->attach_state & PERF_ATTACH_CONTEXT.
Now the biggest problem is that we cannot tell if its a temporary state (move_group / migrate_context) or permanent (exit)...
Urgh
Re: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 38 lines · Peter Zijlstra
Quoted 17 lines · Peter Zijlstra
Quoted 16 lines · Jiri Olsa
Quoted 14 lines · Peter Zijlstra
Quoted 8 lines · Peter Zijlstra
Quoted 2 lines
The thing is; I don't understand those reasons. That commit log doesn't explain.
Ah wait, I finally see. I think we want to fix that exit path, not disallow the cloning.
The thing is, by not allowing this optimization simple things like eg. pipe-test say very expensive.
So its
179033b3e064("perf: AddPERF_EVENT_STATE_EXITstate for events with exited task") that introduces the problem. Before that things would work correctly afaict.Quoted 7 lines
Quoted 3 lines
hum, I dont think so.. because the
perf_remove_from_contextset event toPERF_EVENT_STATE_OFFstate anyway.. thus making any new cloned events disabledUrgh, see I knew I was missing something.
Can't we fix that? Lemme check to see what relies on this.
2e2af50b1fab("perf_events: Disable events when we detach them")Seems to be about it. And I think we should solve that differently, but the best I can come up with ties into the
event->ctxmess we have in that other thread.The thing is, IOC_ENABLE/DISABLE and
read()and such should act (sanely and) independent from the attached state.Its just that the whole
event->ctxmigration mess is making this somewhat hard atm.
Quoted 7 lines
So things like
perf_event_read()should not only checkctx->is_activebut also worry aboutevent->attach_state&PERF_ATTACH_CONTEXT.Now the biggest problem is that we cannot tell if its a temporary state (
move_group/migrate_context) or permanent (exit)...Urgh
I just noticed that we initialize the child state with base parent state not the real (immediate) parent.. which is what we want IMO
I wonder attached patch could fix the issue mentioned in:
1f9a726 perf: Do not allow optimized switch for non-cloned events
now I need to recall what I used to test this ;-)
jirka
---
kernel/events/core.c +2 -1
@@ -7794,6 +7794,7 @@ inherit_event(struct perf_event *parent_event,struct perf_event *group_leader, struct perf_event_context *child_ctx) { + enum perf_event_active_state parent_state = parent_event->state;struct perf_event *child_event;unsigned long flags;
@@ -7827,7 +7828,7 @@ inherit_event(struct perf_event *parent_event,* not its attr.disabled bit. We hold the parent's mutex, * so we won't race with perf_event_{en, dis}able_family. */ -if (parent_event->state >=PERF_EVENT_STATE_INACTIVE ) +if (parent_state >=PERF_EVENT_STATE_INACTIVE ) child_event->state =PERF_EVENT_STATE_INACTIVE ;else child_event->state =PERF_EVENT_STATE_OFF ;
Peter Zijlstra
maintainerRe: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 7 lines · Jiri Olsa
I just noticed that we initialize the child state with base parent state not the real (immediate) parent.. which is what we want IMO
I wonder attached patch could fix the issue mentioned in:
1f9a726 perf: Do not allow optimized switch for non-cloned eventsnow I need to recall what I used to test this ;-)
Ah, very nice! Yes this might just do it.
Quoted 22 lines
---
kernel/events/core.c +2
-1@@ -7794,6 +7794,7 @@ inherit_event(struct perf_event *parent_event,struct perf_event *group_leader, struct perf_event_context *child_ctx) { + enum perf_event_active_state parent_state = parent_event->state;struct perf_event *child_event;unsigned long flags;@@ -7827,7 +7828,7 @@ inherit_event(struct perf_event *parent_event,* not its attr.disabled bit. We hold the parent's mutex, * so we won't race with perf_event_{en, dis}able_family. */ -if (parent_event->state >=PERF_EVENT_STATE_INACTIVE ) +if (parent_state >=PERF_EVENT_STATE_INACTIVE ) child_event->state =PERF_EVENT_STATE_INACTIVE ;else child_event->state =PERF_EVENT_STATE_OFF ;
Peter Zijlstra
maintainerRe: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
In reply to Jiri OlsaQuoted 3 lines · Jiri Olsa
now I need to recall what I used to test this ;-)
Hint, if you'd mentioned that in the changelog.... :-)
Re: [PATCH 1/9] perf: Remove redundant parent context check from context_equiv
Quoted 6 lines · Peter Zijlstra
Quoted 3 lines · Jiri Olsa
now I need to recall what I used to test this ;-)
Hint, if you'd mentioned that in the changelog.... :-)
will try much better this time.. perf test attached ;-) seems like the child state fix is a good one
jirka
---
Quoted 1 line
From
dc1068c627b332b2a5feb4deedfd96db8e056b5fMon Sep 17 00:00:00 2001
From: Jiri Olsa <[email protected]>
Date: Wed, 10 Sep 2014 16:23:51 +0200
Subject: [PATCH] perf tests: Add test for optimized sched switch bug
--- tools/perf/Makefile.perf | 1 + tools/perf/tests/builtin-test.c | 4 + tools/perf/tests/optimized_switch.c | 169 +++++++++++++++++++++++++++++++++++ tools/perf/tests/tests.h | 1 + 4 files changed, 175 insertions(+), 0 deletions(-) create mode 100644 tools/perf/tests/optimized_switch.c
tools/perf/Makefile.perf +1 -0
@@ -427,6 +427,7 @@ LIB_OBJS += $(OUTPUT)tests/mmap-thread-lookup.o LIB_OBJS += $(OUTPUT)tests/thread-mg-share.o LIB_OBJS += $(OUTPUT)tests/switch-tracking.o LIB_OBJS += $(OUTPUT)tests/xyarray.o +LIB_OBJS += $(OUTPUT)tests/optimized_switch.o BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
tools/perf/tests/builtin-test.c +4 -0
@@ -162,6 +162,10 @@ static struct test { .func = test__xyarray, }, { + .desc = "Test optimized switch", + .func = test__optimized_switch, + }, + { .func = NULL, }, };
tools/perf/tests/optimized_switch.c new file +169 -0
169 lines not on this page · raw message
tools/perf/tests/tests.h +1 -0
@@ -50,6 +50,7 @@ int test__hists_output(void);int test__hists_cumulate (void );int test__switch_tracking (void );int test__xyarray (void ); +int test__optimized_switch (void );#if defined(__x86_64__) || defined(__i386__) || defined(__arm__)#ifdef HAVE_DWARF_UNWIND_SUPPORT
Signature · 1 line
1.7.7.6
tip-bot for Jiri Olsa
merge bot[tip:perf/core] Revert "perf: Do not allow optimized switch for non-cloned events"
Revert "perf: Do not allow optimized switch for non-cloned events"
Commit-ID: 802c8a61d4c9c794db863dcabb0006ab001a651b Gitweb: http://git.kernel.org/tip/802c8a61d4c9c794db863dcabb0006ab001a651b Author: Jiri Olsa <[email protected]> AuthorDate: Fri, 12 Sep 2014 13:18:28 +0200 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Sep 2014 14:48:13 +0200
Revert "perf: Do not allow optimized switch for non-cloned events"
This reverts commit 1f9a7268c67f0290837aada443d28fd953ddca90.
With the fix of the initial state for the cloned event we now correctly handle the error described in:
1f9a7268c67f perf: Do not allow optimized switch for non-cloned events
so we can revert it.
I made an automated test for this, but its not suitable for automated perf tests framework. It needs to be customized for each machine (the more cpu the higher numbers for GROUPS/WORKERS/BYTES) and it could take longer time to hit the issue.
--- kernel/events/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/events/core.c +1 -1
@@ -2375,7 +2375,7 @@ static void perf_event_context_sched_out(struct task_struct *task, int ctxn, next_parent =rcu_dereference (next_ctx->parent_ctx);/* If neither context have a parent context; they cannot be clones. */ -if (!parent || !next_parent) +if (!parent && !next_parent) goto unlock;if (next_parent == ctx || next_ctx == parent || next_parent == parent) {
Jiri Olsa
reviewerPATCH v2 2/9 perf: Deny optimized switch for events read by PERF_SAMPLE_READ
In reply to Jiri OlsaThe optimized task context switch for cloned perf events just swaps whole perf event contexts (of current and next process) if it finds them suitable. Events from the 'current' context will now measure data of the 'next' context and vice versa.
This is ok for cases where we are not directly interested in the event->count value of separate child events, like:
- standard sampling, where we take 'period' value for the event count
- counting, where we accumulate all events (children) into a single count value
But in case we read event by using the PERF_SAMPLE_READ sample type, we are interested in direct event->count value measured in specific task. Switching events within tasks for this kind of measurements corrupts data.
Fixing this by setting/unsetting pin_count for perf event context once cloned event with PERF_SAMPLE_READ read is added/removed. The pin_count value != 0 makes the context not suitable for optimized switch.
--- kernel/events/core.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
kernel/events/core.c +12 -0
@@ -1117,6 +1117,12 @@ ctx_group_list(struct perf_event *event, struct perf_event_context *ctx)return &ctx->flexible_groups; } +static bool is_clone_with_read (struct perf_event *event) +{ +return event->parent && + (event->attr.sample_type &PERF_SAMPLE_READ ); +} +/* * Add a event from the lists for its context. * Must be called with ctx->mutex and ctx->lock held.
@@ -1148,6 +1154,9 @@ list_add_event(struct perf_event *event, struct perf_event_context *ctx)if (has_branch_stack (event)) ctx->nr_branch_stack++; +if (is_clone_with_read (event)) + ctx->pin_count++; +list_add_rcu (&event->event_entry, &ctx->event_list);if (!ctx->nr_events)perf_pmu_rotate_start (ctx->pmu);
@@ -1313,6 +1322,9 @@ list_del_event(struct perf_event *event, struct perf_event_context *ctx)if (has_branch_stack (event)) ctx->nr_branch_stack--; +if (is_clone_with_read (event)) + ctx->pin_count--; + ctx->nr_events--;if (event->attr.inherit_stat) ctx->nr_stat--;
Signature · 1 line
1.8.3.1
Peter Zijlstra
maintainerRe: [PATCH 2/9] perf: Deny optimized switch for events read by PERF_SAMPLE_READ
Quoted 17 lines · Jiri Olsa
The optimized task context switch for cloned perf events just swaps whole perf event contexts (of current and next process) if it finds them suitable. Events from the 'current' context will now measure data of the 'next' context and vice versa.
This is ok for cases where we are not directly interested in the
event->countvalue of separate child events, like:
- standard sampling, where we take 'period' value for the event count
- counting, where we accumulate all events (children) into a single count valueBut in case we read event by using the
PERF_SAMPLE_READsample type, we are interested in directevent->countvalue measured in specific task. Switching events within tasks for this kind of measurements corrupts data.
Quoted 4 lines
Fixing this by setting/unsetting
pin_countfor perf event context once cloned event withPERF_SAMPLE_READread is added/removed. Thepin_countvalue != 0 makes the context not suitable for optimized switch.
no.. so the value of the counter is the sum of all the inherited events. It doesn't matter if you flip it or not the sum is not affected.
PERF_SAMPLE_READ should return the value.
Re: [PATCH 2/9] perf: Deny optimized switch for events read by PERF_SAMPLE_READ
Quoted 23 lines · Peter Zijlstra
Quoted 17 lines · Jiri Olsa
The optimized task context switch for cloned perf events just swaps whole perf event contexts (of current and next process) if it finds them suitable. Events from the 'current' context will now measure data of the 'next' context and vice versa.
This is ok for cases where we are not directly interested in the
event->countvalue of separate child events, like:
- standard sampling, where we take 'period' value for the event count
- counting, where we accumulate all events (children) into a single count valueBut in case we read event by using the
PERF_SAMPLE_READsample type, we are interested in directevent->countvalue measured in specific task. Switching events within tasks for this kind of measurements corrupts data.Quoted 4 lines
Fixing this by setting/unsetting
pin_countfor perf event context once cloned event withPERF_SAMPLE_READread is added/removed. Thepin_countvalue != 0 makes the context not suitable for optimized switch.
Quoted 4 lines
no.. so the value of the counter is the sum of all the inherited events. It doesn't matter if you flip it or not the sum is not affected.
PERF_SAMPLE_READshould return the value.
so I want to be able to do the leader sampling over child processes
that means:
- have event group with sampling leader, and the rest of the group events' periods being read on leader's sample via PERF_SAMPLE_READ sample_type
- for each child process created I want it to do the same thing as the parent - sample on leader, read values of other events in group via PERF_SAMPLE_READ
Now, If I let the optimized switch enabled for above config, I'll get wrong data, because the period counts of group events are local to the child process. Optimized switch will move it to another child.
jirka
Peter Zijlstra
maintainerRe: [PATCH 2/9] perf: Deny optimized switch for events read by PERF_SAMPLE_READ
Quoted 5 lines · Jiri Olsa
Quoted 4 lines
no.. so the value of the counter is the sum of all the inherited events. It doesn't matter if you flip it or not the sum is not affected.
PERF_SAMPLE_READshould return the value.
Quoted 8 lines
so I want to be able to do the leader sampling over child processes that means:
- have event group with sampling leader, and the rest of the group events' periods being read on leader's sample viaPERF_SAMPLE_READsample_type
- for each child process created I want it to do the same thing as the parent - sample on leader, read values of other events in group viaPERF_SAMPLE_READ
The thing is, PERF_SAMPLE_READ should return the exact same thing as an actual read(), and for read() the value reported is the sum of all events.
I _think_ I know what you want, and I think I see why you'd want that, but I'm not sure how to best do that.. Hmm.. maybe look at 'perf_event_attr::inherit_stat' that appears to be similar.
At which point read() vs inherit_stat might be 'broken'. Hard to know if anybody uses that :-(
Re: [PATCH 2/9] perf: Deny optimized switch for events read by PERF_SAMPLE_READ
Quoted 16 lines · Peter Zijlstra
Quoted 5 lines · Jiri Olsa
Quoted 4 lines
no.. so the value of the counter is the sum of all the inherited events. It doesn't matter if you flip it or not the sum is not affected.
PERF_SAMPLE_READshould return the value.Quoted 8 lines
so I want to be able to do the leader sampling over child processes that means:
- have event group with sampling leader, and the rest of the group events' periods being read on leader's sample viaPERF_SAMPLE_READsample_type
- for each child process created I want it to do the same thing as the parent - sample on leader, read values of other events in group viaPERF_SAMPLE_READ
Quoted 3 lines
The thing is,
PERF_SAMPLE_READshould return the exact same thing as an actualread(), and forread()the value reported is the sum of all events.
maybe we could make this configurable? like via new per_event_attr::option bit, keeping by default the behaviour you said
Quoted 7 lines
I _think_ I know what you want, and I think I see why you'd want that, but I'm not sure how to best do that.. Hmm.. maybe look at '
perf_event_attr::inherit_stat' that appears to be similar.At which point
read()vsinherit_statmight be 'broken'. Hard to know if anybody uses that :-(
I'll check, thanks
jirka
Jiri Olsa
reviewerPATCH v2 3/9 perf: Allow PERF_FORMAT_GROUP format on inherited events
In reply to Jiri OlsaI assume the reason for this being disabled is the difficulty to read child events once in perf overflow routine, thus the perf_output_read_group function. The read syscall function perf_event_read_group seems to handle this nicely.
My goal is to be able to read all events in group on leader sample by using the PERF_SAMPLE_READ with PERF_FORMAT_GROUP format. Once the monitored process forks, I need the child processes/events do the same and store samples into parents ring buffer.
So I need all events sample just to report their own value (without child events being included). Thus switching the perf_event_count call for simple read of event->count.
--- kernel/events/core.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)
kernel/events/core.c +2 -11
@@ -4608,9 +4608,6 @@ static void perf_output_read_one(struct perf_output_handle *handle,__output_copy (handle, values, n *sizeof (u64)); } -/* -* XXX PERF_FORMAT_GROUP vs inherited events seems difficult. -*/ static void perf_output_read_group (struct perf_output_handle *handle,struct perf_event *event,u64 enabled,u64 running)
@@ -4631,7 +4628,7 @@ static void perf_output_read_group(struct perf_output_handle *handle,if (leader != event) leader->pmu->read (leader); - values[n++] =perf_event_count (leader); + values[n++] =local64_read (&leader->count);if (read_format &PERF_FORMAT_ID ) values[n++] =primary_event_id (leader);
@@ -4644,7 +4641,7 @@ static void perf_output_read_group(struct perf_output_handle *handle, (sub->state == PERF_EVENT_STATE_ACTIVE)) sub->pmu->read(sub); - values[n++] =perf_event_count (sub); + values[n++] =local64_read (&sub->count);if (read_format &PERF_FORMAT_ID ) values[n++] =primary_event_id (sub);
@@ -6953,12 +6950,6 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,local64_set (&hwc->period_left, hwc->sample_period); -/* -* we currently do not support PERF_FORMAT_GROUP on inherited events -*/ -if (attr->inherit && (attr->read_format &PERF_FORMAT_GROUP )) - goto err_ns; - pmu =perf_init_event (event);if (!pmu) goto err_ns;
Signature · 1 line
1.8.3.1
Jiri Olsa
reviewerPATCH v2 4/9 perf tools: Add support to traverse xyarrays
In reply to Jiri OlsaAdding xyarray__for_each define to allow sequentially traverse xyarrays. It will be handy in following patch.
--- tools/perf/Makefile.perf | 1 + tools/perf/tests/builtin-test.c | 4 ++++ tools/perf/tests/tests.h | 1 + tools/perf/tests/xyarray.c | 33 +++++++++++++++++++++++++++++++++ tools/perf/util/xyarray.c | 4 +++- tools/perf/util/xyarray.h | 6 ++++++ 6 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 tools/perf/tests/xyarray.c
tools/perf/Makefile.perf +1 -0
@@ -426,6 +426,7 @@ endif LIB_OBJS += $(OUTPUT)tests/mmap-thread-lookup.o LIB_OBJS += $(OUTPUT)tests/thread-mg-share.o LIB_OBJS += $(OUTPUT)tests/switch-tracking.o +LIB_OBJS += $(OUTPUT)tests/xyarray.o BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
tools/perf/tests/builtin-test.c +4 -0
@@ -158,6 +158,10 @@ static struct test { .func = test__switch_tracking, }, { + .desc = "Test xyarray", + .func = test__xyarray, + }, + { .func = NULL, }, };
tools/perf/tests/tests.h +1 -0
@@ -49,6 +49,7 @@ int test__thread_mg_share(void);int test__hists_output (void );int test__hists_cumulate (void );int test__switch_tracking (void ); +int test__xyarray (void );#if defined(__x86_64__) || defined(__i386__) || defined(__arm__)#ifdef HAVE_DWARF_UNWIND_SUPPORT
tools/perf/tests/xyarray.c new file +33 -0
@@ -0,0 +1,33 @@ +#include "tests.h" +#include "xyarray.h" +#include "debug.h" + +struct krava { +int a; +}; + +#define X 100 +#define Y 100 + +int test__xyarray (void ) +{ +struct xyarray *a; +struct krava *k; +int x, y; + + a =xyarray__new (X ,Y ,sizeof (struct krava )); +TEST_ASSERT_VAL ("failed to allocate xyarray" , a); + +for (x =0 ; x <X ; x++) { +for (y =0 ; y <Y ; y++) { + k =xyarray__entry (a, x, y); + k->a = x *X + y; + } + } + + y =0 ; +xyarray__for_each (a, k) +TEST_ASSERT_VAL ("wrong array value" , k->a == y++); + +return 0 ; +}
tools/perf/util/xyarray.c +3 -1
@@ -4,11 +4,13 @@struct xyarray *xyarray__new (int xlen,int ylen,size_t entry_size) {size_t row_size = ylen * entry_size; -struct xyarray *xy =zalloc (sizeof (*xy) + xlen * row_size); +size_t size = xlen * row_size; +struct xyarray *xy =zalloc (sizeof (*xy) + size);if (xy !=NULL ) { xy->entry_size = entry_size; xy->row_size = row_size; + xy->size = size; }return xy;
tools/perf/util/xyarray.h +6 -0
@@ -6,6 +6,7 @@struct xyarray {size_t row_size;size_t entry_size; +size_t size;char contents[]; };
@@ -17,4 +18,9 @@ static inline void *xyarray__entry(struct xyarray *xy, int x, int y) return &xy->contents[x * xy->row_size + y * xy->entry_size]; } +#define xyarray__for_each (array, entry) \ + for (entry = (void *) &array->contents[0]; \ + (void *) entry < ((void *) array->contents + array->size); \ + entry++) +#endif /* _PERF_XYARRAY_H_ */
Signature · 1 line
1.8.3.1
Jiri Olsa
reviewerPATCH v2 5/9 perf tools: Add pr_warning_once debug macro
In reply to Jiri OlsaAdding pr_warning_once debug macro to display pr_warning message just once. It will be used in following patch.
--- tools/perf/util/debug.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)
tools/perf/util/debug.h +12 -0
@@ -31,6 +31,18 @@ extern int debug_ordered_events;#define pr_debug3 (fmt, ...) pr_debugN(3, pr_fmt(fmt), ##__VA_ARGS__)#define pr_debug4 (fmt, ...) pr_debugN(4, pr_fmt(fmt), ##__VA_ARGS__) +#define pr_once (fmt, pr, ...) \ + do { \ + static int __warned; \ + \ + if (!__warned) { \ + pr(fmt, ##__VA_ARGS__); \ + __warned = 1; \ + } \ + } while (0) + +#define pr_warning_once (fmt, ...) pr_once(fmt, pr_warning, ##__VA_ARGS__) +#define pr_time_N (n, var, t, fmt, ...) \ eprintf_time(n, var, t, fmt, ##__VA_ARGS__)
Signature · 1 line
1.8.3.1
Jiri Olsa
reviewerPATCH v2 6/9 perf tools: Add hash of periods for struct perf_sample_id
In reply to Jiri OlsaWith PERF_FORMAT_GROUP format on inherited events being allowed in kernel, we can now allow leader sampling on inherited events.
But before we actually switch it on, we need to change the sorting of PERF_SAMPLE_READ sample's data. Currently PERF_SAMPLE_READ values are sorted on event id. Now when we'll get data from all children processes we need to add TID as another sort key.
Adding hash of TIDs into each 'struct perf_sample_id' to hold event values for different TIDs.
--- tools/perf/util/evsel.c | 13 +++++++ tools/perf/util/evsel.h | 5 ++- tools/perf/util/session.c | 93 ++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 104 insertions(+), 7 deletions(-)
tools/perf/util/evsel.c +13 -0
@@ -787,8 +787,21 @@ void perf_evsel__free_fd(struct perf_evsel *evsel) evsel->fd =NULL ; } +static void free_sample_id (struct perf_evsel *evsel) +{ +struct perf_sample_id *sid; + +if (evsel->sample_id) { +xyarray__for_each (evsel->sample_id, sid) { +if (sid->periods) +perf_sample_hash__delete (sid->periods); + } + } +} +void perf_evsel__free_id (struct perf_evsel *evsel) { +free_sample_id (evsel);xyarray__delete (evsel->sample_id); evsel->sample_id =NULL ;zfree (&evsel->id);
tools/perf/util/evsel.h +4 -1
@@ -29,6 +29,7 @@ struct perf_counts { };struct perf_evsel ; +struct perf_sample_hash ;/* * Per fd, to map back from PERF_SAMPLE_ID to evsel, only used when there are
@@ -40,9 +41,11 @@ struct perf_sample_id {struct perf_evsel *evsel;/* Holds total ID period value for PERF_SAMPLE_READ processing. */ -u64 period; +struct perf_sample_hash *periods; }; +void perf_sample_hash__delete (struct perf_sample_hash *hash); +/** struct perf_evsel - event selector * * @name - Can be set to retain the original event name passed by the user,
tools/perf/util/session.c +87 -6
116 lines not on this page · raw message
Signature · 1 line
1.8.3.1
Jiri Olsa
reviewerPATCH v2 7/9 perf tools: Allow PERF_FORMAT_GROUP for inherited events
In reply to Jiri OlsaSwithing on leader sampling on inherited events.
Following command will now get data from all children processes:
$ perf record -e '{cycles,cache-misses}:S' <workload>Use following command to display the data:
$ perf report --group
Adding warning for record command if the support is not detected:
$ perf record -e '{cycles,cache-misses}:S,{instructions,branch}:S' kill
Disabling inherit for sample read events, no kernel support.
...--- tools/perf/perf.h | 1 + tools/perf/util/evsel.c | 6 +++++- tools/perf/util/record.c | 12 ++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-)
tools/perf/perf.h +1 -0
@@ -45,6 +45,7 @@ struct record_opts {bool call_graph_enabled;bool group;bool inherit_stat; +bool inherit_format_group;bool no_buffering;bool no_inherit;bool no_inherit_set;
tools/perf/util/evsel.c +5 -1
@@ -586,7 +586,11 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)*/ if (leader->nr_members >1 ) { attr->read_format |=PERF_FORMAT_GROUP ; - attr->inherit =0 ; + attr->inherit = opts->inherit_format_group; +if (!opts->inherit_format_group) { +pr_warning_once ("Disabling inherit for sample read events, " +"no kernel support.\n" ); + } } }
tools/perf/util/record.c +12 -0
@@ -85,6 +85,12 @@ static void perf_probe_comm_exec(struct perf_evsel *evsel) evsel->attr.comm_exec =1 ; } +static void perf_probe_inherit_format_group (struct perf_evsel *evsel) +{ + evsel->attr.inherit =1 ; + evsel->attr.read_format |=PERF_FORMAT_GROUP ; +} +bool perf_can_sample_identifier (void ) {return perf_probe_api (perf_probe_sample_identifier);
@@ -95,6 +101,11 @@ static bool perf_can_comm_exec(void)return perf_probe_api (perf_probe_comm_exec); } +static bool perf_can_inherit_format_group (void ) +{ +return perf_probe_api (perf_probe_inherit_format_group); +} +void perf_evlist__config (struct perf_evlist *evlist,struct record_opts *opts) {struct perf_evsel *evsel;
@@ -111,6 +122,7 @@ void perf_evlist__config(struct perf_evlist *evlist, struct record_opts *opts)if (evlist->cpus->map[0 ] <0 ) opts->no_inherit = true; + opts->inherit_format_group =perf_can_inherit_format_group (); use_comm_exec =perf_can_comm_exec ();evlist__for_each (evlist, evsel) {
Signature · 1 line
1.8.3.1
Jiri Olsa
reviewerPATCH v2 8/9 perf script: Add period data column
In reply to Jiri OlsaAdding period data column to be displayed in perf script. It's possible to get period values using -f option, like:
$ perf script -f comm,tid,time,period,ip,sym,dso
:26019 26019 52414.329088: 3707 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms])
:26019 26019 52414.329088: 44 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms])
:26019 26019 52414.329093: 1987 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms])
:26019 26019 52414.329093: 6 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms])
ls 26019 52414.329442: 537558 3407c0639c _dl_map_object_from_fd (/usr/lib64/ld-2.17.so)
ls 26019 52414.329442: 2099 3407c0639c _dl_map_object_from_fd (/usr/lib64/ld-2.17.so)
ls 26019 52414.330181: 1242100 34080917bb get_next_seq (/usr/lib64/libc-2.17.so)
ls 26019 52414.330181: 3774 34080917bb get_next_seq (/usr/lib64/libc-2.17.so)
ls 26019 52414.331427: 1083662 ffffffff810c7dc2 update_curr ([kernel.kallsyms])
ls 26019 52414.331427: 360 ffffffff810c7dc2 update_curr ([kernel.kallsyms])--- tools/perf/Documentation/perf-script.txt | 2 +- tools/perf/builtin-script.c | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-)
tools/perf/Documentation/perf-script.txt +1 -1
@@ -115,7 +115,7 @@ OPTIONS -f:: --fields:: Comma separated list of fields to print. Options are: - comm, tid, pid, time, cpu, event, trace, ip, sym, dso, addr, symoff, srcline. + comm, tid, pid, time, cpu, event, trace, ip, sym, dso, addr, symoff, srcline, period. Field list can be prepended with the type, trace, sw or hw, to indicate to which event type the field list applies. e.g., -f sw:comm,tid,time,ip,sym and -f trace:time,cpu,trace
tools/perf/builtin-script.c +11 -1
@@ -44,6 +44,7 @@ enum perf_output_field {PERF_OUTPUT_ADDR =1U <<10 ,PERF_OUTPUT_SYMOFFSET =1U <<11 ,PERF_OUTPUT_SRCLINE =1U <<12 , + PERF_OUTPUT_PERIOD = 1U << 13, }; struct output_option {
@@ -63,6 +64,7 @@ struct output_option { {.str = "addr", .field = PERF_OUTPUT_ADDR}, {.str = "symoff", .field = PERF_OUTPUT_SYMOFFSET}, {.str = "srcline", .field = PERF_OUTPUT_SRCLINE}, + {.str = "period", .field = PERF_OUTPUT_PERIOD}, };/* default set to maintain compatibility with current format */
@@ -229,6 +231,11 @@ static int perf_evsel__check_attr(struct perf_evsel *evsel, PERF_OUTPUT_CPU))return -EINVAL ; +if (PRINT_FIELD (PERIOD ) && +perf_evsel__check_stype (evsel,PERF_SAMPLE_PERIOD ,"PERIOD" , +PERF_OUTPUT_PERIOD )) +return -EINVAL ; +return 0 ; }
@@ -448,6 +455,9 @@ static void process_event(union perf_event *event, struct perf_sample *sample,print_sample_start (sample, thread, evsel); +if (PRINT_FIELD (PERIOD )) +printf ("%10" PRIu64" " , sample->period); +if (PRINT_FIELD (EVNAME )) {const char *evname =perf_evsel__name (evsel);printf ("%s: " , evname ? evname :"[unknown]" );
@@ -1544,7 +1554,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused) "comma separated output fields prepend with 'type:'. " "Valid types: hw,sw,trace,raw. " "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso," - "addr,symoff", parse_output_fields), + "addr,symoff,period", parse_output_fields),OPT_BOOLEAN ('a' ,"all-cpus" , &system_wide,"system-wide collection from all CPUs" ), OPT_STRING('S', "symbols", &symbol_conf.sym_list_str, "symbol[,symbol...]",
Signature · 1 line
1.8.3.1
Re: [PATCH 8/9] perf script: Add period data column
Quoted 27 lines · Jiri Olsa
Adding period data column to be displayed in perf script. It's possible to get period values using -f option, like:
$ perf script -f comm,tid,time,period,ip,sym,dso :26019 26019 52414.329088: 3707 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms]) :26019 26019 52414.329088: 44 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms]) :26019 26019 52414.329093: 1987 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms]) :26019 26019 52414.329093: 6 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms]) ls 26019 52414.329442: 537558 3407c0639c _dl_map_object_from_fd (/usr/lib64/ld-2.17.so) ls 26019 52414.329442: 2099 3407c0639c _dl_map_object_from_fd (/usr/lib64/ld-2.17.so) ls 26019 52414.330181: 1242100 34080917bb get_next_seq (/usr/lib64/libc-2.17.so) ls 26019 52414.330181: 3774 34080917bb get_next_seq (/usr/lib64/libc-2.17.so) ls 26019 52414.331427: 1083662 ffffffff810c7dc2 update_curr ([kernel.kallsyms]) ls 26019 52414.331427: 360 ffffffff810c7dc2 update_curr ([kernel.kallsyms])Cc Andi Kleen <[email protected]>Cc Arnaldo Carvalho de Melo <[email protected]>Cc Corey Ashford <[email protected]>Cc David Ahern <[email protected]>Cc Frederic Weisbecker <[email protected]>Cc Ingo Molnar <[email protected]>Cc Jen-Cheng(Tommy) Huang <[email protected]>Cc Namhyung Kim <[email protected]>Cc Paul Mackerras <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Stephane Eranian <[email protected]>Signed-off-by Jiri Olsa <[email protected]>
Quoted 4 lines
--- tools/perf/Documentation/perf-script.txt | 2 +- tools/perf/builtin-script.c | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-)
Re: [PATCH 8/9] perf script: Add period data column
Quoted 33 lines · David Ahern
Quoted 27 lines · Jiri Olsa
Adding period data column to be displayed in perf script. It's possible to get period values using -f option, like:
$ perf script -f comm,tid,time,period,ip,sym,dso :26019 26019 52414.329088: 3707 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms]) :26019 26019 52414.329088: 44 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms]) :26019 26019 52414.329093: 1987 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms]) :26019 26019 52414.329093: 6 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms]) ls 26019 52414.329442: 537558 3407c0639c _dl_map_object_from_fd (/usr/lib64/ld-2.17.so) ls 26019 52414.329442: 2099 3407c0639c _dl_map_object_from_fd (/usr/lib64/ld-2.17.so) ls 26019 52414.330181: 1242100 34080917bb get_next_seq (/usr/lib64/libc-2.17.so) ls 26019 52414.330181: 3774 34080917bb get_next_seq (/usr/lib64/libc-2.17.so) ls 26019 52414.331427: 1083662 ffffffff810c7dc2 update_curr ([kernel.kallsyms]) ls 26019 52414.331427: 360 ffffffff810c7dc2 update_curr ([kernel.kallsyms])Cc Andi Kleen <[email protected]>Cc Arnaldo Carvalho de Melo <[email protected]>Cc Corey Ashford <[email protected]>Cc David Ahern <[email protected]>Cc Frederic Weisbecker <[email protected]>Cc Ingo Molnar <[email protected]>Cc Jen-Cheng(Tommy) Huang <[email protected]>Cc Namhyung Kim <[email protected]>Cc Paul Mackerras <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Stephane Eranian <[email protected]>Signed-off-by Jiri Olsa <[email protected]>Quoted 4 lines
--- tools/perf/Documentation/perf-script.txt | 2 +- tools/perf/builtin-script.c | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-)
Quoted 1 line
Acked-by David Ahern <[email protected]>
Arnaldo,
could you please pick this one and patch 9/9?
just realized this was left behind ;-)
thanks,
jirka
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 8/9] perf script: Add period data column
Em Fri, Oct 17, 2014 at 06:10:06PM +0200, Jiri Olsa escreveu:
Quoted 34 lines · David Ahern
Quoted 33 lines · Jiri Olsa
Quoted 31 lines
Adding period data column to be displayed in perf script. It's possible to get period values using -f option, like:
$ perf script -f comm,tid,time,period,ip,sym,dso :26019 26019 52414.329088: 3707 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms]) :26019 26019 52414.329088: 44 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms]) :26019 26019 52414.329093: 1987 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms]) :26019 26019 52414.329093: 6 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms]) ls 26019 52414.329442: 537558 3407c0639c _dl_map_object_from_fd (/usr/lib64/ld-2.17.so) ls 26019 52414.329442: 2099 3407c0639c _dl_map_object_from_fd (/usr/lib64/ld-2.17.so) ls 26019 52414.330181: 1242100 34080917bb get_next_seq (/usr/lib64/libc-2.17.so) ls 26019 52414.330181: 3774 34080917bb get_next_seq (/usr/lib64/libc-2.17.so) ls 26019 52414.331427: 1083662 ffffffff810c7dc2 update_curr ([kernel.kallsyms]) ls 26019 52414.331427: 360 ffffffff810c7dc2 update_curr ([kernel.kallsyms])Cc Andi Kleen <[email protected]>Cc Arnaldo Carvalho de Melo <[email protected]>Cc Corey Ashford <[email protected]>Cc David Ahern <[email protected]>Cc Frederic Weisbecker <[email protected]>Cc Ingo Molnar <[email protected]>Cc Jen-Cheng(Tommy) Huang <[email protected]>Cc Namhyung Kim <[email protected]>Cc Paul Mackerras <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Stephane Eranian <[email protected]>Signed-off-by Jiri Olsa <[email protected]>--- tools/perf/Documentation/perf-script.txt | 2 +- tools/perf/builtin-script.c | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-)
Quoted 5 lines
Quoted 1 line
Acked-by David Ahern <[email protected]>Arnaldo,
could you please pick this one and patch 9/9?
just realized this was left behind ;-)
Applied both.
tip-bot for Jiri Olsa
merge bot[tip:perf/urgent] perf script: Add period data column
In reply to Jiri Olsaperf script: Add period data column
Commit-ID: 535aeaae7de821ba5d43ee2a204ee667ca95aae4 Gitweb: http://git.kernel.org/tip/535aeaae7de821ba5d43ee2a204ee667ca95aae4 Author: Jiri Olsa <[email protected]> AuthorDate: Mon, 25 Aug 2014 16:45:42 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 17 Oct 2014 15:21:30 -0300
perf script: Add period data column
Adding period data column to be displayed in perf script. It's possible to get period values using -f option, like:
$ perf script -f comm,tid,time,period,ip,sym,dso
:26019 26019 52414.329088: 3707 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms])
:26019 26019 52414.329088: 44 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms])
:26019 26019 52414.329093: 1987 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms])
:26019 26019 52414.329093: 6 ffffffff8105443a native_write_msr_safe ([kernel.kallsyms])
ls 26019 52414.329442: 537558 3407c0639c _dl_map_object_from_fd (/usr/lib64/ld-2.17.so)
ls 26019 52414.329442: 2099 3407c0639c _dl_map_object_from_fd (/usr/lib64/ld-2.17.so)
ls 26019 52414.330181: 1242100 34080917bb get_next_seq (/usr/lib64/libc-2.17.so)
ls 26019 52414.330181: 3774 34080917bb get_next_seq (/usr/lib64/libc-2.17.so)
ls 26019 52414.331427: 1083662 ffffffff810c7dc2 update_curr ([kernel.kallsyms])
ls 26019 52414.331427: 360 ffffffff810c7dc2 update_curr ([kernel.kallsyms])--- tools/perf/Documentation/perf-script.txt | 2 +- tools/perf/builtin-script.c | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-)
tools/perf/Documentation/perf-script.txt +1 -1
@@ -115,7 +115,7 @@ OPTIONS -f:: --fields:: Comma separated list of fields to print. Options are: - comm, tid, pid, time, cpu, event, trace, ip, sym, dso, addr, symoff, srcline. + comm, tid, pid, time, cpu, event, trace, ip, sym, dso, addr, symoff, srcline, period. Field list can be prepended with the type, trace, sw or hw, to indicate to which event type the field list applies. e.g., -f sw:comm,tid,time,ip,sym and -f trace:time,cpu,trace
tools/perf/builtin-script.c +11 -1
@@ -44,6 +44,7 @@ enum perf_output_field {PERF_OUTPUT_ADDR =1U <<10 ,PERF_OUTPUT_SYMOFFSET =1U <<11 ,PERF_OUTPUT_SRCLINE =1U <<12 , + PERF_OUTPUT_PERIOD = 1U << 13, }; struct output_option {
@@ -63,6 +64,7 @@ struct output_option { {.str = "addr", .field = PERF_OUTPUT_ADDR}, {.str = "symoff", .field = PERF_OUTPUT_SYMOFFSET}, {.str = "srcline", .field = PERF_OUTPUT_SRCLINE}, + {.str = "period", .field = PERF_OUTPUT_PERIOD}, };/* default set to maintain compatibility with current format */
@@ -229,6 +231,11 @@ static int perf_evsel__check_attr(struct perf_evsel *evsel, PERF_OUTPUT_CPU))return -EINVAL ; +if (PRINT_FIELD (PERIOD ) && +perf_evsel__check_stype (evsel,PERF_SAMPLE_PERIOD ,"PERIOD" , +PERF_OUTPUT_PERIOD )) +return -EINVAL ; +return 0 ; }
@@ -448,6 +455,9 @@ static void process_event(union perf_event *event, struct perf_sample *sample,print_sample_start (sample, thread, evsel); +if (PRINT_FIELD (PERIOD )) +printf ("%10" PRIu64" " , sample->period); +if (PRINT_FIELD (EVNAME )) {const char *evname =perf_evsel__name (evsel);printf ("%s: " , evname ? evname :"[unknown]" );
@@ -1543,7 +1553,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused) "comma separated output fields prepend with 'type:'. " "Valid types: hw,sw,trace,raw. " "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso," - "addr,symoff", parse_output_fields), + "addr,symoff,period", parse_output_fields),OPT_BOOLEAN ('a' ,"all-cpus" , &system_wide,"system-wide collection from all CPUs" ), OPT_STRING('S', "symbols", &symbol_conf.sym_list_str, "symbol[,symbol...]",
Jiri Olsa
reviewerPATCH v2 9/9 perf script: Add period as a default output column
In reply to Jiri OlsaAdding period as a default output column in script command fo hardware, software and raw events.
If PERF_SAMPLE_PERIOD sample type is defined in perf.data, following will be displayed in perf script output:
$ perf script
ls 8034 57477.887209: 250000 task-clock: ffffffff81361d72 memset ([kernel.kallsyms])
ls 8034 57477.887464: 250000 task-clock: ffffffff816f6d92 _raw_spin_unlock_irqrestore ([kernel.kallsyms])
ls 8034 57477.887708: 250000 task-clock: ffffffff811a94f0 do_munmap ([kernel.kallsyms])
ls 8034 57477.887959: 250000 task-clock: 34080916c6 get_next_seq (/usr/lib64/libc-2.17.so)
ls 8034 57477.888208: 250000 task-clock: 3408079230 _IO_doallocbuf (/usr/lib64/libc-2.17.so)
ls 8034 57477.888717: 250000 task-clock: ffffffff814242c8 n_tty_write ([kernel.kallsyms])
ls 8034 57477.889285: 250000 task-clock: 3408076402 fwrite_unlocked (/usr/lib64/libc-2.17.so)--- tools/perf/builtin-script.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
tools/perf/builtin-script.c +6 -3
@@ -82,7 +82,8 @@ static struct { .fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |PERF_OUTPUT_CPU |PERF_OUTPUT_TIME |PERF_OUTPUT_EVNAME |PERF_OUTPUT_IP | - PERF_OUTPUT_SYM | PERF_OUTPUT_DSO, +PERF_OUTPUT_SYM |PERF_OUTPUT_DSO | + PERF_OUTPUT_PERIOD, .invalid_fields = PERF_OUTPUT_TRACE, },
@@ -93,7 +94,8 @@ static struct { .fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |PERF_OUTPUT_CPU |PERF_OUTPUT_TIME |PERF_OUTPUT_EVNAME |PERF_OUTPUT_IP | - PERF_OUTPUT_SYM | PERF_OUTPUT_DSO, +PERF_OUTPUT_SYM |PERF_OUTPUT_DSO | + PERF_OUTPUT_PERIOD, .invalid_fields = PERF_OUTPUT_TRACE, },
@@ -112,7 +114,8 @@ static struct { .fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |PERF_OUTPUT_CPU |PERF_OUTPUT_TIME |PERF_OUTPUT_EVNAME |PERF_OUTPUT_IP | - PERF_OUTPUT_SYM | PERF_OUTPUT_DSO, +PERF_OUTPUT_SYM |PERF_OUTPUT_DSO | + PERF_OUTPUT_PERIOD, .invalid_fields = PERF_OUTPUT_TRACE, },
Signature · 1 line
1.8.3.1
Re: [PATCH 9/9] perf script: Add period as a default output column
Quoted 15 lines · Jiri Olsa
Adding period as a default output column in script command fo hardware, software and raw events.
If
PERF_SAMPLE_PERIODsample type is defined in perf.data, following will be displayed in perf script output:$ perf script ls 8034 57477.887209: 250000 task-clock: ffffffff81361d72 memset ([kernel.kallsyms]) ls 8034 57477.887464: 250000 task-clock: ffffffff816f6d92 _raw_spin_unlock_irqrestore ([kernel.kallsyms]) ls 8034 57477.887708: 250000 task-clock: ffffffff811a94f0 do_munmap ([kernel.kallsyms]) ls 8034 57477.887959: 250000 task-clock: 34080916c6 get_next_seq (/usr/lib64/libc-2.17.so) ls 8034 57477.888208: 250000 task-clock: 3408079230 _IO_doallocbuf (/usr/lib64/libc-2.17.so) ls 8034 57477.888717: 250000 task-clock: ffffffff814242c8 n_tty_write ([kernel.kallsyms]) ls 8034 57477.889285: 250000 task-clock: 3408076402 fwrite_unlocked (/usr/lib64/libc-2.17.so)
Quoted 12 lines
Cc Andi Kleen<[email protected]>Cc Arnaldo Carvalho de Melo<[email protected]>Cc Corey Ashford<[email protected]>Cc David Ahern<[email protected]>Cc Frederic Weisbecker<[email protected]>Cc Ingo Molnar<[email protected]>Cc Jen-Cheng(Tommy) Huang<[email protected]>Cc Namhyung Kim<[email protected]>Cc Paul Mackerras<[email protected]>Cc Peter Zijlstra<[email protected]>Cc Stephane Eranian<[email protected]>Signed-off-by Jiri Olsa<[email protected]>
Re: [PATCH 9/9] perf script: Add period as a default output column
Quoted 30 lines · David Ahern
Quoted 15 lines · Jiri Olsa
Adding period as a default output column in script command fo hardware, software and raw events.
If
PERF_SAMPLE_PERIODsample type is defined in perf.data, following will be displayed in perf script output:$ perf script ls 8034 57477.887209: 250000 task-clock: ffffffff81361d72 memset ([kernel.kallsyms]) ls 8034 57477.887464: 250000 task-clock: ffffffff816f6d92 _raw_spin_unlock_irqrestore ([kernel.kallsyms]) ls 8034 57477.887708: 250000 task-clock: ffffffff811a94f0 do_munmap ([kernel.kallsyms]) ls 8034 57477.887959: 250000 task-clock: 34080916c6 get_next_seq (/usr/lib64/libc-2.17.so) ls 8034 57477.888208: 250000 task-clock: 3408079230 _IO_doallocbuf (/usr/lib64/libc-2.17.so) ls 8034 57477.888717: 250000 task-clock: ffffffff814242c8 n_tty_write ([kernel.kallsyms]) ls 8034 57477.889285: 250000 task-clock: 3408076402 fwrite_unlocked (/usr/lib64/libc-2.17.so)Quoted 12 lines
Cc Andi Kleen<[email protected]>Cc Arnaldo Carvalho de Melo<[email protected]>Cc Corey Ashford<[email protected]>Cc David Ahern<[email protected]>Cc Frederic Weisbecker<[email protected]>Cc Ingo Molnar<[email protected]>Cc Jen-Cheng(Tommy) Huang<[email protected]>Cc Namhyung Kim<[email protected]>Cc Paul Mackerras<[email protected]>Cc Peter Zijlstra<[email protected]>Cc Stephane Eranian<[email protected]>Signed-off-by Jiri Olsa<[email protected]>
Quoted 1 line
Acked-by David Ahern <[email protected]>
this one (referenced by earlier email) ;-)
thanks,
jirka
tip-bot for Jiri Olsa
merge bot[tip:perf/urgent] perf script: Add period as a default output column
In reply to Jiri Olsaperf script: Add period as a default output column
Commit-ID: e8564b710c6df2c3aeb56c507c22f4bcfa4c0b2d Gitweb: http://git.kernel.org/tip/e8564b710c6df2c3aeb56c507c22f4bcfa4c0b2d Author: Jiri Olsa <[email protected]> AuthorDate: Mon, 25 Aug 2014 16:45:43 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 17 Oct 2014 15:22:19 -0300
perf script: Add period as a default output column
Adding period as a default output column in script command fo hardware, software and raw events.
If PERF_SAMPLE_PERIOD sample type is defined in perf.data, following will be displayed in perf script output:
$ perf script
ls 8034 57477.887209: 250000 task-clock: ffffffff81361d72 memset ([kernel.kallsyms])
ls 8034 57477.887464: 250000 task-clock: ffffffff816f6d92 _raw_spin_unlock_irqrestore ([kernel.kallsyms])
ls 8034 57477.887708: 250000 task-clock: ffffffff811a94f0 do_munmap ([kernel.kallsyms])
ls 8034 57477.887959: 250000 task-clock: 34080916c6 get_next_seq (/usr/lib64/libc-2.17.so)
ls 8034 57477.888208: 250000 task-clock: 3408079230 _IO_doallocbuf (/usr/lib64/libc-2.17.so)
ls 8034 57477.888717: 250000 task-clock: ffffffff814242c8 n_tty_write ([kernel.kallsyms])
ls 8034 57477.889285: 250000 task-clock: 3408076402 fwrite_unlocked (/usr/lib64/libc-2.17.so)--- tools/perf/builtin-script.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
tools/perf/builtin-script.c +6 -3
@@ -82,7 +82,8 @@ static struct { .fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |PERF_OUTPUT_CPU |PERF_OUTPUT_TIME |PERF_OUTPUT_EVNAME |PERF_OUTPUT_IP | - PERF_OUTPUT_SYM | PERF_OUTPUT_DSO, +PERF_OUTPUT_SYM |PERF_OUTPUT_DSO | + PERF_OUTPUT_PERIOD, .invalid_fields = PERF_OUTPUT_TRACE, },
@@ -93,7 +94,8 @@ static struct { .fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |PERF_OUTPUT_CPU |PERF_OUTPUT_TIME |PERF_OUTPUT_EVNAME |PERF_OUTPUT_IP | - PERF_OUTPUT_SYM | PERF_OUTPUT_DSO, +PERF_OUTPUT_SYM |PERF_OUTPUT_DSO | + PERF_OUTPUT_PERIOD, .invalid_fields = PERF_OUTPUT_TRACE, },
@@ -112,7 +114,8 @@ static struct { .fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |PERF_OUTPUT_CPU |PERF_OUTPUT_TIME |PERF_OUTPUT_EVNAME |PERF_OUTPUT_IP | - PERF_OUTPUT_SYM | PERF_OUTPUT_DSO, +PERF_OUTPUT_SYM |PERF_OUTPUT_DSO | + PERF_OUTPUT_PERIOD, .invalid_fields = PERF_OUTPUT_TRACE, },
Quoted 28 lines · Jiri Olsa
hi,
Jen-Cheng(Tommy) Huang reported the leader sampling not working on children processes:
http://www.mail-archive.com/[email protected]/msg01644.htmlThe leader sampling (example below) lets the group leader event (cycles) do the sampling and reads the rest of the group (cache-misses) via
PERF_FORMAT_GROUPformat.Example:
$ perf record -e '{cycles,cache-misses}:S' <workload> $ perf report --groupThe perf report --group allows to see all events group data in single view.
The reason for leader sampling being switched off for inherited events, is that the kernel does no allow
PERF_FORMAT_GROUPformat on inherited events (which is used by leader sampling).I switched on the
PERF_FORMAT_GROUPformat for inherited events with few other fixies in patches:
perf: Deny optimized switch for events read byPERF_SAMPLE_READ
perf: AllowPERF_FORMAT_GROUPformat on inherited eventsAnd I fixed perf tool code to be able to process data from children processes.
Quoted 7 lines
Anyway, I might have missed some other reason why this was never switched on in kernel, so sending this as RFC.
v2 changes:
- detection of the kernel support
- added perf script support for period column
- separate kernel change forcontext_equivchange
and as usuall I forgot.. also reachable in here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/core_format_group
jirka