Re: [PATCH 14/15] perf x86 intel: Add DATALA events into sysfs

[Jiri Olsa]

On Wed, Feb 24, 2016 at 07:59:05AM -0800, Andi Kleen wrote:
> On Wed, Feb 24, 2016 at 09:46:55AM +0100, Jiri Olsa wrote:
> > Adding DATALA events into Haaswell events sysfs
> > directory, so it's easier to use them.
> > 
> > Adding all DATALA events as specified by Intel SDM
> > manual 18.10.3 PEBS Data Address Profiling.
> > 
> > Link: http://lkml.kernel.org/n/tip-dvpk5ys80v41oh5xgqvloviw@xxxxxxxxxxxxxx
> > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > ---
> >  arch/x86/events/intel/core.c | 39 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> > 
> > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> > index a7ec685657a5..199f645aedc1 100644
> > --- a/arch/x86/events/intel/core.c
> > +++ b/arch/x86/events/intel/core.c
> > @@ -3253,6 +3253,26 @@ static __init void intel_ht_bug(void)
> >  EVENT_ATTR_STR(mem-loads,	mem_ld_hsw,	"event=0xcd,umask=0x1,ldlat=3");
> >  EVENT_ATTR_STR(mem-stores,	mem_st_hsw,	"event=0xd0,umask=0x82")
> 
> > +EVENT_ATTR_STR(mem-load-hit-lfb,	hsw_load_hit_lfb,	"event=0xd1,umask=0x40");
> > +EVENT_ATTR_STR(mem-snp-miss,		hsw_snp_miss,		"event=0xd2,umask=0x01");
> > +EVENT_ATTR_STR(mem-snp-hit,		hsw_snp_hit,		"event=0xd2,umask=0x02");
> > +EVENT_ATTR_STR(mem-snp-hitm,		hsw_snp_hitm,		"event=0xd2,umask=0x04");
> > +EVENT_ATTR_STR(mem-snp-none,		hsw_snp_none,		"event=0xd2,umask=0x08");
> 
> I don't think these are well enough defined for generic events. For example what does
> hit-lfb mean generally? Snooping is also a fairly nebulous concept that can change
> between micro architectures.
> 
> Instead of adding more and more badly defined and not really portable 
> generic events I think it would be better to apply the patches that enable
> the full Intel/POWER/other JSON event lists.
> 
> They have been posted multiple times by Sukadev and just need to be reviewed,
> and would be a far better solution to this.
> 
> That would also make them "easy to use".
> 
> This would have the advantage that we actually have documentation 
> what they actually mean in the SDM, and they map to the
> documentation, and people can understand them without needing
> to reverse engineer perf.
> 
> Also these events can have bugs (see e.g. the Haswell Server spec updates)
> and if you can't look them up it can be hard to use.

right, I'll keep that only in my queue.. we discussed with Peter
and he said he'll try to check on JSON patchset state ;-)

thanks,
jirka