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 71928E77188 for ; Fri, 3 Jan 2025 09:13:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E275C6B0083; Fri, 3 Jan 2025 04:13:34 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DD58B6B0085; Fri, 3 Jan 2025 04:13:34 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C27C86B0088; Fri, 3 Jan 2025 04:13:34 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id A13556B0083 for ; Fri, 3 Jan 2025 04:13:34 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 1F4BF806BB for ; Fri, 3 Jan 2025 09:13:34 +0000 (UTC) X-FDA: 82965575448.27.2D03435 Received: from mail-pl1-f179.google.com (mail-pl1-f179.google.com [209.85.214.179]) by imf30.hostedemail.com (Postfix) with ESMTP id 4D24E8001C for ; Fri, 3 Jan 2025 09:11:52 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b="kP/hcsgz"; spf=pass (imf30.hostedemail.com: domain of zhengqi.arch@bytedance.com designates 209.85.214.179 as permitted sender) smtp.mailfrom=zhengqi.arch@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1735895551; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=FvjXC1OjFutvGYgxD2wHa2wTk86a/9lW5eStlVFSRNE=; b=KNPjbAwhn/FZSZzRUJ+7otoKRspWUlsr9EsZFQbl8jeyT0pPbZP4TnOPfM09vTqIMzJkRs XS6qDJPy9STw1NexjokWKtczmZ12DlhyR4Rho0clSCTntvZNNj0vi8UmuS8gF9gjcf8Tfe ywa5L/+bVvsD+lV7NBqvFisZI7KxC/E= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1735895551; a=rsa-sha256; cv=none; b=s/yuy1SaSTbZByu2ppMbdiEeMHt+GSUTv8cU/46hQPqnspDlevowoMPHGg9WdDAk9lJnuw GjBbk5fq8L1Q5btMv9ZyZU7msh6IxqS5aXMkTpL40nZNZG6EGrX1RbgMJjV1enQYbqlWjY YeHZQ3nArQyPNjy42fSMwK6czQzMVeY= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b="kP/hcsgz"; spf=pass (imf30.hostedemail.com: domain of zhengqi.arch@bytedance.com designates 209.85.214.179 as permitted sender) smtp.mailfrom=zhengqi.arch@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com Received: by mail-pl1-f179.google.com with SMTP id d9443c01a7336-2161eb95317so171558165ad.1 for ; Fri, 03 Jan 2025 01:13:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1735895609; x=1736500409; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=FvjXC1OjFutvGYgxD2wHa2wTk86a/9lW5eStlVFSRNE=; b=kP/hcsgz+trZP0oG3rC4qlw3Z+m+AePRUeyI7gd6o2fYkuc9yR8RK+FE02P/qhRX/0 GYLGoRZwn8/WWOyAKza402Dxev01tVh91IZYXvzHvmNPdr53iDio7oaT838AZoHdK42r yaNoTFaD7PEyrXJ1S6Zyzajcv1P5RO4NFSENoymPXQI/2gjznsrpuo+hta0pz+obbSFs UdEobBlRaqavCPl0v791uhgDTJrfhYS3FVCmnHV0lzpSEawnxOWldPWx9QLzDmMF3j/9 yscUJMBeUPK+BOs93lNo6uSqxlnqvKgig157JyM87P5jLGwkj3i1wgX1FyE1opSEzUrR cqIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1735895609; x=1736500409; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=FvjXC1OjFutvGYgxD2wHa2wTk86a/9lW5eStlVFSRNE=; b=gnyfnM61uIvdEySBlb3/bBTCKdsFvtkv+DyywXWA4EriVpBbF581HB4YO0HHGTMk0U RfwVWHl462IFaL5GQgwI47a0vg2WDwp/l45Kt1mUpz0slW3yYr+xnQ/bfpCvAZcefXWT SFbvkbYj+2HUEs/elhtrFD2A0g8ICpFbPhs0B9MrDy1MoR4BDGe8bsLTgL7mrC/XtXMZ O788qqYvLSrbr5rYEwbyC0GA17RBudX213SvsjqWKTASWhlPAVrWqcGV8OOK+yQ8CReC 0yFq70KCFqhr3kz52dZJzWcMBLZm+zrtpOiyCcJFOlzcVCsIFk4Uek2m1buwM/4biRwZ iQ0Q== X-Forwarded-Encrypted: i=1; AJvYcCUzEg3hQRkLl1D2MYFi2i8FL/tn0CQPuUjL7t6hOadNhPikGgKmrDalnxvQsfUbINj/MdVsesSilA==@kvack.org X-Gm-Message-State: AOJu0YyuvOrBUX0Zen8uA9h28wLiVWve8qXl9yZJRe98Zi6HWcAghudr AwYj4czmCcS/BTeW3wljr1y2ud3GGasBYXg12HPFX42MFWO3wH28uudFXL2nFgI= X-Gm-Gg: ASbGnctkLrGzZ/j/VwWIn/7/4fDQehSg1gn0gKAkaIjVrHqWhtuTjrKgO8T4T0evb4J q5w9BJnwSCwIF32SBcCbuwBsJLfKd+eXf1Lf7TarXENSPHUhDVwQQWg62I9ZrhCO1vAw671l7N4 /eS2eT8dgp89uMzTx4nWuKciTpWJT5FvTc6Y/axemrt4bm7KRfcKcSIDraC33CT/R3ClBjL5FC1 MLfscQpNlGGpsnt4gVaB422nlhos+l/kKwrudtX7ctl8VbBVxbTbY6LKvK886+GrsX/OxnAub04 AxiF8w== X-Google-Smtp-Source: AGHT+IEXJPN8gGhA2HYYDOMJlHA0eNtjpiDq/sKLXwKwTWFhE1ZE5FTpP/Am20/VMhak6uDuJsOrhQ== X-Received: by 2002:a17:903:11d2:b0:215:a60d:bcc9 with SMTP id d9443c01a7336-219e6e89536mr708942765ad.2.1735895609241; Fri, 03 Jan 2025 01:13:29 -0800 (PST) Received: from [10.84.148.23] ([203.208.167.148]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-219dc9cde19sm241412365ad.167.2025.01.03.01.13.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 03 Jan 2025 01:13:27 -0800 (PST) Message-ID: Date: Fri, 3 Jan 2025 17:13:12 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 10/15] riscv: pgtable: move pagetable_dtor() to __tlb_remove_table() Content-Language: en-US To: Kevin Brodsky Cc: peterz@infradead.org, agordeev@linux.ibm.com, palmer@dabbelt.com, tglx@linutronix.de, david@redhat.com, jannh@google.com, hughd@google.com, yuzhao@google.com, willy@infradead.org, muchun.song@linux.dev, vbabka@kernel.org, lorenzo.stoakes@oracle.com, akpm@linux-foundation.org, rientjes@google.com, vishal.moola@gmail.com, arnd@arndb.de, will@kernel.org, aneesh.kumar@kernel.org, npiggin@gmail.com, dave.hansen@linux.intel.com, rppt@kernel.org, ryan.roberts@arm.com, linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org, x86@kernel.org, linux-arch@vger.kernel.org, linux-csky@vger.kernel.org, linux-hexagon@vger.kernel.org, loongarch@lists.linux.dev, linux-m68k@lists.linux-m68k.org, linux-mips@vger.kernel.org, linux-openrisc@vger.kernel.org, linux-sh@vger.kernel.org, linux-um@lists.infradead.org References: <0e8f0b3835c15e99145e0006ac1020ae45a2b166.1735549103.git.zhengqi.arch@bytedance.com> <1b09335c-f0b6-4ccb-9800-5fb22f7e8083@arm.com> <7e2c26c8-f5df-4833-a93f-3409b00b58fd@arm.com> From: Qi Zheng In-Reply-To: <7e2c26c8-f5df-4833-a93f-3409b00b58fd@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 4D24E8001C X-Rspam-User: X-Rspamd-Server: rspam07 X-Stat-Signature: uju1ojfkc618aag81upw8tsmkyu9g64c X-HE-Tag: 1735895512-218173 X-HE-Meta: U2FsdGVkX19QBepEomTK5PGaau/XghGrv+ROBJEOgyjtO2h7n23Pt8bkPECCaejpi6X6on2zc4O9xRBesIx3FxVg2iqLvznHRK+XUSmDetiNPZs7NbqQI7uRYkKMHwVZekfspL4fg0YV/VbpI1q/BjpD1477Z65WHd8iztCqV0KaQDU3smGlxcUp3f+yhPYNI+k2h+LN/dGm1p9BTCAmrTGQRniY+deFgpy06uvi0GiN21AkoSPGkHyVMed/vGw+qlPLNwN08bQt76QnsX17+ylp9DwT5QcAqi3UZQWwvFLm1fPE8iagG1WLNiN3SfroKxOhrkxSS9UAfoqWZkmCApelLiLnYH0QZgZM7Kqox3e2XH06V6oMhoaizxfC2leIVEJPkgHI4N7ht7UUowdEAAUpf/e3NebljWOwc2SvSqynKptrGvnaU9IHMD+NDtlMfWC4R6nXkVDqfDH4EUZcbMiW0/XV/TogIIkPkHLqZKMptz06AqRxbs18InRHSHpX1HcFwIVlnzNGVVJUsW2YR2xurx0tHhYwbxxotTnVg1+B8uFkCf2BUCkizTeDpfmwD1lMc9s+KJ+rR4p5qlWjxe9Vv9edlzB87ogaT70gjwT7VOnEyw0cBShhU8CPGDsW+OBNsn30gzeZ5QVs1zjKVxVZvg0cF42SCWallyiGmAcio5F+ykM+NzLbMm2qsRT6bz5GZung7lg8bvFOIKSAUc9j9QmCrFTT5NccpA5uqj30iyfNctV4XNCz6UsJ9oyYRXTR6ABAaYPu4Bj09GegnQTq5BmrsBbjn6FYzHDh9qULVsdiZwscwDc2KeExhdh5LyEq1WeSE0q+QgBU9QVJ0Xr1m/9u4fGnp0Z6mAB3ikmIY3fd4UQhsAQ2ltuYmdoXKENAeMtJ+HkmtHAucG5IXWTQgvsXYRx0TnR2fupo78bgOertuH9pQcQRMc/n0ubDM816p46Uz0WMc+XdIyl 4eIF5tA7 SY8xEsV5fyScu3DgSg3+TwFl/Xt+ts1IGuIKZ85WcD1AkYNjRDnw5ALONX/OzL7JawyJ4Pc172UUO3c6TWUy5MKJrYGgs26aDL17yVexi4Cqjme4PXDXI37rRVXbNsvvpi3qrAVSo+uZOrI8EbkbKcgKC8+ubWcq/lQ40eVpxMpImQxJdFgGUt08gQ5q3dKnG4libohCQcykEsm0MqZ6OTaYRuw9BjqrIfscf9VFmHDAozvaFRDedkN+G01R1QYLiyYVd3U6cGPtf1Otob1GeMS9rt6jBwng8EoKSTHGNh4pTtqiAZesznAnb1ytT3cidmXokKDX93iA31Zbh3BMJO5Eu8JEC1ac4WWkizdbwStEPFbnz0pvyggNEGkAmT+F/cclAFxx9+mNQxvBdr4C6FszKlqxNpm9PzdIewAM5E91nksyq90/H0eWDHzbMp6Ub7PYejxKspPGtzl96fHSUMQkE0eSWEBuqO+X/ 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: List-Subscribe: List-Unsubscribe: On 2025/1/3 16:02, Kevin Brodsky wrote: > On 03/01/2025 04:48, Qi Zheng wrote: >> Hi Kevin, >> >> On 2025/1/3 00:53, Kevin Brodsky wrote: >>> On 30/12/2024 10:07, Qi Zheng wrote: >>>>   static inline void riscv_tlb_remove_ptdesc(struct mmu_gather *tlb, >>>> void *pt) >>>>   { >>>> -    if (riscv_use_sbi_for_rfence()) >>>> +    if (riscv_use_sbi_for_rfence()) { >>>>           tlb_remove_ptdesc(tlb, pt); >>>> -    else >>>> +    } else { >>>> +        pagetable_dtor(pt); >>>>           tlb_remove_page_ptdesc(tlb, pt); >>> >>> I find the imbalance pretty confusing: pagetable_dtor() is called >>> explicitly before using tlb_remove_page() but not tlb_remove_ptdesc(). >>> Doesn't that assume that CONFIG_MMU_GATHER_HAVE_TABLE_FREE is selected? >>> Could we not call pagetable_dtor() from __tlb_batch_free_encoded_pages() >>> to ensure that the dtor is always called just before freeing, and remove >> >> In __tlb_batch_free_encoded_pages(), we can indeed detect PageTable() >> and call pagetable_dtor() to dtor the page table pages. >> But __tlb_batch_free_encoded_pages() is also used to free normal pages >> (not page table pages), so I don't want to add overhead there. > > Interesting, can a tlb batch refer to pages than are not PTPs then? Yes, you can see the caller of __tlb_remove_folio_pages() or tlb_remove_page_size(). > >> >> But now I think maybe we can do this in tlb_remove_page_ptdesc(), like >> this: >> >> diff --git a/arch/csky/include/asm/pgalloc.h >> b/arch/csky/include/asm/pgalloc.h >> index f1ce5b7b28f22..e45c7e91dcbf9 100644 >> --- a/arch/csky/include/asm/pgalloc.h >> +++ b/arch/csky/include/asm/pgalloc.h >> @@ -63,7 +63,6 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm) >> >>  #define __pte_free_tlb(tlb, pte, address)              \ >>  do {                                                   \ >> -       pagetable_dtor(page_ptdesc(pte));               \ >>         tlb_remove_page_ptdesc(tlb, page_ptdesc(pte));  \ >>  } while (0) >> >> [...] >> >> diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h >> index a96d4b440f3da..a59205863f431 100644 >> --- a/include/asm-generic/tlb.h >> +++ b/include/asm-generic/tlb.h >> @@ -506,6 +506,7 @@ static inline void tlb_remove_ptdesc(struct >> mmu_gather *tlb, void *pt) >>  /* Like tlb_remove_ptdesc, but for page-like page directories. */ >>  static inline void tlb_remove_page_ptdesc(struct mmu_gather *tlb, >> struct ptdesc *pt) >>  { >> +       pagetable_dtor(pt); >>         tlb_remove_page(tlb, ptdesc_page(pt)); >>  } > > I think this is an interesting idea, it does make arch code easier to > follow. OTOH it would have been more natural to me to call > pagetable_dtor() from tlb_remove_page(). I can however see that this > doesn't work, because tlb_remove_table() is defined as tlb_remove_page() > if CONFIG_MMU_GATHER_HAVE_TABLE_FREE isn't selected. Which brings me > back to my earlier question: in that case, aren't we missing a call to > pagetable_dtor() when using tlb_remove_table() (or tlb_remove_ptdesc())? Thank you for pointing this out! Now, there are the following architectures selected CONFIG_MMU_GATHER_RCU_TABLE_FREE: 1. arm: select MMU_GATHER_RCU_TABLE_FREE if SMP && ARM_LPAE 2. arm64: select MMU_GATHER_RCU_TABLE_FREE 3. powerpc: select MMU_GATHER_RCU_TABLE_FREE 4. riscv: select MMU_GATHER_RCU_TABLE_FREE if SMP && MMU 5. s390: select MMU_GATHER_RCU_TABLE_FREE 6. sparc: select MMU_GATHER_RCU_TABLE_FREE if SMP 7. x86: select MMU_GATHER_RCU_TABLE_FREE if PARAVIRT If CONFIG_MMU_GATHER_TABLE_FREE is selected, an architecture is expected to provide __tlb_remove_table(). This patch series modifies the __tlb_remove_table() in arm, arm64, riscv, s390 and x86. Among them, arm64 and s390 unconditionally select MMU_GATHER_RCU_TABLE_FREE, so we only need to double-check arm, riscv and x86. For x86, it was called tlb_remove_page() in the non-PARAVIRT case, and I added pagetable_dtor() for it explicitly (see patch #11), so this should be no problem. For riscv, it will only call tlb_remove_ptdesc() in the case of SMP && MMU, so this should be no problem. For arm, the call to pagetable_dtor() is indeed missed in the non-MMU_GATHER_RCU_TABLE_FREE case. This needs to be fixed. But we can't fix this by adding pagetable_dtor() to tlb_remove_table(), because some architectures call tlb_remove_table() but don't support page table statistics, like sparc. So a more direct fix might be: diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h index a59205863f431..0a131444a18ca 100644 --- a/include/asm-generic/tlb.h +++ b/include/asm-generic/tlb.h @@ -500,6 +500,9 @@ static inline void tlb_remove_page(struct mmu_gather *tlb, struct page *page) static inline void tlb_remove_ptdesc(struct mmu_gather *tlb, void *pt) { +#ifndef CONFIG_MMU_GATHER_TABLE_FREE + pagetable_dtor(pt); +#endif tlb_remove_table(tlb, pt); } Or fix it directly in arm? Like the following: diff --git a/arch/arm/include/asm/tlb.h b/arch/arm/include/asm/tlb.h index ea4fbe7b17f6f..cf5d0ca021440 100644 --- a/arch/arm/include/asm/tlb.h +++ b/arch/arm/include/asm/tlb.h @@ -43,6 +43,9 @@ __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte, unsigned long addr) __tlb_adjust_range(tlb, addr - PAGE_SIZE, 2 * PAGE_SIZE); #endif +#ifndef CONFIG_MMU_GATHER_TABLE_FREE + pagetable_dtor(ptdesc); +#endif tlb_remove_ptdesc(tlb, ptdesc); } Thanks, Qi > > - Kevin