[{"id":1762486,"web_url":"http://patchwork.ozlabs.org/comment/1762486/","msgid":"<20170904074636.smsygnxfofr46we5@hirez.programming.kicks-ass.net>","list_archive_url":null,"date":"2017-09-04T07:46:36","subject":"Re: [PATCH v2 net-next 1/4] bpf: add helper\n\tbpf_perf_read_counter_time for perf event array map","submitter":{"id":493,"url":"http://patchwork.ozlabs.org/api/people/493/","name":"Peter Zijlstra","email":"peterz@infradead.org"},"content":"On Fri, Sep 01, 2017 at 10:48:21PM -0700, Yonghong Song wrote:\n> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h\n> index b14095b..5a50808 100644\n> --- a/include/linux/perf_event.h\n> +++ b/include/linux/perf_event.h\n> @@ -898,7 +898,8 @@ perf_event_create_kernel_counter(struct perf_event_attr *attr,\n> \t\t\t\tvoid *context);\n> extern void perf_pmu_migrate_context(struct pmu *pmu,\n> \t\t\t\tint src_cpu, int dst_cpu);\n> -int perf_event_read_local(struct perf_event *event, u64 *value);\n> +int perf_event_read_local(struct perf_event *event, u64 *value,\n> +\t\t\t u64 *enabled, u64 *running);\n> extern u64 perf_event_read_value(struct perf_event *event,\n> \t\t\t\t u64 *enabled, u64 *running);\n> \n\n> diff --git a/kernel/events/core.c b/kernel/events/core.c\n> index 8c01572..20c4039 100644\n> --- a/kernel/events/core.c\n> +++ b/kernel/events/core.c\n> @@ -3670,7 +3670,8 @@ static inline u64 perf_event_count(struct perf_event *event)\n> * will not be local and we cannot read them atomically\n> * - must not have a pmu::count method\n> */\n> -int perf_event_read_local(struct perf_event *event, u64 *value)\n> +int perf_event_read_local(struct perf_event *event, u64 *value,\n> +\t\t\t u64 *enabled, u64 *running)\n> {\n> \tunsigned long flags;\n> \tint ret = 0;\n> @@ -3694,7 +3695,7 @@ int perf_event_read_local(struct perf_event *event, u64 *value)\n> \t * It must not have a pmu::count method, those are not\n> \t * NMI safe.\n> \t */\n> -\tif (event->pmu->count) {\n> +\tif (value && event->pmu->count) {\n> \t\tret = -EOPNOTSUPP;\n> \t\tgoto out;\n> \t}\n\nNo, value _must_ be !NULL. Otherwise you allow getting timestamps\nindependently from the count value and that is broken.\n\nThe {value, enabled, running} tuple is temporally related.\n\n> @@ -3718,10 +3719,16 @@ int perf_event_read_local(struct perf_event *event, u64 *value)\n> \t * or local to this CPU. Furthermore it means its ACTIVE (otherwise\n> \t * oncpu == -1).\n> \t */\n> -\tif (event->oncpu == smp_processor_id())\n> -\t\tevent->pmu->read(event);\n> -\n> -\t*value = local64_read(&event->count);\n> +\tif (value) {\n> +\t\tif (event->oncpu == smp_processor_id())\n> +\t\t\tevent->pmu->read(event);\n> +\t\t*value = local64_read(&event->count);\n> +\t}\n> +\tif (enabled && running) {\n> +\t\tu64 ctx_time = event->shadow_ctx_time + perf_clock();\n> +\t\t*enabled = ctx_time - event->tstamp_enabled;\n> +\t\t*running = ctx_time - event->tstamp_running;\n> +\t}\n> out:\n> \tlocal_irq_restore(flags);\n\nPlease make that something like:\n\n\n\tu64 now = event->shadow_ctx_time + perf_clock();\n\n\tif (enabled)\n\t\t*enabled = now - event->tstamp_enabled;\n\n\tif (event->oncpu == smp_processor_id()) {\n\t\tevent->pmu->read(event);\n\t\tif (running)\n\t\t\t*running = now - event->tstamp_running;\n\t} else {\n\t\t*running = event->total_time_running;\n\t}\n\nAnd I'll fix it up when I make:\n\n https://lkml.kernel.org/r/20170831171837.njnc6r6elsvkl7lt@hirez.programming.kicks-ass.net\n\nhappen.","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=infradead.org header.i=@infradead.org\n\theader.b=\"oG7UMS/v\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xm23q6T3rz9s06\n\tfor ;\n\tMon, 4 Sep 2017 17:46:47 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753299AbdIDHqp (ORCPT );\n\tMon, 4 Sep 2017 03:46:45 -0400","from bombadil.infradead.org ([65.50.211.133]:60970 \"EHLO\n\tbombadil.infradead.org\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753259AbdIDHqo (ORCPT\n\t); Mon, 4 Sep 2017 03:46:44 -0400","from j217100.upc-j.chello.nl ([24.132.217.100]\n\thelo=hirez.programming.kicks-ass.net)\n\tby bombadil.infradead.org with esmtpsa (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dom5d-0003PJ-VX; Mon, 04 Sep 2017 07:46:38 +0000","by hirez.programming.kicks-ass.net (Postfix, from userid 1000)\n\tid 0E33F202A064C; Mon, 4 Sep 2017 09:46:36 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=infradead.org; s=bombadil.20170209;\n\th=In-Reply-To:Content-Type:MIME-Version\n\t:References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To:\n\tContent-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:\n\tResent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:\n\tList-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive;\n\tbh=Wvwcsh5foxD5Ae8ue5vVQZYjWgxknwfiK3cc21j8b04=;\n\tb=oG7UMS/vb1a+rGQSSEcykwCjy\n\txdaivO8RYNEHfg79r2VOm5kpRc/8O7ey4qG5vK1IpBjq4+LBnJDty+FvEpf7a6T/3H3pmoz/fwhIQ\n\tkyVEbaDzCxeQABWM8WwGM/xgFKMtFdNyytGPM7dtsNZHWyDEVtz0NW7LfM1MxoM/4ob4jo23mh705\n\tzOQUBk5xmw/BP5Tv7uArHAvelUZLIEBLUYEQ7EI7TxkCwT3mhXCgPYLQGo864NAIr2jpn1IIz0xnd\n\tKS7w3gLJLGAO6tmsBGr0aTFd84tECBhij8jxb8b5vefjbJ5mdNeNyIwOs1uJI//sLS9olA0C4oewg\n\tJFOwty1ZQ==;","Date":"Mon, 4 Sep 2017 09:46:36 +0200","From":"Peter Zijlstra ","To":"Yonghong Song ","Cc":"rostedt@goodmis.org, ast@fb.com, daniel@iogearbox.net,\n\tnetdev@vger.kernel.org, kernel-team@fb.com","Subject":"Re: [PATCH v2 net-next 1/4] bpf: add helper\n\tbpf_perf_read_counter_time for perf event array map","Message-ID":"<20170904074636.smsygnxfofr46we5@hirez.programming.kicks-ass.net>","References":"<20170902054824.371962-1-yhs@fb.com>\n\t<20170902054824.371962-2-yhs@fb.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170902054824.371962-2-yhs@fb.com>","User-Agent":"NeoMutt/20170609 (1.8.3)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]