From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1780FC2BD09 for ; Fri, 28 Jun 2024 17:25:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=VuOgHg2hOWWGNT8nlbTJa/36fahl3bUk9HtnL9Usc04=; b=GiHkBrYBLv6FBqfWAhHH7ruywJ iP5xkLoS+YJ/l6MKcqviIlBy+HLvCcGRlFLccKzlXTaJ4NxMHEifOf4kNgHwf9rv4FjBbtJn3+OnU uOQHtMM0hEetU50ByLsReJFDkcqV+NZ/n+OByUKUBWgh7YSivunueC7m+EMbQCeVUaAtyRDzrqx5u oFtA1MNswvPUMQbRdXL1XqQOBdSypKaGuke97DxSLKsUmiWCLkvwDDem2qVyIyea+c1IDntPwiCFV 5xeCaDkrr6/k0rSy6lKMj6GzZjwl4jIJGxw7X7G3WjfDvhYyN3WGKR3QDlf2Vw85HcJJSHnYSP3ji a4Axurrw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sNFLX-0000000EUo4-0MEs; Fri, 28 Jun 2024 17:25:15 +0000 Received: from sin.source.kernel.org ([145.40.73.55]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sNFLM-0000000EUkj-0O61 for linux-arm-kernel@lists.infradead.org; Fri, 28 Jun 2024 17:25:06 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 3B9FCCE4233; Fri, 28 Jun 2024 17:25:02 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 60717C116B1; Fri, 28 Jun 2024 17:25:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1719595501; bh=AKDp42mRsUBja/ZLr+8Y1Iqms9ORd+ytnrDKdKS7MIg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=fahcjlzO7Dqk88dt4n1sOJ3BAs41JLixAADOHyJdH1tdvF4vPVb70vNM2lEYg82EX lUIB9ziaAEx23iFz/0dMlDSxHLQrQ+7vVGXIZxV+7ughBdShqrnkAqExIeAznTNTyf f1jAseVHD4vNp3CCYuUhvJGVzMsl7zf7s4Nssmo3m6oZxYkZ900Wuyw8KAgYiV+X75 JTlTzKlbnwage8O2OokM5LfGCbiICwrSKlGqgGTc4jxXnI0/S6h9LWxg1OfmVQxAgH X7Xm4NPthPo71OYampk8m6uAJq1TeR3vZrzMrdabHkJ6Yu6hwrHKNqznf99H1YZkKW 3pq/u56GHHRpQ== Date: Fri, 28 Jun 2024 10:24:58 -0700 From: Namhyung Kim To: Ian Rogers Cc: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Adrian Hunter , Kan Liang , John Garry , Will Deacon , James Clark , Mike Leach , Leo Yan , Suzuki K Poulose , Yicong Yang , Jonathan Cameron , Nick Terrell , Nick Desaulniers , Oliver Upton , Anshuman Khandual , Song Liu , Ilkka Koskinen , Huacai Chen , Yanteng Si , Sun Haiyong , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 00/27] Constify tool pointers Message-ID: References: <20240626203630.1194748-1-irogers@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20240626203630.1194748-1-irogers@google.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240628_102504_520664_633DCF8E X-CRM114-Status: GOOD ( 23.37 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Jun 26, 2024 at 01:36:02PM -0700, Ian Rogers wrote: > struct perf_tool provides a set of function pointers that are called > through when processing perf data. To make filling the pointers less > cumbersome, if they are NULL perf_tools__fill_defaults will add > default do nothing implementations. > > This change refactors struct perf_tool to have an init function that > provides the default implementation. The special use of NULL and > perf_tools__fill_defaults are removed. As a consequence the tool > pointers can then all be made const, which better reflects the > behavior a particular perf command would expect of the tool and to > some extent can reduce the cognitive load on someone working on a > command. I thought you actually wanted to make the tool const (rodata) but it seems you leave it as is but treat it as const. I'm curious if we can change the event delivery code something like: if (tool->func) tool->func(...); else stub_func(...); Then probably we don't need to touch the tool and make it const. Thoughts? Thanks, Namhyung > > v2: Remove dummy tool initialization [Adrian] and make zero sized. Add > cs-etm fix for address sanitizer build, found necessary when > testing dummy tool change. > > Ian Rogers (27): > perf auxevent: Zero size dummy tool > perf cs-etm: Fix address sanitizer dso build failure > perf tool: Constify tool pointers > perf tool: Move fill defaults into tool.c > perf tool: Add perf_tool__init > perf kmem: Use perf_tool__init > perf buildid-list: Use perf_tool__init > perf kvm: Use perf_tool__init > perf lock: Use perf_tool__init > perf evlist: Use perf_tool__init > perf record: Use perf_tool__init > perf c2c: Use perf_tool__init > perf script: Use perf_tool__init > perf inject: Use perf_tool__init > perf report: Use perf_tool__init > perf stat: Use perf_tool__init > perf annotate: Use perf_tool__init > perf sched: Use perf_tool__init > perf mem: Use perf_tool__init > perf timechart: Use perf_tool__init > perf diff: Use perf_tool__init > perf data convert json: Use perf_tool__init > perf data convert ctf: Use perf_tool__init > perf test event_update: Ensure tools is initialized > perf kwork: Use perf_tool__init > perf tool: Remove perf_tool__fill_defaults > perf session: Constify tool > > tools/perf/arch/x86/util/event.c | 4 +- > tools/perf/bench/synthesize.c | 2 +- > tools/perf/builtin-annotate.c | 44 ++-- > tools/perf/builtin-buildid-list.c | 10 + > tools/perf/builtin-c2c.c | 33 ++- > tools/perf/builtin-diff.c | 30 ++- > tools/perf/builtin-evlist.c | 10 +- > tools/perf/builtin-inject.c | 159 +++++++------ > tools/perf/builtin-kmem.c | 20 +- > tools/perf/builtin-kvm.c | 19 +- > tools/perf/builtin-kwork.c | 33 ++- > tools/perf/builtin-lock.c | 41 ++-- > tools/perf/builtin-mem.c | 37 +-- > tools/perf/builtin-record.c | 47 ++-- > tools/perf/builtin-report.c | 67 +++--- > tools/perf/builtin-sched.c | 50 ++-- > tools/perf/builtin-script.c | 106 ++++----- > tools/perf/builtin-stat.c | 26 +-- > tools/perf/builtin-timechart.c | 25 +- > tools/perf/builtin-top.c | 2 +- > tools/perf/builtin-trace.c | 4 +- > tools/perf/tests/cpumap.c | 6 +- > tools/perf/tests/dlfilter-test.c | 2 +- > tools/perf/tests/dwarf-unwind.c | 2 +- > tools/perf/tests/event_update.c | 9 +- > tools/perf/tests/stat.c | 6 +- > tools/perf/tests/thread-map.c | 2 +- > tools/perf/util/Build | 1 + > tools/perf/util/arm-spe.c | 14 +- > tools/perf/util/auxtrace.c | 12 +- > tools/perf/util/auxtrace.h | 20 +- > tools/perf/util/bpf-event.c | 4 +- > tools/perf/util/build-id.c | 34 +-- > tools/perf/util/build-id.h | 8 +- > tools/perf/util/cs-etm.c | 24 +- > tools/perf/util/data-convert-bt.c | 34 ++- > tools/perf/util/data-convert-json.c | 47 ++-- > tools/perf/util/dso.h | 10 + > tools/perf/util/event.c | 54 +++-- > tools/perf/util/event.h | 38 ++-- > tools/perf/util/header.c | 6 +- > tools/perf/util/header.h | 4 +- > tools/perf/util/hisi-ptt.c | 6 +- > tools/perf/util/intel-bts.c | 14 +- > tools/perf/util/intel-pt.c | 15 +- > tools/perf/util/jitdump.c | 4 +- > tools/perf/util/s390-cpumsf.c | 11 +- > tools/perf/util/session.c | 342 ++-------------------------- > tools/perf/util/session.h | 6 +- > tools/perf/util/synthetic-events.c | 80 +++---- > tools/perf/util/synthetic-events.h | 70 +++--- > tools/perf/util/tool.c | 294 ++++++++++++++++++++++++ > tools/perf/util/tool.h | 18 +- > tools/perf/util/tsc.c | 2 +- > 54 files changed, 967 insertions(+), 1001 deletions(-) > create mode 100644 tools/perf/util/tool.c > > -- > 2.45.2.741.gdbec12cfda-goog >