From: Nathan Chancellor <nathan@kernel.org>
To: Hugh Dickins <hughd@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Mike Kravetz <mike.kravetz@oracle.com>,
Mike Rapoport <rppt@kernel.org>,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
Matthew Wilcox <willy@infradead.org>,
David Hildenbrand <david@redhat.com>,
Suren Baghdasaryan <surenb@google.com>,
Qi Zheng <zhengqi.arch@bytedance.com>,
Peter Zijlstra <peterz@infradead.org>,
Russell King <linux@armlinux.org.uk>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Greg Ungerer <gerg@linux-m68k.org>,
Michal Simek <monstr@monstr.eu>,
Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
Helge Deller <deller@gmx.de>,
John David Anglin <dave.anglin@bell.net>,
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
Michael Ellerman <mpe@ellerman.id.au>,
Alexandre Ghiti <alexghiti@rivosinc.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Heiko Carstens <hca@linux.ibm.com>,
Christian Borntraeger <borntraeger@linux.ibm.com>,
Claudio Imbrenda <imbrenda@linux.ibm.com>,
Alexander Gordeev <agordeev@linux.ibm.com>,
John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
"David S. Miller" <davem@davemloft.net>,
Chris Zankel <chris@zankel.net>,
Max Filippov <jcmvbkbc@gmail.com>,
x86@kernel.org, linux-arm-kernel@lists.infradead.org,
linux-ia64@vger.kernel.org, linux-m68k@lists.linux-m68k.org,
linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org,
linux-s390@vger.kernel.org, linux-sh@vger.kernel.org,
sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
Subject: Re: [PATCH v2 07/23] mips: update_mmu_cache() can replace __update_tlb()
Date: Thu, 15 Jun 2023 08:50:59 -0700 [thread overview]
Message-ID: <20230615155059.GB3665766@dev-arch.thelio-3990X> (raw)
In-Reply-To: <344a4da-3890-45fd-607e-b5f85ca6ad48@google.com>
On Wed, Jun 14, 2023 at 10:43:30PM -0700, Hugh Dickins wrote:
> On Wed, 14 Jun 2023, Hugh Dickins wrote:
> > On Wed, 14 Jun 2023, Nathan Chancellor wrote:
> > >
> > > I just bisected a crash while powering down a MIPS machine in QEMU to
> > > this change as commit 8044511d3893 ("mips: update_mmu_cache() can
> > > replace __update_tlb()") in linux-next.
> >
> > Thank you, Nathan, that's very helpful indeed. This patch certainly knew
> > that it wanted testing, and I'm glad to hear that it is now seeing some.
> >
> > While powering down? The messages below look like it was just coming up,
> > but no doubt that's because you were bisecting (or because I'm unfamiliar
> > with what messages to expect there). It's probably irrelevant information,
> > but I wonder whether the (V)machine worked well enough for a while before
> > you first powered down and spotted the problem, or whether it's never got
> > much further than trying to run init (busybox)? I'm trying to get a feel
> > for whether the problem occurs under common or uncommon conditions.
Ugh sorry, I have been looking into too many bugs lately and got my
wires crossed :) this is indeed a problem when running init (which is
busybox, this is a simple Buildroot file system).
> > > Unfortunately, I can still
> > > reproduce it with the existing fix you have for this change on the
> > > mailing list, which is present in next-20230614.
> >
> > Right, that later fix was only for a build warning, nothing functional
> > (or at least I hoped that it wasn't making any functional difference).
> >
> > Thanks a lot for the detailed instructions below: unfortunately, those
> > would draw me into a realm of testing I've never needed to enter before,
> > so a lot of time spent on setup and learning. Usually, I just stare at
> > the source.
> >
> > What this probably says is that I should revert most my cleanup there,
> > and keep as close to the existing code as possible. But some change is
> > needed, and I may need to understand (or have a good guess at) what was
> > going wrong, to decide what kind of retreat will be successful.
> >
> > Back to the source for a while: I hope I'll find examples in nearby MIPS
> > kernel source (and git history), which will hint at the right way forward.
> > Then send you a patch against next-20230614 to try, when I'm reasonably
> > confident that it's enough to satisfy my purpose, but likely not to waste
> > your time.
>
> I'm going to take advantage of your good nature by attaching
> two alternative patches, either to go on top of next-20230614.
>
> mips1.patch,
> arch/mips/mm/tlb-r4k.c | 12 +-----------
> 1 file changed, 1 insertion(+), 11 deletions(-)
>
> is by far my favourite. I couldn't see anything wrong with what's
> already there for mips, but it seems possible that (though I didn't
> find it) somewhere calls update_mmu_cache_pmd() on a page table. So
> mips1.patch restores the pmd_huge() check, and cleans up further by
> removing the silly pgdp, p4dp, pudp, pmdp stuff: the pointer has now
> been passed in by the caller, why walk the tree again? I should have
> done it this way before.
>
> But if that doesn't work, then I'm afraid it will have to be
> mips2.patch,
> arch/mips/include/asm/pgtable.h | 15 ++++++++++++---
> arch/mips/mm/tlb-r3k.c | 5 ++---
> arch/mips/mm/tlb-r4k.c | 27 ++++++++++++++++++---------
> 3 files changed, 32 insertions(+), 15 deletions(-)
>
> which reverts all of the original patch and its build warning fix,
> and does a pte_unmap() to balance the silly pte_offset_map() there;
> with an apologetic comment for this being about the only place in
> the tree where I have no idea what to do if ptep were NULL.
>
> I do hope that you find the first fixes the breakage; but if not, then
I hate to be the bearer of bad news but the first patch did not fix the
breakage, I see the same issue.
> I even more fervently hope that the second will, despite my hating it.
> Touch wood for the first, fingers crossed for the second, thanks,
Thankfully, the second one does. Thanks for the quick and thoughtful
responses!
Cheers,
Nathan
next prev parent reply other threads:[~2023-06-15 15:52 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-08 19:07 [PATCH v2 00/23] arch: allow pte_offset_map[_lock]() to fail Hugh Dickins
2023-06-08 19:10 ` [PATCH v2 01/23] arm: " Hugh Dickins
2023-06-08 19:11 ` [PATCH v2 02/23] arm64: allow pte_offset_map() " Hugh Dickins
2023-06-08 19:13 ` [PATCH v2 03/23] arm64/hugetlb: pte_alloc_huge() pte_offset_huge() Hugh Dickins
2023-06-08 19:14 ` [PATCH v2 04/23] ia64/hugetlb: " Hugh Dickins
2023-06-08 19:15 ` [PATCH v2 05/23] m68k: allow pte_offset_map[_lock]() to fail Hugh Dickins
2023-06-08 19:16 ` [PATCH v2 06/23] microblaze: allow pte_offset_map() " Hugh Dickins
2023-06-08 19:17 ` [PATCH v2 07/23] mips: update_mmu_cache() can replace __update_tlb() Hugh Dickins
2023-06-09 8:08 ` [PATCH v2 07/23 fix] mips: update_mmu_cache() can replace __update_tlb(): fix Hugh Dickins
2023-06-14 23:17 ` [PATCH v2 07/23] mips: update_mmu_cache() can replace __update_tlb() Nathan Chancellor
2023-06-15 0:26 ` Hugh Dickins
2023-06-15 5:43 ` Hugh Dickins
2023-06-15 15:50 ` Nathan Chancellor [this message]
2023-06-15 21:22 ` Hugh Dickins
2023-06-15 23:02 ` [PATCH v2 07/23 replacement] mips: add pte_unmap() to balance pte_offset_map() Hugh Dickins
2023-06-17 3:54 ` Yu Zhao
2023-06-18 20:57 ` Yu Zhao
2023-06-15 22:07 ` [PATCH v2 07/23] mips: update_mmu_cache() can replace __update_tlb() Yu Zhao
2023-06-08 19:18 ` [PATCH v2 08/23] parisc: add pte_unmap() to balance get_ptep() Hugh Dickins
2023-06-19 3:55 ` Helge Deller
2023-06-08 19:20 ` [PATCH v2 09/23] parisc: unmap_uncached_pte() use pte_offset_kernel() Hugh Dickins
2023-06-08 19:21 ` [PATCH v2 10/23] parisc/hugetlb: pte_alloc_huge() pte_offset_huge() Hugh Dickins
2023-06-08 19:22 ` [PATCH v2 11/23] powerpc: kvmppc_unmap_free_pmd() pte_offset_kernel() Hugh Dickins
2023-06-08 19:23 ` [PATCH v2 12/23] powerpc: allow pte_offset_map[_lock]() to fail Hugh Dickins
2023-06-08 19:24 ` [PATCH v2 13/23] powerpc/hugetlb: pte_alloc_huge() Hugh Dickins
2023-06-08 19:25 ` [PATCH v2 14/23] riscv/hugetlb: pte_alloc_huge() pte_offset_huge() Hugh Dickins
2023-06-08 19:27 ` [PATCH v2 15/23] s390: allow pte_offset_map_lock() to fail Hugh Dickins
2023-06-13 11:45 ` Claudio Imbrenda
2023-06-08 19:29 ` [PATCH v2 16/23] s390: gmap use pte_unmap_unlock() not spin_unlock() Hugh Dickins
2023-06-08 19:30 ` [PATCH v2 17/23] sh/hugetlb: pte_alloc_huge() pte_offset_huge() Hugh Dickins
2023-06-08 19:31 ` [PATCH v2 18/23] sparc/hugetlb: " Hugh Dickins
2023-06-08 19:32 ` [PATCH v2 19/23] sparc: allow pte_offset_map() to fail Hugh Dickins
2023-06-08 19:33 ` [PATCH v2 20/23] sparc: iounit and iommu use pte_offset_kernel() Hugh Dickins
2023-06-08 19:35 ` [PATCH v2 21/23] x86: Allow get_locked_pte() to fail Hugh Dickins
2023-06-08 19:36 ` [PATCH v2 22/23] x86: sme_populate_pgd() use pte_offset_kernel() Hugh Dickins
2023-06-08 19:37 ` [PATCH v2 23/23] xtensa: add pte_unmap() to balance pte_offset_map() Hugh Dickins
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230615155059.GB3665766@dev-arch.thelio-3990X \
--to=nathan@kernel.org \
--cc=agordeev@linux.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=alexghiti@rivosinc.com \
--cc=aneesh.kumar@linux.ibm.com \
--cc=borntraeger@linux.ibm.com \
--cc=catalin.marinas@arm.com \
--cc=chris@zankel.net \
--cc=dave.anglin@bell.net \
--cc=davem@davemloft.net \
--cc=david@redhat.com \
--cc=deller@gmx.de \
--cc=geert@linux-m68k.org \
--cc=gerg@linux-m68k.org \
--cc=glaubitz@physik.fu-berlin.de \
--cc=hca@linux.ibm.com \
--cc=hughd@google.com \
--cc=imbrenda@linux.ibm.com \
--cc=jcmvbkbc@gmail.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-m68k@lists.linux-m68k.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-parisc@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mike.kravetz@oracle.com \
--cc=monstr@monstr.eu \
--cc=mpe@ellerman.id.au \
--cc=palmer@dabbelt.com \
--cc=peterz@infradead.org \
--cc=rppt@kernel.org \
--cc=sparclinux@vger.kernel.org \
--cc=surenb@google.com \
--cc=tsbogend@alpha.franken.de \
--cc=will@kernel.org \
--cc=willy@infradead.org \
--cc=x86@kernel.org \
--cc=zhengqi.arch@bytedance.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).