From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756223AbbGPVmj (ORCPT ); Thu, 16 Jul 2015 17:42:39 -0400 Received: from e19.ny.us.ibm.com ([129.33.205.209]:53435 "EHLO e19.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756135AbbGPVmh (ORCPT ); Thu, 16 Jul 2015 17:42:37 -0400 X-Helo: d01dlp01.pok.ibm.com X-MailFrom: sukadev@linux.vnet.ibm.com X-RcptTo: sparclinux@vger.kernel.org Date: Thu, 16 Jul 2015 14:41:24 -0700 From: Sukadev Bhattiprolu To: Peter Zijlstra Cc: Ingo Molnar , Arnaldo Carvalho de Melo , Michael Ellerman , linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org, sparclinux@vger.kernel.org Subject: Re: [PATCH v3 5/8] perf: Split perf_event_read_value() Message-ID: <20150716214124.GB31962@us.ibm.com> References: <1436929315-28520-1-git-send-email-sukadev@linux.vnet.ibm.com> <1436929315-28520-6-git-send-email-sukadev@linux.vnet.ibm.com> <20150716211219.GN3644@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150716211219.GN3644@twins.programming.kicks-ass.net> X-Operating-System: Linux 2.0.32 on an i486 User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15071621-0057-0000-0000-000000C0888E Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Peter Zijlstra [peterz@infradead.org] wrote: | On Tue, Jul 14, 2015 at 08:01:52PM -0700, Sukadev Bhattiprolu wrote: | > Move the part of perf_event_read_value() that computes the event | > counts and event times into a new function, perf_event_compute(). | > | > This would allow us to call perf_event_compute() independently. | > | > Signed-off-by: Sukadev Bhattiprolu | > | > Changelog[v3] | > Rather than move perf_event_read() into callers and then | > rename, just move the computations into a separate function | > (redesign to address comment from Peter Zijlstra). | > --- | | Changelog[] bits go here, below the '---' where they get discarded. Sorry. Will fix it. | | > kernel/events/core.c | 37 ++++++++++++++++++++++++------------- | > 1 file changed, 24 insertions(+), 13 deletions(-) | > | > diff --git a/kernel/events/core.c b/kernel/events/core.c | > index 44fb89d..b1e9a42 100644 | > --- a/kernel/events/core.c | > +++ b/kernel/events/core.c | > @@ -3704,6 +3704,29 @@ static int perf_release(struct inode *inode, struct file *file) | > return 0; | > } | > | > +static u64 perf_event_compute(struct perf_event *event, u64 *enabled, | > + u64 *running) | | This is a horrible name, 'compute' what? We are aggregating event counts and time for children. Would perf_event_aggregate() or perf_event_aggregate_children() be better? | | > +{ | > + struct perf_event *child; | > + u64 total; | > + | > + total = perf_event_count(event); | > + | > + *enabled += event->total_time_enabled + | > + atomic64_read(&event->child_total_time_enabled); | > + *running += event->total_time_running + | > + atomic64_read(&event->child_total_time_running); | > + | | lockdep_assert_held(&event->child_mutex); OK. Thanks for the comments. Sukadev