Message ID | 20231114170038.381634-1-ubizjak@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:6358:a59:b0:164:83eb:24d7 with SMTP id 25csp2061636rwb; Tue, 14 Nov 2023 09:01:36 -0800 (PST) X-Google-Smtp-Source: AGHT+IH2+owcpWjQF7SeNWrbzGE7YkJt5V8x1yQno9gprUFe864FSgyx9zNX6ukVrLJ8Z58cinFC X-Received: by 2002:a17:902:8b85:b0:1cc:68a5:f397 with SMTP id ay5-20020a1709028b8500b001cc68a5f397mr2837707plb.51.1699981296088; Tue, 14 Nov 2023 09:01:36 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1699981296; cv=none; d=google.com; s=arc-20160816; b=bXN32eSEkuOCyr8P4O0n5ocujkrRuLzMoFvFyxILxrEiNE9uI/VvxQ9ltU8SPtaWBZ mAmG9VOkxCoYRyY6sjxe2ybbte5HFTKhTP0Nvc1cWT+yel91UPINnw7h2DUeXkpAjTvm zJvIdiDvvB8h+6hsZwZOq3GzZHsLe/q2TmH3w0XgNzMGRFs4dUnMZtVxJ9r6W86dWzVQ bJbsQKswyhUmcu8xbQo/h6qIeAVIcIKn5rTdIJFbQhMMXblveOVAYUBFQIWTtqw6iaX7 Bvh7fBV/pz1U6hIN5xFOHnIzOFFwZoFqglyJKKtQDAr5zLVXlUbn0e8ebDnXkkofNhAH AioA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=aQal8qUMEmhK7Dkdqvrsuy8gJSftsXg0qDExwqH+0QI=; fh=1oj0bM0B3SWQTZx0yZbA9H0/wQw1I3gpmMu2F2juP1Q=; b=ldLDF7VXuMXl+QnCl0xDaYJbYKvrpeS2CpPCA9Fw99N0gflRiMzpxdmFC+SEiON4zT 47dy9Jh92oR/THCoMVh+T2JqQU76DmxMqgW4iukmIAMQtgR6ECVr9nAupdOfGb6yZnxV 8G+HoPLf6mwB6FXymjWjoCov/t7+jGnb5cCvzmV/rXyQoOUXa2xE6cVN77tRAS9ONPna 3rN5a+MjVATUMAaCSKNwvcy6pFqoxBwsmpMbROyQYzG8rxidcAKCm1AiJlhdBcXbfVAX r3B++1vq0kFN3WUcoPl69lzn00q/rtyUFXohYCe8O6VVyJ7yEK1erEI1H8sSarqjsEpB pEqA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=dzLiiK1k; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from agentk.vger.email (agentk.vger.email. [2620:137:e000::3:2]) by mx.google.com with ESMTPS id e7-20020a170902b78700b001bbca0a8393si8036458pls.56.2023.11.14.09.01.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Nov 2023 09:01:36 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=dzLiiK1k; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id 2526A8043CA1; Tue, 14 Nov 2023 09:01:19 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232229AbjKNRA5 (ORCPT <rfc822;lhua1029@gmail.com> + 29 others); Tue, 14 Nov 2023 12:00:57 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36036 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229607AbjKNRAy (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 14 Nov 2023 12:00:54 -0500 Received: from mail-lj1-x232.google.com (mail-lj1-x232.google.com [IPv6:2a00:1450:4864:20::232]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 206B911D; Tue, 14 Nov 2023 09:00:51 -0800 (PST) Received: by mail-lj1-x232.google.com with SMTP id 38308e7fff4ca-2c79d8b67f3so64643591fa.0; Tue, 14 Nov 2023 09:00:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1699981249; x=1700586049; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=aQal8qUMEmhK7Dkdqvrsuy8gJSftsXg0qDExwqH+0QI=; b=dzLiiK1kOBV1tbNT+t7ndRYhKTf/rz5Y5rabfXCLPrMpO5XNGEeuOj8NYFnxlPMA9x uyuHE3u2vTZz3M37CYtzRSBJMOljQzx2OLN5a+uvLkGabbo540Av7b+ReaGqO97yHICa QFBq/At/u9j5prQe8Cm0z+UNljLE+ysDWdYf3d5OYSgmryh2XnWxkLlZRVvIrCAczEPJ JNeu64XZfjMKEM4bR6aTzsXOPIkX87W8HofEVjz1du/3lz9swS0z/+oBvlrARQS1HpLb Fp9m5TOm9RHxyJCEwD06CNrp4bikqPbHdhsKHrpO9tZt8eDo08gOMGBzhaBp1BhLsyVN SEXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699981249; x=1700586049; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=aQal8qUMEmhK7Dkdqvrsuy8gJSftsXg0qDExwqH+0QI=; b=hS4Rc9YJEKLm8fcqneem7dXVNlhAcMMzxezDsW1i76++Dp5xQvfaDBiQgOo3OslS43 EE3TPZDMuztV0Qy/dJ5oZPUREZRcHf/c+6nb6k9ssiUtpi0roLBMaM8/i/kh47ao5GmA hl5n4aKgbg1bzL3OD8njn3PD8rP7Z2LOd3yPdpftZNgdJf1qMs8aQUvgfVDrG2/xsvHg 3toA2IB9CDisgrX5Kwirr4lLEGJA8nhI6qS8J1mqa+C+j3aP6uyjN1EwUzj23BK6mkpo J1gg4awUUiSoMqmo1tihA6/IhrXnEzqVzaRW8wKakw9UmyuNWGgTrgpQBW7MUOykiwvl YmNw== X-Gm-Message-State: AOJu0Yz4+9MKvl64UZgXbO2LFqdpN0dYBcTBc9/T9mkimpPLamB0QNnn N1iNwzNwIEWvNa2SqZFx5xyJWqaVbR9rEFuB X-Received: by 2002:a05:651c:2204:b0:2c5:52d:c9ff with SMTP id y4-20020a05651c220400b002c5052dc9ffmr2432095ljq.10.1699981248844; Tue, 14 Nov 2023 09:00:48 -0800 (PST) Received: from localhost.localdomain ([46.248.82.114]) by smtp.gmail.com with ESMTPSA id k40-20020a05600c1ca800b0040a5e69482esm2465315wms.11.2023.11.14.09.00.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Nov 2023 09:00:48 -0800 (PST) From: Uros Bizjak <ubizjak@gmail.com> To: linux-hyperv@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org Cc: Uros Bizjak <ubizjak@gmail.com>, "K. Y. Srinivasan" <kys@microsoft.com>, Haiyang Zhang <haiyangz@microsoft.com>, Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>, "H. Peter Anvin" <hpa@zytor.com> Subject: [PATCH] x86/hyperv: Use atomic_try_cmpxchg() to micro-optimize hv_nmi_unknown() Date: Tue, 14 Nov 2023 17:59:28 +0100 Message-ID: <20231114170038.381634-1-ubizjak@gmail.com> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Tue, 14 Nov 2023 09:01:19 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1782559587484515084 X-GMAIL-MSGID: 1782559587484515084 |
Series |
x86/hyperv: Use atomic_try_cmpxchg() to micro-optimize hv_nmi_unknown()
|
|
Commit Message
Uros Bizjak
Nov. 14, 2023, 4:59 p.m. UTC
Use atomic_try_cmpxchg() instead of atomic_cmpxchg(*ptr, old, new) == old
in hv_nmi_unknown(). On x86 the CMPXCHG instruction returns success in
the ZF flag, so this change saves a compare after CMPXCHG. The generated
asm code improves from:
3e: 65 8b 15 00 00 00 00 mov %gs:0x0(%rip),%edx
45: b8 ff ff ff ff mov $0xffffffff,%eax
4a: f0 0f b1 15 00 00 00 lock cmpxchg %edx,0x0(%rip)
51: 00
52: 83 f8 ff cmp $0xffffffff,%eax
55: 0f 95 c0 setne %al
to:
3e: 65 8b 15 00 00 00 00 mov %gs:0x0(%rip),%edx
45: b8 ff ff ff ff mov $0xffffffff,%eax
4a: f0 0f b1 15 00 00 00 lock cmpxchg %edx,0x0(%rip)
51: 00
52: 0f 95 c0 setne %al
No functional change intended.
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Wei Liu <wei.liu@kernel.org>
Cc: Dexuan Cui <decui@microsoft.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
---
arch/x86/kernel/cpu/mshyperv.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
Comments
From: Uros Bizjak <ubizjak@gmail.com> Sent: Tuesday, November 14, 2023 8:59 AM > > Use atomic_try_cmpxchg() instead of atomic_cmpxchg(*ptr, old, new) == old > in hv_nmi_unknown(). On x86 the CMPXCHG instruction returns success in > the ZF flag, so this change saves a compare after CMPXCHG. The generated > asm code improves from: > > 3e: 65 8b 15 00 00 00 00 mov %gs:0x0(%rip),%edx > 45: b8 ff ff ff ff mov $0xffffffff,%eax > 4a: f0 0f b1 15 00 00 00 lock cmpxchg %edx,0x0(%rip) > 51: 00 > 52: 83 f8 ff cmp $0xffffffff,%eax > 55: 0f 95 c0 setne %al > > to: > > 3e: 65 8b 15 00 00 00 00 mov %gs:0x0(%rip),%edx > 45: b8 ff ff ff ff mov $0xffffffff,%eax > 4a: f0 0f b1 15 00 00 00 lock cmpxchg %edx,0x0(%rip) > 51: 00 > 52: 0f 95 c0 setne %al > > No functional change intended. > > Cc: "K. Y. Srinivasan" <kys@microsoft.com> > Cc: Haiyang Zhang <haiyangz@microsoft.com> > Cc: Wei Liu <wei.liu@kernel.org> > Cc: Dexuan Cui <decui@microsoft.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@kernel.org> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> > --- > arch/x86/kernel/cpu/mshyperv.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/cpu/mshyperv.c > b/arch/x86/kernel/cpu/mshyperv.c index e6bba12c759c..01fa06dd06b6 > 100644 > --- a/arch/x86/kernel/cpu/mshyperv.c > +++ b/arch/x86/kernel/cpu/mshyperv.c > @@ -262,11 +262,14 @@ static uint32_t __init ms_hyperv_platform(void) > static int hv_nmi_unknown(unsigned int val, struct pt_regs *regs) { > static atomic_t nmi_cpu = ATOMIC_INIT(-1); > + unsigned int old_cpu, this_cpu; > > if (!unknown_nmi_panic) > return NMI_DONE; > > - if (atomic_cmpxchg(&nmi_cpu, -1, raw_smp_processor_id()) != -1) > + old_cpu = -1; > + this_cpu = raw_smp_processor_id(); > + if (!atomic_try_cmpxchg(&nmi_cpu, &old_cpu, this_cpu)) > return NMI_HANDLED; > > return NMI_DONE; > -- > 2.41.0 The change looks correct to me. But is there any motivation other than saving 3 bytes of generated code? This is not a performance sensitive path. And the change adds 3 lines of source code. So I wonder if the change is worth the churn. In any case, Reviewed-by: Michael Kelley <mhklinux@outlook.com>
On Wed, Nov 15, 2023 at 6:19 PM Michael Kelley <mhklinux@outlook.com> wrote: > > From: Uros Bizjak <ubizjak@gmail.com> Sent: Tuesday, November 14, 2023 8:59 AM > > > > Use atomic_try_cmpxchg() instead of atomic_cmpxchg(*ptr, old, new) == old > > in hv_nmi_unknown(). On x86 the CMPXCHG instruction returns success in > > the ZF flag, so this change saves a compare after CMPXCHG. The generated > > asm code improves from: > > > > 3e: 65 8b 15 00 00 00 00 mov %gs:0x0(%rip),%edx > > 45: b8 ff ff ff ff mov $0xffffffff,%eax > > 4a: f0 0f b1 15 00 00 00 lock cmpxchg %edx,0x0(%rip) > > 51: 00 > > 52: 83 f8 ff cmp $0xffffffff,%eax > > 55: 0f 95 c0 setne %al > > > > to: > > > > 3e: 65 8b 15 00 00 00 00 mov %gs:0x0(%rip),%edx > > 45: b8 ff ff ff ff mov $0xffffffff,%eax > > 4a: f0 0f b1 15 00 00 00 lock cmpxchg %edx,0x0(%rip) > > 51: 00 > > 52: 0f 95 c0 setne %al > > > > No functional change intended. > > > > Cc: "K. Y. Srinivasan" <kys@microsoft.com> > > Cc: Haiyang Zhang <haiyangz@microsoft.com> > > Cc: Wei Liu <wei.liu@kernel.org> > > Cc: Dexuan Cui <decui@microsoft.com> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: Ingo Molnar <mingo@kernel.org> > > Cc: Borislav Petkov <bp@alien8.de> > > Cc: Dave Hansen <dave.hansen@linux.intel.com> > > Cc: "H. Peter Anvin" <hpa@zytor.com> > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> > > --- > > arch/x86/kernel/cpu/mshyperv.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kernel/cpu/mshyperv.c > > b/arch/x86/kernel/cpu/mshyperv.c index e6bba12c759c..01fa06dd06b6 > > 100644 > > --- a/arch/x86/kernel/cpu/mshyperv.c > > +++ b/arch/x86/kernel/cpu/mshyperv.c > > @@ -262,11 +262,14 @@ static uint32_t __init ms_hyperv_platform(void) > > static int hv_nmi_unknown(unsigned int val, struct pt_regs *regs) { > > static atomic_t nmi_cpu = ATOMIC_INIT(-1); > > + unsigned int old_cpu, this_cpu; > > > > if (!unknown_nmi_panic) > > return NMI_DONE; > > > > - if (atomic_cmpxchg(&nmi_cpu, -1, raw_smp_processor_id()) != -1) > > + old_cpu = -1; > > + this_cpu = raw_smp_processor_id(); > > + if (!atomic_try_cmpxchg(&nmi_cpu, &old_cpu, this_cpu)) > > return NMI_HANDLED; > > > > return NMI_DONE; > > -- > > 2.41.0 > > The change looks correct to me. But is there any motivation other > than saving 3 bytes of generated code? This is not a performance > sensitive path. And the change adds 3 lines of source code. So > I wonder if the change is worth the churn. Yes, I was trying to make the function more easy to understand and similar to nmi_panic() from kernel/panic.c. I had also the idea of using CPU_INVALID #define instead of -1, but IMO, the above works as well. > In any case, > > Reviewed-by: Michael Kelley <mhklinux@outlook.com> Thanks, Uros.
On Wed, Nov 15, 2023 at 09:58:29PM +0100, Uros Bizjak wrote: > On Wed, Nov 15, 2023 at 6:19 PM Michael Kelley <mhklinux@outlook.com> wrote: > > > > From: Uros Bizjak <ubizjak@gmail.com> Sent: Tuesday, November 14, 2023 8:59 AM > > > > > > Use atomic_try_cmpxchg() instead of atomic_cmpxchg(*ptr, old, new) == old > > > in hv_nmi_unknown(). On x86 the CMPXCHG instruction returns success in > > > the ZF flag, so this change saves a compare after CMPXCHG. The generated > > > asm code improves from: > > > > > > 3e: 65 8b 15 00 00 00 00 mov %gs:0x0(%rip),%edx > > > 45: b8 ff ff ff ff mov $0xffffffff,%eax > > > 4a: f0 0f b1 15 00 00 00 lock cmpxchg %edx,0x0(%rip) > > > 51: 00 > > > 52: 83 f8 ff cmp $0xffffffff,%eax > > > 55: 0f 95 c0 setne %al > > > > > > to: > > > > > > 3e: 65 8b 15 00 00 00 00 mov %gs:0x0(%rip),%edx > > > 45: b8 ff ff ff ff mov $0xffffffff,%eax > > > 4a: f0 0f b1 15 00 00 00 lock cmpxchg %edx,0x0(%rip) > > > 51: 00 > > > 52: 0f 95 c0 setne %al > > > > > > No functional change intended. > > > > > > Cc: "K. Y. Srinivasan" <kys@microsoft.com> > > > Cc: Haiyang Zhang <haiyangz@microsoft.com> > > > Cc: Wei Liu <wei.liu@kernel.org> > > > Cc: Dexuan Cui <decui@microsoft.com> > > > Cc: Thomas Gleixner <tglx@linutronix.de> > > > Cc: Ingo Molnar <mingo@kernel.org> > > > Cc: Borislav Petkov <bp@alien8.de> > > > Cc: Dave Hansen <dave.hansen@linux.intel.com> > > > Cc: "H. Peter Anvin" <hpa@zytor.com> > > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> > > > --- > > > arch/x86/kernel/cpu/mshyperv.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/x86/kernel/cpu/mshyperv.c > > > b/arch/x86/kernel/cpu/mshyperv.c index e6bba12c759c..01fa06dd06b6 > > > 100644 > > > --- a/arch/x86/kernel/cpu/mshyperv.c > > > +++ b/arch/x86/kernel/cpu/mshyperv.c > > > @@ -262,11 +262,14 @@ static uint32_t __init ms_hyperv_platform(void) > > > static int hv_nmi_unknown(unsigned int val, struct pt_regs *regs) { > > > static atomic_t nmi_cpu = ATOMIC_INIT(-1); > > > + unsigned int old_cpu, this_cpu; > > > > > > if (!unknown_nmi_panic) > > > return NMI_DONE; > > > > > > - if (atomic_cmpxchg(&nmi_cpu, -1, raw_smp_processor_id()) != -1) > > > + old_cpu = -1; > > > + this_cpu = raw_smp_processor_id(); > > > + if (!atomic_try_cmpxchg(&nmi_cpu, &old_cpu, this_cpu)) > > > return NMI_HANDLED; > > > > > > return NMI_DONE; > > > -- > > > 2.41.0 > > > > The change looks correct to me. But is there any motivation other > > than saving 3 bytes of generated code? This is not a performance > > sensitive path. And the change adds 3 lines of source code. So > > I wonder if the change is worth the churn. > > Yes, I was trying to make the function more easy to understand and > similar to nmi_panic() from kernel/panic.c. I had also the idea of > using CPU_INVALID #define instead of -1, but IMO, the above works as > well. > > > In any case, > > > > Reviewed-by: Michael Kelley <mhklinux@outlook.com> Applied to hyperv-fixes. Uros, just so you know, DKIM verification failed when I used b4 to apply this patch. You may want to check your email setup. For such a simple patch I'm not worried about spoofing authorship, and I also checked the same email address had sent similar patches before. Thanks, Wei. > > Thanks, > Uros.
On Wed, Nov 22, 2023 at 4:52 AM Wei Liu <wei.liu@kernel.org> wrote: > > On Wed, Nov 15, 2023 at 09:58:29PM +0100, Uros Bizjak wrote: > > On Wed, Nov 15, 2023 at 6:19 PM Michael Kelley <mhklinux@outlook.com> wrote: > > > > > > From: Uros Bizjak <ubizjak@gmail.com> Sent: Tuesday, November 14, 2023 8:59 AM > > > > > > > > Use atomic_try_cmpxchg() instead of atomic_cmpxchg(*ptr, old, new) == old > > > > in hv_nmi_unknown(). On x86 the CMPXCHG instruction returns success in > > > > the ZF flag, so this change saves a compare after CMPXCHG. The generated > > > > asm code improves from: > > > > > > > > 3e: 65 8b 15 00 00 00 00 mov %gs:0x0(%rip),%edx > > > > 45: b8 ff ff ff ff mov $0xffffffff,%eax > > > > 4a: f0 0f b1 15 00 00 00 lock cmpxchg %edx,0x0(%rip) > > > > 51: 00 > > > > 52: 83 f8 ff cmp $0xffffffff,%eax > > > > 55: 0f 95 c0 setne %al > > > > > > > > to: > > > > > > > > 3e: 65 8b 15 00 00 00 00 mov %gs:0x0(%rip),%edx > > > > 45: b8 ff ff ff ff mov $0xffffffff,%eax > > > > 4a: f0 0f b1 15 00 00 00 lock cmpxchg %edx,0x0(%rip) > > > > 51: 00 > > > > 52: 0f 95 c0 setne %al > > > > > > > > No functional change intended. > > > > > > > > Cc: "K. Y. Srinivasan" <kys@microsoft.com> > > > > Cc: Haiyang Zhang <haiyangz@microsoft.com> > > > > Cc: Wei Liu <wei.liu@kernel.org> > > > > Cc: Dexuan Cui <decui@microsoft.com> > > > > Cc: Thomas Gleixner <tglx@linutronix.de> > > > > Cc: Ingo Molnar <mingo@kernel.org> > > > > Cc: Borislav Petkov <bp@alien8.de> > > > > Cc: Dave Hansen <dave.hansen@linux.intel.com> > > > > Cc: "H. Peter Anvin" <hpa@zytor.com> > > > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> > > > > --- > > > > arch/x86/kernel/cpu/mshyperv.c | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/arch/x86/kernel/cpu/mshyperv.c > > > > b/arch/x86/kernel/cpu/mshyperv.c index e6bba12c759c..01fa06dd06b6 > > > > 100644 > > > > --- a/arch/x86/kernel/cpu/mshyperv.c > > > > +++ b/arch/x86/kernel/cpu/mshyperv.c > > > > @@ -262,11 +262,14 @@ static uint32_t __init ms_hyperv_platform(void) > > > > static int hv_nmi_unknown(unsigned int val, struct pt_regs *regs) { > > > > static atomic_t nmi_cpu = ATOMIC_INIT(-1); > > > > + unsigned int old_cpu, this_cpu; > > > > > > > > if (!unknown_nmi_panic) > > > > return NMI_DONE; > > > > > > > > - if (atomic_cmpxchg(&nmi_cpu, -1, raw_smp_processor_id()) != -1) > > > > + old_cpu = -1; > > > > + this_cpu = raw_smp_processor_id(); > > > > + if (!atomic_try_cmpxchg(&nmi_cpu, &old_cpu, this_cpu)) > > > > return NMI_HANDLED; > > > > > > > > return NMI_DONE; > > > > -- > > > > 2.41.0 > > > > > > The change looks correct to me. But is there any motivation other > > > than saving 3 bytes of generated code? This is not a performance > > > sensitive path. And the change adds 3 lines of source code. So > > > I wonder if the change is worth the churn. > > > > Yes, I was trying to make the function more easy to understand and > > similar to nmi_panic() from kernel/panic.c. I had also the idea of > > using CPU_INVALID #define instead of -1, but IMO, the above works as > > well. > > > > > In any case, > > > > > > Reviewed-by: Michael Kelley <mhklinux@outlook.com> > > Applied to hyperv-fixes. > > Uros, just so you know, DKIM verification failed when I used b4 to apply > this patch. You may want to check your email setup. Strange, because I didn't touch the mailer and git config for months... and recently I have sent many patches this way without problems. Thanks, Uros.
On Wed, Nov 22, 2023 at 1:31 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > On Wed, Nov 22, 2023 at 4:52 AM Wei Liu <wei.liu@kernel.org> wrote: > > > > On Wed, Nov 15, 2023 at 09:58:29PM +0100, Uros Bizjak wrote: > > > On Wed, Nov 15, 2023 at 6:19 PM Michael Kelley <mhklinux@outlook.com> wrote: > > > > > > > > From: Uros Bizjak <ubizjak@gmail.com> Sent: Tuesday, November 14, 2023 8:59 AM > > > > > > > > > > Use atomic_try_cmpxchg() instead of atomic_cmpxchg(*ptr, old, new) == old > > > > > in hv_nmi_unknown(). On x86 the CMPXCHG instruction returns success in > > > > > the ZF flag, so this change saves a compare after CMPXCHG. The generated > > > > > asm code improves from: > > > > > > > > > > 3e: 65 8b 15 00 00 00 00 mov %gs:0x0(%rip),%edx > > > > > 45: b8 ff ff ff ff mov $0xffffffff,%eax > > > > > 4a: f0 0f b1 15 00 00 00 lock cmpxchg %edx,0x0(%rip) > > > > > 51: 00 > > > > > 52: 83 f8 ff cmp $0xffffffff,%eax > > > > > 55: 0f 95 c0 setne %al > > > > > > > > > > to: > > > > > > > > > > 3e: 65 8b 15 00 00 00 00 mov %gs:0x0(%rip),%edx > > > > > 45: b8 ff ff ff ff mov $0xffffffff,%eax > > > > > 4a: f0 0f b1 15 00 00 00 lock cmpxchg %edx,0x0(%rip) > > > > > 51: 00 > > > > > 52: 0f 95 c0 setne %al > > > > > > > > > > No functional change intended. > > > > > > > > > > Cc: "K. Y. Srinivasan" <kys@microsoft.com> > > > > > Cc: Haiyang Zhang <haiyangz@microsoft.com> > > > > > Cc: Wei Liu <wei.liu@kernel.org> > > > > > Cc: Dexuan Cui <decui@microsoft.com> > > > > > Cc: Thomas Gleixner <tglx@linutronix.de> > > > > > Cc: Ingo Molnar <mingo@kernel.org> > > > > > Cc: Borislav Petkov <bp@alien8.de> > > > > > Cc: Dave Hansen <dave.hansen@linux.intel.com> > > > > > Cc: "H. Peter Anvin" <hpa@zytor.com> > > > > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> > > > > > --- > > > > > arch/x86/kernel/cpu/mshyperv.c | 5 ++++- > > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/arch/x86/kernel/cpu/mshyperv.c > > > > > b/arch/x86/kernel/cpu/mshyperv.c index e6bba12c759c..01fa06dd06b6 > > > > > 100644 > > > > > --- a/arch/x86/kernel/cpu/mshyperv.c > > > > > +++ b/arch/x86/kernel/cpu/mshyperv.c > > > > > @@ -262,11 +262,14 @@ static uint32_t __init ms_hyperv_platform(void) > > > > > static int hv_nmi_unknown(unsigned int val, struct pt_regs *regs) { > > > > > static atomic_t nmi_cpu = ATOMIC_INIT(-1); > > > > > + unsigned int old_cpu, this_cpu; > > > > > > > > > > if (!unknown_nmi_panic) > > > > > return NMI_DONE; > > > > > > > > > > - if (atomic_cmpxchg(&nmi_cpu, -1, raw_smp_processor_id()) != -1) > > > > > + old_cpu = -1; > > > > > + this_cpu = raw_smp_processor_id(); > > > > > + if (!atomic_try_cmpxchg(&nmi_cpu, &old_cpu, this_cpu)) > > > > > return NMI_HANDLED; > > > > > > > > > > return NMI_DONE; > > > > > -- > > > > > 2.41.0 > > > > > > > > The change looks correct to me. But is there any motivation other > > > > than saving 3 bytes of generated code? This is not a performance > > > > sensitive path. And the change adds 3 lines of source code. So > > > > I wonder if the change is worth the churn. > > > > > > Yes, I was trying to make the function more easy to understand and > > > similar to nmi_panic() from kernel/panic.c. I had also the idea of > > > using CPU_INVALID #define instead of -1, but IMO, the above works as > > > well. > > > > > > > In any case, > > > > > > > > Reviewed-by: Michael Kelley <mhklinux@outlook.com> > > > > Applied to hyperv-fixes. > > > > Uros, just so you know, DKIM verification failed when I used b4 to apply > > this patch. You may want to check your email setup. > > Strange, because I didn't touch the mailer and git config for > months... and recently I have sent many patches this way without > problems. This one [1] checks OK, so it looks like some transient issue with gmail. [1] https://lore.kernel.org/lkml/20231120153419.3045-1-ubizjak@gmail.com/ Thanks, Uros.
November 21, 2023 at 10:51 PM, "Wei Liu" <wei.liu@kernel.org> wrote: > Uros, just so you know, DKIM verification failed when I used b4 to apply > this patch. You may want to check your email setup. This is not actually Uros's fault. Recently, Gmail started adding a forced expiration field to their DKIM signatures, via the x= field: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1699981249; x=1700586049; darn=vger.kernel.org; ^^^^^^^^^^^^^ This gives the signature an enforced validity of only 7 days. Since the original message was sent on November 14 and you're retrieving it on November 21, this causes the DKIM check to fail. I need to figure out how to make b4 ignore the x= field, because it's not relevant for our purposes, but the library we're using for DKIM doesn't currently have any mechanism to do so. I will open an RFE with them in the hopes that we can get this implemented. Regards, -K
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c index e6bba12c759c..01fa06dd06b6 100644 --- a/arch/x86/kernel/cpu/mshyperv.c +++ b/arch/x86/kernel/cpu/mshyperv.c @@ -262,11 +262,14 @@ static uint32_t __init ms_hyperv_platform(void) static int hv_nmi_unknown(unsigned int val, struct pt_regs *regs) { static atomic_t nmi_cpu = ATOMIC_INIT(-1); + unsigned int old_cpu, this_cpu; if (!unknown_nmi_panic) return NMI_DONE; - if (atomic_cmpxchg(&nmi_cpu, -1, raw_smp_processor_id()) != -1) + old_cpu = -1; + this_cpu = raw_smp_processor_id(); + if (!atomic_try_cmpxchg(&nmi_cpu, &old_cpu, this_cpu)) return NMI_HANDLED; return NMI_DONE;