| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 11 Aug 2009 21:40:51 +0200
From: stephane eranian <eranian@...glemail.com>
To: Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc: Ingo Molnar <mingo@...e.hu>, LKML <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Thomas Gleixner <tglx@...utronix.de>,
Robert Richter <robert.richter@....com>,
Paul Mackerras <paulus@...ba.org>,
Andi Kleen <andi@...stfloor.org>,
Maynard Johnson <mpjohn@...ibm.com>,
Carl Love <cel@...ibm.com>,
Corey J Ashford <cjashfor@...ibm.com>,
Philip Mucci <mucci@...s.utk.edu>,
Dan Terpstra <terpstra@...s.utk.edu>,
perfmon2-devel <perfmon2-devel@...ts.sourceforge.net>
Subject: Re: perf_counters issue with PERF_SAMPLE_GROUP
On Tue, Aug 11, 2009 at 6:05 PM, Peter Zijlstra<a.p.zijlstra@...llo.nl> wrote:
> On Tue, 2009-08-11 at 17:41 +0200, stephane eranian wrote:
>> Hi,
>>
>> It seems to me there is a problem with the group counter values
>> when you use PERF_SAMPLE_GROUP. The counts are bogus
>> for all events.
>>
>> Test case is pretty simple:
>> - single group, 2 events
>> - sampling on PERF_COUNT_HW_CYCLES
>> - other event is PERF_COUNT_HW_CYCLES
>> - leader has SAMPLE_IP|SAMPLE_GROUP
>> - no inheritance
>> - single thread
>> - using sampling in one shot mode with PERF_COUNTER_IOC_REFRESH
>> - all events but leader start with disabled = 0 (i.e., enabled)
>> - sampling period is 240000000 (cycles)
>>
>> Notification 1: ip=0x401300 39100608 PERF_COUNT_HW_CPU_CYCLES (12)
>> Notification 2: ip=0x401300 17991616 PERF_COUNT_HW_CPU_CYCLES (12)
>> Notification 3: ip=0x401300 17981248 PERF_COUNT_HW_CPU_CYCLES (12)
>> Notification 4: ip=0x401300 9409478912 PERF_COUNT_HW_CPU_CYCLES (12)
>>
>> I would expect the value for the 2nd event to be close to 240000000.
>> But instead,
>> it is going up and down. The IP, nr and id (12) fields are correct, so
>> the parsing of
>> the buffer is correct. This is with the latest from Linus's 2.6.31-rc5.
>
> Could have broken somewhere along the line, the group stuff doesn't get
> tested a lot, if at all.
>
> perf used to have some support for it, not sure what the current state
> is.
>
> You seem to have forgotten to append your test.c though :-)
>
Can't send you the program because it uses extra bits and pieces
which are hard to remove. Otherwise I would have send it already.
But I think it boils down to the following piece of code in
perf_counter_output():
leader = counter->group_leader;
list_for_each_entry(sub, &leader->sibling_list, list_entry) {
if (sub != counter)
sub->pmu->read(sub);
group_entry.id = primary_counter_id(sub);
group_entry.counter = atomic64_read(&sub->count);
perf_output_put(&handle, group_entry);
}
>> Related to PERF_SAMPLE_GROUP, I believe there is some information missing.
>> You need to provide the TIMING information because in the case of SAMPLE_GROUP
>> you'd like to be able to scale the values of the counters you are
>> collecting. And you
>> need the timing at the moment, the sample was recorded not later.
>
> Right, so something like the below, possibly complemented with having
> PERF_COUNTER_IOC_RESET also reset the run-times?
>
Yes, but don't you have a namespace issue between PERF_FORMAT_* and
PERF_SAMPLE_* in the patch below? I would think you want to keep them separate.
I am also wondering about why one would want one timing value and not the other.
In other words, why not group them under a single name. But maybe it is harder
to return more than one u64 per PERF_FORMAT?
> ---
> include/linux/perf_counter.h | 3 +++
> kernel/perf_counter.c | 20 ++++++++++++++++++--
> 2 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/perf_counter.h b/include/linux/perf_counter.h
> index 2b36afe..44a056b 100644
> --- a/include/linux/perf_counter.h
> +++ b/include/linux/perf_counter.h
> @@ -365,10 +365,13 @@ enum perf_event_type {
> * { u64 period; } && PERF_SAMPLE_PERIOD
> *
> * { u64 nr;
> + * { u64 time_enabled; } && PERF_FORMAT_ENABLED
> + * { u64 time_running; } && PERF_FORMAT_RUNNING
> * { u64 id, val; } cnt[nr]; } && PERF_SAMPLE_GROUP
> *
> * { u64 nr,
> * u64 ips[nr]; } && PERF_SAMPLE_CALLCHAIN
> + *
> * { u32 size;
> * char data[size];}&& PERF_SAMPLE_RAW
> * };
> diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
> index e26d2fc..e61e701 100644
> --- a/kernel/perf_counter.c
> +++ b/kernel/perf_counter.c
> @@ -2636,6 +2636,7 @@ void perf_counter_output(struct perf_counter *counter, int nmi,
> {
> int ret;
> u64 sample_type = counter->attr.sample_type;
> + u64 read_format = counter->attr.read_format;
> struct perf_output_handle handle;
> struct perf_event_header header;
> u64 ip;
> @@ -2703,6 +2704,10 @@ void perf_counter_output(struct perf_counter *counter, int nmi,
> if (sample_type & PERF_SAMPLE_GROUP) {
> header.size += sizeof(u64) +
> counter->nr_siblings * sizeof(group_entry);
> + if (read_format & PERF_FORMAT_ENABLED)
> + header.size += sizeof(u64);
> + if (read_format & PERF_FORMAT_RUNNING)
> + header.size += sizeof(u64);
> }
>
> if (sample_type & PERF_SAMPLE_CALLCHAIN) {
> @@ -2765,9 +2770,20 @@ void perf_counter_output(struct perf_counter *counter, int nmi,
> */
> if (sample_type & PERF_SAMPLE_GROUP) {
> struct perf_counter *leader, *sub;
> - u64 nr = counter->nr_siblings;
> + u64 val;
> +
> + val = counter->nr_siblings;
> + perf_output_put(&handle, val);
>
> - perf_output_put(&handle, nr);
> + if (read_format & PERF_FORMAT_ENABLED) {
> + val = counter->total_time_enabled;
> + perf_output_put(&handle, val);
> + }
> +
> + if (read_format & PERF_FORMAT_RUNNING) {
> + val = counter->total_time_running;
> + perf_output_put(&handle, val);
> + }
>
> leader = counter->group_leader;
> list_for_each_entry(sub, &leader->sibling_list, list_entry) {
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists