Messages in this thread | | | Date | Fri, 18 Nov 2016 14:23:46 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH] perf/x86/uncore: Allow single pmu/box within events group |
| |
On Fri, Nov 18, 2016 at 01:53:54PM +0100, Peter Zijlstra wrote: > On Fri, Nov 18, 2016 at 01:33:25PM +0100, Jiri Olsa wrote: > > On Fri, Nov 18, 2016 at 12:28:50PM +0100, Peter Zijlstra wrote: > > > On Fri, Nov 18, 2016 at 01:15:28AM +0100, Jiri Olsa wrote: > > > > Current uncore_validate_group code expects all events within > > > > the group to have same pmu. > > > > > > > > This leads to constraint code using wrong boxes which leads > > > > in my case to touching uninitialized spinlocks, but could > > > > be probably worse.. depends on type and box details. > > > > > > > > I get lockdep warning below for following perf stat: > > > > # perf stat -vv -e '{uncore_cbox_0/config=0x0334/,uncore_qpi_0/event=1/}' -a sleep 1 > > > > > > Hurm, we shouldn't be allowing that in the first place I think. > > > > > > > > > Let me stare at the generic group code, the intent was to only allow > > > software events to mix with hw events, nothing else. > > > > yep, that's what's happening now.. but after the event_init callback > > Ah yes indeed. Its the is_uncore_event() test in uncore_collect_event() > that's too lenient, that allows us to mix events from various uncore > boxes. > > Would something like so fix things too? Because that is the point of > is_uncore_event() in collect(), to only collect events for _that_ pmu.
that looks ok.. I'll run the test
jirka
| |