From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 23 Jul 2015 09:45:09 +0200 From: Peter Zijlstra Subject: Re: [PATCH v3 5/8] perf: Split perf_event_read_value() Message-ID: <20150723074509.GD25159@twins.programming.kicks-ass.net> References: <1436929315-28520-1-git-send-email-sukadev@linux.vnet.ibm.com> <1436929315-28520-6-git-send-email-sukadev@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <1436929315-28520-6-git-send-email-sukadev@linux.vnet.ibm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , List-Archive: To: Sukadev Bhattiprolu 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 List-ID: 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). > --- > 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) > +{ > + 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); > + > + list_for_each_entry(child, &event->child_list, child_list) { > + perf_event_read(child); Sure we don't want that.. > + total += perf_event_count(child); > + *enabled += child->total_time_enabled; > + *running += child->total_time_running; > + } > + > + return total; > +} From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Zijlstra Date: Thu, 23 Jul 2015 07:45:09 +0000 Subject: Re: [PATCH v3 5/8] perf: Split perf_event_read_value() Message-Id: <20150723074509.GD25159@twins.programming.kicks-ass.net> List-Id: References: <1436929315-28520-1-git-send-email-sukadev@linux.vnet.ibm.com> <1436929315-28520-6-git-send-email-sukadev@linux.vnet.ibm.com> In-Reply-To: <1436929315-28520-6-git-send-email-sukadev@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Sukadev Bhattiprolu 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 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). > --- > 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) > +{ > + 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); > + > + list_for_each_entry(child, &event->child_list, child_list) { > + perf_event_read(child); Sure we don't want that.. > + total += perf_event_count(child); > + *enabled += child->total_time_enabled; > + *running += child->total_time_running; > + } > + > + return total; > +}