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 BE9FEC4345F for ; Wed, 1 May 2024 17:13:22 +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:MIME-Version:Message-ID:Date:References :In-Reply-To:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=zLX/uXaxmo9aMAEIlvSpNKS0eOD0cbc8Cvg31+3auno=; b=2GSqnRB3eCkEDI ZicJISzPUCUM5mjd2NHUYztHt9XwBoqXp9RsEvjX+Sz49Dw/YCDNl8SbtsAOywGFHFWNKKNjFSAl/ nx8ly0H/5wzEDUU83xWxSenpdF/G1VTGbIrpVyPXtwFzqUgcj2yFYuMuMZfkFK4AOBK9KmIsrvJwo 5Z1J+QNY28iuh/Hbe924dSx00eANQSvyXjhT2bI7/w89XHX63/03IKAZVoRVb6nL07EkH2Rpo+MUu C57wG2bjsHPEKF1OmSFOgzpy4t7BeoGZ4rwtNipqX+udChNd7PyjhxSt2PXW7BbvjUCultq8qKkaN QfiT6RtiIqRfDO44/YQQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s2DVz-0000000AHNR-233x; Wed, 01 May 2024 17:13:07 +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 1s2DVv-0000000AHLX-1t9l for linux-arm-kernel@lists.infradead.org; Wed, 01 May 2024 17:13:05 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 86CF9CE1376; Wed, 1 May 2024 17:13:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 01CF5C4AF18; Wed, 1 May 2024 17:12:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1714583579; bh=zbcbIeexpIPCHZ07qIjb2xvy0LAIrIkIli7fZrlkkPs=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=mzDOwakc6N/hsFge1akpD47JDrwLOLSaetMZZCmYVmWU9z77Y3smwP96XGF/jK7Lu e7aAtkFjfydVufECpNDfIBHWdjYLU5hKhaz5RPKM9JwQsu6SdXL95X1b4aQVOD3NVk J6hT+7VPC4XZG9m6xxKi1PErdm/GfALSq528wIRxRRg2MhxptlhXxxzxZWYjn9Q1L7 IBxowFFy6xHRYUuv6LOSXvKz+vVjFv9m4akDdidqK1TMxbjCx+3JXJ/qxTuqELRZ+B FfRfP6x0R/RjQc3x9X+Or2FuHqCNDtMyeUB/TUedwngviJWWsqkwHygxDRteaXOtAb Ukrj8mq6vgeHA== From: Puranjay Mohan To: Mark Rutland Cc: Catalin Marinas , Will Deacon , Sumit Garg , Stephen Boyd , Douglas Anderson , "Peter Zijlstra (Intel)" , Thomas Gleixner , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, bpf@vger.kernel.org, Ard Biesheuvel Subject: Re: [PATCH] arm64: implement raw_smp_processor_id() using thread_info In-Reply-To: References: <20240501154236.10236-1-puranjay@kernel.org> Date: Wed, 01 May 2024 17:12:52 +0000 Message-ID: MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240501_101304_060370_B86A01DD X-CRM114-Status: GOOD ( 30.89 ) 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 Mark Rutland writes: > Hi Puranjay, > > On Wed, May 01, 2024 at 03:42:36PM +0000, Puranjay Mohan wrote: >> ARM64 defines THREAD_INFO_IN_TASK which means the cpu id can be found >> from current_thread_info()->cpu. > > Nice! > > This is something that we'd wanted to do, but there were some historical > reasons that prevented that. I think it'd be worth describing that in the > commit message, e.g. > > | Historically, arm64 implemented raw_smp_processor_id() as a read of > | current_thread_info()->cpu. This changed when arm64 moved thread_info into > | task struct, as at the time CONFIG_THREAD_INFO_IN_TASK made core code use > | thread_struct::cpu for the cpu number, and due to header dependencies > | prevented using this in raw_smp_processor_id(). As a workaround, we moved to > | using a percpu variable in commit: > | > | 57c82954e77fa12c ("arm64: make cpu number a percpu variable") > | > | Since then, thread_info::cpu was reintroduced, and core code was made to use > | this in commits: > | > | 001430c1910df65a ("arm64: add CPU field to struct thread_info") > | bcf9033e5449bdca ("sched: move CPU field back into thread_info if THREAD_INFO_IN_TASK=y") > | > | Consequently it is possible to use current_thread_info()->cpu again. > >> Implement raw_smp_processor_id() using the above. This decreases the >> number of emitted instructions like in the following example: >> >> Dump of assembler code for function bpf_get_smp_processor_id: >> 0xffff8000802cd608 <+0>: nop >> 0xffff8000802cd60c <+4>: nop >> 0xffff8000802cd610 <+8>: adrp x0, 0xffff800082138000 >> 0xffff8000802cd614 <+12>: mrs x1, tpidr_el1 >> 0xffff8000802cd618 <+16>: add x0, x0, #0x8 >> 0xffff8000802cd61c <+20>: ldrsw x0, [x0, x1] >> 0xffff8000802cd620 <+24>: ret >> >> After this patch: >> >> Dump of assembler code for function bpf_get_smp_processor_id: >> 0xffff8000802c9130 <+0>: nop >> 0xffff8000802c9134 <+4>: nop >> 0xffff8000802c9138 <+8>: mrs x0, sp_el0 >> 0xffff8000802c913c <+12>: ldr w0, [x0, #24] >> 0xffff8000802c9140 <+16>: ret >> >> A microbenchmark[1] was built to measure the performance improvement >> provided by this change. It calls the following function given number of >> times and finds the runtime overhead: >> >> static noinline int get_cpu_id(void) >> { >> return smp_processor_id(); >> } >> >> Run the benchmark like: >> modprobe smp_processor_id nr_function_calls=1000000000 >> >> +--------------------------+------------------------+ >> | | Number of Calls | Time taken | >> +--------+-----------------+------------------------+ >> | Before | 1000000000 | 1602888401ns | >> +--------+-----------------+------------------------+ >> | After | 1000000000 | 1206212658ns | >> +--------+-----------------+------------------------+ >> | Difference (decrease) | 396675743ns (24.74%) | >> +---------------------------------------------------+ >> >> This improvement is in this very specific microbenchmark but it proves >> the point. >> >> The percpu variable cpu_number is left as it is because it is used in >> set_smp_ipi_range() >> >> [1] https://github.com/puranjaymohan/linux/commit/77d3fdd >> >> Signed-off-by: Puranjay Mohan >> --- >> arch/arm64/include/asm/smp.h | 8 ++------ >> 1 file changed, 2 insertions(+), 6 deletions(-) >> >> diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h >> index efb13112b408..88fd2ab805ec 100644 >> --- a/arch/arm64/include/asm/smp.h >> +++ b/arch/arm64/include/asm/smp.h >> @@ -34,13 +34,9 @@ >> DECLARE_PER_CPU_READ_MOSTLY(int, cpu_number); >> >> /* >> - * We don't use this_cpu_read(cpu_number) as that has implicit writes to >> - * preempt_count, and associated (compiler) barriers, that we'd like to avoid >> - * the expense of. If we're preemptible, the value can be stale at use anyway. >> - * And we can't use this_cpu_ptr() either, as that winds up recursing back >> - * here under CONFIG_DEBUG_PREEMPT=y. >> + * This relies on THREAD_INFO_IN_TASK, but arm64 defines that unconditionally. >> */ >> -#define raw_smp_processor_id() (*raw_cpu_ptr(&cpu_number)) >> +#define raw_smp_processor_id() (current_thread_info()->cpu) > > I think we can (and should) delete the comment entirely. Sure, I will add the information to the commit message and remove this comment in the next version. I think it would be useful to remove the cpu_number percpu variable as well. We can use &irq_stat in place of &cpu_number in set_smp_ipi_range() in the calls to request_percpu_nmi/irq() as this is just a dummy value and ipi_handler() doesn't use it. There are no other users of cpu_number. Thanks, Puranjay _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel