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 BF8FBC52D7B for ; Tue, 13 Aug 2024 14:39:49 +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:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=UtQ1YB4RrIk11ETTD2r10cRbUnQqCHQoAwSrZok6NMo=; b=zpnEomzvFAejNUpP3aHdIpFFdH QrAl/FndNWARyzuSfCH3syIHPx5laJp8yztPydMjv72ktao5RjiqStMhJD4hHAkgHINg7Dh7T0ROu ATUh4dDFbcVPv5NpQhfdjvmF1zLw2de97E6Kdh96TPeeRw1zoZllwqceO2jfRNA1jPfMyX+p6bLIT 5wrSa84ZZKtZ99lKmnR8d80T06wQdNy4GN5znzXSaj6yM3skXPLNTrs308rBD60djuyAowucRNkmS vfmu7MgC+KDSoQl15j9UXTWnSJ15ZtgQYUGn1M/JKdhEZiZK12ZDrjjbQINaMxPh5/IpmxyeoEJED DsjDihRw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sdsgR-000000043CX-2BMC; Tue, 13 Aug 2024 14:39:35 +0000 Received: from mail-wm1-x32c.google.com ([2a00:1450:4864:20::32c]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sdsfp-0000000433J-1Jj7 for linux-arm-kernel@lists.infradead.org; Tue, 13 Aug 2024 14:38:58 +0000 Received: by mail-wm1-x32c.google.com with SMTP id 5b1f17b1804b1-428243f928cso39287905e9.3 for ; Tue, 13 Aug 2024 07:38:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1723559935; x=1724164735; darn=lists.infradead.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=UtQ1YB4RrIk11ETTD2r10cRbUnQqCHQoAwSrZok6NMo=; b=PpWu6FpcuQ1omMCROJkZ6yjq3SmGmuIDIEQkGVbYJtxzXz8sh5x1rAACEoQgPe40Bo NWVt5pdfxP5vQJuFAkxiv8hlA0YqykYDjGFfhSr1BTtyKgLyC10PjiCSxPfkIkChIOCd 1tHlzTxIxXrg8Vw0MEZ+SqNRasdh01svHocTS1MUgBomPbXPMpKjRv0eJeunQyQX1HM6 s8RBxWyWdp5+AR9UmXvNrEkBjpeYKbLcGXJKWTxzaMHA1Apw8sTBpFlkR+cR9iLZZfid ZAuzb3H5DBdifRLhTYBCc4Z3kgh7ayyR138RjFzXX48u5iyNk2ECmFuqZKRjtZ9YubfY 81jw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723559935; x=1724164735; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=UtQ1YB4RrIk11ETTD2r10cRbUnQqCHQoAwSrZok6NMo=; b=nxf0msmTNR/Khj8GkG6KPwrRSUwJ/KG9GUz3yBe+vwP9C2ApXbcv+98o211BT8lW3O dMlPCY/I9gQn2upk7RaxCxyRkezcc/71mzhunyYplGPW0DaxoBuPLgq66OzxKuhMAhgV eKyATqZMNR2d9lMvdT0LZSotyy5Sm+Lpw6xb6b9UK9YYRBXzlR31CZFQ4myB5c4Ui2dJ 6HU5Ue++G26wFjW6rO3/nkLk6ddEBg5Xv7HuXPJXCjdu2ZF9Nc6KVvZgee8jf8mH+AKs DiOj1RjWBKLkcTOnCRf5Yb5Q37jhzNJy6e21wxzgA2n18AI8WC4fd3NAMljGdNGpVIQm 5lmw== X-Forwarded-Encrypted: i=1; AJvYcCUlbgrNV0owPwi+07LsyziJTLHlO/N6U5rEMSi66lQ01PIYr1wiHLSverVoQQ/Z4QYat/16QIBSVrhCMG7IGmsjub4hTyg9OI2SAdSzq2fZBvJKQvY= X-Gm-Message-State: AOJu0YwXDZZMUCYlo7uXeDBof3IvQp3RHevhMljpihyqfvZlbBeCfV9h jVVcUkUlNPlxFWpcc7MEBk+PkGLzDDc2tUxi38wAQvStsgC622R2Di6PM5/xCtQ= X-Google-Smtp-Source: AGHT+IGZ3q4qbGnDy3Ugzxb9kudMaZUjN4anbfPXoXoEcqrmhBSoqtwtLTteBZDVMdUtj+hl9W7U2w== X-Received: by 2002:a5d:40cc:0:b0:367:947a:a491 with SMTP id ffacd0b85a97d-3716cd028b6mr2751743f8f.26.1723559935045; Tue, 13 Aug 2024 07:38:55 -0700 (PDT) Received: from [192.168.1.3] ([89.47.253.130]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-36e4f0a7111sm10604674f8f.117.2024.08.13.07.38.54 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 13 Aug 2024 07:38:54 -0700 (PDT) Message-ID: <76ee9150-36fa-4dfc-ba9f-8a10df580c92@linaro.org> Date: Tue, 13 Aug 2024 15:38:53 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/7] perf stat: Initialize instead of overwriting clock event To: Ian Rogers Cc: linux-perf-users@vger.kernel.org, John Garry , Will Deacon , Mike Leach , Leo Yan , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Namhyung Kim , Mark Rutland , Alexander Shishkin , Jiri Olsa , Adrian Hunter , "Liang, Kan" , Yang Jihong , Ze Gao , Dominique Martinet , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org References: <20240813132323.98728-1-james.clark@linaro.org> <20240813132323.98728-2-james.clark@linaro.org> Content-Language: en-US From: James Clark In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240813_073857_363057_A5BA2442 X-CRM114-Status: GOOD ( 26.28 ) 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 13/08/2024 3:28 pm, Ian Rogers wrote: > On Tue, Aug 13, 2024 at 6:24 AM James Clark wrote: >> >> This overwrite relies on the clock event remaining at index 0 and is >> quite a way down from where the array is initialized, making it easy to >> miss. Just initialize it with the correct clock event to begin with. >> >> Signed-off-by: James Clark >> --- >> tools/perf/builtin-stat.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c >> index 1f92445f7480..a65f58f8783f 100644 >> --- a/tools/perf/builtin-stat.c >> +++ b/tools/perf/builtin-stat.c >> @@ -1817,7 +1817,9 @@ static int add_default_attributes(void) >> { >> struct perf_event_attr default_attrs0[] = { >> >> - { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_TASK_CLOCK }, >> + { .type = PERF_TYPE_SOFTWARE, .config = target__has_cpu(&target) ? >> + PERF_COUNT_SW_CPU_CLOCK : >> + PERF_COUNT_SW_TASK_CLOCK }, > > Hand crafting perf_event_attr when we have an event name to > perf_event_atttr parser doesn't make sense. Doing things this way > means we need to duplicate logic between event parsing and these > default configurations. The default configurations are also using > legacy events which of course are broken on Apple ARM M? (albeit for > hardware events, here it is software). Event and metric parsing has to > worry about things like grouping topdown events. All-in-all let's have > one way to do things, event parsing, otherwise this code is going to > end up reinventing all the workarounds the event parsing has to have. > Lots of struct perf_event_attr also contribute to binary size. > > If you are worried about a cycles event being opened on arm_dsu PMUs, > there is this patch: > https://lore.kernel.org/lkml/20240525152927.665498-1-irogers@google.com/ > > Thanks, > Ian Hi Ian, Is this comment related to this patch specifically or is it more of a general comment? This patch doesn't really make any actual changes other than move one line of code from one place to another. James