From patchwork Mon May 6 12:53:56 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Ellerman X-Patchwork-Id: 241635 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from ozlabs.org (localhost [IPv6:::1]) by ozlabs.org (Postfix) with ESMTP id 5412F2C022A for ; Mon, 6 May 2013 22:54:25 +1000 (EST) Received: by ozlabs.org (Postfix) id F1F3C2C00F0; Mon, 6 May 2013 22:53:57 +1000 (EST) Delivered-To: linuxppc-dev@ozlabs.org Received: from [192.168.1.2] (124-171-106-22.dyn.iinet.net.au [124.171.106.22]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPSA id BFA9F2C00EC; Mon, 6 May 2013 22:53:57 +1000 (EST) Message-ID: <1367844836.26741.1.camel@concordia> Subject: Re: [PATCH] powerpc, perf: Clear out branch entries to avoid any previous stale values From: Michael Ellerman To: Anshuman Khandual Date: Mon, 06 May 2013 22:53:56 +1000 In-Reply-To: <1367827480-5375-1-git-send-email-khandual@linux.vnet.ibm.com> References: <1367827480-5375-1-git-send-email-khandual@linux.vnet.ibm.com> X-Mailer: Evolution 3.6.2-0ubuntu0.1 Mime-Version: 1.0 Cc: linuxppc-dev@ozlabs.org, mikey@neuling.org, linux-kernel@vger.kernel.org X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Mon, 2013-05-06 at 13:34 +0530, Anshuman Khandual wrote: > The 'to' field inside branch entries might contain stale values from previous > PMU interrupt instances which had indirect branches. So clear all the values > before reading a fresh set of BHRB entries after a PMU interrupt. > > Signed-off-by: Anshuman Khandual > --- > arch/powerpc/perf/core-book3s.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c > index c627843..09db68d 100644 > --- a/arch/powerpc/perf/core-book3s.c > +++ b/arch/powerpc/perf/core-book3s.c > @@ -1590,6 +1590,8 @@ static void record_and_restart(struct perf_event *event, unsigned long val, > if (event->attr.sample_type & PERF_SAMPLE_BRANCH_STACK) { > struct cpu_hw_events *cpuhw; > cpuhw = &__get_cpu_var(cpu_hw_events); > + memset(cpuhw->bhrb_entries, 0, > + sizeof(struct perf_branch_entry) * BHRB_MAX_ENTRIES); > power_pmu_bhrb_read(cpuhw); > data.br_stack = &cpuhw->bhrb_stack; > } Wouldn't it be just as effective, and less overhead, to set .to = 0; in the else branch in power_pmu_bhrb_read() ? eg: cheers diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c index c627843..30af11a4 100644 --- a/arch/powerpc/perf/core-book3s.c +++ b/arch/powerpc/perf/core-book3s.c @@ -1516,6 +1516,7 @@ void power_pmu_bhrb_read(struct cpu_hw_events *cpuhw) } else { /* Update address, flags for current entry */ cpuhw->bhrb_entries[u_index].from = addr; + cpuhw->bhrb_entries[u_index].to = 0; cpuhw->bhrb_entries[u_index].mispred = pred; cpuhw->bhrb_entries[u_index].predicted = ~pred;