Received: by 2002:a05:7412:4e10:b0:e2:908c:2ebd with SMTP id gb16csp70736rdb; Tue, 7 Nov 2023 00:34:31 -0800 (PST) X-Google-Smtp-Source: AGHT+IEqLXigzyWkYGBMxNpphxBxL1FNHRGL6mm9qNU0yiXDs4IFu2MqF9W/kNl+54WA/iRvJlnE X-Received: by 2002:a17:903:1250:b0:1cc:7ec0:bc8a with SMTP id u16-20020a170903125000b001cc7ec0bc8amr16438263plh.66.1699346070655; Tue, 07 Nov 2023 00:34:30 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1699346070; cv=none; d=google.com; s=arc-20160816; b=L2yMhZ91hbINimaRjpxpi34r+uVfGJSdIs9YDbUR4CAKgwSA1KgH9chyLijR7lqIhV YWgZc9bC1cOIckvuNAruNm54yEjpw5XJRv0PYhhXYu9689hfnCWgITzFYmYmELrTDEPh goOg2sMqDg/qk9Xoxovoz9hcAlmji6k+L4qHx7P50sh0OpcA0CGrY3V3++/fahMKnYyA c36q+7N+lBhA007k62olYiPitCK31YP21Bvn22wPVbTQHZLUlibWrlamSbE2EZE0zLvg L3tOy7X4mgvF/MSUA4lH07eZwQZP5EXUygXtZS9BTj4UZ/mf0OZJkUX3D2QxmKBw7Wzm 3+0w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:message-id:subject:cc :to:from:dkim-signature:dkim-signature:date; bh=zWJk9nL8xlrcEUZ19TjgnfyXI3oFOA97vzHt9B/TpHU=; fh=8+1zfhualYUyDTauH/O4akYyr9/4UEeckInISzpvZkI=; b=rIwkG3BsNhp9mGC9t5MjRH8RtGpXso1j/dvk6STT+DOcN41ahc+96GfIzWvcEnL8qt Io+DcNKO3l+Z7u0tTB3HXKz1O0KOygibt5f7oJaGVUZhuzdB3V6oqOY4unv0xBlJviaK XGgz2IjmVPtIm/meuj8gFFC3gjx6tp+U7PcFyt+gzGMgQgGfGGI35lRsbPXRokFb+G+4 YyKjkj9i6jZxet9KVmMjis9kbF4vVVuvwRt5VVBZjmHZwKgbcyTha05LWT0oKiIZADxE 05yEOZCMC84rmNtFK+Do2QVoSZ4U9xVuGaarMOVry+iHgEEWsgjSLVd/TOCY11x0Da/V c/NQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=2BAOVEM2; dkim=neutral (no key) header.i=@linutronix.de header.s=2020e; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Return-Path: Received: from snail.vger.email (snail.vger.email. [23.128.96.37]) by mx.google.com with ESMTPS id g2-20020a170902e38200b001bf5c12ea06si9401699ple.404.2023.11.07.00.34.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Nov 2023 00:34:30 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37; Authentication-Results: mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=2BAOVEM2; dkim=neutral (no key) header.i=@linutronix.de header.s=2020e; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 57DBE811F13C; Tue, 7 Nov 2023 00:34:29 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233766AbjKGIeV (ORCPT + 99 others); Tue, 7 Nov 2023 03:34:21 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50878 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233747AbjKGIeT (ORCPT ); Tue, 7 Nov 2023 03:34:19 -0500 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EF20FBD for ; Tue, 7 Nov 2023 00:34:15 -0800 (PST) Date: Tue, 7 Nov 2023 09:34:11 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1699346053; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zWJk9nL8xlrcEUZ19TjgnfyXI3oFOA97vzHt9B/TpHU=; b=2BAOVEM2MkhWFzYNhU0L0NWAgz71dhhwB/EyqNU3aPKvT/3kM5Xr7bz9jweX0fuVrm++Cl jJrRF3OH0M0pdXlr2jEbXyWMtJ4xZk4OoqBgPwYz1qkkkbKJ7aX05Fq8dmCZgEu/pb7YZH ZSMq3m5BetHLlW9LU2z7Q5hnPDzInWJKnRdTBRBvn0usqMgO7Mdrf65DdKEX67yoVboafl 556MILwGAOPrnSgzHOcIKzkHipW7rRJODJSl5dbgo/AiIukwnLfV3dydZWzZ29eQcGwvPe Z2fAHCLqOq/ihgFQuWwk+Bnkh5T98+AmQSdQaCzVXwuLd3D6D0biXSAiBOsojw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1699346053; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zWJk9nL8xlrcEUZ19TjgnfyXI3oFOA97vzHt9B/TpHU=; b=l8ekf5952TYSX13DM2PgLJGVKnksuDGDa4y/Hi+ZLOejIQVBdacpg1hHI+cGLPnzdyXPtB 9HNl1WjpJDpY0hCw== From: Sebastian Andrzej Siewior To: Evan Green Cc: Palmer Dabbelt , Jisheng Zhang , David Laight , Albert Ou , Andrew Jones , Anup Patel , =?utf-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= , Conor Dooley , Greentime Hu , Heiko Stuebner , Ley Foon Tan , Marc Zyngier , Palmer Dabbelt , Paul Walmsley , Sunil V L , linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org Subject: Re: [PATCH v3] RISC-V: Probe misaligned access speed in parallel Message-ID: <20231107083411.J3ga0YmA@linutronix.de> References: <20231106225855.3121724-1-evan@rivosinc.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <20231106225855.3121724-1-evan@rivosinc.com> X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Tue, 07 Nov 2023 00:34:29 -0800 (PST) On 2023-11-06 14:58:55 [-0800], Evan Green wrote: > Probing for misaligned access speed takes about 0.06 seconds. On a > system with 64 cores, doing this in smp_callin() means it's done > serially, extending boot time by 3.8 seconds. That's a lot of boot time. >=20 > Instead of measuring each CPU serially, let's do the measurements on > all CPUs in parallel. If we disable preemption on all CPUs, the > jiffies stop ticking, so we can do this in stages of 1) everybody > except core 0, then 2) core 0. The allocations are all done outside of > on_each_cpu() to avoid calling alloc_pages() with interrupts disabled. >=20 > For hotplugged CPUs that come in after the boot time measurement, > register CPU hotplug callbacks, and do the measurement there. Interrupts > are enabled in those callbacks, so they're fine to do alloc_pages() in. I think this is dragged out of proportion. I would do this (if needed can can't be identified by CPU-ID or so) on boot CPU only. If there is evidence/ proof/ blessing from the high RiscV council that different types of CPU cores are mixed together then this could be extended. You brought Big-Little up in the other thread. This is actually known. Same as with hyper-threads on x86, you know which CPU is the core and which hyper thread (CPU) belongs to it. So in terms of BigLittle you _could_ limit this to one Big and one Little core instead running it on all. But this is just my few on this. From PREEMPT_RT's point of view, the way you restructured the memory allocation should work now. > Reported-by: Jisheng Zhang > Closes: https://lore.kernel.org/all/mhng-9359993d-6872-4134-83ce-c97debe1= cf9a@palmer-ri-x1c9/T/#mae9b8f40016f9df428829d33360144dc5026bcbf > Fixes: 584ea6564bca ("RISC-V: Probe for unaligned access speed") > Signed-off-by: Evan Green >=20 >=20 > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeatur= e.c > index 6a01ded615cd..fe59e18dbd5b 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c =E2=80=A6 > =20 > -static int __init check_unaligned_access_boot_cpu(void) > +/* Measure unaligned access on all CPUs present at boot in parallel. */ > +static int check_unaligned_access_all_cpus(void) > { > - check_unaligned_access(0); > + unsigned int cpu; > + unsigned int cpu_count =3D num_possible_cpus(); > + struct page **bufs =3D kzalloc(cpu_count * sizeof(struct page *), > + GFP_KERNEL); kcalloc(). For beauty reasons you could try a reverse xmas tree.=20 > + > + if (!bufs) { > + pr_warn("Allocation failure, not measuring misaligned performance\n"); > + return 0; > + } > + > + /* > + * Allocate separate buffers for each CPU so there's no fighting over > + * cache lines. > + */ > + for_each_cpu(cpu, cpu_online_mask) { > + bufs[cpu] =3D alloc_pages(GFP_KERNEL, MISALIGNED_BUFFER_ORDER); > + if (!bufs[cpu]) { > + pr_warn("Allocation failure, not measuring misaligned performance\n"); > + goto out; > + } > + } > + > + /* Check everybody except 0, who stays behind to tend jiffies. */ > + on_each_cpu(check_unaligned_access_nonboot_cpu, bufs, 1); comments! _HOW_ do you ensure that CPU0 is left out? You don't. CPU0 does this and the leaves which is a waste. Using on_each_cpu_cond() could deal with this. And you have the check within the wrapper (check_unaligned_access_nonboot_cpu()) anyway. > + /* Check core 0. */ > + smp_call_on_cpu(0, check_unaligned_access, bufs[0], true); Now that comment is obvious. If you want to add a comment, why not state why CPU0 has to be done last? > + > + /* Setup hotplug callback for any new CPUs that come online. */ > + cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "riscv:online", > + riscv_online_cpu, NULL); Instead riscv:online you could use riscv:unaliged_check or something that pin points the callback to something obvious. This is exported via sysfs. Again, comment is obvious. For that to make sense would require RiscV to support physical-hotplug. For KVM like environment (where you can plug in CPUs later) this probably doesn't make sense at all. Why not? Because - without explicit CPU pinning your slow/ fast CPU mapping (host <-> guest) could change if the scheduler on the host moves the threads around. - without explicit task offload and resource partitioning on the host your guest thread might get interrupt during the measurement. This is done during boot so chances are high that it runs 100% of its time slice and will be preempted once other tasks on the host ask for CPU run time. Both points mean, that the results may not be accurate over time. Maybe a KVM-hint if this is so important (along with other restrictions). > + > +out: > unaligned_emulation_finish(); > + for_each_cpu(cpu, cpu_online_mask) { > + if (bufs[cpu]) > + __free_pages(bufs[cpu], MISALIGNED_BUFFER_ORDER); > + } > + > + kfree(bufs); > return 0; > } > =20 > -arch_initcall(check_unaligned_access_boot_cpu); > +arch_initcall(check_unaligned_access_all_cpus); > =20 > void riscv_user_isa_enable(void) > { Sebastian