linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
	Ariel Marcovitch <arielmarcovitch@gmail.com>,
	Mike Rapoport <rppt@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>
Cc: "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"benh@kernel.crashing.org" <benh@kernel.crashing.org>,
	"paulus@samba.org" <paulus@samba.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: False positive kmemleak report for dtb properties names on powerpc
Date: Tue, 12 Apr 2022 16:47:47 +1000	[thread overview]
Message-ID: <87pmlm6bn0.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <a9d13878-7820-d41c-9357-135094c8357f@csgroup.eu>

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Hi Ariel
>
> Le 09/04/2022 à 15:47, Ariel Marcovitch a écrit :
>> Hi Christophe, did you get the chance to look at this?
>
> I tested something this morning, it works for me, see below
>
>> 
>> On 23/03/2022 21:06, Mike Rapoport wrote:
>>> Hi Catalin,
>>>
>>> On Wed, Mar 23, 2022 at 05:22:38PM +0000, Catalin Marinas wrote:
>>>> Hi Ariel,
>>>>
>>>> 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 kernel
>>>>> config)
>
> ...
>
>>>>> I don't suppose I can just shuffle the calls in setup_arch() around, 
>>>>> so I
>>>>> wanted to hear your opinions first
>>>> 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 
>>> setting
>>> 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.
>
> I think memblock is already available at the end of MMU_init() on PPC32 
> and at the end of early_setup() on PPC64. It means it is ready when we 
> enter setup_arch().
>
> I tested the change below, it works for me, I don't get any kmemleak 
> report anymore.
>
> diff --git a/arch/powerpc/kernel/setup-common.c 
> b/arch/powerpc/kernel/setup-common.c
> index 518ae5aa9410..9f4e50b176c9 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -840,6 +840,9 @@ void __init setup_arch(char **cmdline_p)
>   	/* Set a half-reasonable default so udelay does something sensible */
>   	loops_per_jiffy = 500000000 / HZ;
>
> +	/* Parse memory topology */
> +	mem_topology_setup();
> +
>   	/* Unflatten the device-tree passed by prom_init or kexec */
>   	unflatten_device_tree();

The 64-bit/NUMA version of mem_topology_setup() requires the device tree
to be unflattened, so I don't think that can work.

Setting max_low_pfn etc in MMU_init() as Mike suggested seems more
likely to work.

But we might need to set it again in mem_topology_setup() though, so
that things that change memblock_end_of_DRAM() are reflected, eg. memory
limit or crash dump?

cheers


  reply	other threads:[~2022-04-12  6:47 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-18 19:45 Ariel Marcovitch
2022-03-23 17:22 ` Catalin Marinas
2022-03-23 19:06   ` Mike Rapoport
2022-04-09 13:47     ` Ariel Marcovitch
2022-04-11  9:10       ` Christophe Leroy
2022-04-12  6:47         ` Michael Ellerman [this message]
2022-04-12 17:56           ` Mike Rapoport
2022-02-24 22:27 Ariel Marcovitch
2022-03-18 19:44 ` Ariel Marcovitch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87pmlm6bn0.fsf@mpe.ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=akpm@linux-foundation.org \
    --cc=arielmarcovitch@gmail.com \
    --cc=benh@kernel.crashing.org \
    --cc=catalin.marinas@arm.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=paulus@samba.org \
    --cc=rppt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox