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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AB033CD1284 for ; Tue, 2 Apr 2024 22:44:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=53tLenlJ48QE7daMdgM6i6kju3zHCyN8hZRSzCUP/5s=; b=pD3PZUhdo+8Yyk RO28SIKqbpsb6NmVDb0OOehqnvvqIoQQeh4g8Gp5Sic6q+MgWmsEHgyr0cMIlQD8W/cNhLHJXp0Na yohEymJPzzZJgusRlO6fzfaNMyufGcEJkYlafotrMl40Do+btg9r+NAXOHjLO3cyLqXQmcoS/SGE/ jJeeFOhfYyvLzQsF9g1U6b0PeHqVNGnaJUEXFgu0Chrce6/DTon1/DW4qULX2IA63xEvJ85f/5Hym hxcDwC3UYl8nkRGrYMZ+X4weoP7MPl5C95fzaMuRgoZYAJkzuFe5+W8poxOfJ9BKUq4BUMcCRnFY4 0zcAmYJQNGJFAYKi5LEA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rrmrQ-0000000D3KP-21r7; Tue, 02 Apr 2024 22:44:08 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rrmrN-0000000D3I7-3kEn for linux-arm-kernel@lists.infradead.org; Tue, 02 Apr 2024 22:44:07 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1712097843; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=msm30zqdysTm36DA/rZCqzTJxXxvA+rydv8lKPCKRNo=; b=S9PxDmS7cMP4xVVO6g3JrkBP3FB1ohuU/ePpi1mO0OcndetwgzH2rn7dZrTBVwyg//HdL9 BqBBNxkQWVczr9dKhppoZH4dCphZ0+NFVi04ogbSa6UTEefj5OHhMrFWrm//sDLhwtJema 1PfDFmObfd+ZUbne8wLNEPYpoRTI3V8= Received: from mail-ot1-f71.google.com (mail-ot1-f71.google.com [209.85.210.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-520-BESQqdirM6inhLYmzoCS-Q-1; Tue, 02 Apr 2024 18:44:01 -0400 X-MC-Unique: BESQqdirM6inhLYmzoCS-Q-1 Received: by mail-ot1-f71.google.com with SMTP id 46e09a7af769-6e6b0efc193so2532559a34.0 for ; Tue, 02 Apr 2024 15:44:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712097840; x=1712702640; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=msm30zqdysTm36DA/rZCqzTJxXxvA+rydv8lKPCKRNo=; b=PCDiq7oVsjtZ45/y8JQiq4c60DHDLJvJM5vKAv5phprvZnpd3zk+r1bQPuRtDvjWfI 9rh89Uoopf3R55ooZdlNh/buFdrzNOsSQc+1VPHhIpWaJhiOxAlrgN6rxegvWzka+dz/ /XlrHWApuT8PIeoARMH7MCQ8+PFtlOy7e9LDSrq4HkZipmA1ImAJSuLoLipqumxoNaqc Q+6QAVcF82KDz8vW/U3uDeFcEELoblUHREi1hgr/jxnNi5RvBjHYJ5vRjPv7odHvouVa v4ChgqmZk9euyXFmXlWu2kiDBQ4XxTa7MIsuSohBpLFhYLdUGprptrXjWtIwesbVTgbu iuqA== X-Forwarded-Encrypted: i=1; AJvYcCVxDHKLQUCi2GySz/ZHMxnUMi4eixRT1KLJOv7w5C5Sq7IZONd8zBS7MvwiKSf3S2Cp9XV93PHymoQLy9DPt1Ugw3DhyBAis3BnasLgb1kzw+ymT8A= X-Gm-Message-State: AOJu0YzvGpD/VnB1n4cal+QMFNuRuVBGgTuCh18343X/cleS+VB0h/up 1KAfX35JDlx1TThH10zFWGZ09tYhqWj/VkUMlwogeOV5yiENPo0HVQ7YGtZ7C+lc8ogIN6ntwd7 tIZgFJx2wZVRF4ndHuZfhyt3E4se1YIjv+nCohJ2V3mhLoG3cQtCBJyi4fNSpR6OTe1olB+h2 X-Received: by 2002:a05:6808:15a1:b0:3c4:f2b2:2c20 with SMTP id t33-20020a05680815a100b003c4f2b22c20mr4238901oiw.2.1712097840192; Tue, 02 Apr 2024 15:44:00 -0700 (PDT) X-Google-Smtp-Source: AGHT+IF6a1W8aoV0zchhQZ+4+B4MiM4kTW0SJ53OaThQIv9a42FW90adjoyx8moZXDxiuklchxwa7w== X-Received: by 2002:a05:6808:15a1:b0:3c4:f2b2:2c20 with SMTP id t33-20020a05680815a100b003c4f2b22c20mr4238865oiw.2.1712097839620; Tue, 02 Apr 2024 15:43:59 -0700 (PDT) Received: from x1n ([99.254.121.117]) by smtp.gmail.com with ESMTPSA id u6-20020a05620a120600b00789e8860ef7sm4617106qkj.121.2024.04.02.15.43.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Apr 2024 15:43:59 -0700 (PDT) Date: Tue, 2 Apr 2024 18:43:56 -0400 From: Peter Xu To: Nathan Chancellor Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Yang Shi , "Kirill A . Shutemov" , Mike Kravetz , John Hubbard , Michael Ellerman , Andrew Jones , Muchun Song , linux-riscv@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, Christophe Leroy , Andrew Morton , Christoph Hellwig , Lorenzo Stoakes , Matthew Wilcox , Rik van Riel , linux-arm-kernel@lists.infradead.org, Andrea Arcangeli , David Hildenbrand , "Aneesh Kumar K . V" , Vlastimil Babka , James Houghton , Jason Gunthorpe , Mike Rapoport , Axel Rasmussen , Huacai Chen , WANG Xuerui , loongarch@lists.linux.dev Subject: Re: [PATCH v4 05/13] mm/arch: Provide pud_pfn() fallback Message-ID: References: <20240327152332.950956-1-peterx@redhat.com> <20240327152332.950956-6-peterx@redhat.com> <20240402190549.GA706730@dev-arch.thelio-3990X> MIME-Version: 1.0 In-Reply-To: <20240402190549.GA706730@dev-arch.thelio-3990X> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240402_154406_077954_610AE2E0 X-CRM114-Status: GOOD ( 42.38 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Apr 02, 2024 at 12:05:49PM -0700, Nathan Chancellor wrote: > Hi Peter (and LoongArch folks), > > On Wed, Mar 27, 2024 at 11:23:24AM -0400, peterx@redhat.com wrote: > > From: Peter Xu > > > > The comment in the code explains the reasons. We took a different approach > > comparing to pmd_pfn() by providing a fallback function. > > > > Another option is to provide some lower level config options (compare to > > HUGETLB_PAGE or THP) to identify which layer an arch can support for such > > huge mappings. However that can be an overkill. > > > > Cc: Mike Rapoport (IBM) > > Cc: Matthew Wilcox > > Reviewed-by: Jason Gunthorpe > > Signed-off-by: Peter Xu > > --- > > arch/riscv/include/asm/pgtable.h | 1 + > > arch/s390/include/asm/pgtable.h | 1 + > > arch/sparc/include/asm/pgtable_64.h | 1 + > > arch/x86/include/asm/pgtable.h | 1 + > > include/linux/pgtable.h | 10 ++++++++++ > > 5 files changed, 14 insertions(+) > > > > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h > > index 20242402fc11..0ca28cc8e3fa 100644 > > --- a/arch/riscv/include/asm/pgtable.h > > +++ b/arch/riscv/include/asm/pgtable.h > > @@ -646,6 +646,7 @@ static inline unsigned long pmd_pfn(pmd_t pmd) > > > > #define __pud_to_phys(pud) (__page_val_to_pfn(pud_val(pud)) << PAGE_SHIFT) > > > > +#define pud_pfn pud_pfn > > static inline unsigned long pud_pfn(pud_t pud) > > { > > return ((__pud_to_phys(pud) & PUD_MASK) >> PAGE_SHIFT); > > diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h > > index 1a71cb19c089..6cbbe473f680 100644 > > --- a/arch/s390/include/asm/pgtable.h > > +++ b/arch/s390/include/asm/pgtable.h > > @@ -1414,6 +1414,7 @@ static inline unsigned long pud_deref(pud_t pud) > > return (unsigned long)__va(pud_val(pud) & origin_mask); > > } > > > > +#define pud_pfn pud_pfn > > static inline unsigned long pud_pfn(pud_t pud) > > { > > return __pa(pud_deref(pud)) >> PAGE_SHIFT; > > diff --git a/arch/sparc/include/asm/pgtable_64.h b/arch/sparc/include/asm/pgtable_64.h > > index 4d1bafaba942..26efc9bb644a 100644 > > --- a/arch/sparc/include/asm/pgtable_64.h > > +++ b/arch/sparc/include/asm/pgtable_64.h > > @@ -875,6 +875,7 @@ static inline bool pud_leaf(pud_t pud) > > return pte_val(pte) & _PAGE_PMD_HUGE; > > } > > > > +#define pud_pfn pud_pfn > > static inline unsigned long pud_pfn(pud_t pud) > > { > > pte_t pte = __pte(pud_val(pud)); > > diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h > > index cefc7a84f7a4..273f7557218c 100644 > > --- a/arch/x86/include/asm/pgtable.h > > +++ b/arch/x86/include/asm/pgtable.h > > @@ -234,6 +234,7 @@ static inline unsigned long pmd_pfn(pmd_t pmd) > > return (pfn & pmd_pfn_mask(pmd)) >> PAGE_SHIFT; > > } > > > > +#define pud_pfn pud_pfn > > static inline unsigned long pud_pfn(pud_t pud) > > { > > phys_addr_t pfn = pud_val(pud); > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > > index 600e17d03659..75fe309a4e10 100644 > > --- a/include/linux/pgtable.h > > +++ b/include/linux/pgtable.h > > @@ -1817,6 +1817,16 @@ typedef unsigned int pgtbl_mod_mask; > > #define pte_leaf_size(x) PAGE_SIZE > > #endif > > > > +/* > > + * We always define pmd_pfn for all archs as it's used in lots of generic > > + * code. Now it happens too for pud_pfn (and can happen for larger > > + * mappings too in the future; we're not there yet). Instead of defining > > + * it for all archs (like pmd_pfn), provide a fallback. > > + */ > > +#ifndef pud_pfn > > +#define pud_pfn(x) ({ BUILD_BUG(); 0; }) > > +#endif > > + > > /* > > * Some architectures have MMUs that are configurable or selectable at boot > > * time. These lead to variable PTRS_PER_x. For statically allocated arrays it > > -- > > 2.44.0 > > > > This BUILD_BUG() triggers for LoongArch with their defconfig, so it > seems like they need to provide an implementation of pud_pfn()? > > In function 'follow_huge_pud', > inlined from 'follow_pud_mask' at mm/gup.c:1075:10, > inlined from 'follow_p4d_mask' at mm/gup.c:1105:9, > inlined from 'follow_page_mask' at mm/gup.c:1151:10: > include/linux/compiler_types.h:460:45: error: call to '__compiletime_assert_382' declared with attribute error: BUILD_BUG failed > 460 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) > | ^ > include/linux/compiler_types.h:441:25: note: in definition of macro '__compiletime_assert' > 441 | prefix ## suffix(); \ > | ^~~~~~ > include/linux/compiler_types.h:460:9: note: in expansion of macro '_compiletime_assert' > 460 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) > | ^~~~~~~~~~~~~~~~~~~ > include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert' > 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) > | ^~~~~~~~~~~~~~~~~~ > include/linux/build_bug.h:59:21: note: in expansion of macro 'BUILD_BUG_ON_MSG' > 59 | #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed") > | ^~~~~~~~~~~~~~~~ > include/linux/pgtable.h:1887:23: note: in expansion of macro 'BUILD_BUG' > 1887 | #define pud_pfn(x) ({ BUILD_BUG(); 0; }) > | ^~~~~~~~~ > mm/gup.c:679:29: note: in expansion of macro 'pud_pfn' > 679 | unsigned long pfn = pud_pfn(pud); > | ^~~~~~~ I actually tested this without hitting the issue (even though I didn't mention it in the cover letter..). I re-kicked the build test, it turns out my "make alldefconfig" on loongarch will generate a config with both HUGETLB=n && THP=n, while arch/loongarch/configs/loongson3_defconfig has THP=y (which I assume was the one above build used). I didn't further check how "make alldefconfig" generated the config; a bit surprising that it didn't fetch from there. (and it also surprises me that this BUILD_BUG can trigger.. I used to try triggering it elsewhere but failed..) For loongarch the best thing is not compile in follow_huge_pud(), as it doesn't support pud dax, neither does it support pud hugetlb. However again that may require some more CONFIG_* options to declare the level one arch supports on HUGETLB_PAGE. Here maybe the simplest (and it should also cover all the rest archs on similar issues if ever possible to happen) is we remove the BUILD_BUG() and explain why. It should be safe for loongarch too here in this case to not defined it until properly supported. Thanks, ===8<=== >From 585f34aa3d5b12cd2186367b0882d4293f792062 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Tue, 2 Apr 2024 18:31:07 -0400 Subject: [PATCH] fixup! mm/arch: provide pud_pfn() fallback Signed-off-by: Peter Xu --- include/linux/pgtable.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h index fa8f92f6e2d7..0f4b2faa1d71 100644 --- a/include/linux/pgtable.h +++ b/include/linux/pgtable.h @@ -1882,9 +1882,13 @@ typedef unsigned int pgtbl_mod_mask; * code. Now it happens too for pud_pfn (and can happen for larger * mappings too in the future; we're not there yet). Instead of defining * it for all archs (like pmd_pfn), provide a fallback. + * + * Note that returning 0 here means any arch that didn't define this can + * get severely wrong when it hits a real pud leaf. It's arch's + * responsibility to properly define it when a huge pud is possible. */ #ifndef pud_pfn -#define pud_pfn(x) ({ BUILD_BUG(); 0; }) +#define pud_pfn(x) 0 #endif /* -- 2.44.0 -- Peter Xu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel