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 X-Spam-Level: X-Spam-Status: No, score=-13.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4E0BAC47082 for ; Tue, 8 Jun 2021 15:36:12 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 04DA26128E for ; Tue, 8 Jun 2021 15:36:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 04DA26128E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=SIbJ9QS4/qhrN4/CMRqRaUL9/JGU9we9jfSFq1wrP2A=; b=x1ItoFznf7UfBN 0swvh0PPgvlVcBYZDabXc/+3cDqCb9v86gavTmTTblZj4mluq5imQW/RJn/DEOPG0+MHjWaxxidji GTBerkb6Z9XS3w33eidKLThd+Z6Tek3MjdQqIpExSht0oJjHzcgG5qAiqTSeKl4dbhH/qwXEW/01n 6OS+8NiRLDlQT88+gPUJNaqAXkPyYC3RYLMCUmKmQQPTx71/4va/YJAFBf08IVykT22Va6seWpKRt 8JgxyZVRPhbvQ8lshlzSxVRTFFoNtHOgY5/l8FId9d0uFFFcIoIflsN6S1RPFiDPSkpnJJFT0uMID wEJEQ99/2/Khn3l7Zicg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lqdkU-009FR5-1C; Tue, 08 Jun 2021 15:34:38 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lqdkL-009FPQ-Jr for linux-arm-kernel@lists.infradead.org; Tue, 08 Jun 2021 15:34:31 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 328426D; Tue, 8 Jun 2021 08:34:28 -0700 (PDT) Received: from C02TD0UTHF1T.local (unknown [10.57.5.29]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5BCC13F73D; Tue, 8 Jun 2021 08:34:27 -0700 (PDT) Date: Tue, 8 Jun 2021 16:34:24 +0100 From: Mark Rutland To: Leo Yan Cc: Will Deacon , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 1/2] arm64: perf: Correct per-thread mode if the event is not supported Message-ID: <20210608153424.GD16585@C02TD0UTHF1T.local> References: <20210608145228.36595-1-leo.yan@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210608145228.36595-1-leo.yan@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210608_083429_784800_5EA71CA2 X-CRM114-Status: GOOD ( 33.56 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Jun 08, 2021 at 10:52:27PM +0800, Leo Yan wrote: > When the perf tool runs in per-thread mode, armpmu_event_init() defers > to handle events in armpmu_add(), the main reason is the selected PMU in > the init phase can mismatch with the CPUs when the profiled task > is scheduled on. > > For example, on an Arm big.LTTILE platform with two clusters, every > cluster has its dedicated PMU; the event initialization happens on the > LITTLE cluster and its corresponding PMU is selected, but the profiled > task is scheduled on big cluster, it's deferred to handle this case in > armpmu_add(). > > Usually, we should report failure in the first place so this can allow > users to easily locate the issue they are facing. For the per-thread > mode, the profiled task can be migrated on any CPU, therefore the event > can be enabled on any CPU. In other words, if a PMU detects it fails to > support the process-following event, it can directly returns -EOPNOTSUPP > so can stop profiling. > > This patch adds the checking for per-thread mode, if the event is not > supported, return -EOPNOTSUPP. I don't understand the rationale for this patch. We call armpmu_event_init() from perf_try_init_event(), and if we return *any* error code that will be returned to userspace, or at least that used to be the case. What problem are you trying to solve here? Is this some fallout of commit: 55bcf6ef314ae8ba ("perf: Extend PERF_TYPE_HARDWARE and PERF_TYPE_HW_CACHE") ... ? Thanks, Mark. > > Signed-off-by: Leo Yan > --- > drivers/perf/arm_pmu.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c > index d4f7f1f9cc77..aedea060ca8b 100644 > --- a/drivers/perf/arm_pmu.c > +++ b/drivers/perf/arm_pmu.c > @@ -502,9 +502,9 @@ static int armpmu_event_init(struct perf_event *event) > /* > * Reject CPU-affine events for CPUs that are of a different class to > * that which this PMU handles. Process-following events (where > - * event->cpu == -1) can be migrated between CPUs, and thus we have to > - * reject them later (in armpmu_add) if they're scheduled on a > - * different class of CPU. > + * event->cpu == -1) can be migrated between CPUs, and thus we will > + * reject them when map_event() detects absent entry at below or later > + * (in armpmu_add) if they're scheduled on a different class of CPU. > */ > if (event->cpu != -1 && > !cpumask_test_cpu(event->cpu, &armpmu->supported_cpus)) > @@ -514,8 +514,16 @@ static int armpmu_event_init(struct perf_event *event) > if (has_branch_stack(event)) > return -EOPNOTSUPP; > > - if (armpmu->map_event(event) == -ENOENT) > + if (armpmu->map_event(event) == -ENOENT) { > + /* > + * Process-following event is not supported on current PMU, > + * returns -EOPNOTSUPP to stop perf at the initialization > + * phase. > + */ > + if (event->cpu == -1) > + return -EOPNOTSUPP; > return -ENOENT; > + } > > return __hw_perf_event_init(event); > } > -- > 2.25.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel