Re: [RFCv2 4/4] perf: util: support sysfs supported_cpumask file

[Jiri Olsa]

On Mon, Jul 18, 2016 at 04:00:45PM +0100, Mark Rutland wrote:
> On Mon, Jul 18, 2016 at 04:30:18PM +0200, Jiri Olsa wrote:
> > On Fri, Jul 15, 2016 at 11:08:13AM +0100, Mark Rutland wrote:
> > > For system PMUs, the perf tools have long expected a cpumask file under
> > > sysfs, describing the single CPU which they support events being
> > 
> > single cpu? it's cpumask.. 
> 
> Indeed.
> 
> The issue is that in practice, due to an internal inconsistency the
> perf tools only work work when a single CPU is described in the mask.
> More details below (and in patch 1).
> 
> > > opened/handled on. Prior patches in this series have reworked this
> > > support to support multiple CPUs in a mask, as is required to handle
> > > heterogeneous CPU PMUs.
> > > 
> > > Unfortunately, adding a cpumask file to CPU PMUs would break existing
> > > userspace. Prior to this series, perf record will refuse to open events,
> > 
> > I'm lost.. we already have 'cpumask' file under pmu..
> 
> Sorry, I should spell out the problem more concretely:
> 
> When manipulating events, the tools sometimes use evsel->cpus, and other
> times evlist->cpus. Sometimes, the two are used inconsistently, which
> only works if they are the same size and/or describe the same CPUs.
> Patch 1 fixes an instance of this, where the inconsistency results in
> treating uninitialised memory as perf event FDs.
> 
> In the absence of a PMU cpumask file, the evsel's cpumask is initialised
> to that of the evlist, so things line up.
> 
> Currently the only PMUs which happen to expose a cpumask are uncore
> PMUs, which in practice only describe a single CPU.
> 
> When recording system-wide, various parts of the perf tools assume a
> single CPU, regardless of evlist->cpus, for the purpose of manipulating
> events. This happens to make uncore PMUs work, avoiding the
> inconsistency.
> 
> Were we to just add a 'cpumask' file to our CPU PMUs, we would break
> existing userspace (e.g. hitting the issue fixed in patch 1).

so you're saying that perf is broken once pmu's cpumask
contains more than single cpu, is that right?

we should fix that, not make workarounds.. I'll go check,
I might be still missing something ;-)

would be great to have some automated test for this stuff 

thanks,
jirka