Re: [GIT PULL] Performance Counters for Linux

[stephane eranian]

Hi,

On Fri, Jun 12, 2009 at 12:28 PM, Ingo Molnar<mingo@xxxxxxx> wrote:
>
>>
>> On Thu, Jun 11, 2009 at 6:03 PM, Ingo Molnar <mingo@xxxxxxx> wrote:
>>
>> > The counter concept got objected to in past discussions on lkml,
>> > by DaveM and by Stephane Eranian (i've Cc:-ed them) - so this
>> > code was not eligible for linux-next testing - nevertheless we
>> > gave it good testing on PowerPC and x86 and i've done a wide
>> > cross-build test as well to try to make sure it breaks no other
>> > architecture.
>>
>> I don't think you can quote me saying "I object to this code".
>> [...]
>
> I never saw you retract/change this negative opinion of yours about
> the whole separate-counters concept:
>
> Â" In summary, although the idea of simplifying tools by moving the
> Â Âcomplexity elsewhere is legitimate, pushing it down to the
> Â Âkernel is the wrong approach in my opinion, perfmon has avoided
> Â Âthat as much as possible for good reasons. "
>
> Â Âhttp://lkml.org/lkml/2008/12/5/359
>

I, indeed, did not retract because I still have reservations about the approach
even after 6 months of intense development.

> Do you like the concept now? That would be great news - you have a
> lot of experience with various PMU details and we could certainly
> welcome help with the perf tool and with the kernel side of
> perfcounters!
>

I still have reservations. I could be convinced, though. But for that to happen,
there are a couple of milestones that need to be reached:
        - Full Intel Nehalem support: core PMU, uncore PMU, LBR, PEBS
(incl. load latency),
          offcore_response.
        - Full Intel Itanium 2 dual-core (Montecito) support: D-EAR,
I-EAR, opcode matching, range
          restrictions, user level control

Those represent very advanced and very useful PMUs. Having implemented
user and kernel
support for both of  them, I can attest that they challenge any
interfaces. But perfmon is the proof
that those can be  exposed with their full strength thru a generic
kernel API. Therefore, I am
relatively hopeful, there should be a way to expose them through your API.

Another important consequence of your design is that the event
assignment logic is in the kernel.
As discussed early on, this can be quite complicated. Today, you only
have very partial support
for architected Intel X86 and AMD64 processors (I know about Power). I
am sure you will update
this shortly. But I think getting complete support for Intel Nehalem
and Itanium 2 in that area is
another important milestone.

Concerning help, I am sure you realize I am already helping you out by posting
detailed reviews. I have yet to see anybody else posting this kind of
information
concerning your API. I will keep posting as I find new issues. My goal is not to
torpedo this API, it's already upstream anyway, but instead I am
trying to ensure
it does what I want based on my experience developing tools, talking with PMU
architects and feedback from tool developers.

I think we could have a much more constructive working relationship if
people showed
some more respect for the work I and many others have done. Perfmon
certainly has
issues and could be implemented better. You certainly have better
skills than me in that
area. I have no problem admitting that. But I do not think perfmon
deserves the kind of
comments I have seen, repeated over and over, from you and Zijlstra
since December.
Regardless of your personal opinion, perfmon deserves some credit for
what it has offered
to many people around the world. If it had been as bad as you
described it, it could not
possibly have supported all the PMUs and their advanced features.
Nobody would have
used it. But this is not what happened.

>> [...] ÂI posted a detailed review of the API and implementation on
>> X86 outlining lots of issues. Some got fixed, but many others are
>> left unresolved at this point. And I will post some more shortly.
>
> Hm, Peter replied to you mail a week ago, in detail. We addressed a
> good number of issues pointed out by you, and we credited you for
> them:
>
> earth4:~/tip> git log v2.6.30..linus | grep 'Reported-by: Stephane Eranian'
> Â ÂReported-by: Stephane Eranian <eranian@xxxxxxxxxxxxxx>
> Â ÂReported-by: Stephane Eranian <eranian@xxxxxxxxxxxxxx>
> Â ÂReported-by: Stephane Eranian <eranian@xxxxxxxxxxxxxx>
> Â ÂReported-by: Stephane Eranian <eranian@xxxxxxxxxxxxxx>
> Â ÂReported-by: Stephane Eranian <eranian@xxxxxxxxxxxxxx>
> Â ÂReported-by: Stephane Eranian <eranian@xxxxxxxxxxxxxx>
> Â ÂReported-by: Stephane Eranian <eranian@xxxxxxxxxxxxxx>
>
I know. I appreciate that. I wish you had also acknowledged the fact
that I suggested that you split the config field into type and config fields
in my initial posting. I had to discover this change by looking at the GIT
log.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/