Re: [PATCH v2 02/11] perf core: export swevent hrtimer helpers

[Cody P Schafer <cody@linux.vnet.ibm.com>]

On 02/26/2014 12:29 AM, Peter Zijlstra wrote:
> On Tue, Feb 25, 2014 at 01:38:31PM -0800, Cody P Schafer wrote:
>> On 02/25/2014 02:20 AM, Peter Zijlstra wrote:
>>> On Tue, Feb 25, 2014 at 02:33:26PM +1100, Michael Ellerman wrote:
>>>> On Fri, 2014-14-02 at 22:02:06 UTC, Cody P Schafer wrote:
>>>>> Export the swevent hrtimer helpers currently only used in events/core.c
>>>>> to allow the addition of architecture specific sw-like pmus.
>>>>
>>>> Peter, Ingo, can we get your ACK on this please?
>>>
>>> How are they used? I saw some usage in patch 9 or so; but its not
>>> explained anywhere. All patches have non-existent Changelogs and the few
>>> comments that are there are pretty hardware specific.
>>>
>>> So please do tell; what do you need this for?
>>
>>  From this patch's change log:
>>
>>> Export the swevent hrtimer helpers currently only used in events/core.c to allow the addition of architecture specific sw-like pmus.
>>
>> The key part here is "architecture specific sw-like pmus", where the
>> announcement explains why these pmus are sw-like:
>
> I don't read announcements for crucial patch details; announcements are
> lost and therefore unimportant.

And I'll be sure to elaborate further in the changelog next time (if I 
don't drop this change entirely).
This is the first comment I've got on this particular patch.

>>> The counters supplied by these interfaces are continually counting and never
>>> need to be (and cannot be) disabled or enabled. They additionally do not
>>> generate any interrupts. This makes them in some regards similar to software
>>> counters, and as a result their implimentation shares some common code (which
>>> an initial patch exposes) with the sw counters.
>>
>> Essentially, these pmus just provide access to a big array of counters which
>> don't generate interrupts, and are all 64bit (and assumed to never
>> overflow). Rather than duplicate the code that we already have for managing
>> timing when reading from counters that don't have interrupts (the functions
>> that are exposed by this patch), I've reused it.
>
> So note that all the software counters generate interrupts in their own
> measuring domain. The hrtimer ones measure time and generate time based
> interrupts, the event based ones generate 'interrupts' on their events.
>
> What you have here is a hw pmu without interrupt capability. That's
> fine, they don't get to generate interrupt. We have plenty of those
> already.
>
> But what you propose to do is add interrupt in another domain entirely.
> That's not fine. Don't do that.

Ok, so it looks like I misunderstood the need for an interrupt. The 
intention in using the swevent_hrtimer code was to enable setting up the 
events as frequency sampled. After taking another look at the gpci and 
24x7 pmus, I'm forbidding sampling events anyhow in event init, so the 
timer code isn't even taken advantage of. I'll drop this patch in the 
next set.

>
> You also try and conceal this information; so you suck.
>