linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mike Rapoport <rppt@kernel.org>
To: Sang-Heon Jeon <ekffu200098@gmail.com>
Cc: akpm@linux-foundation.org, djbw@kernel.org, mingo@kernel.org,
	linux-mm@kvack.org, Donghyeon Lee <asd142513@gmail.com>,
	Munhui Chae <mochae@student.42seoul.kr>
Subject: Re: [RFC PATCH v2] mm/fake-numa: fix under-allocation detection in uniform split
Date: Thu, 16 Apr 2026 21:45:07 +0300	[thread overview]
Message-ID: <aeEuM41NLMr9HGac@kernel.org> (raw)
In-Reply-To: <CABFDxMGYBkZpAeH8zbZsQA3giN9v8dyHPq=TCZf2EZbppMT24w@mail.gmail.com>

On Fri, Apr 17, 2026 at 12:25:54AM +0900, Sang-Heon Jeon wrote:
> On Thu, Apr 16, 2026 at 11:36 PM Mike Rapoport <rppt@kernel.org> wrote:
> >
> > On Thu, Apr 16, 2026 at 07:25:58PM +0900, Sang-Heon Jeon wrote:
> > > When split NUMA node uniformly, split_nodes_size_interleave_uniform()
> > > returns the next absolute node ID, not the number of nodes created.
> > >
> > > The existing under-allocation detection logic compares next absolute node
> > > ID (ret) and request count (n), which only works when nid starts at 0.
> > >
> > > For example, on a system with 2 physical NUMA nodes (node 0: 2GB, node
> > > 1: 128MB) and numa=fake=8U, 8 fake nodes are successfully created from
> > > node 0 and split_nodes_size_interleave_uniform() returns 8. For node 1,
> > > fake node nid starts at 8, but only 4 fake nodes are created due to
> > > current FAKE_NODE_MIN_SIZE being 32MB, and
> > > split_nodes_size_interleave_uniform() returns 12. By existing
> > > under-allocation detection logic, "ret < n" (12 < 8) is false, so the
> > > under-allocation will not be detected.
> > >
> > > Fix under-allocation detection logic to compare the number of actually
> > > created nodes (ret - nid) against the request count (n).
> > >
> > > Also, fix the outdated comment to match the actual return value.
> > >
> > > Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> > > Reported-by: Donghyeon Lee <asd142513@gmail.com>
> > > Reported-by: Munhui Chae <mochae@student.42seoul.kr>
> > > Fixes: cc9aec03e58f ("x86/numa_emulation: Introduce uniform split capability") # 4.19
> > > ---
> > >  mm/numa_emulation.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/mm/numa_emulation.c b/mm/numa_emulation.c
> > > index 703c8fa05048..c1d0a76aef64 100644
> > > --- a/mm/numa_emulation.c
> > > +++ b/mm/numa_emulation.c
> > > @@ -214,7 +214,7 @@ static u64 uniform_size(u64 max_addr, u64 base, u64 hole, int nr_nodes)
> > >   * Sets up fake nodes of `size' interleaved over physical nodes ranging from
> > >   * `addr' to `max_addr'.
> > >   *
> > > - * Returns zero on success or negative on error.
> > > + * Returns absolute node ID on success or negative on error.
> > >   */
> > >  static int __init split_nodes_size_interleave_uniform(struct numa_meminfo *ei,
> > >                                             struct numa_meminfo *pi,
> > > @@ -416,7 +416,7 @@ void __init numa_emulation(struct numa_meminfo *numa_meminfo, int numa_dist_cnt)
> > >                                       n, &pi.blk[0], nid);
> > >                       if (ret < 0)
> > >                               break;
> > > -                     if (ret < n) {
> > > +                     if (ret - nid < n) {
> > >                               pr_info("%s: phys: %d only got %d of %ld nodes, failing\n",
> > >                                               __func__, i, ret, n);
> >
> > The error message also should be updated, now it prints the last node ID
> > rather than number of created nodes. I think it's worse creating a
> > temporary variable for ret - nid to make the code clearer.
> 
> Is "worse" a typo of "worth"?

Yes, my bad :)
 
> And thanks for catching. I totally agree that the error message needs
> to be updated.
> 
> > I'd also recommend running qemu without and with your patch and verifying
> > it works as intended.
> 
> In the next version, I'll try to include qemu based test results.

Thanks!
 
> Best Regards,
> Sang-Heon Jeon

-- 
Sincerely yours,
Mike.


      parent reply	other threads:[~2026-04-16 18:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-16 10:25 Sang-Heon Jeon
2026-04-16 10:36 ` Sang-Heon Jeon
2026-04-16 14:29 ` Mike Rapoport
2026-04-16 15:10   ` Sang-Heon Jeon
2026-04-16 14:36 ` Mike Rapoport
2026-04-16 15:25   ` Sang-Heon Jeon
2026-04-16 18:25     ` Donghyeon Lee
2026-04-16 18:45     ` Mike Rapoport [this message]

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=aeEuM41NLMr9HGac@kernel.org \
    --to=rppt@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=asd142513@gmail.com \
    --cc=djbw@kernel.org \
    --cc=ekffu200098@gmail.com \
    --cc=linux-mm@kvack.org \
    --cc=mingo@kernel.org \
    --cc=mochae@student.42seoul.kr \
    /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