From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751499AbdALLsR (ORCPT ); Thu, 12 Jan 2017 06:48:17 -0500 Received: from foss.arm.com ([217.140.101.70]:44294 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750796AbdALLsQ (ORCPT ); Thu, 12 Jan 2017 06:48:16 -0500 Date: Thu, 12 Jan 2017 11:47:16 +0000 From: Mark Rutland To: David Carrillo-Cisneros , Peter Zijlstra Cc: linux-kernel , "x86@kernel.org" , Ingo Molnar , Thomas Gleixner , Andi Kleen , Kan Liang , Borislav Petkov , Srinivas Pandruvada , Dave Hansen , Vikas Shivappa , Arnaldo Carvalho de Melo , Vince Weaver , Paul Turner , Stephane Eranian Subject: Re: [RFC 2/6] perf/core: add a rb-tree index to inactive_groups Message-ID: <20170112114716.GC10615@leverpostej> References: <20170110102502.106187-1-davidcc@google.com> <20170110102502.106187-3-davidcc@google.com> <20170110141408.GC19704@leverpostej> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 10, 2017 at 12:20:00PM -0800, David Carrillo-Cisneros wrote: > On Tue, Jan 10, 2017 at 6:14 AM, Mark Rutland wrote: > > On Tue, Jan 10, 2017 at 02:24:58AM -0800, David Carrillo-Cisneros wrote: > > For example, on a big.LITTLE system, big and little CPU PMUs share the > > same context, but their events are mutually incompatible. On big CPUs we > > only want to consider the sub-tree of big events, and on little CPUs we > > only want to consider little events. Hence, we need to be abel to search > > by PMU. > > I see it now. So, if PMU were added to the rb-tree keys. How can the > generic code know what's the PMU of the current CPU? I'm not immediately sure. We might need to augment struct pmu or perf_event_context with information such that we can determine that. That's not something I'd considered in great detail, and I'm not sure if peter had something in mind. > > For SW PMUs, pmu::add() should never fail, and regardless of the order > > of the list we should be able to pmu::add() all events. Given that, why > > does the manner in which rotation occurs matter for SW PMUs? > > > >> Another complicatino is that using ctx->time (or timestamp) implies that > >> groups added during the same context switch may not have unique key. > >> This increases the complexity of that finds all events in the rb-tree > >> that are within a time interval. > > > > Could you elaborate on this? I don't understand what the problem is > > here. If we need uniqueness where {pmu,cpu,runtime} are equal, can't we > > extend the comparison to {pmu,cpu,runtime,event pointer}? That way > > everything we need is already implicit in the event, and we don't need > > perf_event::rbtree_key nor do we need > > perf_event_context::nr_inactive_added. > > Yes, we could extend the comparison. But I am trying to keep the key a > u64 to speed up things. > > I found it easier to simply create a counter and use it as an equivalent to > (timestamp, unique id). Both ways induce the same order of events. As I mentioned before, I believe that Peter's intent was to consider runtime, rather than a last-scheduled timestamp, so I don't think the counter is equivalent. It might be that either way is fine; I'll leave it to Peter to weigh in. Do we have any benchmark figures either way? Thanks, Mark.