Unlike normal hardware PMCs, the 24x7 counters in Power8 are stored in
memory and accessed via a hypervisor call(HCALL). A major aspect of the
HCALL is that it allows retireving _several_ counters at once (unlike
regular PMCs, which are read one at a time). By reading several counters
at once, we can get a more consistent snapshot of the system.
This patchset extends the transaction interface to accomplish submitting
several events to the PMU and have the PMU read them all at once. User is
expected to submit the set of events they want to read as an "event group".
In the kernel, we submit each event to the PMU using the following logic
(from Peter Zijlstra).
pmu->start_txn(pmu, PMU_TXN_READ);
leader->read();
for_each_sibling()
sibling->read();
pmu->commit_txn();
where:
- the ->read()s queue events to be submitted to the hypervisor, and,
- the ->commit_txn() issues the HCALL, retrieves the result and
updates the event count.
Architectures/PMUs that don't need/implement PMU_TXN_READ type of transactions,
simply ignore the ->start_txn() and ->commit_txn() and continue to read the
counters one at a time in the ->read() call.
Compile/touch tested on x86. Need help testing on s390 and Sparc.
Thanks to Peter Zijlstra for his input.
Changelog [v3]
- Simple changes/reorg of patchset to split/rename functions
- [Peter Zijlstra] Save the transaction flags in ->start_txn() and
drop the flags parameter from ->commit_txn() and ->cancel_txn().
- [Peter Zijlstra] The nop txn interfaces don't need to disable/enable
PMU for PERF_PMU_TXN_READ transactions.
Changelog [v2]
- Use the transaction interface unconditionally to avoid special-case
code. Architectures/PMUs that don't need the READ transaction types
simply ignore the ->start_txn() and ->commit_txn() calls.
Peter Zijlstra (Intel) (1):
perf: Rename perf_event_read_{one,group}, perf_read_hw
Sukadev Bhattiprolu (7):
powerpc/perf/hv-24x7: Whitespace - fix parameter alignment
powerpc/perf/hv-24x7: Simplify extracting counter from result buffer
perf: Add a flags parameter to pmu txn interfaces
perf: Split perf_event_read() and perf_event_count()
perf: Split perf_event_read_value()
perf: Define PMU_TXN_READ interface
powerpc/perf/hv-24x7: Use PMU_TXN_READ interface
arch/powerpc/perf/core-book3s.c | 25 ++++-
arch/powerpc/perf/hv-24x7.c | 186 ++++++++++++++++++++++++++++++++++----
arch/s390/kernel/perf_cpum_cf.c | 24 ++++-
arch/sparc/kernel/perf_event.c | 19 +++-
arch/x86/kernel/cpu/perf_event.c | 27 +++++-
arch/x86/kernel/cpu/perf_event.h | 1 +
include/linux/perf_event.h | 15 ++-
kernel/events/core.c | 143 +++++++++++++++++++++++------
8 files changed, 389 insertions(+), 51 deletions(-)
--
1.7.9.5
@@ -1125,8 +1125,8 @@ static unsigned long single_24x7_request(struct perf_event *event, u64 *count)}/* process result from hcall */-resb=&result_buffer->results[0];-*count=be64_to_cpu(resb->elements[0].element_data[0]);+val=result_buffer->results[0].elements[0].element_data[0];+*count=be64_to_cpu(val);out:put_cpu_var(hv_24x7_reqb);
Currently, the PMU interface allows reading only one counter at a time.
But some PMUs like the 24x7 counters in Power, support reading several
counters at once. To leveage this functionality, extend the transaction
interface to support a "transaction type".
The first type, PERF_PMU_TXN_ADD, refers to the existing transactions,
i.e. used to _schedule_ all the events on the PMU as a group. A second
transaction type, PERF_PMU_TXN_READ, will be used in a follow-on patch,
by the 24x7 counters to read several counters at once.
Extend the transaction interfaces to the PMU to accept a 'txn_flags'
parameter and use this parameter to ignore any transactions that are
not of type PERF_PMU_TXN_ADD.
Thanks to Peter Zijlstra for his input.
Signed-off-by: Sukadev Bhattiprolu <redacted>
Changelog[v3]
- [Peter Zijlstra] Ensure the nop_txn interfaces disable/enable
PMU only for TXN_ADD transactions.
- [Peter Zijlstra] Cache the flags parameter in ->start_txn() and
drop the flags parameter from ->commit_txn() and ->cancel_txn().
---
arch/powerpc/perf/core-book3s.c | 25 ++++++++++++++++++-
arch/s390/kernel/perf_cpum_cf.c | 24 +++++++++++++++++-
arch/sparc/kernel/perf_event.c | 19 +++++++++++++-
arch/x86/kernel/cpu/perf_event.c | 27 ++++++++++++++++++--
arch/x86/kernel/cpu/perf_event.h | 1 +
include/linux/perf_event.h | 14 ++++++++---
kernel/events/core.c | 51 +++++++++++++++++++++++++++++++++++---
7 files changed, 150 insertions(+), 11 deletions(-)
@@ -604,9 +620,15 @@ static void cpumf_pmu_cancel_txn(struct pmu *pmu)*/staticintcpumf_pmu_commit_txn(structpmu*pmu){+inttxn_flags;structcpu_hw_events*cpuhw=this_cpu_ptr(&cpu_hw_events);u64state;+txn_flags=cpuhw->txn_flags;+cpuhw->txn_flags=0;+if(txn_flags&~PERF_PMU_TXN_ADD)+return0;+/* check if the updated state can be scheduled */state=cpuhw->state&~((1<<CPUMF_LCCTL_ENABLE_SHIFT)-1);state>>=CPUMF_LCCTL_ENABLE_SHIFT;
perf_event_read() does two things:
- call the PMU to read/update the counter value, and
- compute the total count of the event and its children
Not all callers need both. perf_event_reset() for instance needs the
first piece but doesn't need the second. Similarly, when we implement
the ability to read a group of events using the transaction interface,
we would need the two pieces done independently.
Break up perf_event_read() and have it just read/update the counter
and have the callers compute the total count if necessary.
Signed-off-by: Sukadev Bhattiprolu <redacted>
---
kernel/events/core.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
Define a new PERF_PMU_TXN_READ interface to read a group of counters
at once. Note that we use this interface with all PMUs.
PMUs that implement this interface use the ->read() operation to _queue_
the counters to be read and use ->commit_txn() to actually read all the
queued counters at once.
PMUs that don't implement PERF_PMU_TXN_READ ignore ->start_txn() and
->commit_txn() and continue to read counters one at a time.
Thanks to input from Peter Zijlstra.
Signed-off-by: Sukadev Bhattiprolu <redacted>
---
include/linux/perf_event.h | 1 +
kernel/events/core.c | 35 +++++++++++++++++++++++++++++++++--
2 files changed, 34 insertions(+), 2 deletions(-)
The 24x7 counters in Powerpc allow monitoring a large number of counters
simultaneously. They also allow reading several counters in a single
HCALL so we can get a more consistent snapshot of the system.
Use the PMU's transaction interface to monitor and read several event
counters at once. The idea is that users can group several 24x7 events
into a single group of events. We use the following logic to submit
the group of events to the PMU and read the values:
pmu->start_txn() // Initialize before first event
for each event in group
pmu->read(event); // Queue each event to be read
pmu->commit_txn() // Read/update all queuedcounters
The ->commit_txn() also updates the event counts in the respective
perf_event objects. The perf subsystem can then directly get the
event counts from the perf_event and can avoid submitting a new
->read() request to the PMU.
Thanks to input from Peter Zijlstra.
Signed-off-by: Sukadev Bhattiprolu <redacted>
Changelog[v3]
[Peter Zijlstra] Save the transaction state in ->start_txn() and
drop the flags parameter from ->commit_txn() and ->cancel_txn().
[Peter Zijlstra] The nop txn interfaces don't need to disable/enable
PMU for PERF_PMU_TXN_READ transactions.
---
arch/powerpc/perf/hv-24x7.c | 160 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 157 insertions(+), 3 deletions(-)
@@ -1257,6 +1309,105 @@ static int h_24x7_event_add(struct perf_event *event, int flags)return0;}+/*+*24x7countersonlysupportREADtransactions.Theyare+*alwayscountinganddontneed/supportADDtransactions.+*Cachetheflags,butotherwiseignoretransactionsthat+*arenotPERF_PMU_TXN_READ.+*/+staticvoidh_24x7_event_start_txn(structpmu*pmu,intflags)+{+structhv_24x7_request_buffer*request_buffer;+structhv_24x7_data_result_buffer*result_buffer;++/* We should not be called if we are already in a txn */+WARN_ON_ONCE(h24x7hw.txn_flags);++h24x7hw.txn_flags=flags;+if(flags&~PERF_PMU_TXN_READ)+return;++request_buffer=(void*)&hv_24x7_txn_reqb[0];+result_buffer=(void*)&hv_24x7_txn_resb[0];++init_24x7_request(request_buffer,result_buffer);+}++/*+*Cleanuptransactionstate.+*+*NOTE:Ignorestateofrequestandresultbuffersfornow.+*Wewillinitializethemduringthenextread/txn.+*/+staticvoidreset_txn(void)+{+h24x7hw.txn_err=0;+h24x7hw.txn_flags=0;+}++/*+*24x7countersonlysupportREADtransactions.Theyarealwayscounting+*anddontneed/supportADDtransactions.Clear->txn_flagsbutotherwise+*ignoretransactionsthatarenotoftypePERF_PMU_TXN_READ.+*+*ForREADtransactions,submitallpending24x7requests(i.erequests+*thatwerequeuedbyh_24x7_event_read()),tothehypervisorandupdate+*theeventcounts.+*/+staticinth_24x7_event_commit_txn(structpmu*pmu)+{+structhv_24x7_request_buffer*request_buffer;+structhv_24x7_data_result_buffer*result_buffer;+structhv_24x7_result*resb;+structperf_event*event;+u64count;+inti,ret;++WARN_ON_ONCE(!h24x7hw.txn_flags);++ret=0;+if(h24x7hw.txn_flags&~PERF_PMU_TXN_READ)+gotoout;++ret=h24x7hw.txn_err;+if(h24x7hw.txn_err)+gotoout;++request_buffer=(void*)&hv_24x7_txn_reqb[0];+result_buffer=(void*)&hv_24x7_txn_resb[0];++ret=make_24x7_request(request_buffer,result_buffer);+if(ret){+log_24x7_hcall(request_buffer,result_buffer,ret);+gotoout;+}++/* Update event counts from hcall */+for(i=0;i<request_buffer->num_requests;i++){+resb=&result_buffer->results[i];+count=be64_to_cpu(resb->elements[0].element_data[0]);+event=h24x7hw.events[i];+h24x7hw.events[i]=NULL;+update_event_count(event,count);+}++out:+reset_txn();+returnret;+}++/*+*24x7countersonlysupportREADtransactions.Theyarealwayscounting+*anddontneed/supportADDtransactions.However,regardlessoftype+*oftransaction,allweneedtodoiscleanup,sowedon'thavetocheck+*thetypeoftransaction.+*/+staticvoidh_24x7_event_cancel_txn(structpmu*pmu)+{+WARN_ON_ONCE(!h24x7hw.txn_flags);+reset_txn();+}+staticstructpmuh_24x7_pmu={.task_ctx_nr=perf_invalid_context,
Move the part of perf_event_read_value() that computes the event
counts and event times into a new function, perf_event_compute().
This would allow us to call perf_event_compute() independently.
Signed-off-by: Sukadev Bhattiprolu <redacted>
Changelog[v3]
Rather than move perf_event_read() into callers and then
rename, just move the computations into a separate function
(redesign to address comment from Peter Zijlstra).
---
kernel/events/core.c | 37 ++++++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 13 deletions(-)
From: Peter Zijlstra <peterz@infradead.org> Date: 2015-07-16 21:12:32
On Tue, Jul 14, 2015 at 08:01:52PM -0700, Sukadev Bhattiprolu wrote:
Move the part of perf_event_read_value() that computes the event
counts and event times into a new function, perf_event_compute().
This would allow us to call perf_event_compute() independently.
Signed-off-by: Sukadev Bhattiprolu <redacted>
Changelog[v3]
Rather than move perf_event_read() into callers and then
rename, just move the computations into a separate function
(redesign to address comment from Peter Zijlstra).
---
Changelog[] bits go here, below the '---' where they get discarded.
Peter Zijlstra [peterz@infradead.org] wrote:
| On Tue, Jul 14, 2015 at 08:01:52PM -0700, Sukadev Bhattiprolu wrote:
| > Move the part of perf_event_read_value() that computes the event
| > counts and event times into a new function, perf_event_compute().
| >
| > This would allow us to call perf_event_compute() independently.
| >
| > Signed-off-by: Sukadev Bhattiprolu [off-list ref]
| >
| > Changelog[v3]
| > Rather than move perf_event_read() into callers and then
| > rename, just move the computations into a separate function
| > (redesign to address comment from Peter Zijlstra).
| > ---
|
| Changelog[] bits go here, below the '---' where they get discarded.
Sorry. Will fix it.
|
| > kernel/events/core.c | 37 ++++++++++++++++++++++++-------------
| > 1 file changed, 24 insertions(+), 13 deletions(-)
| >
| > diff --git a/kernel/events/core.c b/kernel/events/core.c
| > index 44fb89d..b1e9a42 100644
| > --- a/kernel/events/core.c
| > +++ b/kernel/events/core.c
| > @@ -3704,6 +3704,29 @@ static int perf_release(struct inode *inode, struct file *file)
| > return 0;
| > }
| >
| > +static u64 perf_event_compute(struct perf_event *event, u64 *enabled,
| > + u64 *running)
|
| This is a horrible name, 'compute' what?
We are aggregating event counts and time for children.
Would perf_event_aggregate() or perf_event_aggregate_children()
be better?
|
| > +{
| > + struct perf_event *child;
| > + u64 total;
| > +
| > + total = perf_event_count(event);
| > +
| > + *enabled += event->total_time_enabled +
| > + atomic64_read(&event->child_total_time_enabled);
| > + *running += event->total_time_running +
| > + atomic64_read(&event->child_total_time_running);
| > +
|
| lockdep_assert_held(&event->child_mutex);
OK. Thanks for the comments.
Sukadev
From: Peter Zijlstra <peterz@infradead.org> Date: 2015-07-16 22:20:24
On Tue, Jul 14, 2015 at 08:01:54PM -0700, Sukadev Bhattiprolu wrote:
+/*
+ * Use the transaction interface to read the group of events in @leader.
+ * PMUs like the 24x7 counters in Power, can use this to queue the events
+ * in the ->read() operation and perform the actual read in ->commit_txn.
+ *
+ * Other PMUs can ignore the ->start_txn and ->commit_txn and read each
+ * PMU directly in the ->read() operation.
+ */
+static int perf_event_read_group(struct perf_event *leader)
+{
+ int ret;
+ struct perf_event *sub;
+ struct pmu *pmu;
+
+ pmu = leader->pmu;
+
+ pmu->start_txn(pmu, PERF_PMU_TXN_READ);
+
+ perf_event_read(leader);
There should be a lockdep assert with that list iteration.
Peter Zijlstra [peterz@infradead.org] wrote:
| On Tue, Jul 14, 2015 at 08:01:54PM -0700, Sukadev Bhattiprolu wrote:
| > +/*
| > + * Use the transaction interface to read the group of events in @leader.
| > + * PMUs like the 24x7 counters in Power, can use this to queue the events
| > + * in the ->read() operation and perform the actual read in ->commit_txn.
| > + *
| > + * Other PMUs can ignore the ->start_txn and ->commit_txn and read each
| > + * PMU directly in the ->read() operation.
| > + */
| > +static int perf_event_read_group(struct perf_event *leader)
| > +{
| > + int ret;
| > + struct perf_event *sub;
| > + struct pmu *pmu;
| > +
| > + pmu = leader->pmu;
| > +
| > + pmu->start_txn(pmu, PERF_PMU_TXN_READ);
| > +
| > + perf_event_read(leader);
|
| There should be a lockdep assert with that list iteration.
|
| > + list_for_each_entry(sub, &leader->sibling_list, group_entry)
| > + perf_event_read(sub);
| > +
| > + ret = pmu->commit_txn(pmu);
Peter,
I have a situation :-)
We are trying to use the following interface:
start_txn(pmu, PERF_PMU_TXN_READ);
perf_event_read(leader);
list_for_each(sibling, &leader->sibling_list, group_entry)
perf_event_read(sibling)
pmu->commit_txn(pmu);
with the idea that the PMU driver would save the type of transaction in
->start_txn() and use in ->read() and ->commit_txn().
But since ->start_txn() and the ->read() operations could happen on different
CPUs (perf_event_read() uses the event->oncpu to schedule a call), the PMU
driver cannot use a per-cpu variable to save the state in ->start_txn().
I tried using a pmu-wide global, but that would also need us to hold a mutex
to serialize access to that global. The problem is ->start_txn() can be
called from an interrupt context for the TXN_ADD transactions (I got the
following backtrace during testing)
mutex_lock_nested+0x504/0x520 (unreliable)
h_24x7_event_start_txn+0x3c/0xd0
group_sched_in+0x70/0x230
ctx_sched_in.isra.63+0x150/0x230
__perf_install_in_context+0x1c8/0x1e0
remote_function+0x7c/0xa0
flush_smp_call_function_queue+0xb0/0x1d0
smp_ipi_demux+0x88/0xf0
icp_hv_ipi_action+0x54/0xc0
handle_irq_event_percpu+0x98/0x2b0
handle_percpu_irq+0x7c/0xc0
generic_handle_irq+0x4c/0x80
__do_irq+0x7c/0x190
call_do_irq+0x14/0x24
do_IRQ+0x8c/0x100
hardware_interrupt_common+0x168/0x180
--- interrupt: 501 at .plpar_hcall_norets+0x14/0x20
Basically stuck trying to save the txn type in ->start_txn() and retrieve in
->read().
Couple of options I can think of are:
- having ->start_txn() return a handle that should then be passed in
with ->read() (yuck) and ->commit_txn().
- serialize the READ transaction for the PMU in perf_event_read_group()
with a new pmu->txn_mutex:
mutex_lock(&pmu->txn_mutex);
pmu->start_txn()
list_for_each_entry(sub, &leader->sibling_list, group_entry)
perf_event_read(sub);
ret = pmu->commit_txn(pmu);
mutex_unlock(&pmu->txn_mutex);
such serialization would be ok with 24x7 counters (they are system
wide counters anyway) We could maybe skip the mutex for PMUs that
don't implement TXN_READ interface.
or is there better way?
Sukadev
From: Peter Zijlstra <peterz@infradead.org> Date: 2015-07-22 05:55:16
On Tue, Jul 21, 2015 at 06:50:45PM -0700, Sukadev Bhattiprolu wrote:
We are trying to use the following interface:
start_txn(pmu, PERF_PMU_TXN_READ);
perf_event_read(leader);
list_for_each(sibling, &leader->sibling_list, group_entry)
perf_event_read(sibling)
pmu->commit_txn(pmu);
with the idea that the PMU driver would save the type of transaction in
->start_txn() and use in ->read() and ->commit_txn().
But since ->start_txn() and the ->read() operations could happen on different
CPUs (perf_event_read() uses the event->oncpu to schedule a call), the PMU
driver cannot use a per-cpu variable to save the state in ->start_txn().
or is there better way?
I've not woken up yet, and not actually fully read the email, but can
you stuff the entire above chunk inside the IPI?
I think you could then actually optimize __perf_event_read() as well,
because all these events should be on the same context, so no point in
calling update_*time*() for every event or so.
Peter Zijlstra [peterz@infradead.org] wrote:
| I've not woken up yet, and not actually fully read the email, but can
| you stuff the entire above chunk inside the IPI?
|
| I think you could then actually optimize __perf_event_read() as well,
| because all these events should be on the same context, so no point in
| calling update_*time*() for every event or so.
|
Do you mean something like this (will move the rename to a separate
patch before posting):
--
From e8eddb5d3877ebdb3b71213a00aaa980f4010dd0 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <redacted>
Date: Tue, 7 Jul 2015 21:45:23 -0400
Subject: [PATCH 1/1] perf: Define PMU_TXN_READ interface
Define a new PERF_PMU_TXN_READ interface to read a group of counters
at once. Note that we use this interface with all PMUs.
PMUs that implement this interface use the ->read() operation to _queue_
the counters to be read and use ->commit_txn() to actually read all the
queued counters at once.
PMUs that don't implement PERF_PMU_TXN_READ ignore ->start_txn() and
->commit_txn() and continue to read counters one at a time.
Thanks to input from Peter Zijlstra.
Signed-off-by: Sukadev Bhattiprolu <redacted>
---
Changelog[v5]
[Peter Zijlstra] Ensure the entire transaction happens on the same CPU.
Changelog[v4]
[Peter Zijlstra] Add lockdep_assert_held() in perf_event_read_group()
---
include/linux/perf_event.h | 1 +
kernel/events/core.c | 72 +++++++++++++++++++++++++++++++++++++-------
2 files changed, 62 insertions(+), 11 deletions(-)
From: Peter Zijlstra <peterz@infradead.org> Date: 2015-07-23 07:45:21
On Tue, Jul 14, 2015 at 08:01:52PM -0700, Sukadev Bhattiprolu wrote:
quoted hunk
Move the part of perf_event_read_value() that computes the event
counts and event times into a new function, perf_event_compute().
This would allow us to call perf_event_compute() independently.
Signed-off-by: Sukadev Bhattiprolu <redacted>
Changelog[v3]
Rather than move perf_event_read() into callers and then
rename, just move the computations into a separate function
(redesign to address comment from Peter Zijlstra).
---
kernel/events/core.c | 37 ++++++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 13 deletions(-)
From: Peter Zijlstra <peterz@infradead.org> Date: 2015-07-23 08:04:55
On Wed, Jul 22, 2015 at 04:19:16PM -0700, Sukadev Bhattiprolu wrote:
Peter Zijlstra [peterz@infradead.org] wrote:
| I've not woken up yet, and not actually fully read the email, but can
| you stuff the entire above chunk inside the IPI?
|
| I think you could then actually optimize __perf_event_read() as well,
| because all these events should be on the same context, so no point in
| calling update_*time*() for every event or so.
|
Do you mean something like this (will move the rename to a separate
patch before posting):
More like so.. please double check, I've not even had tea yet.
Peter Zijlstra [peterz@infradead.org] wrote:
| On Wed, Jul 22, 2015 at 04:19:16PM -0700, Sukadev Bhattiprolu wrote:
| > Peter Zijlstra [peterz@infradead.org] wrote:
| > | I've not woken up yet, and not actually fully read the email, but can
| > | you stuff the entire above chunk inside the IPI?
| > |
| > | I think you could then actually optimize __perf_event_read() as well,
| > | because all these events should be on the same context, so no point in
| > | calling update_*time*() for every event or so.
| > |
| >
| > Do you mean something like this (will move the rename to a separate
| > patch before posting):
|
| More like so.. please double check, I've not even had tea yet.
Yeah, I realized I had ignored the 'event->cpu' spec.
Will try this out. Thanks,
Sukadev
Peter Zijlstra [peterz@infradead.org] wrote:
| On Tue, Jul 14, 2015 at 08:01:52PM -0700, Sukadev Bhattiprolu wrote:
| > Move the part of perf_event_read_value() that computes the event
| > counts and event times into a new function, perf_event_compute().
| >
| > This would allow us to call perf_event_compute() independently.
| >
| > Signed-off-by: Sukadev Bhattiprolu [off-list ref]
| >
| > Changelog[v3]
| > Rather than move perf_event_read() into callers and then
| > rename, just move the computations into a separate function
| > (redesign to address comment from Peter Zijlstra).
| > ---
| > kernel/events/core.c | 37 ++++++++++++++++++++++++-------------
| > 1 file changed, 24 insertions(+), 13 deletions(-)
| >
| > diff --git a/kernel/events/core.c b/kernel/events/core.c
| > index 44fb89d..b1e9a42 100644
| > --- a/kernel/events/core.c
| > +++ b/kernel/events/core.c
| > @@ -3704,6 +3704,29 @@ static int perf_release(struct inode *inode, struct file *file)
| > return 0;
| > }
| >
| > +static u64 perf_event_compute(struct perf_event *event, u64 *enabled,
| > + u64 *running)
| > +{
| > + struct perf_event *child;
| > + u64 total;
| > +
| > + total = perf_event_count(event);
| > +
| > + *enabled += event->total_time_enabled +
| > + atomic64_read(&event->child_total_time_enabled);
| > + *running += event->total_time_running +
| > + atomic64_read(&event->child_total_time_running);
| > +
| > + list_for_each_entry(child, &event->child_list, child_list) {
| > + perf_event_read(child);
|
| Sure we don't want that..
So if say x86 calls perf_event_read_value() the current upstream code
makes the perf_event_read(child).
If we remove this, then it would be a change in behavior?
I have commented it out and have TODO in the latest patchset. Pls
review and let me know if we should drop this read (and the TODO)
of the child event.
Sukadev
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-08-03 01:35:12
On Wed, 2015-15-07 at 03:01:49 UTC, Sukadev Bhattiprolu wrote:
Simplify code that extracts a 24x7 counter from the HCALL's result buffer.
Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Sukadev Bhattiprolu <redacted>