Messages in this thread | | | Date | Tue, 11 Aug 2009 23:08:56 +0200 | Subject | Re: perf_counters issue with PERF_SAMPLE_GROUP | From | stephane eranian <> |
| |
On Tue, Aug 11, 2009 at 10:55 PM, Peter Zijlstra<a.p.zijlstra@chello.nl> wrote: > On Tue, 2009-08-11 at 21:40 +0200, stephane eranian wrote: > >> > 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. > > Those other bits aren't open source? tskk :-) >
You don't know me well! They will be but they are not ready yet.
>> 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); >> } > > Well, likely, but nothing obviously wrong stands out there, so now I get > to write a reproduces to see what's going wrong. >
>> >> 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. > > Maybe, otoh we've consistently used it whenever exposing the timing > data. > How do you make sure the bits used by the PERF_FORMAT_* stuff does not collide with PERF_SAMPLE_*?
>> 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? > > Not really, Paul did it like that initially and we've been consistently > doing it like that -- changing it now is a bit late. >
But aren't you going to change the cpu, pid target stuff we discussed a couple of weeks ago anyway? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |