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 X-Spam-Level: X-Spam-Status: No, score=-16.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2C3AFC433ED for ; Wed, 19 May 2021 07:56:33 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B353361364 for ; Wed, 19 May 2021 07:56:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B353361364 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id C6D1F6B006C; Wed, 19 May 2021 03:56:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C0B2C6B006E; Wed, 19 May 2021 03:56:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 76B846B0070; Wed, 19 May 2021 03:56:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0174.hostedemail.com [216.40.44.174]) by kanga.kvack.org (Postfix) with ESMTP id 312966B006C for ; Wed, 19 May 2021 03:56:31 -0400 (EDT) Received: from smtpin09.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id C2E08A8E0 for ; Wed, 19 May 2021 07:56:30 +0000 (UTC) X-FDA: 78157223340.09.E51E739 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf30.hostedemail.com (Postfix) with ESMTP id D2E80E0001AC for ; Wed, 19 May 2021 07:56:29 +0000 (UTC) Received: by mail.kernel.org (Postfix) with ESMTPSA id 865536135C; Wed, 19 May 2021 07:56:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1621410989; bh=vzuZ6DVeJnJZRawEfq/YK/8biq6FoO9rWVr+hhO2TdI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=nAAggeRznSsTt0ljgd8qH4LEEi5XDSBzp4rVMk5zukA738lq6fv1dNfXf51STG4kr DRx6N2dHDyFgZ1kq7DJwuEVw2TqD3WxVdWPpd7oL9yuthkjnrpoxBlyp1qEj4LItbC wp2iDFG8LeD1f8TSWjkLgZB+xFv/lvInts129BGIBDivENl3vICSbXr2TTlJICIw8P jny67z9voNS99WtuBM8iAhByV50kbHYFbpSBLu/fJVgmXLx2m0zyHn1vzkDVIWMvB0 lSiWWFMwXforpcqYzUl2Svrg3vFyNunOQActhh1d3zqd7lVY1J6q/xdsM+/W2zvY1B kUN3P1Mx8GC2A== Date: Wed, 19 May 2021 10:56:20 +0300 From: Mike Rapoport To: Miles Chen Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, wsd_upstream@mediatek.com, Catalin Marinas , Will Deacon , Mark Rutland Subject: Re: [PATCH] mm/sparse: fix check_usemap_section_nr warnings Message-ID: References: <20210511093114.15123-1-miles.chen@mediatek.com> <1621408729.12301.34.camel@mtkswgap22> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1621408729.12301.34.camel@mtkswgap22> Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=nAAggeRz; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf30.hostedemail.com: domain of rppt@kernel.org designates 198.145.29.99 as permitted sender) smtp.mailfrom=rppt@kernel.org X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: D2E80E0001AC X-Stat-Signature: b3yyefniadjfjo58ctbrwki18tfxh8pf X-HE-Tag: 1621410989-259176 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, May 19, 2021 at 03:18:49PM +0800, Miles Chen wrote: > On Wed, 2021-05-19 at 08:30 +0300, Mike Rapoport wrote: > > (add arm64 people) > > > > On Tue, May 11, 2021 at 05:31:14PM +0800, Miles Chen wrote: > > > In current implementation of node_data, if CONFIG_NEED_MULTIPLE_NODES=y, > > > node_data is allocated by kzmalloc. If CONFIG_NEED_MULTIPLE_NODES=n, > > > > The node structures are allocated from memblock rather than kzmalloc, so > > I'd suggest to use "dynamically allocated": > > > > ... node_data is dynamically allocated > > > no problem. "dynamically allocated" is better. > > > > > > we use a global variable named "contig_page_data". > > > > > > If CONFIG_DEBUG_VIRTUAL is not enabled. __pa() can handle both kzalloc and > > > > ^ coma > > > > And here as well, rather then mention kzalloc > > > ok, will fix this in v3 > > > > > ... __pa() can handle both dynamic allocation and symbol cases. > > > > > symbol cases. But if CONFIG_DEBUG_VIRTUAL is set, we will have the > > > "virt_to_phys used for non-linear address" warning when booting. > > > > > > To fix it, create a small function to handle both translation. > > > > More generally, I wonder how other architectures than support DEBUG_VIRTUAL > > cope with this? > > arm64 support DEBUG_VIRTUAL since v4.11. Before v4.11, a driver can get > incorrect PA by using __pa() for non-linear addresses and create > hard-to-debug issues. > > arm32...I remember an old issue in ext4: __pa() is used to convert > non-linear kmap'ed addresses (when CONFIG_HIGHMEM=y). > > There are multiple architectures which support CONFIG_DEBUG_VIRTUAL. > It helps people to catch non-linear addresses at the first place. > > find arch -name 'physaddr.c' > arch/arm/mm/physaddr.c > arch/arm64/mm/physaddr.c > arch/mips/mm/physaddr.c > arch/riscv/mm/physaddr.c > arch/x86/mm/physaddr.c Right, but it seems they check for different things on different architectures... Just to make it clear, I'm Ok with your patch. All I'm saying is that we probably need to revisit DEBUG_VIRTUAL to make it consistent across architectures. > > > > Maybe such lack of consistency between debug and no-debug version of > > arm64::virt_to_phys() is what needs to be fixed at the first place? > > > > > Warning message: > > > [ 0.000000] ------------[ cut here ]------------ > > > [ 0.000000] virt_to_phys used for non-linear address: (____ptrval____) (contig_page_data+0x0/0x1c00) > > > [ 0.000000] WARNING: CPU: 0 PID: 0 at arch/arm64/mm/physaddr.c:15 __virt_to_phys+0x58/0x68 > > > [ 0.000000] Modules linked in: > > > [ 0.000000] CPU: 0 PID: 0 Comm: swapper Tainted: G W 5.13.0-rc1-00074-g1140ab592e2e #3 > > > [ 0.000000] Hardware name: linux,dummy-virt (DT) > > > [ 0.000000] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO BTYPE=--) > > > [ 0.000000] pc : __virt_to_phys+0x58/0x68 > > > [ 0.000000] lr : __virt_to_phys+0x54/0x68 > > > [ 0.000000] sp : ffff800011833e70 > > > [ 0.000000] x29: ffff800011833e70 x28: 00000000418a0018 x27: 0000000000000000 > > > [ 0.000000] x26: 000000000000000a x25: ffff800011b70000 x24: ffff800011b70000 > > > [ 0.000000] x23: fffffc0001c00000 x22: ffff800011b70000 x21: 0000000047ffffb0 > > > [ 0.000000] x20: 0000000000000008 x19: ffff800011b082c0 x18: ffffffffffffffff > > > [ 0.000000] x17: 0000000000000000 x16: ffff800011833bf9 x15: 0000000000000004 > > > [ 0.000000] x14: 0000000000000fff x13: ffff80001186a548 x12: 0000000000000000 > > > [ 0.000000] x11: 0000000000000000 x10: 00000000ffffffff x9 : 0000000000000000 > > > [ 0.000000] x8 : ffff8000115c9000 x7 : 737520737968705f x6 : ffff800011b62ef8 > > > [ 0.000000] x5 : 0000000000000000 x4 : 0000000000000001 x3 : 0000000000000000 > > > [ 0.000000] x2 : 0000000000000000 x1 : ffff80001159585e x0 : 0000000000000058 > > > [ 0.000000] Call trace: > > > [ 0.000000] __virt_to_phys+0x58/0x68 > > > [ 0.000000] check_usemap_section_nr+0x50/0xfc > > > [ 0.000000] sparse_init_nid+0x1ac/0x28c > > > [ 0.000000] sparse_init+0x1c4/0x1e0 > > > [ 0.000000] bootmem_init+0x60/0x90 > > > [ 0.000000] setup_arch+0x184/0x1f0 > > > [ 0.000000] start_kernel+0x78/0x488 > > > [ 0.000000] ---[ end trace f68728a0d3053b60 ]--- > > > > > > Signed-off-by: Miles Chen > > > --- > > > mm/sparse.c | 13 +++++++++++-- > > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > > > diff --git a/mm/sparse.c b/mm/sparse.c > > > index b2ada9dc00cb..55c18aff3e42 100644 > > > --- a/mm/sparse.c > > > +++ b/mm/sparse.c > > > @@ -344,6 +344,15 @@ size_t mem_section_usage_size(void) > > > return sizeof(struct mem_section_usage) + usemap_size(); > > > } > > > > > > +static inline phys_addr_t pgdat_to_phys(struct pglist_data *pgdat) > > > +{ > > > +#ifndef CONFIG_NEED_MULTIPLE_NODES > > > + return __pa_symbol(pgdat); > > > +#else > > > + return __pa(pgdat); > > > +#endif > > > +} > > > + > > > #ifdef CONFIG_MEMORY_HOTREMOVE > > > static struct mem_section_usage * __init > > > sparse_early_usemaps_alloc_pgdat_section(struct pglist_data *pgdat, > > > @@ -362,7 +371,7 @@ sparse_early_usemaps_alloc_pgdat_section(struct pglist_data *pgdat, > > > * from the same section as the pgdat where possible to avoid > > > * this problem. > > > */ > > > - goal = __pa(pgdat) & (PAGE_SECTION_MASK << PAGE_SHIFT); > > > + goal = pgdat_to_phys(pgdat) & (PAGE_SECTION_MASK << PAGE_SHIFT); > > > limit = goal + (1UL << PA_SECTION_SHIFT); > > > nid = early_pfn_to_nid(goal >> PAGE_SHIFT); > > > again: > > > @@ -390,7 +399,7 @@ static void __init check_usemap_section_nr(int nid, > > > } > > > > > > usemap_snr = pfn_to_section_nr(__pa(usage) >> PAGE_SHIFT); > > > - pgdat_snr = pfn_to_section_nr(__pa(pgdat) >> PAGE_SHIFT); > > > + pgdat_snr = pfn_to_section_nr(pgdat_to_phys(pgdat) >> PAGE_SHIFT); > > > if (usemap_snr == pgdat_snr) > > > return; > > > > > > -- > > > 2.18.0 > > > -- Sincerely yours, Mike.