Message ID | 20230627221430.464073-1-dinguyen@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp8508012vqr; Tue, 27 Jun 2023 15:27:09 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ52PHL13snRiPqZq1GVsUBkS8Q4cAEfmuKLFDcd/UQt5ZzcimTN406m5n7GdF38pPTqbmhx X-Received: by 2002:a17:907:d0f:b0:94e:e30e:7245 with SMTP id gn15-20020a1709070d0f00b0094ee30e7245mr32137521ejc.8.1687904829455; Tue, 27 Jun 2023 15:27:09 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687904829; cv=none; d=google.com; s=arc-20160816; b=JXT5AxssWg3fZZWVzLox0coXCGRaE6CaU3sKOWCPCaWHL1C0wH2enoXnMhzyAXH0cO BGRaqA0BAZejHncssbw7gF+ZUzXPBsZ0gmSh67iniIWHiWabCpc7b8hslSpfef7h+h0H TWdnXR+AoZLOCUzG0+98HfSQYkmR0u+W2qVPDQySDxx0Otjn8N5Az6hdtxuM1GKtBLJf 2tUTJ2rsO9+jhxBOZuvlQrAMQgUNTCbBYeCc7a6p9DiaqUZVfr9OTamd+dbG/lYd49Nl KelJt4KGE5uxOPDK2O5PfKHocaPFaiUZoIAs0yGZfZX86PlU7QbsVzJ71/PYlK9F6m2l 1l4A== 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=cCON27nm8AIsfHyiqIUBZqaRT6yjniJrnZgg4IRWLN0=; fh=vLTTGeauCc/MPTDragkfHJSrt+nMhs/klsnkkc2xxyE=; b=dFS25xTjxRcLAnBr2IkfB5t702ebzV1l3UQOzGKAKbCmJi974T/42n0yh1fEp2j9s4 PU0821HWob/Q4afVz8c77jcyVMXUa3J3sNtDjDHY0tHDsvpUon0ShUG1UvXT89Ig7z+G bZ8WGjLARZuaVFJ9rC5pCczJ3ThzZDVhUn3n0E5WXKjRwuLSw3EhsPvMnRDl/RBbnoIo hN/QmE+Yb62IR4rcbOXJxvuEXfGgRv8pwy2KbOMXBlZH7s6u2yxO8X3Co61BtTp0Z6im 1gO2b97WRnHQVgLFHRSaJGhWuknF8FqKJHlRJT653E2QoJlAs/gGC9n9LVcs2DcmMObP VGcQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=XerVoOK0; 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; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s27-20020a170906169b00b0098f564f636esi3374003ejd.132.2023.06.27.15.26.42; Tue, 27 Jun 2023 15:27:09 -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=@kernel.org header.s=k20201202 header.b=XerVoOK0; 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; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229593AbjF0WOy (ORCPT <rfc822;nicolai.engesland@gmail.com> + 99 others); Tue, 27 Jun 2023 18:14:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51546 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229437AbjF0WOr (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 27 Jun 2023 18:14:47 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1B9571FCD for <linux-kernel@vger.kernel.org>; Tue, 27 Jun 2023 15:14:45 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 638AB61239 for <linux-kernel@vger.kernel.org>; Tue, 27 Jun 2023 22:14:45 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9AC2FC433C0; Tue, 27 Jun 2023 22:14:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1687904084; bh=PWkigw51oTYP+QIZeGiN4X++ycxgw6EELB6XK+Bx1vw=; h=From:To:Cc:Subject:Date:From; b=XerVoOK0uI9k692hzjtE2HMcyTWHTJoxtigXk8K/TM2i4cLAstZzJUvN/wvW5VQV7 9CbGCLwjBq2zY7ai8tLoMx/tvETP2XXCmcmTK3awbJjL2I9YG4n6J62BVOLq4NO0kQ qra5WzkLK+50FtAsXyEjZ6YjX2ohn6iibEdHs3OQwfuwWntIv2WbCiSEiB58hwkll/ Qa4SAp3+YAuUlBT3CQTMHCuhGHU9Tu9tyYdQHc2QhsIBGcDQV8oT/L0ogXTM2smj4I W4TK3yme41VLqb6d6m+0X3U2tc9zeGn2FJk00ofzWBKbg1WOw6/R2iyeh3VaAxbPfl v/KFm8MsUuItw== From: Dinh Nguyen <dinguyen@kernel.org> To: torvalds@linux-foundation.org Cc: dinguyen@kernel.org, linux-kernel@vger.kernel.org, linux@roeck-us.net, vishal.moola@gmail.com, akpm@linux-foundation.org, sfr@canb.auug.org.au Subject: [PATCH] Revert "nios2: Convert __pte_free_tlb() to use ptdescs" Date: Tue, 27 Jun 2023 17:14:30 -0500 Message-Id: <20230627221430.464073-1-dinguyen@kernel.org> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, 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: <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?1769896494457700432?= X-GMAIL-MSGID: =?utf-8?q?1769896494457700432?= |
Series |
Revert "nios2: Convert __pte_free_tlb() to use ptdescs"
|
|
Commit Message
Dinh Nguyen
June 27, 2023, 10:14 p.m. UTC
This reverts commit 6ebe94baa2b9ddf3ccbb7f94df6ab26234532734.
The patch "nios2: Convert __pte_free_tlb() to use ptdescs" was supposed
to go together with a patchset that Vishal Moola had planned taking it
through the mm tree. By just having this patch, all NIOS2 builds are
broken.
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
arch/nios2/include/asm/pgalloc.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Comments
On Tue, 27 Jun 2023 at 15:14, Dinh Nguyen <dinguyen@kernel.org> wrote: > > This reverts commit 6ebe94baa2b9ddf3ccbb7f94df6ab26234532734. > > The patch "nios2: Convert __pte_free_tlb() to use ptdescs" was supposed > to go together with a patchset that Vishal Moola had planned taking it > through the mm tree. By just having this patch, all NIOS2 builds are > broken. This is now at least the third time just this merge window where some base tree was broken, and people thought that linux-next is some kind of testing ground for it all. NO! Linux-next is indeed for testing, and for finding situations where there are interactions between different trees. But linux-next is *not* a replacement for "this tree has to work on its own". THAT testing needs to be done independently, and *before* a tree hits linux-next. It is *NOT* ok to say "this will work in combination with that other tree". EVERY SINGLE TREE needs to work on its own, because otherwise you cannot bisect the end result sanely. We apparently had the NIOS2 tree being broken. And the RCU tree was broken. And the KUnit tree was broken. In all those cases, the base tree did not compile properly on its own, and linux-next "magically fixed" it by either having Stephen Rothwell literally fix the build breakage by hand, or by having some other tree hide the problem. This is very much not ok. I'm not sure why it happened so much this release, but this needs to stop. People need to realize that you can't just throw shit at the wall and see if it sticks. You need to test your own trees *first*, and *independently* of other peoples trees. Then, if you have done basic testing, you can then have it in linux-next and that hopefully then finds any issues with bad interactions with other trees, and maybe also ends up getting more coverage testing on odd architectures and with odd configurations. But linux-next must not in *any* way be a replacement for doing basic testing on your own tree first. Linus
On 6/27/23 15:35, Linus Torvalds wrote: > On Tue, 27 Jun 2023 at 15:14, Dinh Nguyen <dinguyen@kernel.org> wrote: >> >> This reverts commit 6ebe94baa2b9ddf3ccbb7f94df6ab26234532734. >> >> The patch "nios2: Convert __pte_free_tlb() to use ptdescs" was supposed >> to go together with a patchset that Vishal Moola had planned taking it >> through the mm tree. By just having this patch, all NIOS2 builds are >> broken. > > This is now at least the third time just this merge window where some > base tree was broken, and people thought that linux-next is some kind > of testing ground for it all. > > NO! > > Linux-next is indeed for testing, and for finding situations where > there are interactions between different trees. > > But linux-next is *not* a replacement for "this tree has to work on > its own". THAT testing needs to be done independently, and *before* a > tree hits linux-next. > > It is *NOT* ok to say "this will work in combination with that other > tree". EVERY SINGLE TREE needs to work on its own, because otherwise > you cannot bisect the end result sanely. > > We apparently had the NIOS2 tree being broken. And the RCU tree was > broken. And the KUnit tree was broken. > Actually, this one is broken in linux-next as well because it was pulled into it, but the context patches needed to make it work (compile) are not there. It is also broken in next/pending-fixes for the same reason. Only this happened so quick that by the time I noticed and reported and argued that, no, I did not try to apply this patch on its own, the pull request into mainline was already sent and applied. Problem with linux-next is that it is so badly broken that it would take a full-time position to track down all its failures. Then there are those last-minute patches added in the week (or days) before the commit window opens which break it again. This is one example, but there is at least one more in linux-next (and pending-fixes); see https://kerneltests.org/builders/next-sh-pending-fixes/builds/822/steps/buildcommand/logs/stdio Guenter > In all those cases, the base tree did not compile properly on its own, > and linux-next "magically fixed" it by either having Stephen Rothwell > literally fix the build breakage by hand, or by having some other tree > hide the problem. > > This is very much not ok. > > I'm not sure why it happened so much this release, but this needs to > stop. People need to realize that you can't just throw shit at the > wall and see if it sticks. You need to test your own trees *first*, > and *independently* of other peoples trees. > > Then, if you have done basic testing, you can then have it in > linux-next and that hopefully then finds any issues with bad > interactions with other trees, and maybe also ends up getting more > coverage testing on odd architectures and with odd configurations. > > But linux-next must not in *any* way be a replacement for doing basic > testing on your own tree first. > > Linus
On Tue, Jun 27, 2023 at 03:35:45PM -0700, Linus Torvalds wrote: > On Tue, 27 Jun 2023 at 15:14, Dinh Nguyen <dinguyen@kernel.org> wrote: > > > > This reverts commit 6ebe94baa2b9ddf3ccbb7f94df6ab26234532734. > > > > The patch "nios2: Convert __pte_free_tlb() to use ptdescs" was supposed > > to go together with a patchset that Vishal Moola had planned taking it > > through the mm tree. By just having this patch, all NIOS2 builds are > > broken. > > This is now at least the third time just this merge window where some > base tree was broken, and people thought that linux-next is some kind > of testing ground for it all. > > NO! > > Linux-next is indeed for testing, and for finding situations where > there are interactions between different trees. > > But linux-next is *not* a replacement for "this tree has to work on > its own". THAT testing needs to be done independently, and *before* a > tree hits linux-next. > > It is *NOT* ok to say "this will work in combination with that other > tree". EVERY SINGLE TREE needs to work on its own, because otherwise > you cannot bisect the end result sanely. > > We apparently had the NIOS2 tree being broken. And the RCU tree was > broken. And the KUnit tree was broken. > > In all those cases, the base tree did not compile properly on its own, > and linux-next "magically fixed" it by either having Stephen Rothwell > literally fix the build breakage by hand, or by having some other tree > hide the problem. > > This is very much not ok. > > I'm not sure why it happened so much this release, but this needs to > stop. People need to realize that you can't just throw shit at the > wall and see if it sticks. You need to test your own trees *first*, > and *independently* of other peoples trees. > > Then, if you have done basic testing, you can then have it in > linux-next and that hopefully then finds any issues with bad > interactions with other trees, and maybe also ends up getting more > coverage testing on odd architectures and with odd configurations. > > But linux-next must not in *any* way be a replacement for doing basic > testing on your own tree first. On the off-chance that it helps someone else avoid my stupid mistakes, here is exactly how I messed this up so badly: 1. This API-name-change series went well, except for the usual lagging changes. This *should* not be a problem, as you simply leave the old API in however long it takes for the change to get in. 2. At some point -next was a single-argument kfree_rcu()-free zone. So I queued the offending commit on the -rcu tree's rcu/next branch, followed by a revert for my own testing. The idea was to make new uses fail in -next testing. So far, so good. 3. I noticed that -next was now free of kfree_rcu() calls. At this point, I made three stupid mistakes: a. I failed to wait for mainline itself to be free of the single-argument kfree_rcu(), thus pulling the offending single-argument kfree_rcu() removal commit into my pull request a merge window too soon. This is of course especially stupid since I tend to send you the RCU pull request early. b. I failed to identify exactly which -next commit eliminated single-argument kfree_rcu(). Had I done so, I would have seen that this was in fact Stephen's rcu/next merge commit, which was never going to go to mainline. c. Worst yet, out of force of habit, I left the revert from #2 above in my testing, thus failing to see the -rcu failure due to that remaining single-argument kfree_rcu() call. So a combination of three stupid mistakes on my part made the RCU happen. As you say, testing *exactly* the commit heading up the pull request merged with your master branch would have spotted this, and I will of course make sure that I do this in the future. And again, please accept my apologies for this mess. Thanx, Paul
On 6/27/23 16:23, Guenter Roeck wrote: > On 6/27/23 15:35, Linus Torvalds wrote: >> On Tue, 27 Jun 2023 at 15:14, Dinh Nguyen <dinguyen@kernel.org> wrote: >>> >>> This reverts commit 6ebe94baa2b9ddf3ccbb7f94df6ab26234532734. >>> >>> The patch "nios2: Convert __pte_free_tlb() to use ptdescs" was supposed >>> to go together with a patchset that Vishal Moola had planned taking it >>> through the mm tree. By just having this patch, all NIOS2 builds are >>> broken. >> >> This is now at least the third time just this merge window where some >> base tree was broken, and people thought that linux-next is some kind >> of testing ground for it all. >> >> NO! >> >> Linux-next is indeed for testing, and for finding situations where >> there are interactions between different trees. >> >> But linux-next is *not* a replacement for "this tree has to work on >> its own". THAT testing needs to be done independently, and *before* a >> tree hits linux-next. >> >> It is *NOT* ok to say "this will work in combination with that other >> tree". EVERY SINGLE TREE needs to work on its own, because otherwise >> you cannot bisect the end result sanely. >> >> We apparently had the NIOS2 tree being broken. And the RCU tree was >> broken. And the KUnit tree was broken. >> > > Actually, this one is broken in linux-next as well because it was pulled > into it, but the context patches needed to make it work (compile) are not > there. It is also broken in next/pending-fixes for the same reason. > > Only this happened so quick that by the time I noticed and reported > and argued that, no, I did not try to apply this patch on its own, > the pull request into mainline was already sent and applied. > > Problem with linux-next is that it is so badly broken that it would take > a full-time position to track down all its failures. Then there are those > last-minute patches added in the week (or days) before the commit window > opens which break it again. This is one example, but there is at least > one more in linux-next (and pending-fixes); see > https://kerneltests.org/builders/next-sh-pending-fixes/builds/822/steps/buildcommand/logs/stdio > And now the broken (never compiled) patch made it into mainline and breaks the sh:dreamcast_defconfig build there. Yes, it does happen a lot that builds are temporarily broken in mainline because patch series are split up among maintainers and submitted to mainline without regard of buildability. I have learned to live with that and don't normally report it because I know (ok, hope) it is going to be fixed by the end of the commit window. I personally find patch series - typically doing some cleanup - which are not even build tested on the affected architectures much more annoying. Guenter
diff --git a/arch/nios2/include/asm/pgalloc.h b/arch/nios2/include/asm/pgalloc.h index ce6bb8e74271..ecd1657bb2ce 100644 --- a/arch/nios2/include/asm/pgalloc.h +++ b/arch/nios2/include/asm/pgalloc.h @@ -28,10 +28,10 @@ static inline void pmd_populate(struct mm_struct *mm, pmd_t *pmd, extern pgd_t *pgd_alloc(struct mm_struct *mm); -#define __pte_free_tlb(tlb, pte, addr) \ - do { \ - pagetable_pte_dtor(page_ptdesc(pte)); \ - tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte))); \ +#define __pte_free_tlb(tlb, pte, addr) \ + do { \ + pgtable_pte_page_dtor(pte); \ + tlb_remove_page((tlb), (pte)); \ } while (0) #endif /* _ASM_NIOS2_PGALLOC_H */