From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754475AbcH3H0P (ORCPT ); Tue, 30 Aug 2016 03:26:15 -0400 Received: from merlin.infradead.org ([205.233.59.134]:42122 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751035AbcH3H0M (ORCPT ); Tue, 30 Aug 2016 03:26:12 -0400 Date: Tue, 30 Aug 2016 09:26:03 +0200 From: Peter Zijlstra To: Jiri Olsa Cc: Vegard Nossum , Thomas Gleixner , Stephane Eranian , Vince Weaver , Ingo Molnar , David Carrillo-Cisneros , "H. Peter Anvin" , Kan Liang , Arnaldo Carvalho de Melo , Paul Turner , Linus Torvalds , LKML , Alexander Shishkin , linux-tip-commits@vger.kernel.org Subject: Re: [tip:perf/core] perf/core: Check return value of the perf_event_read() IPI Message-ID: <20160830072603.GG10168@twins.programming.kicks-ass.net> References: <1471467307-61171-2-git-send-email-davidcc@google.com> <20160822071737.GF4349@krava> <20160822082932.GA13171@krava> <20160822103823.GA2271@krava> <20160829100309.GS10121@twins.programming.kicks-ass.net> <20160829130213.GF10168@twins.programming.kicks-ass.net> <20160829185841.GA27077@krava> <20160830064724.GX10153@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160830064724.GX10153@twins.programming.kicks-ass.net> 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 On Tue, Aug 30, 2016 at 08:47:24AM +0200, Peter Zijlstra wrote: > If oncpu is not valid, the sched_out that made it invalid will have > updated the event count and we're good. > > All I'll leave is an explicit comment that we've ignored the > smp_call_function_single() return value on purpose. Something like so.. --- kernel/events/core.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/kernel/events/core.c b/kernel/events/core.c index 3f07e6cfc1b6..a35cbc382b2c 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -3576,12 +3576,21 @@ static int perf_event_read(struct perf_event *event, bool group) local_cpu = get_cpu(); cpu_to_read = find_cpu_to_read(event, local_cpu); + + /* + * Purposely ignore the smp_call_function_single() return + * value. + * + * If event->oncpu isn't a valid CPU it means the event got + * scheduled out and that will have updated the event count. + * + * Therefore, either way, we'll have an up-to-date event count + * after this. + */ + (void)smp_call_function_single(cpu_to_read, __perf_event_read, &data, 1); put_cpu(); - ret = smp_call_function_single(cpu_to_read, __perf_event_read, &data, 1); - /* The event must have been read from an online CPU: */ - WARN_ON_ONCE(ret); - ret = ret ? : data.ret; + ret = data.ret; } else if (event->state == PERF_EVENT_STATE_INACTIVE) { struct perf_event_context *ctx = event->ctx; unsigned long flags;