Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941272AbcKXTC6 (ORCPT ); Thu, 24 Nov 2016 14:02:58 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:55406 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935677AbcKXTC5 (ORCPT ); Thu, 24 Nov 2016 14:02:57 -0500 Date: Thu, 24 Nov 2016 20:02:42 +0100 From: Peter Zijlstra To: Mark Rutland Cc: kan.liang@intel.com, mingo@redhat.com, acme@kernel.org, linux-kernel@vger.kernel.org, alexander.shishkin@linux.intel.com, tglx@linutronix.de, namhyung@kernel.org, jolsa@kernel.org, adrian.hunter@intel.com, wangnan0@huawei.com, andi@firstfloor.org Subject: Re: [PATCH 02/14] perf/x86: output NMI overhead Message-ID: <20161124190242.GK3092@twins.programming.kicks-ass.net> References: <1479894292-16277-1-git-send-email-kan.liang@intel.com> <1479894292-16277-3-git-send-email-kan.liang@intel.com> <20161124161712.GA2444@remoulade> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161124161712.GA2444@remoulade> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1800 Lines: 51 On Thu, Nov 24, 2016 at 04:19:09PM +0000, Mark Rutland wrote: > > static void > > event_sched_out(struct perf_event *event, > > struct perf_cpu_context *cpuctx, > > - struct perf_event_context *ctx) > > + struct perf_event_context *ctx, > > + bool log_overhead) > > Boolean parameter are always confusing. Why not pass the flags directly? That > way we can pass *which* overhead to log, and make the callsites easier to > understand. > > > event->tstamp_stopped = tstamp; > > - event->pmu->del(event, 0); > > + event->pmu->del(event, log_overhead ? PERF_EF_LOG : 0); > > ... which we could pass on here. > > > @@ -1835,20 +1835,21 @@ event_sched_out(struct perf_event *event, > > static void > > group_sched_out(struct perf_event *group_event, > > struct perf_cpu_context *cpuctx, > > - struct perf_event_context *ctx) > > + struct perf_event_context *ctx, > > + bool log_overhead) > > Likewise. > > > @@ -1872,7 +1873,7 @@ __perf_remove_from_context(struct perf_event *event, > > { > > unsigned long flags = (unsigned long)info; > > > > - event_sched_out(event, cpuctx, ctx); > > + event_sched_out(event, cpuctx, ctx, false); > > if (flags & DETACH_GROUP) > > perf_group_detach(event); > > list_del_event(event, ctx); > > @@ -1918,9 +1919,9 @@ static void __perf_event_disable(struct perf_event *event, > > update_cgrp_time_from_event(event); > > update_group_times(event); > > if (event == event->group_leader) > > - group_sched_out(event, cpuctx, ctx); > > + group_sched_out(event, cpuctx, ctx, true); > > else > > - event_sched_out(event, cpuctx, ctx); > > + event_sched_out(event, cpuctx, ctx, true); > > Why does this differ from __perf_remove_from_context()? So I'm not a great fan of sprinkling all this through the core code at all.