linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Bernardo C. Gutierrez Cantu" <bercantu@amazon.de>
To: <akpm@linux-foundation.org>
Cc: <bercantu@amazon.de>, <dwmw2@infradead.org>,
	<linux-kernel@vger.kernel.org>, <linux-mm@kvack.org>,
	<lkp@intel.com>, <rppt@kernel.org>, <yajun.deng@linux.dev>
Subject: Re: [PATCH] mm: memblock: Fix arguments passed to memblock_set_node()
Date: Mon, 28 Apr 2025 09:13:33 +0000	[thread overview]
Message-ID: <20250428091333.35120-1-bercantu@amazon.de> (raw)
In-Reply-To: <20250425180539.2b780a8b3d0958fcc2e8a500@linux-foundation.org>

On Sat, 26 Apr 2025 03:05:39 +0200 Andrew Morton <akpm@linux-foundation.org> wrote:
> On Fri, 25 Apr 2025 10:20:03 +0000 "Bernardo C. Gutierrez Cantu" <bercantu@amazon.de> wrote:
> 
> > memblock_set_node() receives a `base` and a `size` arguments, but we are
> > passing the `start` and `end` of the memory regions when iterating over
> > them in memmap_init_reserved_pages() to set their node ids.
> > 
> > This results in the function setting the node ids for the reserved memory
> > regions in `[base, base + base + size)` instead of `[base, base + size)`.
> > 
> > Pass `start` and `size`, so that we iterate over the correct range.
> > 
> > Fixes: 61167ad5fecd ("mm: pass nid to reserve_bootmem_region()")
> > 
> > ...
> >
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -2196,7 +2196,7 @@ static void __init memmap_init_reserved_pages(void)
> >  		if (memblock_is_nomap(region))
> >  			reserve_bootmem_region(start, end, nid);
> >  
> > -		memblock_set_node(start, end, &memblock.reserved, nid);
> > +		memblock_set_node(start, region->size, &memblock.reserved, nid);
> >  	}
> >  
> >  	/*
> 
> What were the runtime effects of this bug?

I found the bug via code introspection while trying to learn how the boot time
memory allocation of Linux worked. I was not actively pursuing to fix a runtime
issue that I could then prove was fixed by this.

But I see that in most cases this bug should be mostly benign (which could
explain why it was not caught before). In kernels compiled with CONFIG_NUMA, the
memblock_set_node() function would iterate over more memblock regions and
potentially set incorrect node ids, which would then be overwritten with the
correct node ids once the caller function memmap_init_reserved_pages() iterates
over the next regions. So the resultant node ids after initialization should
still be correct.

It could still impact the length of the mm init process on kernel boot, but I
did not measure the improvement, as it is highly dependent on the memory
configuration system, and in my specific system under test, it was neglible.



Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597



  reply	other threads:[~2025-04-28  9:14 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-25 10:02 [PATCH v4] mm: pass nid to reserve_bootmem_region() Gutierrez Cantu, Bernardo
2025-04-25 10:20 ` [PATCH] mm: memblock: Fix arguments passed to memblock_set_node() Bernardo C. Gutierrez Cantu
2025-04-25 14:18   ` David Woodhouse
2025-04-25 14:37     ` Jiaxun Yang
2025-04-26  1:05   ` Andrew Morton
2025-04-28  9:13     ` Bernardo C. Gutierrez Cantu [this message]
2025-04-26  8:22   ` Mike Rapoport
2025-04-28  9:53     ` Bernardo C. Gutierrez Cantu

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=20250428091333.35120-1-bercantu@amazon.de \
    --to=bercantu@amazon.de \
    --cc=akpm@linux-foundation.org \
    --cc=dwmw2@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lkp@intel.com \
    --cc=rppt@kernel.org \
    --cc=yajun.deng@linux.dev \
    /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