From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753163AbcKRNZA (ORCPT ); Fri, 18 Nov 2016 08:25:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36062 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753128AbcKRNXt (ORCPT ); Fri, 18 Nov 2016 08:23:49 -0500 Date: Fri, 18 Nov 2016 14:23:46 +0100 From: Jiri Olsa To: Peter Zijlstra Cc: "Liang, Kan" , Andi Kleen , Vince Weaver , lkml , Ingo Molnar , Thomas Gleixner Subject: Re: [PATCH] perf/x86/uncore: Allow single pmu/box within events group Message-ID: <20161118132346.GA17918@krava> References: <20161118001528.GA28231@krava> <20161118112850.GO3117@twins.programming.kicks-ass.net> <20161118123325.GA15416@krava> <20161118125354.GQ3117@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161118125354.GQ3117@twins.programming.kicks-ass.net> User-Agent: Mutt/1.7.1 (2016-10-04) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Fri, 18 Nov 2016 13:23:48 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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