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). The difference in naming allows new userspace to do the right thing while not breaking existing userspace, though I agree it's somewhat clunky. > > and perf stat may unexpectedly block at exit time. In the absence of a > > cpumask, perf stat is functional. > > > > To address this, this patch adds support for a new file, > > supported_cpumask, which can be used to describe heterogeneous CPUs, > > without the risk of breaking existing userspace binaries. > > is there kernel patch adding supported_cpumask support? Modulo the naming, a patch exists [1]. We were holding off adding that until we'd figured out how to address breaking existing userspace [2]. Thanks, Mark. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438239.html [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/441953.html
