From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 61B812737E0 for ; Mon, 1 Jun 2026 05:02:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780290129; cv=none; b=f1MWSCdJ2mE3AP0fZSzpbJLoG1ghC4tuhVCGKhI5HW2ou8+icPv0kiqCR3lemzDj9PycedT8+bq5h17jR+fmV3EoXr0xkQcpxlATnbcbLKBJYvsvIn0OSLtBQCVL/y1Q5xNag3/84QPCdj9GdytalHZuGCwekmuU+NHhcrBk/5Y= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780290129; c=relaxed/simple; bh=eLhjNs220y5uHRNrMiXUfKBzDSiUq0OzoNZWUdU02Vw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=eIHsPuA/pLAgyWwpcUHMVC9Ic49IoBkcHw6Udwc37L17tlI4lDW8cku8cXjrCTDlLD34Va9Z3KsZv7AD/GzfZ45R/SnNwxdxB1tuVhKrzvFUH5WATlzZYcBI2SQBIG7bZSlGfUdQlo1cCeE2mfeJPo4VNC76oKQaQfy/3/1Iz8w= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=MTu7+33H; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="MTu7+33H" Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 52F1E1E8D; Sun, 31 May 2026 22:02:01 -0700 (PDT) Received: from [10.164.18.45] (unknown [10.164.18.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4F0283F93E; Sun, 31 May 2026 22:02:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1780290126; bh=eLhjNs220y5uHRNrMiXUfKBzDSiUq0OzoNZWUdU02Vw=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=MTu7+33HfwoK52MCslYpG45Jblw0DwK/LDvbi0t4eOltNGt78kgVEvlJRGfrJBx/W gREOZmYytTMWcgIDztMaljJNvbxuLR+2oyvy65dX1Exw5c7S3L22pTBevM+cEJqcTO c02jMugrIxXuWmnKfxvueYmYWbDyni8NrIDmIew0= Message-ID: <944a2ca8-c3de-4053-97cc-34e9b4f08b77@arm.com> Date: Mon, 1 Jun 2026 10:31:59 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC V2 07/14] arm64/mm: Route all pgtable reads via pxxval_get() To: Ryan Roberts , linux-arm-kernel@lists.infradead.org Cc: Catalin Marinas , Will Deacon , Mark Rutland , Lorenzo Stoakes , Andrew Morton , David Hildenbrand , Mike Rapoport , Linu Cherian , Usama Arif , linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20260513044547.4128549-1-anshuman.khandual@arm.com> <20260513044547.4128549-8-anshuman.khandual@arm.com> Content-Language: en-US From: Anshuman Khandual In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 27/05/26 7:41 PM, Ryan Roberts wrote: > On 13/05/2026 05:45, Anshuman Khandual wrote: >> Define arm64 platform specific implementations for new pXdp_get() helpers. >> These resolve into READ_ONCE(), thus ensuring required single copy atomic >> semantics for the page table entry reads. >> >> In future this infrastructure can be used for D128 to maintain single copy >> atomicity semantics with inline asm blocks. >> >> Cc: Catalin Marinas >> Cc: Will Deacon >> Cc: Ryan Roberts >> Cc: Mark Rutland >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual >> --- >> Changes in RFC V2: >> >> - Renamed all ptdesc_ instances as pxxval_ instead >> - Moved arm64 pgtable header READ_ONCE() replacements here in this patch >> >> arch/arm64/include/asm/pgtable.h | 38 +++++++++++++++++++++++++++----- >> 1 file changed, 32 insertions(+), 6 deletions(-) >> >> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h >> index cefe8ab86acd..72da582e8d12 100644 >> --- a/arch/arm64/include/asm/pgtable.h >> +++ b/arch/arm64/include/asm/pgtable.h >> @@ -84,6 +84,32 @@ static inline void arch_leave_lazy_mmu_mode(void) >> arch_flush_lazy_mmu_mode(); >> } >> >> +#define pxxval_get(x) READ_ONCE(x) > > I think this would be better as an inline (or __force_inline) function, which > takes a pointer, and dereferences it. READ_ONCE() is special - it's modifying > how the dereference should be done, so it makes sense that the dereference is > done in the caller. > >> + >> +#define pmdp_get pmdp_get >> +static inline pmd_t pmdp_get(pmd_t *pmdp) >> +{ >> + return pxxval_get(*pmdp); > > This is weird to read. It would be clearer as: > > return pxxval_get(pmdp); But being a macro enables pxxval_get() to be used for all page table levels as the dereference is done in the callers. Otherwise we would need separate inline functions which take the corresponding pointers for each pgtable level. Just wondering - would that be any better ? Although pxxval_get(*pmdp) format does look bit weird to read. > > Thanks, > Ryan > >> +} >> + >> +#define pudp_get pudp_get >> +static inline pud_t pudp_get(pud_t *pudp) >> +{ >> + return pxxval_get(*pudp); >> +} >> + >> +#define p4dp_get p4dp_get >> +static inline p4d_t p4dp_get(p4d_t *p4dp) >> +{ >> + return pxxval_get(*p4dp); >> +} >> + >> +#define pgdp_get pgdp_get >> +static inline pgd_t pgdp_get(pgd_t *pgdp) >> +{ >> + return pxxval_get(*pgdp); >> +} >> + >> #ifdef CONFIG_TRANSPARENT_HUGEPAGE >> #define __HAVE_ARCH_FLUSH_PMD_TLB_RANGE >> >> @@ -380,7 +406,7 @@ static inline void __set_pte(pte_t *ptep, pte_t pte) >> >> static inline pte_t __ptep_get(pte_t *ptep) >> { >> - return READ_ONCE(*ptep); >> + return pxxval_get(*ptep); >> } >> >> extern void __sync_icache_dcache(pte_t pteval); >> @@ -1011,7 +1037,7 @@ static inline phys_addr_t pud_offset_phys(p4d_t *p4dp, unsigned long addr) >> { >> BUG_ON(!pgtable_l4_enabled()); >> >> - return p4d_page_paddr(READ_ONCE(*p4dp)) + pud_index(addr) * sizeof(pud_t); >> + return p4d_page_paddr(p4dp_get(p4dp)) + pud_index(addr) * sizeof(pud_t); >> } >> >> static inline >> @@ -1025,7 +1051,7 @@ pud_t *pud_offset_lockless(p4d_t *p4dp, p4d_t p4d, unsigned long addr) >> >> static inline pud_t *pud_offset(p4d_t *p4dp, unsigned long addr) >> { >> - return pud_offset_lockless(p4dp, READ_ONCE(*p4dp), addr); >> + return pud_offset_lockless(p4dp, p4dp_get(p4dp), addr); >> } >> #define pud_offset pud_offset >> >> @@ -1134,7 +1160,7 @@ static inline phys_addr_t p4d_offset_phys(pgd_t *pgdp, unsigned long addr) >> { >> BUG_ON(!pgtable_l5_enabled()); >> >> - return pgd_page_paddr(READ_ONCE(*pgdp)) + p4d_index(addr) * sizeof(p4d_t); >> + return pgd_page_paddr(pgdp_get(pgdp)) + p4d_index(addr) * sizeof(p4d_t); >> } >> >> static inline >> @@ -1148,7 +1174,7 @@ p4d_t *p4d_offset_lockless(pgd_t *pgdp, pgd_t pgd, unsigned long addr) >> >> static inline p4d_t *p4d_offset(pgd_t *pgdp, unsigned long addr) >> { >> - return p4d_offset_lockless(pgdp, READ_ONCE(*pgdp), addr); >> + return p4d_offset_lockless(pgdp, pgdp_get(pgdp), addr); >> } >> >> static inline p4d_t *p4d_set_fixmap(unsigned long addr) >> @@ -1346,7 +1372,7 @@ static inline bool pmdp_test_and_clear_young(struct vm_area_struct *vma, >> unsigned long address, pmd_t *pmdp) >> { >> /* Operation applies to PMD table entry only if FEAT_HAFT is enabled */ >> - VM_WARN_ON(pmd_table(READ_ONCE(*pmdp)) && !system_supports_haft()); >> + VM_WARN_ON(pmd_table(pmdp_get(pmdp)) && !system_supports_haft()); >> return __ptep_test_and_clear_young(vma, address, (pte_t *)pmdp); >> } >> #endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_ARCH_HAS_NONLEAF_PMD_YOUNG */ >