Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
From:   Michael Kelley <mhklinux@outlook.com>
To:     Uros Bizjak <ubizjak@gmail.com>,
        "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     "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: RE: [PATCH] x86/hyperv: Use atomic_try_cmpxchg() to micro-optimize
 hv_nmi_unknown()
Thread-Topic: [PATCH] x86/hyperv: Use atomic_try_cmpxchg() to micro-optimize
 hv_nmi_unknown()
Thread-Index: AQHaFxwmvIq1AgDxz0+8P8PcuNcfxbB7nX2A
Date:   Wed, 15 Nov 2023 17:19:04 +0000
Message-ID: <SN6PR02MB41570168279C428D385ADCB0D4B1A@SN6PR02MB4157.namprd02.prod.outlook.com>
References: <20231114170038.381634-1-ubizjak@gmail.com>
In-Reply-To: <20231114170038.381634-1-ubizjak@gmail.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?3+LfY6zz6VB0puxRKAX69GQs8yhYnUVtm4NrO4lsXauMnRKGStESncJtuL0z?=
 =?us-ascii?Q?jip31zZy1yKYHCA23G5bESfzXMb/Ostk2LrXBJQgEpPoZ/P8UyI0IGVDvDAB?=
 =?us-ascii?Q?uiWOgnke91zJFbNMs+XykA8zO/loGtlklL/NM9ADtDs294Zn0NojCT0oyd5m?=
 =?us-ascii?Q?V7sBs6Go3kMPJIAPbXCTisV9J5pA0EI3MhlwyAwKS9PYV3h9ZuS6Za80stvV?=
 =?us-ascii?Q?if4/GATy/RKGysm27N6idxQufISyJADwF+mcuLX5X+JGFhDJhzEUP8Ryqq3y?=
 =?us-ascii?Q?vSaZIoLtYyJUbSoz9yQx6CWfTyyWT6jqUoV0GH/1G/nfpwQBtxElJg6ER5H5?=
 =?us-ascii?Q?P/Z9uOjbOuMOEm4fPsq7U29ZjnQFaLvS5reZt7AOY4krgdy9AZqwFv6Rmmf8?=
 =?us-ascii?Q?+uH6TRuUnnHr8qlrzqIJj0KFynsug4RGIiMB9+Lrx3Gjq7QghGaYcJxxgI3i?=
 =?us-ascii?Q?jBb4fshPRsFWzLittblhGDXKWAGKb+CQGxx5+ytSQ420VH3SdMXFuoVRKxPr?=
 =?us-ascii?Q?4Bv7AKGYmfzNjgL992+A6iae/If/oEytw9Q7DAfNs7mOpoO76oz0TJGn5qk8?=
 =?us-ascii?Q?9avXtVRFW+pFmgIKzvqIkV6WiLaO8PYKga/xcpdjntbX8l3FJ1qtWlYrFEhD?=
 =?us-ascii?Q?SPUSOdE6OLXIzJlRKToYoY93Nkga6jyFcl2KQsr27qS5WM8E9OK0MWt4XLyu?=
 =?us-ascii?Q?e23RpB4jCvBEjFhHiPKZfpEfKo1uSk1AOXA+XV7UoRvaDw5COOnvCDD8YFwr?=
 =?us-ascii?Q?TNCufbzLXuUX8m7vWD2Td6XBjFGnIeGlp3GtFJW8XQDLYZnpU58QQkVsGFgT?=
 =?us-ascii?Q?R/OcKmLR2RK86jEGlTwQg2O3DO3mz7KVnHt8KRXiRh50e7rHR2Xr3F+UJKGL?=
 =?us-ascii?Q?ryndMwMzeuF1M6QhWCjz7VpHtjbafUmDCND9UfJEvgShNM9un50Ndlvo0/SZ?=
 =?us-ascii?Q?eMLf0344a/ShvYctAoJ2UMSIbMttEbmIUiTypto+fJm6IkPMRFg5LZznDsty?=
 =?us-ascii?Q?w8zrLN5cz3whQj7RqazudV20TefUzErAbUYkxW5Oud3j2W+0z9OatJkuLm8o?=
 =?us-ascii?Q?6Txcq3/DxweDclZS2S/UFZ4xG3hUSfHL3QXfCW7g2A5zR7XPrzQMfjxzqsvJ?=
 =?us-ascii?Q?JUGsuo0vuGQqeSuGRnNtZzZo+btG0go2178QBYsujoQlTNLLU/EG7lP/IO9g?=
 =?us-ascii?Q?5ZG6z60U5CmZ4cqB/oYMDeJrR/zCC7lI5elsqg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR02MB4157.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: 86165ba8-f85a-4917-f8a6-08dbe5fef793
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Nov 2023 17:19:04.0693
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR02MB9209
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]); Wed, 15 Nov 2023 09:19:18 -0800 (PST)

From: Uros Bizjak <ubizjak@gmail.com> Sent: Tuesday, November 14, 2023 8:59=
 AM
>=20
> Use atomic_try_cmpxchg() instead of atomic_cmpxchg(*ptr, old, new) =3D=3D=
 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:
>=20
>   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
>=20
> to:
>=20
>   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
>=20
> No functional change intended.
>=20
> 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(-)
>=20
> 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 =3D ATOMIC_INIT(-1);
> +	unsigned int old_cpu, this_cpu;
>=20
>  	if (!unknown_nmi_panic)
>  		return NMI_DONE;
>=20
> -	if (atomic_cmpxchg(&nmi_cpu, -1, raw_smp_processor_id()) !=3D -1)
> +	old_cpu =3D -1;
> +	this_cpu =3D raw_smp_processor_id();
> +	if (!atomic_try_cmpxchg(&nmi_cpu, &old_cpu, this_cpu))
>  		return NMI_HANDLED;
>=20
>  	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>