Message ID | 20221022114425.168036718@infradead.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4242:0:0:0:0:0 with SMTP id s2csp1169169wrr; Sat, 22 Oct 2022 04:52:44 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4q+iwuBnwtsSUzYh5U9XJSMo2XeFkGsUtw8iWGHTsvTMmIRCA/FUiCuNLzkXyzFyV6pnPh X-Received: by 2002:a17:903:246:b0:179:b6d0:f8fd with SMTP id j6-20020a170903024600b00179b6d0f8fdmr23937298plh.124.1666439564035; Sat, 22 Oct 2022 04:52:44 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666439564; cv=none; d=google.com; s=arc-20160816; b=yKOcUqvv6aKqZimWOEioZJ1o3I95iofdyRgbGhLSZA/TlbP750eB5KZLGbR9QOoznf 3qDGU17dzZVu2LROCOmDuulsTpSkN251dema7X5UxqkstuEJzgEiykgz5X4yQYc+vv4r tZbcTw1u283LC+41naavQhs+c77Y6MWgJ68zMoGUaXNJfuTxa6yivwx32471a0LBJQeG L/+XL3x9TDWC1yAutByW1HaBEBiY4Zta9U2qiCYqTFO6FruUQ7cYodSAa2Z7BT0TVTcw xensTW8W9FHbwFKjKxpaSWO008q8fqv0dttrADeiOll7EwV3GxmGv7yOuHlRncvsZ989 Vvfg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:references:subject:cc:to:from:date :user-agent:message-id:dkim-signature; bh=kfJUOBrrmjy4JsdmvOUplpu2MGBZGnH8F3TsabZggcA=; b=FUY23TzKccBccpgg7ltkv4mKdjHjc/wGxZ7QSZEMqtKYd2F8tnwc7Z5r08/mBMnNGC O8k7fqJoLq3Riy/yguktX4h3q1SAwAnv6YGqOn1ccRRPQJTcqPmFx7GuL7tueCXJggxi Cg6FjuxmY85lFIZrd/ZKJLcmC/afx5eGIyUwxQsvtI/X+E6ed9cgVMMnyCqdv4mSLQG5 jMOHOrt7yPHLVJPmyV6jcKi/use+aWJhLbgBKJcF7KD5REktY2wiYjH+7VJLsmAKdmGD 49ZmpZ50PemRRfnewlM4NS4Fn3SiqGxVKLaFYDh3qMEHwXFdRSLiE5yBgjHx6OpQGQ8M Bp1w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@infradead.org header.s=desiato.20200630 header.b=TZtejNuX; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s1-20020a62e701000000b0056196446366si25366371pfh.226.2022.10.22.04.52.31; Sat, 22 Oct 2022 04:52:44 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@infradead.org header.s=desiato.20200630 header.b=TZtejNuX; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229975AbiJVLs7 (ORCPT <rfc822;pwkd43@gmail.com> + 99 others); Sat, 22 Oct 2022 07:48:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43380 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229765AbiJVLst (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sat, 22 Oct 2022 07:48:49 -0400 Received: from desiato.infradead.org (desiato.infradead.org [IPv6:2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 317C52505C3 for <linux-kernel@vger.kernel.org>; Sat, 22 Oct 2022 04:48:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=Content-Type:MIME-Version:References: Subject:Cc:To:From:Date:Message-ID:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:In-Reply-To; bh=kfJUOBrrmjy4JsdmvOUplpu2MGBZGnH8F3TsabZggcA=; b=TZtejNuXM7kl3MOXWKy8aaU/tk e118lDcxQsCXKrtcjfNaX3hhzfxtkXwSWz763aZgaWRoymxnOariKlXTCOTFQ1zOXOH9zc1OTUFRv 8azjLpfaOOQEKL6SRwBny2sFE0p0l69+LTKxpFmQ79VJ+ySU5VFdBl2ClPmUNW6KfJP7IiEb2xjC9 tsNtJRffbNAX/xOBgv9twdHzjXxpXTWw7dPoKQlb47QCW08TuhULthxwrfwPLHr8YayH5Eek3Idiz D6G/kMsRd5f8hFWCtPSwW5VwwMGGMZa4EtEoRiAGDeP8Ppb2+SjV6sDe0sQYqyLmn55ceyQHrM481 m9ajaoKQ==; Received: from j130084.upc-j.chello.nl ([24.132.130.84] helo=noisy.programming.kicks-ass.net) by desiato.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1omCzM-005XdT-Vx; Sat, 22 Oct 2022 11:48:29 +0000 Received: from hirez.programming.kicks-ass.net (hirez.programming.kicks-ass.net [192.168.1.225]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (Client did not present a certificate) by noisy.programming.kicks-ass.net (Postfix) with ESMTPS id C85DC3030FD; Sat, 22 Oct 2022 13:48:26 +0200 (CEST) Received: by hirez.programming.kicks-ass.net (Postfix, from userid 0) id 4299A28B8E512; Sat, 22 Oct 2022 13:48:26 +0200 (CEST) Message-ID: <20221022114425.168036718@infradead.org> User-Agent: quilt/0.66 Date: Sat, 22 Oct 2022 13:14:14 +0200 From: Peter Zijlstra <peterz@infradead.org> To: x86@kernel.org, willy@infradead.org, torvalds@linux-foundation.org, akpm@linux-foundation.org Cc: linux-kernel@vger.kernel.org, peterz@infradead.org, linux-mm@kvack.org, aarcange@redhat.com, kirill.shutemov@linux.intel.com, jroedel@suse.de, ubizjak@gmail.com Subject: [PATCH 11/13] x86_64: Remove pointless set_64bit() usage References: <20221022111403.531902164@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_NONE 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: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747388532412691646?= X-GMAIL-MSGID: =?utf-8?q?1747388532412691646?= |
Series |
Clean up pmd_get_atomic() and i386-PAE
|
|
Commit Message
Peter Zijlstra
Oct. 22, 2022, 11:14 a.m. UTC
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) <peterz@infradead.org>
---
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(-)
Comments
On Sat, Oct 22, 2022 at 4:48 AM Peter Zijlstra <peterz@infradead.org> 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. Thanks. That was really confusing code, using set_64bit() in exactly the situation where it was _not_ needed. Linus
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) <peterz@infradead.org> > --- > 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)); > > @@ -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
On Thu, Nov 3, 2022 at 8:09 PM Nathan Chancellor <nathan@kernel.org> 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) <peterz@infradead.org> > > --- > > 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. Uros. > > > > @@ -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
On Thu, Nov 03, 2022 at 08:23:41PM +0100, Uros Bizjak wrote: > On Thu, Nov 3, 2022 at 8:09 PM Nathan Chancellor <nathan@kernel.org> 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) <peterz@infradead.org> > > > --- > > > 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
On Thu, Nov 3, 2022 at 12:36 PM Nathan Chancellor <nathan@kernel.org> wrote: > > 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. That looks obviously correct. Except in this case "obviously correct patch" is to some very non-obvious code, and I think the whole code around it is very very questionable. I had to actually go check that this code can only be enabled on x86-64 (because "IRQ_REMAP" has a "depends on X86_64" on it), because it also uses cmpxchg_double and that now exists on x86-32 too (but only does 64 bits, not 128 bits, of course). Now, to make things even more confusing, I think that #if defined(CONFIG_HAVE_CMPXCHG_DOUBLE) has *never* made sense, since it's always enabled for x86. HOWEVER - there were actually early AMD x86-64 machines that didn't have CMPXCHG16B. So the conditional kind of makes sense, but doing it based on CONFIG_HAVE_CMPXCHG_DOUBLE does not. It turns out that we do do this all correctly, except we do it at boot time, with a test for boot_cpu_has(X86_FEATURE_CX16): /* * Note: GA (128-bit IRTE) mode requires cmpxchg16b supports. * XT, GAM also requires GA mode. Therefore, we need to * check cmpxchg16b support before enabling them. */ if (!boot_cpu_has(X86_FEATURE_CX16) || ... but that #ifdef has apparenrly never been valid (I didn't go back and see if we at some point had a config entry for those old CPUs). And even after I checked *that*, I then checked the 'struct irte' to check that it's actually properly defined, and it isn't. Considering that this all requires 16-byte alignment to work, I think that type should also be marked as being 16-byte aligned. In fact, I wonder if we should aim to actually force compile-time checking, because right now we have VM_BUG_ON((unsigned long)(p1) % (2 * sizeof(long))); VM_BUG_ON((unsigned long)((p1) + 1) != (unsigned long)(p2)); in our x86-64 __cmpxchg_double() macro, and honestly, that first one might be better as a compile time check of __alignof__, and the second one shouldn't exisrt at all because our interface shouldn't be using two different pointers when the only possible use is for one single aligned value. If somebody actually wants the old m68k type of "DCAS" that did a cmpxchg on two actually *different* pointers, we should call it somethign else (and that's not what any current architecture does). So honestly, just looking at this trivially correct patch, I got into a rats nest of horribly wrong code. Nasty. Linus
On Thu, Nov 03, 2022 at 01:39:17PM -0700, Linus Torvalds wrote: > On Thu, Nov 3, 2022 at 12:36 PM Nathan Chancellor <nathan@kernel.org> wrote: > > > > 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. > > That looks obviously correct. I'll force-push tip/x86/mm with the fix from Nathan and I'll try and look into the rest of the trainwreck tomorrow with the brain awake -- after I figure out that other fail Nathan reported :/ Sorry for breaking all your machines Nate.
On Thu, Nov 03, 2022 at 01:39:17PM -0700, Linus Torvalds wrote: > And even after I checked *that*, I then checked the 'struct irte' to > check that it's actually properly defined, and it isn't. Considering > that this all requires 16-byte alignment to work, I think that type > should also be marked as being 16-byte aligned. > > In fact, I wonder if we should aim to actually force compile-time > checking, because right now we have > > VM_BUG_ON((unsigned long)(p1) % (2 * sizeof(long))); > VM_BUG_ON((unsigned long)((p1) + 1) != (unsigned long)(p2)); > > in our x86-64 __cmpxchg_double() macro, and honestly, that first one > might be better as a compile time check of __alignof__, and the second > one shouldn't exisrt at all because our interface shouldn't be using > two different pointers when the only possible use is for one single > aligned value. So cmpxchg_double() does a cmpxchg on a double long value and is currently supported by: i386, x86_64, arm64 and s390. On all those, except i386, two longs are u128. So how about we introduce u128 and cmpxchg128 -- then it directly mirrors the u64 and cmpxchg64 usage we already have. It then also naturally imposses the alignment thing. Afaict the only cmpxchg_double() users are: arch/s390/kernel/perf_cpum_sf.c drivers/iommu/amd/iommu.c drivers/iommu/intel/irq_remapping.c mm/slub.c Of those slub.c is the only one that cares about 32bit and would need some 'compat' code to pick between cmpxchg64 / cmpxchg128, but it already has everything wrapped in helpers so that shouldn't be too big of a worry. Then we can convert these few users over and simply delete the whole cmpxchg_double() thing.
On Fri, Nov 4, 2022 at 9:01 AM Peter Zijlstra <peterz@infradead.org> wrote: > > So cmpxchg_double() does a cmpxchg on a double long value and is > currently supported by: i386, x86_64, arm64 and s390. > > On all those, except i386, two longs are u128. > > So how about we introduce u128 and cmpxchg128 -- then it directly > mirrors the u64 and cmpxchg64 usage we already have. It then also > naturally imposses the alignment thing. Ack, except that we might have some "u128" users that do *not* necessarily want any alignment thing. But maybe we could at least start with an u128 type that is marked as being fully aligned, and if some other user comes in down the line that wants relaxed alignment we can call it "u128_unaligned" or something. That would have avoided the pain we have on x86-32 where "u64" in a UAPI structure is differently aligned than it is on actual 64-bit architectures. > Of those slub.c is the only one that cares about 32bit and would need > some 'compat' code to pick between cmpxchg64 / cmpxchg128, but it > already has everything wrapped in helpers so that shouldn't be too big > of a worry. Ack. Special case, and we can call it something very clear, and in fact it would clean up the slab code too if we actually used some explicit type for this. Right now, slab does /* Double-word boundary */ void *freelist; /* first free object */ union { unsigned long counters; struct { where the alignment constraints are just a comment, and then it passes pointers to the two words around manually. But it should be fairly straightforward to make it use an actual properly typed thing, and using an unnamed union member, do something that takes an explicitly aligned "two word" thing instead. IOW, make the 'compat' case *not* use those stupid two separate pointers (that have to be consecutive and aligned), but actually do something like struct cmpxchg_double_long { unsigned long a,b; } __aligned(2*sizeof(long)); and then the slab case can use that union of that and the existing "freelist+word-union" to make this all not just a comment to people, but something the compiler sees too. Linus
On Fri, Nov 04, 2022 at 10:15:08AM -0700, Linus Torvalds wrote: > On Fri, Nov 4, 2022 at 9:01 AM Peter Zijlstra <peterz@infradead.org> wrote: > > > > So cmpxchg_double() does a cmpxchg on a double long value and is > > currently supported by: i386, x86_64, arm64 and s390. > > > > On all those, except i386, two longs are u128. > > > > So how about we introduce u128 and cmpxchg128 -- then it directly > > mirrors the u64 and cmpxchg64 usage we already have. It then also > > naturally imposses the alignment thing. > > Ack, except that we might have some "u128" users that do *not* > necessarily want any alignment thing. > > But maybe we could at least start with an u128 type that is marked as > being fully aligned, and if some other user comes in down the line > that wants relaxed alignment we can call it "u128_unaligned" or > something. Hm, sounds maybe not so nice for another use case: arithmetic code that makes use of u128 for efficient computations, but otherwise has no particular alignment requirements. For example, `typedef __uint128_t u128;` in: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/crypto/poly1305-donna64.c https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/crypto/curve25519-hacl64.c I always thought it'd be nice to see that typedef alongside the others in the shared kernel headers, but figured the requirement for 64-bit and libgcc for some operations on some architectures made it a bit less general purpose, so I never proposed it. Jason
On Sat, Nov 05, 2022 at 02:29:47PM +0100, Jason A. Donenfeld wrote: > On Fri, Nov 04, 2022 at 10:15:08AM -0700, Linus Torvalds wrote: > > On Fri, Nov 4, 2022 at 9:01 AM Peter Zijlstra <peterz@infradead.org> wrote: > > > > > > So cmpxchg_double() does a cmpxchg on a double long value and is > > > currently supported by: i386, x86_64, arm64 and s390. > > > > > > On all those, except i386, two longs are u128. > > > > > > So how about we introduce u128 and cmpxchg128 -- then it directly > > > mirrors the u64 and cmpxchg64 usage we already have. It then also > > > naturally imposses the alignment thing. > > > > Ack, except that we might have some "u128" users that do *not* > > necessarily want any alignment thing. > > > > But maybe we could at least start with an u128 type that is marked as > > being fully aligned, and if some other user comes in down the line > > that wants relaxed alignment we can call it "u128_unaligned" or > > something. > > Hm, sounds maybe not so nice for another use case: arithmetic code that > makes use of u128 for efficient computations, but otherwise has > no particular alignment requirements. For example, `typedef __uint128_t > u128;` in: Natural alignment is... natural. Making it unaligned is quite mad. That whole u64 is not naturally aligned on i386 thing Linus referred to is a sodding pain in the backside. If the code has no alignment requirements, natural alignment is as good as any. And if it does have requirements, you can use u128_unaligned. Also: $ ./align 16, 16 --- #include <stdio.h> int main(int argx, char **argv) { __int128 a; printf("%d, %d\n", sizeof(a), __alignof(a)); return 0; }
On Sat, Nov 05, 2022 at 04:14:19PM +0100, Peter Zijlstra wrote: > Also: > > $ ./align > 16, 16 > > --- > > #include <stdio.h> > > int main(int argx, char **argv) > { > __int128 a; > > printf("%d, %d\n", sizeof(a), __alignof(a)); > return 0; > } zx2c4@thinkpad /tmp $ x86_64-linux-musl-gcc -O2 a.c -static && qemu-x86_64 ./a.out 16, 16 zx2c4@thinkpad /tmp $ aarch64-linux-musl-gcc -O2 a.c -static && qemu-aarch64 ./a.out 16, 16 zx2c4@thinkpad /tmp $ powerpc64le-linux-musl-gcc -O2 a.c -static && qemu-ppc64le ./a.out 16, 16 zx2c4@thinkpad /tmp $ s390x-linux-musl-gcc -O2 a.c -static && qemu-s390x ./a.out 16, 8 zx2c4@thinkpad /tmp $ riscv64-linux-musl-gcc -O2 a.c -static && qemu-riscv64 ./a.out 16, 16 Er, yea, you're right. Looks like of these, it's just s390x, so whatever. Jason
From: Peter Zijlstra > Sent: 05 November 2022 15:14 > > On Sat, Nov 05, 2022 at 02:29:47PM +0100, Jason A. Donenfeld wrote: > > On Fri, Nov 04, 2022 at 10:15:08AM -0700, Linus Torvalds wrote: > > > On Fri, Nov 4, 2022 at 9:01 AM Peter Zijlstra <peterz@infradead.org> wrote: > > > > > > > > So cmpxchg_double() does a cmpxchg on a double long value and is > > > > currently supported by: i386, x86_64, arm64 and s390. > > > > > > > > On all those, except i386, two longs are u128. > > > > > > > > So how about we introduce u128 and cmpxchg128 -- then it directly > > > > mirrors the u64 and cmpxchg64 usage we already have. It then also > > > > naturally imposses the alignment thing. > > > > > > Ack, except that we might have some "u128" users that do *not* > > > necessarily want any alignment thing. > > > > > > But maybe we could at least start with an u128 type that is marked as > > > being fully aligned, and if some other user comes in down the line > > > that wants relaxed alignment we can call it "u128_unaligned" or > > > something. > > > > Hm, sounds maybe not so nice for another use case: arithmetic code that > > makes use of u128 for efficient computations, but otherwise has > > no particular alignment requirements. For example, `typedef __uint128_t > > u128;` in: > > Natural alignment is... natural. Making it unaligned is quite mad. That > whole u64 is not naturally aligned on i386 thing Linus referred to is a > sodding pain in the backside. > > If the code has no alignment requirements, natural alignment is as good > as any. And if it does have requirements, you can use u128_unaligned. > > Also: > > $ ./align > 16, 16 > > --- > > #include <stdio.h> > > int main(int argx, char **argv) > { > __int128 a; > > printf("%d, %d\n", sizeof(a), __alignof(a)); > return 0; > } Well, __alignof() doesn't return the required value. (cf 'long long' on 32bit x86). But the alignment of __int128 is 16 :-) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Fri, Nov 04, 2022 at 10:15:08AM -0700, Linus Torvalds wrote: > On Fri, Nov 4, 2022 at 9:01 AM Peter Zijlstra <peterz@infradead.org> wrote: > > > > So cmpxchg_double() does a cmpxchg on a double long value and is > > currently supported by: i386, x86_64, arm64 and s390. > > > > On all those, except i386, two longs are u128. > > > > So how about we introduce u128 and cmpxchg128 -- then it directly > > mirrors the u64 and cmpxchg64 usage we already have. It then also > > naturally imposses the alignment thing. > > Ack Out now: https://lkml.kernel.org/r/20221219153525.632521981@infradead.org
--- 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)); @@ -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);