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 CFFDEEB64D9 for ; Thu, 15 Jun 2023 00:27:02 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 760C06B0072; Wed, 14 Jun 2023 20:27:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 710AB6B0074; Wed, 14 Jun 2023 20:27:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5D9188E0001; Wed, 14 Jun 2023 20:27:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 521EF6B0072 for ; Wed, 14 Jun 2023 20:27:02 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 1A03F1C8884 for ; Thu, 15 Jun 2023 00:27:02 +0000 (UTC) X-FDA: 80903092284.12.79B63FF Received: from mail-yw1-f182.google.com (mail-yw1-f182.google.com [209.85.128.182]) by imf06.hostedemail.com (Postfix) with ESMTP id 428EB180009 for ; Thu, 15 Jun 2023 00:27:00 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=WRHrIu3g; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf06.hostedemail.com: domain of hughd@google.com designates 209.85.128.182 as permitted sender) smtp.mailfrom=hughd@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686788820; a=rsa-sha256; cv=none; b=bmiwGJGMGXwTTeTjlUDeMCugT6Kj/dclnk9TVBpPPJRVpawBhpKKBRU//FKxic9WcL4nr3 P64qzWliA7Mt6tfwAzQZ78pls5QaMTIZVfOah6W0Htheij3DwyysvbkBjqrvEvIbsoU2nC HlMWa67ZuxAVSFaf+8o+XRxJwK4CeYk= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=WRHrIu3g; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf06.hostedemail.com: domain of hughd@google.com designates 209.85.128.182 as permitted sender) smtp.mailfrom=hughd@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686788820; 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=DnWlt5Rs3KXon7VRWQ16gguJVnALQ3ZmklO6Frv1x8M=; b=TWA/SkcoSmxteOBsEz5gSlccSy/h2TuatyjO17AsavdyycW/bDoJL+9MCrSf659vPEM5wR Y97PX+9A0ddE2CA0tOQ9vsy5Oaju/90DvGdE6RA6F1Lgr+KPzDBGpMH0Fpl3pVSVPPJQHZ sdRfsFd/BuAdBvNagss2OIoIGHT4zWg= Received: by mail-yw1-f182.google.com with SMTP id 00721157ae682-56ff9cc91b4so15081917b3.0 for ; Wed, 14 Jun 2023 17:26:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1686788819; x=1689380819; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=DnWlt5Rs3KXon7VRWQ16gguJVnALQ3ZmklO6Frv1x8M=; b=WRHrIu3gJ2wGOTVICPuRYrvd1c7L0vJmrlkgymQr/8tluUcC03co9peWifCQEP1BsY n1bcKbEoAy3csFFfNr5H+pB6uC7G+M6NDOY8RplOtmKbo0R9oGV+MU9bx9fwzUqBE6qz UoP7JSR/WImIB+BSu++yiJd2ccucSuR3F/8O1ueWLj3eeb3W7W3deV4ilVi0Hz02Zt93 TjoBiZtPZ+PXM7D8IHAIhSy81RgpuYrpZDW9wXrDupoXxD94l+mSwBro2iU8DdUEQdKI M+gCDPVaDv5mY02cBNtASY+U3YkMeuVD/52uz72mGD+cYhnjUb+L8OIbARxgrlvrjLd5 w90A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686788819; x=1689380819; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=DnWlt5Rs3KXon7VRWQ16gguJVnALQ3ZmklO6Frv1x8M=; b=cknWnQoAk3k/h0UjG3UuAYt7MgsFkAcw8wuEBQHblKI8WrkNTSWez1xPIc4CbKcRZt 6rORHanOQ6zeQ4g1dLEdZxcz92vTGjs1wbMZ2YO+zwPKfqNV67OrtIw4iTkg/cXyFsBd ByCMApFQUjBPQNWWqPGst3jJBvAdjL/9cwc7K37iKilaMqI6McwrUxv7KrW3KzezMk+I 8Y2JVEbO/pX99xqWfKCTkPQTws8Ez1UEigF1vkNZ3CqQUt5NsZ0AWR/7N/Svm6vZHkgE NWZKgUX9cyxjjEnbKObX6vhswIiuOmWFad14u1AchVA4qrtjv/cuZSmKPcjueB+Mfyfb 3p0Q== X-Gm-Message-State: AC+VfDxoIzwL7OA4VJWs/vsCIFM0u4OneQc9G03y7g55EhH3RSiryZmm bMvg1ed43scYg0akKW55LosT7g== X-Google-Smtp-Source: ACHHUZ7bmVwEeref/EmuNgObqE0Cze454MmZYUDZGEjkaQsMh2xmkr5i3f3fpHWj6dmJHfcSbaICaw== X-Received: by 2002:a81:8744:0:b0:56d:31bb:7388 with SMTP id x65-20020a818744000000b0056d31bb7388mr3076697ywf.49.1686788819041; Wed, 14 Jun 2023 17:26:59 -0700 (PDT) Received: from ripple.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id h81-20020a815354000000b0055a881abfc3sm2421337ywb.135.2023.06.14.17.26.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Jun 2023 17:26:57 -0700 (PDT) Date: Wed, 14 Jun 2023 17:26:43 -0700 (PDT) From: Hugh Dickins X-X-Sender: hugh@ripple.attlocal.net To: Nathan Chancellor cc: Hugh Dickins , Andrew Morton , Mike Kravetz , Mike Rapoport , "Kirill A. Shutemov" , Matthew Wilcox , David Hildenbrand , Suren Baghdasaryan , Qi Zheng , Peter Zijlstra , Russell King , Catalin Marinas , Will Deacon , Geert Uytterhoeven , Greg Ungerer , Michal Simek , Thomas Bogendoerfer , Helge Deller , John David Anglin , "Aneesh Kumar K.V" , Michael Ellerman , Alexandre Ghiti , Palmer Dabbelt , Heiko Carstens , Christian Borntraeger , Claudio Imbrenda , Alexander Gordeev , John Paul Adrian Glaubitz , "David S. Miller" , Chris Zankel , Max Filippov , 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() In-Reply-To: <20230614231758.GA1503611@dev-arch.thelio-3990X> Message-ID: References: <178970b0-1539-8aac-76fd-972c6c46ec17@google.com> <20230614231758.GA1503611@dev-arch.thelio-3990X> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 428EB180009 X-Stat-Signature: dhsbirswreagnu5u7cc3zadfgrtzsb5r X-HE-Tag: 1686788820-213726 X-HE-Meta: U2FsdGVkX1+jpk/+gnSAfA6ahMkB1/hbUphUYPuWOJMKGO1BL++70Ba3ymrz+J0hT21+evdWG9WkZUIc6YNYSgg5tAXKi2nwLHsZK0R2OcTzv5Zng5dpPstZ/K2TH7jHq5+ovMm41UizuJYUSTarVpvJETkjmIpacwqyiTKXv8t8AucMUgBz5tIAso4sq/gQuMZoMAUR4ewvVK/LCXIUjb2g7gsjMVxh/b0w35PUr69zqz9hecc1J9kIbddEoW9UYtwq2sExCQffQO4ii6uNOg3TFowVeKi8UWYMtQDoRZKuSrOoiWCLFoG6hu7aTAByBMRpdw9NKatb9CsJuyv03a19bLvxL42U1D5ujHNgXmBSewFBfFxisgrJjFSQ/rCUfQ7ADQDpSTW37oq7MOc4ZCyvX9ObBIo/C3cAlxXeT9dqUDL+WdK41yAZ/3Nw4RHwn0z4yBztSknAKKAhxmf656vdGQ/8p2aDVeMOcq4lD3Bte5buwclVhUYOWv7KKYnkfJhE7YBT6GHQWjCvVqiQt8X/uQCaROqzJ9NY02R0tlQdpHrwLXWeCV8XtacUhgGodrF4Sy21ZGXYZEBodgNqlquEWwev8aZiXvokTFW+Trs/BNQQMWUBq/EFGx1KF58Pj58+71D1mdkJyLvHqW0vtgTdQr2TH3dGAA9MP2qDaP0DR7+uxR0EWJUjhPqSUdoybtrRjpkzVMKwLAF685vtTvLnpxn3KvZMdKYYiEad06e5MLmmFBVt4fvVqdIUC+XHg1hwaTxROs+YhgxCDV0b1KcotBOCmoDfLpoMupU7rppH8qcLwEib2JwBMpD+kNFZodEitn2RM7Tq34NB0w/wUmfdkKrBbFIPBELu+iCzqz6QbhGgaimZzhRItAW6eZ7MUqdyv+5XLWW5y0QReFcF3K/Jtz4MBHaET6sf4co6oRVo/D/sm4m4LLHQ6VzAtMF12cEaINs6xqYLYSxiRNc sgzVNBAF VaWLSQWv5F93ks7Rlx+XsutY3/gwIYMOQRERjN7ZxMDTIzz+tEc3fsngIkTUpSABTuvUVfSqx/y1EkcJzx8HytADIxwv5fHH0/zz86zQbw9mt7Cu0W7j0rHAbUMRK9d4MDdYGGSiR2SOw9zx9abbQM8TuSMfUNXtvu3kCanXZVqPU2B7UXCvZMII9c861AyN/ECoDd3q8KDD/Yh4EsYqGGykdtVqch0y+0211RjXyeUVTgyGPMpRpnVTRHintnoeutAL0t3Bpg1rsf+S2F3Zyud5wm0XjP+l8QfGHYfb3jNV3ueh3CCZH7sUmlW2ReGQYebFdDmlzxVTdFT0g4qlQeKSXBwRSRBNH+gLOVYMvYIyrea7DFcPkmcutgE2NjBZKwj+Q9f1uSGYlBsep5zuwIONogjcUts8SfLMrojKM/kYbgKUGw2z1nMyPTYeyYzThkoH07H5j8Z5YPVcs4Wkkv9r9qxuEl0QM19hRIvRCY4+XkdkECmGju0WxtI3DLaJvnyHAmVY22yTjdaw= 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 Wed, 14 Jun 2023, Nathan Chancellor wrote: > Hi Hugh, > > On Thu, Jun 08, 2023 at 12:17:24PM -0700, Hugh Dickins wrote: > > Don't make update_mmu_cache() a wrapper around __update_tlb(): call it > > directly, and use the ptep (or pmdp) provided by the caller, instead of > > re-calling pte_offset_map() - which would raise a question of whether a > > pte_unmap() is needed to balance it. > > > > Check whether the "ptep" provided by the caller is actually the pmdp, > > instead of testing pmd_huge(): or test pmd_huge() too and warn if it > > disagrees? This is "hazardous" territory: needs review and testing. > > > > Signed-off-by: Hugh Dickins > > --- > > arch/mips/include/asm/pgtable.h | 15 +++------------ > > arch/mips/mm/tlb-r3k.c | 5 +++-- > > arch/mips/mm/tlb-r4k.c | 9 +++------ > > 3 files changed, 9 insertions(+), 20 deletions(-) > > > > diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h > > index 574fa14ac8b2..9175dfab08d5 100644 > > --- a/arch/mips/include/asm/pgtable.h > > +++ b/arch/mips/include/asm/pgtable.h > > @@ -565,15 +565,8 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte) > > } > > #endif > > > > -extern void __update_tlb(struct vm_area_struct *vma, unsigned long address, > > - pte_t pte); > > - > > -static inline void update_mmu_cache(struct vm_area_struct *vma, > > - unsigned long address, pte_t *ptep) > > -{ > > - pte_t pte = *ptep; > > - __update_tlb(vma, address, pte); > > -} > > +extern void update_mmu_cache(struct vm_area_struct *vma, > > + unsigned long address, pte_t *ptep); > > > > #define __HAVE_ARCH_UPDATE_MMU_TLB > > #define update_mmu_tlb update_mmu_cache > > @@ -581,9 +574,7 @@ static inline void update_mmu_cache(struct vm_area_struct *vma, > > static inline void update_mmu_cache_pmd(struct vm_area_struct *vma, > > unsigned long address, pmd_t *pmdp) > > { > > - pte_t pte = *(pte_t *)pmdp; > > - > > - __update_tlb(vma, address, pte); > > + update_mmu_cache(vma, address, (pte_t *)pmdp); > > } > > > > /* > > diff --git a/arch/mips/mm/tlb-r3k.c b/arch/mips/mm/tlb-r3k.c > > index 53dfa2b9316b..e5722cd8dd6d 100644 > > --- a/arch/mips/mm/tlb-r3k.c > > +++ b/arch/mips/mm/tlb-r3k.c > > @@ -176,7 +176,8 @@ void local_flush_tlb_page(struct vm_area_struct *vma, unsigned long page) > > } > > } > > > > -void __update_tlb(struct vm_area_struct *vma, unsigned long address, pte_t pte) > > +void update_mmu_cache(struct vm_area_struct *vma, > > + unsigned long address, pte_t *ptep) > > { > > unsigned long asid_mask = cpu_asid_mask(¤t_cpu_data); > > unsigned long flags; > > @@ -203,7 +204,7 @@ void __update_tlb(struct vm_area_struct *vma, unsigned long address, pte_t pte) > > BARRIER; > > tlb_probe(); > > idx = read_c0_index(); > > - write_c0_entrylo0(pte_val(pte)); > > + write_c0_entrylo0(pte_val(*ptep)); > > write_c0_entryhi(address | pid); > > if (idx < 0) { /* BARRIER */ > > tlb_write_random(); > > diff --git a/arch/mips/mm/tlb-r4k.c b/arch/mips/mm/tlb-r4k.c > > index 1b939abbe4ca..c96725d17cab 100644 > > --- a/arch/mips/mm/tlb-r4k.c > > +++ b/arch/mips/mm/tlb-r4k.c > > @@ -290,14 +290,14 @@ void local_flush_tlb_one(unsigned long page) > > * updates the TLB with the new pte(s), and another which also checks > > * for the R4k "end of page" hardware bug and does the needy. > > */ > > -void __update_tlb(struct vm_area_struct * vma, unsigned long address, pte_t pte) > > +void update_mmu_cache(struct vm_area_struct *vma, > > + unsigned long address, pte_t *ptep) > > { > > unsigned long flags; > > pgd_t *pgdp; > > p4d_t *p4dp; > > pud_t *pudp; > > pmd_t *pmdp; > > - pte_t *ptep; > > int idx, pid; > > > > /* > > @@ -326,10 +326,9 @@ void __update_tlb(struct vm_area_struct * vma, unsigned long address, pte_t pte) > > idx = read_c0_index(); > > #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT > > /* this could be a huge page */ > > - if (pmd_huge(*pmdp)) { > > + if (ptep == (pte_t *)pmdp) { > > unsigned long lo; > > write_c0_pagemask(PM_HUGE_MASK); > > - ptep = (pte_t *)pmdp; > > lo = pte_to_entrylo(pte_val(*ptep)); > > write_c0_entrylo0(lo); > > write_c0_entrylo1(lo + (HPAGE_SIZE >> 7)); > > @@ -344,8 +343,6 @@ void __update_tlb(struct vm_area_struct * vma, unsigned long address, pte_t pte) > > } else > > #endif > > { > > - ptep = pte_offset_map(pmdp, address); > > - > > #if defined(CONFIG_PHYS_ADDR_T_64BIT) && defined(CONFIG_CPU_MIPS32) > > #ifdef CONFIG_XPA > > write_c0_entrylo0(pte_to_entrylo(ptep->pte_high)); > > -- > > 2.35.3 > > > > 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. > 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. Thanks, until later, Hugh > > I can reproduce it with the GCC 13.1.0 on kernel.org [1]. > > $ make -skj"$(nproc)" ARCH=mips CROSS_COMPILE=mips-linux- mrproper malta_defconfig vmlinux > > $ qemu-system-mipsel \ > -display none \ > -nodefaults \ > -cpu 24Kf \ > -machine malta \ > -kernel vmlinux \ > -initrd rootfs.cpio \ > -m 512m \ > -serial mon:stdio > ... > Linux version 6.4.0-rc6-next-20230614 (nathan@dev-arch.thelio-3990X) (mips-linux-gcc (GCC) 13.1.0, GNU ld (GNU Binutils) 2.40) #1 SMP Wed Jun 14 16:13:02 MST 2023 > ... > Run /init as init process > process '/bin/busybox' started with executable stack > do_page_fault(): sending SIGSEGV to init for invalid read access from 0000003c > epc = 77b893dc in ld-uClibc-1.0.39.so[77b84000+8000] > ra = 77b8930c in ld-uClibc-1.0.39.so[77b84000+8000] > Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b > ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]--- > > The rootfs is available at [2] if it is needed. I am more than happy to > provide additional information or test patches if necessary. > > [1]: https://mirrors.edge.kernel.org/pub/tools/crosstool/ > [2]: https://github.com/ClangBuiltLinux/boot-utils/releases/download/20230609-194440/mipsel-rootfs.cpio.zst > > Cheers, > Nathan