Re: Basic perf PMU support for Haswell v14

Re: Basic perf PMU support for Haswell v14

[Ingo Molnar]

* Andi Kleen <andi@firstfloor.org> wrote:

> v13:
> Various white space changes.
> v14:
> Rebased to 3.10-rc6

FYI, I'll have a look at your patches in the coming days.

As I warned you in the past your very slow, piecemail-wise, feet-dragging 
response to review feedback which has resulted in a supposedly simple perf 
driver submission reach an absurdly high version count of 14 over 6+ 
months has understandably put your patch-queue to the end of my (sadly 
long) TODO list - there's no need for you to keep pestering me or PeterZ 
over private mail when I'll take yet another look...

Thanks,

	Ingo

Re: Basic perf PMU support for Haswell v14

[Ingo Molnar]

* Andi Kleen <andi@firstfloor.org> wrote:

> Contains support for:
> - Basic Haswell PMU and PEBS support
> - Late unmasking of the PMI
> - mem-loads/stores support
> 
> v2: Addressed Stephane's feedback. See individual patches for details.
> v3: now even more bite-sized. Qualifier constraints merged earlier.
> v4: Rename some variables, add some comments and other minor changes.
> Add some Reviewed/Tested-bys.
> v5: Address some minor review feedback. Port to latest perf/core
> v6: Add just some variable names, add comments, edit descriptions, some
> more testing, rebased to latest perf/core
> v7: Expand comment
> v8: Rename structure field.
> v9: No wide counters, but add basic LBRs. Add some more 
> constraints. Rebase to 3.9rc1
> v10: Change some whitespace. Rebase to 3.9rc3
> v11: Rebase to perf/core. Fix extra regs. Rename INTX.
> v12: Rebase to 3.10-rc2
> Add mem-loads/stores support for parity with Sandy Bridge.
> Fix fixed counters (Thanks Ingo!)
> Make late ack optional
> Export new config bits in sysfs.
> Minor changes
> v13:
> Various white space changes.
> v14:
> Rebased to 3.10-rc6

Ok, this series looks good at first sight: you've slightly overdone the 
checkpatch style changes without checking that the result is actually an 
improvement - I've fixed those few places.

I'll give it a bit of testing.

Thanks,

	Ingo

Re: Basic perf PMU support for Haswell v14

[Andi Kleen]

> Ok, this series looks good at first sight: you've slightly overdone the 
> checkpatch style changes without checking that the result is actually an 
> improvement - I've fixed those few places.
> 
> I'll give it a bit of testing.

Thanks for testing and merging.

I still have some other patches to enable other features,
on top of this.

I'll repost them in hopefully logical groups. 

If you want them grouped in some particular way or other
requirements please let me know.

https://git.kernel.org/cgit/linux/kernel/git/ak/linux-misc.git/log/?h=hsw/pmu6

FWIW the outstanding patches do:

- Some changes to make CPU event aliases more user friendly:

print aliases in perf list, avoid the need to specify cpu//,
automatically enable PEBS for mem-loads/stores,
Allow aliases to specify "precise"

This is all not Haswell specific, but makes
the TSX changes below a lot more user friendly.

Most of this is actually user mode only, except for the
"precise" alias changes.

- TSX changes: perf stat -T, TSX event aliases, support for 
reporting the abort cause from PEBS, plus the user tools
support for all of this

- Full width counting (generally useful, but also avoids
a TSX checkpoint problem)

- Support TSX filtering in the the LBR, plus a workaround
for the duplicated abort record.

Plus also on top (not in my git tree):

- Zheng's call graph lbr mode patches, to allow user mode -g without
frame pointers or full dwarf. This has a few known problems, but is still
useful enough that it should be available as a option.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: Basic perf PMU support for Haswell v14

[Ingo Molnar]

* Andi Kleen <andi@firstfloor.org> wrote:

> > Ok, this series looks good at first sight: you've slightly overdone the 
> > checkpatch style changes without checking that the result is actually an 
> > improvement - I've fixed those few places.
> > 
> > I'll give it a bit of testing.
> 
> Thanks for testing and merging.
> 
> I still have some other patches to enable other features,
> on top of this.
> 
> I'll repost them in hopefully logical groups. 
> 
> If you want them grouped in some particular way or other
> requirements please let me know.
> 
> https://git.kernel.org/cgit/linux/kernel/git/ak/linux-misc.git/log/?h=hsw/pmu6
> 
> FWIW the outstanding patches do:
> 
> - Some changes to make CPU event aliases more user friendly:
> 
> print aliases in perf list, avoid the need to specify cpu//,
> automatically enable PEBS for mem-loads/stores,
> Allow aliases to specify "precise"
> 
> This is all not Haswell specific, but makes
> the TSX changes below a lot more user friendly.
> 
> Most of this is actually user mode only, except for the
> "precise" alias changes.

Yeah. This I think looks good mostly.

> - TSX changes: perf stat -T, TSX event aliases, support for 
> reporting the abort cause from PEBS, plus the user tools
> support for all of this

Yes.

> - Full width counting (generally useful, but also avoids
> a TSX checkpoint problem)

I'd suggest putting this in first and testing it ASAP, so that any TSX 
patches don't have to worry about it. I'd expect it to just work.

[ You could also now add the patch that standardizes APIC-Ack handling in 
  the NMI handler, removing the Haswell specific quirk. If it causes any 
  problems it's bisectable. ]

> - Support TSX filtering in the the LBR, plus a workaround
> for the duplicated abort record.

Ok.

Please resend all of these on top of latest -tip.

My main and pretty much only complaint regarding the merge process of the 
'basic' Haswell patches in the past few months is that you should try to 
avoid the trivial problems preemptively so that maintainers don't have to 
waste time on dealing with them again and again. Also, when a particular 
category of problem is pointed out try to fix similar instances in other 
pending patches as well - not just strictly the exact problems that have 
been pointed out.

> Plus also on top (not in my git tree):
> 
> - Zheng's call graph lbr mode patches, to allow user mode -g without 
> frame pointers or full dwarf. This has a few known problems, but is 
> still useful enough that it should be available as a option.

This looks useful too, please send these changes in a separate submission 
with all call graph people Cc:-ed: Peter, Frederic, Arnaldo, Jiri, 
Namhyung.

Thanks,

	Ingo