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 F0577C2BB3F for ; Sat, 18 Nov 2023 18:45:42 +0000 (UTC) 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=0fj9JxRUKaEXfBq8zMGCBs+45N0ZT2oKFkpcSc07GXk=; b=YFIJcMMrprs0+s q9w6Rgrw+laUef6MWUExrGa3Z4t2Io3RQf0Tz6c8TVlOYsy4ohbBxzVjTu4AVvQqI9R2n3IpsPnnh cqvHTUUJrjHES4r8SI6SRerlUCHizLusKofWkjftX0SOuyz14KM6yq+9BdgS0Rlh0+wNGbPoOMwHa cJ2hCEew8yvxSmCFmLf6pV1oh7BXTGFW8pt4w5GNCQ6v0GFYzLXoGhozdM2se/MSBswiStSPmowaQ Lph7m24XAwLA6V45my+PcmXH5mP+5CMwjTORgwapLCBjYGIJwKGnBUMPuBW/bt2FDzCEWgiLkdi3B 4bDh3g7wPMX05GhKBGlw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r4QJf-0090TZ-1B; Sat, 18 Nov 2023 18:45:15 +0000 Received: from mail-yb1-xb34.google.com ([2607:f8b0:4864:20::b34]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1r4QJc-0090ST-32 for linux-arm-kernel@lists.infradead.org; Sat, 18 Nov 2023 18:45:14 +0000 Received: by mail-yb1-xb34.google.com with SMTP id 3f1490d57ef6-da37522a363so2967222276.0 for ; Sat, 18 Nov 2023 10:45:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700333111; x=1700937911; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=bW2tiTIrBWAXiy30jb0LKEp8e44RWdEZj57EYCLluIw=; b=coHJ3M6hNZxg+Nv/JQugWuAPS9GK7TMUbMmNxP1jHi4stT0rz0pWlshWM6Sipr3byy ry+1a1EyCXujWmhAizu+H5plBJMuLbn5jsIfocBPKKJRCFCmkqBmOa66p7TZYqbdZidJ N8s6OkWsotYT9BL84RXUAHJao9jA5DdEETnoGg9WcbO4eYKJeh67JbMz/5/I3UL+pbJ2 bRdR+8XNOG0SxeXvF/hrhcE8FINm0Tq7Rt/fSY3JyKFuXqmGt+BzXBFL9zBehq2+QeX6 8mcnaKqvawaPV4m9OeuaglVFIDR/VV51hXtVhIQR9yAXUTyFfY54TKzP2KMqBS4RsyG1 nprw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700333111; x=1700937911; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=bW2tiTIrBWAXiy30jb0LKEp8e44RWdEZj57EYCLluIw=; b=NB4h7lZ7tSp2ZHlJwtUbnOPXpVCzXmQLDW0HB2BOwK1MLOsPug9yewQQffwNH8MYEE nDZjjsBea6RBiB+0aJ+XNEP1cAtg3aSYkK2it85RqfgzNSuwPu5dgx2aaIE/nNWiKpPk wruCi7I0cg/Tp2Pqnvmc1+XT0pFlfnQCizQFK4yZYufvdzWEMEwhIx3Ls6R2uAaCBai+ Rx0EtHWtaq40ztYUUNg6J25RoBZCsC23V6LAzOcJXPI+cnehYm9vsxDBWMBE3wJKKUa9 2Y9XmIkeaZeo6OR1WY5L0MCQpvBg4oTXkQTBeDiiuUoncdQTRbUIDwzCobpElPo0warL pLqQ== X-Gm-Message-State: AOJu0YwZRHJtwLbSPtqtn4fhuIbx/MQztws9MGm3YXyi9Ka2mr0hahhn XC1QFXF11Z1n2Rfm2a5GWiA= X-Google-Smtp-Source: AGHT+IHiqNQImhu8dUqW/vDSumeQYp/EBUMQCRNHeIgCB3E0iPG/RuSSIO/uJ7jxZ62gFFvlGS1/tw== X-Received: by 2002:a25:d091:0:b0:db0:1338:8a64 with SMTP id h139-20020a25d091000000b00db013388a64mr2719699ybg.57.1700333111258; Sat, 18 Nov 2023 10:45:11 -0800 (PST) Received: from localhost ([2601:344:8301:57f0:48a9:bd4c:868d:dc97]) by smtp.gmail.com with ESMTPSA id b79-20020a253452000000b00da076458395sm1065426yba.43.2023.11.18.10.45.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Nov 2023 10:45:10 -0800 (PST) Date: Sat, 18 Nov 2023 10:45:10 -0800 From: Yury Norov To: Marc Zyngier Cc: linux-kernel@vger.kernel.org, Will Deacon , Mark Rutland , linux-arm-kernel@lists.infradead.org, Jan Kara , Mirsad Todorovac , Matthew Wilcox , Rasmus Villemoes , Andy Shevchenko , Maxim Kuvyrkov , Alexey Klimov Subject: Re: [PATCH 31/34] drivers/perf: optimize m1_pmu_get_event_idx() by using find_bit() API Message-ID: References: <20231118155105.25678-1-yury.norov@gmail.com> <20231118155105.25678-32-yury.norov@gmail.com> <87cyw6ykes.wl-maz@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87cyw6ykes.wl-maz@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231118_104513_004066_2CA72001 X-CRM114-Status: GOOD ( 26.54 ) 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 Sat, Nov 18, 2023 at 06:40:43PM +0000, Marc Zyngier wrote: > On Sat, 18 Nov 2023 15:51:02 +0000, > Yury Norov wrote: > > > > The function searches used_mask for a bit in a for-loop bit by bit. > > We can do it faster by using atomic find_and_set_bit(). > > Sure, let's do things fast. Correctness is overrated anyway. > > > > > The comment to the function says that it searches for the first free > > counter, but obviously for_each_set_bit() searches for the first set > > counter. > > No it doesn't. It iterates over the counters the event can count on. > > > The following test_and_set_bit() tries to enable already set > > bit, which is weird. > > Maybe you could try to actually read the code? > > > > > This patch, by using find_and_set_bit(), fixes this automatically. > > This doesn't fix anything, but instead actively breaks the driver. > > > > > Fixes: a639027a1be1 ("drivers/perf: Add Apple icestorm/firestorm CPU PMU driver") > > Signed-off-by: Yury Norov > > --- > > drivers/perf/apple_m1_cpu_pmu.c | 8 ++------ > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/perf/apple_m1_cpu_pmu.c b/drivers/perf/apple_m1_cpu_pmu.c > > index cd2de44b61b9..2d50670ffb01 100644 > > --- a/drivers/perf/apple_m1_cpu_pmu.c > > +++ b/drivers/perf/apple_m1_cpu_pmu.c > > @@ -447,12 +447,8 @@ static int m1_pmu_get_event_idx(struct pmu_hw_events *cpuc, > > * counting on the PMU at any given time, and by placing the > > * most constraining events first. > > */ > > - for_each_set_bit(idx, &affinity, M1_PMU_NR_COUNTERS) { > > - if (!test_and_set_bit(idx, cpuc->used_mask)) > > - return idx; > > - } > > - > > - return -EAGAIN; > > + idx = find_and_set_bit(cpuc->used_mask, M1_PMU_NR_COUNTERS); > > + return idx < M1_PMU_NR_COUNTERS ? idx : -EAGAIN; > > So now you're picking any possible counter, irrespective of the > possible affinity of the event. This is great. Ok, I'll drop the patch. Sorry. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel