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 D2389C433EF for ; Wed, 23 Mar 2022 19:06:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F24636B0085; Wed, 23 Mar 2022 15:06:41 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id ED5516B0087; Wed, 23 Mar 2022 15:06:41 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D9BBF8D0001; Wed, 23 Mar 2022 15:06:41 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0051.hostedemail.com [216.40.44.51]) by kanga.kvack.org (Postfix) with ESMTP id C88016B0085 for ; Wed, 23 Mar 2022 15:06:41 -0400 (EDT) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 7EB979F870 for ; Wed, 23 Mar 2022 19:06:41 +0000 (UTC) X-FDA: 79276582602.27.0DB274B Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf30.hostedemail.com (Postfix) with ESMTP id DC4FF8000C for ; Wed, 23 Mar 2022 19:06:40 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id BDBFC614FC; Wed, 23 Mar 2022 19:06:39 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9E151C340E8; Wed, 23 Mar 2022 19:06:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1648062399; bh=JTywm6TBkPk9Sn2XVlScT+GDNPgFoMES3sgO/uLu1sU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=f87Yr9sgcYpaiRt4fOOBg9OZElxAtXcrdoJCKtRWi4XUxlSmB7o0SVf4LPzUieoCh jeQ9sGfp4Z/E9Qv2px2x/uW/GOzddxodJxdpuQbee1MwaV9o4nZsFKa2vUkhB/AwVB CbQsAUzA2kKE0uAQb+kiCD47voGzPVNJmtTR4pJA18Hwpk3CDx28Pz3GPU7L199xW1 RXNVxB9pt46ehcobG6p8cK5jB78yvMxwR3S+hdCV0UXa7b1dBRg9PPIpHxP4o0mAIz 8wJzf7VThlUfbSyLx09fkmQHY3aOBmg+2AEQBZQRPeKyz9uHhdDAHOEjqv8dNd/xRZ 5JtwvHvcD3/Rw== Date: Wed, 23 Mar 2022 21:06:30 +0200 From: Mike Rapoport To: Catalin Marinas Cc: Ariel Marcovitch , Christophe Leroy , akpm@linux-foundation.org, mpe@ellerman.id.au, benh@kernel.crashing.org, paulus@samba.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org Subject: Re: False positive kmemleak report for dtb properties names on powerpc Message-ID: References: <9dd08bb5-f39e-53d8-f88d-bec598a08c93@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: X-Stat-Signature: z3x6pgxmr3t78ou6ybsetmzin5h86wut Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=f87Yr9sg; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf30.hostedemail.com: domain of rppt@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=rppt@kernel.org X-Rspam-User: X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: DC4FF8000C X-HE-Tag: 1648062400-52905 Content-Transfer-Encoding: quoted-printable 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: Hi Catalin, On Wed, Mar 23, 2022 at 05:22:38PM +0000, Catalin Marinas wrote: > Hi Ariel, >=20 > On Fri, Feb 18, 2022 at 09:45:51PM +0200, Ariel Marcovitch wrote: > > I was running a powerpc 32bit kernel (built using > > qemu_ppc_mpc8544ds_defconfig > > buildroot config, with enabling DEBUGFS+KMEMLEAK+HIGHMEM in the kerne= l > > config) > > on qemu and invoked the kmemleak scan (twice. for some reason the fir= st time > > wasn't enough). > [...] > > I got 97 leak reports, all similar to the following: > [...] > > memblock_alloc lets kmemleak know about the allocated memory using > > kmemleak_alloc_phys (in mm/memblock.c:memblock_alloc_range_nid()). > >=20 > > The problem is with the following code (mm/kmemleak.c): > >=20 > > ```c > >=20 > > void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min= _count, > > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0 gfp_t gfp) > > { > > =A0=A0=A0=A0=A0=A0=A0 if (!IS_ENABLED(CONFIG_HIGHMEM) || PHYS_PFN(phy= s) < max_low_pfn) > > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 kmemleak_alloc(__va(phy= s), size, min_count, gfp); > > } > >=20 > > ``` > >=20 > > When CONFIG_HIGHMEM is enabled, the pfn of the allocated memory is ch= ecked > > against max_low_pfn, to make sure it is not in the HIGHMEM zone. > >=20 > > However, when called through unflatten_device_tree(), max_low_pfn is = not yet > > initialized in powerpc. > >=20 > > max_low_pfn is initialized (when NUMA is disabled) in > > arch/powerpc/mm/mem.c:mem_topology_setup() which is called only after > > unflatten_device_tree() is called in the same function (setup_arch())= . > >=20 > > Because max_low_pfn is global it is 0 before initialization, so as fa= r as > > kmemleak_alloc_phys() is concerned, every memory is HIGHMEM (: and th= e > > allocated memory is not tracked by kmemleak, causing references to ob= jects > > allocated later with kmalloc() to be ignored and these objects are ma= rked as > > leaked. >=20 > Thanks for digging into this. It looks like the logic doesn't work (not > sure whether it ever worked). >=20 > > I actually tried to find out whether this happen on other arches as w= ell, > > and it seems like arm64 also have this problem when dtb is used inste= ad of > > acpi, although I haven't had the chance to confirm this. >=20 > arm64 doesn't enable CONFIG_HIGHMEM, so it's not affected. >=20 > > I don't suppose I can just shuffle the calls in setup_arch() around, = so I > > wanted to hear your opinions first >=20 > I think it's better if we change the logic than shuffling the calls. > IIUC MEMBLOCK_ALLOC_ACCESSIBLE means that __va() works on the phys > address return by memblock, so something like below (untested): MEMBLOCK_ALLOC_ACCESSIBLE means "anywhere", see commit e63075a3c937 ("memblock: Introduce default allocation limit and use it to replace explicit ones"), so it won't help to detect high memory. If I remember correctly, ppc initializes memblock *very* early, so settin= g max_low_pfn along with lowmem_end_addr in arch/powerpc/mm/init_32::MMU_init() makes sense to me. Maybe ppc folks have other ideas... I've added Christophe who works on ppc32 these days. =20 > -------------8<---------------------- > diff --git a/mm/kmemleak.c b/mm/kmemleak.c > index 7580baa76af1..f3599e857c13 100644 > --- a/mm/kmemleak.c > +++ b/mm/kmemleak.c > @@ -1127,8 +1127,7 @@ EXPORT_SYMBOL(kmemleak_no_scan); > void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min_= count, > gfp_t gfp) > { > - if (!IS_ENABLED(CONFIG_HIGHMEM) || PHYS_PFN(phys) < max_low_pfn) > - kmemleak_alloc(__va(phys), size, min_count, gfp); > + kmemleak_alloc(__va(phys), size, min_count, gfp); > } > EXPORT_SYMBOL(kmemleak_alloc_phys); > =20 > diff --git a/mm/memblock.c b/mm/memblock.c > index b12a364f2766..2515bd4331e8 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -1397,7 +1397,7 @@ phys_addr_t __init memblock_alloc_range_nid(phys_= addr_t size, > * Skip kmemleak for those places like kasan_init() and > * early_pgtable_alloc() due to high volume. > */ > - if (end !=3D MEMBLOCK_ALLOC_NOLEAKTRACE) > + if (end =3D=3D MEMBLOCK_ALLOC_ACCESSIBLE) This change would enable kmemleak for KASAN on arm and arm64 that AFAIR caused OOM in kmemleak and it also will limit tracking only to allocation= s that do not specify 'end' explicitly ;-)=20 > /* > * The min_count is set to 0 so that memblock allocated > * blocks are never reported as leaks. This is because many > -------------8<---------------------- >=20 > But I'm not sure whether we'd now miss some genuine allocations where > the memblock limit is different from MEMBLOCK_ALLOC_ACCESSIBLE but stil= l > within the lowmem limit. If the above works, we can probably get rid of > some other kmemleak callbacks in the kernel. >=20 > Adding Mike for any input on memblock. >=20 > --=20 > Catalin --=20 Sincerely yours, Mike.