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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9E5F9C433FE for ; Thu, 3 Nov 2022 19:36:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C56FA6B0072; Thu, 3 Nov 2022 15:36:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BDFD56B0073; Thu, 3 Nov 2022 15:36:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A59216B0074; Thu, 3 Nov 2022 15:36:03 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 9055F6B0072 for ; Thu, 3 Nov 2022 15:36:03 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 667614050D for ; Thu, 3 Nov 2022 19:36:03 +0000 (UTC) X-FDA: 80093136606.30.9829B0D Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf04.hostedemail.com (Postfix) with ESMTP id C8DE140009 for ; Thu, 3 Nov 2022 19:36:02 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id CF419B82960; Thu, 3 Nov 2022 19:36:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CCC06C433C1; Thu, 3 Nov 2022 19:35:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1667504159; bh=zcoG/+WvByXxxHPPM3wHg/JURpe0ekEhkjLQoWAGhZI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=qUgD8oT9cbo3l/wCfTbjVuXwW6h5DNa8vDkMjOVwoVzb7ap7fvlX6Npne6FNzY1LL +JoF4UyvL8q7C7D9pl1XfEWIsY22dareueuT+c957nvXJRGSKWiX/ikAWEvDav4NAR 3PBavyPvHY8mG42ZLNvy1JYg/JTSSSZ0ag2l8N/KdQ4wtJqaYHfKYK598+MSf1lNgu PKEbfiEWZCsBrskIvAR/sFWeY5VhsPGiV3HsG9visFtaw+ovlyJHVOBFXxR4CvpAvL B19YyqQbnN15ZJcR88G8uDbHPa8jGXZA5+Qfzs8HSUZtYH74KWAA5aMMdxlDABnVve BY8e0vGH/fbKw== Date: Thu, 3 Nov 2022 12:35:57 -0700 From: Nathan Chancellor To: Uros Bizjak Cc: Peter Zijlstra , x86@kernel.org, willy@infradead.org, torvalds@linux-foundation.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, aarcange@redhat.com, kirill.shutemov@linux.intel.com, jroedel@suse.de Subject: Re: [PATCH 11/13] x86_64: Remove pointless set_64bit() usage Message-ID: References: <20221022111403.531902164@infradead.org> <20221022114425.168036718@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1667504163; a=rsa-sha256; cv=none; b=PbdBbAp1HoIo8/c2UWgVgpTkAqvOL4hiF21eMB5qJoSryI+t/KQOghM68OPTE6MHJ6hkIp nWvu4lbTTHthUrJPX6SpqmB+u+UtSiZETlmr4vjZlthW6rPtcK6IrFRotEJzfZARYUQj4M HJIBrl3mSYF/TmLftSBkoh6n/bSVOb0= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=qUgD8oT9; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf04.hostedemail.com: domain of nathan@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=nathan@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1667504163; h=from:from:sender: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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=+tExboWj0XnJuZHqm08zHT7StJVVWnj3UovJSH3R6GY=; b=cZCL12i4Yqy8kjJ29IxMZoYgK7SlxgqihBIFBSLTHlF1HZ9j2tXyWwjbh1yIlrIUr9rUWG JA4BKw8rRH2YcxHwonwy0OkQ8fuQlxpfN68wOn3uR1cd8kfT0/XRQ0lYHz6bz3QTQolpkW 79BI9m8M1bvTgXvINK+CJvIzERyVm8c= X-Stat-Signature: 6dcono7ypy9dakt4zt6fgkp4sjmxnhdf X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: C8DE140009 X-Rspam-User: Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=qUgD8oT9; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf04.hostedemail.com: domain of nathan@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=nathan@kernel.org X-HE-Tag: 1667504162-871188 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Nov 03, 2022 at 08:23:41PM +0100, Uros Bizjak wrote: > On Thu, Nov 3, 2022 at 8:09 PM Nathan Chancellor wrote: > > > > Hi Peter, > > > > On Sat, Oct 22, 2022 at 01:14:14PM +0200, Peter Zijlstra wrote: > > > The use of set_64bit() in X86_64 only code is pretty pointless, seeing > > > how it's a direct assignment. Remove all this nonsense. > > > > > > Additionally, since x86_64 unconditionally has HAVE_CMPXCHG_DOUBLE, > > > there is no point in even having that fallback. > > > > > > Signed-off-by: Peter Zijlstra (Intel) > > > --- > > > arch/um/include/asm/pgtable-3level.h | 8 -------- > > > arch/x86/include/asm/cmpxchg_64.h | 5 ----- > > > drivers/iommu/intel/irq_remapping.c | 10 ++-------- > > > 3 files changed, 2 insertions(+), 21 deletions(-) > > > > > > --- a/arch/um/include/asm/pgtable-3level.h > > > +++ b/arch/um/include/asm/pgtable-3level.h > > > @@ -58,11 +58,7 @@ > > > #define pud_populate(mm, pud, pmd) \ > > > set_pud(pud, __pud(_PAGE_TABLE + __pa(pmd))) > > > > > > -#ifdef CONFIG_64BIT > > > -#define set_pud(pudptr, pudval) set_64bit((u64 *) (pudptr), pud_val(pudval)) > > > -#else > > > #define set_pud(pudptr, pudval) (*(pudptr) = (pudval)) > > > -#endif > > > > > > static inline int pgd_newpage(pgd_t pgd) > > > { > > > @@ -71,11 +67,7 @@ static inline int pgd_newpage(pgd_t pgd) > > > > > > static inline void pgd_mkuptodate(pgd_t pgd) { pgd_val(pgd) &= ~_PAGE_NEWPAGE; } > > > > > > -#ifdef CONFIG_64BIT > > > -#define set_pmd(pmdptr, pmdval) set_64bit((u64 *) (pmdptr), pmd_val(pmdval)) > > > -#else > > > #define set_pmd(pmdptr, pmdval) (*(pmdptr) = (pmdval)) > > > -#endif > > > > > > static inline void pud_clear (pud_t *pud) > > > { > > > --- a/arch/x86/include/asm/cmpxchg_64.h > > > +++ b/arch/x86/include/asm/cmpxchg_64.h > > > @@ -2,11 +2,6 @@ > > > #ifndef _ASM_X86_CMPXCHG_64_H > > > #define _ASM_X86_CMPXCHG_64_H > > > > > > -static inline void set_64bit(volatile u64 *ptr, u64 val) > > > -{ > > > - *ptr = val; > > > -} > > > - > > > #define arch_cmpxchg64(ptr, o, n) \ > > > ({ \ > > > BUILD_BUG_ON(sizeof(*(ptr)) != 8); \ > > > --- a/drivers/iommu/intel/irq_remapping.c > > > +++ b/drivers/iommu/intel/irq_remapping.c > > > @@ -173,7 +173,6 @@ static int modify_irte(struct irq_2_iomm > > > index = irq_iommu->irte_index + irq_iommu->sub_handle; > > > irte = &iommu->ir_table->base[index]; > > > > > > -#if defined(CONFIG_HAVE_CMPXCHG_DOUBLE) > > > if ((irte->pst == 1) || (irte_modified->pst == 1)) { > > > bool ret; > > > > > > @@ -187,11 +186,6 @@ static int modify_irte(struct irq_2_iomm > > > * same as the old value. > > > */ > > > WARN_ON(!ret); > > > - } else > > > -#endif > > > - { > > > - set_64bit(&irte->low, irte_modified->low); > > > - set_64bit(&irte->high, irte_modified->high); > > > } > > > __iommu_flush_cache(iommu, irte, sizeof(*irte)); > > It looks to me that the above part should not be removed, but > set_64bit should be substituted with WRITE_ONCE. Only #if/#endif lines > should be removed. Thanks, I also realized that only a couple minutes after I sent my initial message. I just got done testing the following diff, which resolves my issue. Peter, would you like a formal patch or did you want to squash it in to the original change? diff --git a/drivers/iommu/intel/irq_remapping.c b/drivers/iommu/intel/irq_remapping.c index 4216cafd67e7..5d176168bb76 100644 --- a/drivers/iommu/intel/irq_remapping.c +++ b/drivers/iommu/intel/irq_remapping.c @@ -186,6 +186,9 @@ static int modify_irte(struct irq_2_iommu *irq_iommu, * same as the old value. */ WARN_ON(!ret); + } else { + WRITE_ONCE(irte->low, irte_modified->low); + WRITE_ONCE(irte->high, irte_modified->high); } __iommu_flush_cache(iommu, irte, sizeof(*irte)); > > > > > > @@ -249,8 +243,8 @@ static int clear_entries(struct irq_2_io > > > end = start + (1 << irq_iommu->irte_mask); > > > > > > for (entry = start; entry < end; entry++) { > > > - set_64bit(&entry->low, 0); > > > - set_64bit(&entry->high, 0); > > > + WRITE_ONCE(entry->low, 0); > > > + WRITE_ONCE(entry->high, 0); > > > } > > > bitmap_release_region(iommu->ir_table->bitmap, index, > > > irq_iommu->irte_mask); > > > > > > > > > > This commit is now in -next as commit 0475a2d10fc7 ("x86_64: Remove > > pointless set_64bit() usage") and I just bisect a boot failure on my > > Intel test desktop to it. > > > > # bad: [81214a573d19ae2fa5b528286ba23cd1cb17feec] Add linux-next specific files for 20221103 > > # good: [8e5423e991e8cd0988d0c4a3f4ac4ca1af7d148a] Merge tag 'parisc-for-6.1-2' of git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux > > git bisect start '81214a573d19ae2fa5b528286ba23cd1cb17feec' '8e5423e991e8cd0988d0c4a3f4ac4ca1af7d148a' > > # good: [8c13089d26d070fef87a64b48191cb7ae6dfbdb2] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git > > git bisect good 8c13089d26d070fef87a64b48191cb7ae6dfbdb2 > > # bad: [1bba8e9d15551d2f1c304d8f9d5c647a5b54bfc0] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git > > git bisect bad 1bba8e9d15551d2f1c304d8f9d5c647a5b54bfc0 > > # good: [748c419c7ade509684ce5bcf74f50e13e0447afd] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git > > git bisect good 748c419c7ade509684ce5bcf74f50e13e0447afd > > # good: [0acc81a3bf9f875c5ef03037ff5431d37f536f05] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git > > git bisect good 0acc81a3bf9f875c5ef03037ff5431d37f536f05 > > # bad: [c0fb84e0698d2ce57f9391c7f4112f6e17676f99] Merge branch into tip/master: 'x86/cleanups' > > git bisect bad c0fb84e0698d2ce57f9391c7f4112f6e17676f99 > > # good: [7212c34aac1ec6abadf8b439824c8307ef0dd338] Merge branch 'x86/core' into x86/paravirt, to resolve conflicts > > git bisect good 7212c34aac1ec6abadf8b439824c8307ef0dd338 > > # good: [e1f2ac1d285d963a783a027a1b109420b07f30c1] Merge branch into tip/master: 'x86/cpu' > > git bisect good e1f2ac1d285d963a783a027a1b109420b07f30c1 > > # good: [306b75edbf25b86fe8189a4f96c217e49483f8ae] Merge branch into tip/master: 'x86/cleanups' > > git bisect good 306b75edbf25b86fe8189a4f96c217e49483f8ae > > # good: [8f28b415703e1935457a4bf0be7f03dc5471d09f] mm: Rename GUP_GET_PTE_LOW_HIGH > > git bisect good 8f28b415703e1935457a4bf0be7f03dc5471d09f > > # bad: [0475a2d10fc7ced3268cd0f0551390b5858f90cd] x86_64: Remove pointless set_64bit() usage > > git bisect bad 0475a2d10fc7ced3268cd0f0551390b5858f90cd > > # good: [a677802d5b0258f93f54620e1cd181b56547c36c] x86/mm/pae: Don't (ab)use atomic64 > > git bisect good a677802d5b0258f93f54620e1cd181b56547c36c > > # good: [533627610ae7709572a4fac1393fb61153e2a5b3] x86/mm/pae: Be consistent with pXXp_get_and_clear() > > git bisect good 533627610ae7709572a4fac1393fb61153e2a5b3 > > # first bad commit: [0475a2d10fc7ced3268cd0f0551390b5858f90cd] x86_64: Remove pointless set_64bit() usage > > # good: [533627610ae7709572a4fac1393fb61153e2a5b3] x86/mm/pae: Be consistent with pXXp_get_and_clear() > > git bisect good 533627610ae7709572a4fac1393fb61153e2a5b3 > > # first bad commit: [0475a2d10fc7ced3268cd0f0551390b5858f90cd] x86_64: Remove pointless set_64bit() usage > > > > Unfortunately, I see no output on the screen it is attached to so I > > assume it is happening pretty early during the boot sequence, which will > > probably make getting logs somewhat hard. I can provide information > > about the system if that would help reveal anything. If there is > > anything I can test, I am more than happy to do so. > > > > Cheers, > > Nathan