[PATCH 0/2] (no cover subject)

[PATCH 0/2] (no cover subject)

[pratyush.brahma]

mm/numa_emulation: Code refactoring to improve modularity and
readability

This series intends to improve the code readability by using
existing macros instead of hardcoded values for size and improves
the modularity by moving the size calculation for emulated blocks
to a separate function.

Signed-off-by: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
---
Pratyush Brahma (2):
      mm/numa_emulation: Refactor NUMA emulation size handling to use kernel macros
      mm/numa_emulation: Move the size calculation in split_nodes_interleave() to a separate function

 mm/numa_emulation.c | 47 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 17 deletions(-)
---
base-commit: 4ff71af020ae59ae2d83b174646fc2ad9fcd4dc4
change-id: 20250926-numa-emu-6e6c27bd8f6f

Best regards,
--  
Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>

[PATCH 1/2] mm/numa_emulation: Refactor NUMA emulation size handling to use kernel macros

[pratyush.brahma]

From: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>

Replace hard‑coded 32 MB constant with SZ_32M and apply ALIGN_DOWN for
clearer calculations and maintainability.  Update the related hash mask
usage accordingly to improve readability.

Signed-off-by: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
---
 mm/numa_emulation.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/mm/numa_emulation.c b/mm/numa_emulation.c
index 703c8fa05048..2a335b3dd46a 100644
--- a/mm/numa_emulation.c
+++ b/mm/numa_emulation.c
@@ -10,8 +10,7 @@
 #include <asm/numa.h>
 #include <acpi/acpi_numa.h>
 
-#define FAKE_NODE_MIN_SIZE	((u64)32 << 20)
-#define FAKE_NODE_MIN_HASH_MASK	(~(FAKE_NODE_MIN_SIZE - 1UL))
+#define FAKE_NODE_MIN_SIZE	SZ_32M
 
 int emu_nid_to_phys[MAX_NUMNODES];
 static char *emu_cmdline __initdata;
@@ -112,10 +111,10 @@ static int __init split_nodes_interleave(struct numa_meminfo *ei,
 	 * Calculate the number of big nodes that can be allocated as a result
 	 * of consolidating the remainder.
 	 */
-	big = ((size & ~FAKE_NODE_MIN_HASH_MASK) * nr_nodes) /
+	big = ((size & (FAKE_NODE_MIN_SIZE - 1UL)) * nr_nodes) /
 		FAKE_NODE_MIN_SIZE;
 
-	size &= FAKE_NODE_MIN_HASH_MASK;
+	size = ALIGN_DOWN(size, FAKE_NODE_MIN_SIZE);
 	if (!size) {
 		pr_err("Not enough memory for each node.  "
 			"NUMA emulation disabled.\n");

-- 
2.34.1

[PATCH 2/2] mm/numa_emulation: Move the size calculation in split_nodes_interleave() to a separate function

[pratyush.brahma]

From: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>

The size calculation in split_nodes_interleave() has several nuances.
Move it to a separate function to improve code modularity and
simplify the readability of split_nodes_interleave().

Signed-off-by: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
---
 mm/numa_emulation.c | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/mm/numa_emulation.c b/mm/numa_emulation.c
index 2a335b3dd46a..882c349c2a0f 100644
--- a/mm/numa_emulation.c
+++ b/mm/numa_emulation.c
@@ -76,6 +76,34 @@ static int __init emu_setup_memblk(struct numa_meminfo *ei,
 	return 0;
 }
 
+static void __init __calc_split_params(u64 addr, u64 max_addr,
+		int nr_nodes, u64 *psize, int *pbig)
+{
+	u64 size, usable_size;
+	int big;
+
+	/* total usable memory (skip holes) */
+	usable_size  = max_addr - addr - mem_hole_size(addr, max_addr);
+
+	/*
+	 * Calculate target node size.  x86_32 freaks on __udivdi3() so do
+	 * the division in ulong number of pages and convert back.
+	 */
+	size = PFN_PHYS((unsigned long)(usable_size >> PAGE_SHIFT) / nr_nodes);
+
+	/*
+	 * Calculate the number of big nodes that can be allocated as a result
+	 * of consolidating the remainder.
+	 */
+	big = ((size & (FAKE_NODE_MIN_SIZE - 1UL)) * nr_nodes) / FAKE_NODE_MIN_SIZE;
+
+	/* Align the base size down to the minimum granularity */
+	size = ALIGN_DOWN(size, FAKE_NODE_MIN_SIZE);
+
+	*psize = size;
+	*pbig  = big;
+}
+
 /*
  * Sets up nr_nodes fake nodes interleaved over physical nodes ranging from addr
  * to max_addr.
@@ -100,21 +128,7 @@ static int __init split_nodes_interleave(struct numa_meminfo *ei,
 		nr_nodes = MAX_NUMNODES;
 	}
 
-	/*
-	 * Calculate target node size.  x86_32 freaks on __udivdi3() so do
-	 * the division in ulong number of pages and convert back.
-	 */
-	size = max_addr - addr - mem_hole_size(addr, max_addr);
-	size = PFN_PHYS((unsigned long)(size >> PAGE_SHIFT) / nr_nodes);
-
-	/*
-	 * Calculate the number of big nodes that can be allocated as a result
-	 * of consolidating the remainder.
-	 */
-	big = ((size & (FAKE_NODE_MIN_SIZE - 1UL)) * nr_nodes) /
-		FAKE_NODE_MIN_SIZE;
-
-	size = ALIGN_DOWN(size, FAKE_NODE_MIN_SIZE);
+	__calc_split_params(addr, max_addr, nr_nodes, &size, &big);
 	if (!size) {
 		pr_err("Not enough memory for each node.  "
 			"NUMA emulation disabled.\n");

-- 
2.34.1

Re: [PATCH 2/2] mm/numa_emulation: Move the size calculation in split_nodes_interleave() to a separate function

[Christophe Leroy]

Le 26/09/2025 à 12:34, pratyush.brahma@oss.qualcomm.com a écrit :
> From: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
> 
> The size calculation in split_nodes_interleave() has several nuances.
> Move it to a separate function to improve code modularity and
> simplify the readability of split_nodes_interleave().
> 
> Signed-off-by: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
> ---
>   mm/numa_emulation.c | 44 +++++++++++++++++++++++++++++---------------
>   1 file changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/mm/numa_emulation.c b/mm/numa_emulation.c
> index 2a335b3dd46a..882c349c2a0f 100644
> --- a/mm/numa_emulation.c
> +++ b/mm/numa_emulation.c
> @@ -76,6 +76,34 @@ static int __init emu_setup_memblk(struct numa_meminfo *ei,
>   	return 0;
>   }
>   
> +static void __init __calc_split_params(u64 addr, u64 max_addr,
> +		int nr_nodes, u64 *psize, int *pbig)
> +{
> +	u64 size, usable_size;
> +	int big;
> +
> +	/* total usable memory (skip holes) */
> +	usable_size  = max_addr - addr - mem_hole_size(addr, max_addr);
> +
> +	/*
> +	 * Calculate target node size.  x86_32 freaks on __udivdi3() so do
> +	 * the division in ulong number of pages and convert back.
> +	 */
> +	size = PFN_PHYS((unsigned long)(usable_size >> PAGE_SHIFT) / nr_nodes);
> +
> +	/*
> +	 * Calculate the number of big nodes that can be allocated as a result
> +	 * of consolidating the remainder.
> +	 */
> +	big = ((size & (FAKE_NODE_MIN_SIZE - 1UL)) * nr_nodes) / FAKE_NODE_MIN_SIZE;
> +
> +	/* Align the base size down to the minimum granularity */
> +	size = ALIGN_DOWN(size, FAKE_NODE_MIN_SIZE);
> +
> +	*psize = size;
> +	*pbig  = big;

Having to return simple type values through pointers is usually the 
start of proplems.Whenever possible you shouldn't returning simple types 
via pointers.

Your function is void, it could return size instead.

And big seems independant, could be returned by another function.

> +}
> +
>   /*
>    * Sets up nr_nodes fake nodes interleaved over physical nodes ranging from addr
>    * to max_addr.
> @@ -100,21 +128,7 @@ static int __init split_nodes_interleave(struct numa_meminfo *ei,
>   		nr_nodes = MAX_NUMNODES;
>   	}
>   
> -	/*
> -	 * Calculate target node size.  x86_32 freaks on __udivdi3() so do
> -	 * the division in ulong number of pages and convert back.
> -	 */
> -	size = max_addr - addr - mem_hole_size(addr, max_addr);
> -	size = PFN_PHYS((unsigned long)(size >> PAGE_SHIFT) / nr_nodes);
> -
> -	/*
> -	 * Calculate the number of big nodes that can be allocated as a result
> -	 * of consolidating the remainder.
> -	 */
> -	big = ((size & (FAKE_NODE_MIN_SIZE - 1UL)) * nr_nodes) /
> -		FAKE_NODE_MIN_SIZE;
> -
> -	size = ALIGN_DOWN(size, FAKE_NODE_MIN_SIZE);
> +	__calc_split_params(addr, max_addr, nr_nodes, &size, &big);
>   	if (!size) {
>   		pr_err("Not enough memory for each node.  "
>   			"NUMA emulation disabled.\n");
> 

Re: [PATCH 2/2] mm/numa_emulation: Move the size calculation in split_nodes_interleave() to a separate function

[Pratyush Brahma]

[-- Attachment #1: Type: text/plain, Size: 4178 bytes --]

On Fri, Sep 26, 2025 at 7:50 PM Christophe Leroy <
christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 26/09/2025 à 12:34, pratyush.brahma@oss.qualcomm.com a écrit :
> > From: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
> >
> > The size calculation in split_nodes_interleave() has several nuances.
> > Move it to a separate function to improve code modularity and
> > simplify the readability of split_nodes_interleave().
> >
> > Signed-off-by: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
> > ---
> >   mm/numa_emulation.c | 44 +++++++++++++++++++++++++++++---------------
> >   1 file changed, 29 insertions(+), 15 deletions(-)
> >
> > diff --git a/mm/numa_emulation.c b/mm/numa_emulation.c
> > index 2a335b3dd46a..882c349c2a0f 100644
> > --- a/mm/numa_emulation.c
> > +++ b/mm/numa_emulation.c
> > @@ -76,6 +76,34 @@ static int __init emu_setup_memblk(struct
numa_meminfo *ei,
> >       return 0;
> >   }
> >
> > +static void __init __calc_split_params(u64 addr, u64 max_addr,
> > +             int nr_nodes, u64 *psize, int *pbig)
> > +{
> > +     u64 size, usable_size;
> > +     int big;
> > +
> > +     /* total usable memory (skip holes) */
> > +     usable_size  = max_addr - addr - mem_hole_size(addr, max_addr);
> > +
> > +     /*
> > +      * Calculate target node size.  x86_32 freaks on __udivdi3() so do
> > +      * the division in ulong number of pages and convert back.
> > +      */
> > +     size = PFN_PHYS((unsigned long)(usable_size >> PAGE_SHIFT) /
nr_nodes);
> > +
> > +     /*
> > +      * Calculate the number of big nodes that can be allocated as a
result
> > +      * of consolidating the remainder.
> > +      */
> > +     big = ((size & (FAKE_NODE_MIN_SIZE - 1UL)) * nr_nodes) /
FAKE_NODE_MIN_SIZE;
> > +
> > +     /* Align the base size down to the minimum granularity */
> > +     size = ALIGN_DOWN(size, FAKE_NODE_MIN_SIZE);
> > +
> > +     *psize = size;
> > +     *pbig  = big;
>
> Having to return simple type values through pointers is usually the
> start of proplems.Whenever possible you shouldn't returning simple types
> via pointers.
Thanks Christophe for your comments. Can you please help me understand what
kind of
problems can we run into so I can be mindful of this going forward?
>
> Your function is void, it could return size instead.
Sure, it can be done.
>
> And big seems independant, could be returned by another function.
Had included big in this function as it was calculated before we align the
size to
FAKE_NODE_MIN_SIZE. If we move the calculation of big to a separate
function,
it would compute the value after the alignment of size, which would always
render
big as zero, wouldn't it?

And if I move the calculation of big to a separate function which takes in
the precomputed
size value as input and call it within the new helper, then I would still
have to return big
from this new helper, won't I?

Please let me know if I am missing something.
>
> > +}
> > +
> >   /*
> >    * Sets up nr_nodes fake nodes interleaved over physical nodes
ranging from addr
> >    * to max_addr.
> > @@ -100,21 +128,7 @@ static int __init split_nodes_interleave(struct
numa_meminfo *ei,
> >               nr_nodes = MAX_NUMNODES;
> >       }
> >
> > -     /*
> > -      * Calculate target node size.  x86_32 freaks on __udivdi3() so do
> > -      * the division in ulong number of pages and convert back.
> > -      */
> > -     size = max_addr - addr - mem_hole_size(addr, max_addr);
> > -     size = PFN_PHYS((unsigned long)(size >> PAGE_SHIFT) / nr_nodes);
> > -
> > -     /*
> > -      * Calculate the number of big nodes that can be allocated as a
result
> > -      * of consolidating the remainder.
> > -      */
> > -     big = ((size & (FAKE_NODE_MIN_SIZE - 1UL)) * nr_nodes) /
> > -             FAKE_NODE_MIN_SIZE;
> > -
> > -     size = ALIGN_DOWN(size, FAKE_NODE_MIN_SIZE);
> > +     __calc_split_params(addr, max_addr, nr_nodes, &size, &big);
> >       if (!size) {
> >               pr_err("Not enough memory for each node.  "
> >                       "NUMA emulation disabled.\n");
> >
>

[-- Attachment #2: Type: text/html, Size: 5334 bytes --]

Re: [PATCH 2/2] mm/numa_emulation: Move the size calculation in split_nodes_interleave() to a separate function

[Pratyush Brahma]

On Mon, Sep 29, 2025 at 12:46 PM Pratyush Brahma
<pratyush.brahma@oss.qualcomm.com> wrote:
>
>
>
> On Fri, Sep 26, 2025 at 7:50 PM Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> >
> >
> >
> > Le 26/09/2025 à 12:34, pratyush.brahma@oss.qualcomm.com a écrit :
> > > From: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
> > >
> > > The size calculation in split_nodes_interleave() has several nuances.
> > > Move it to a separate function to improve code modularity and
> > > simplify the readability of split_nodes_interleave().
> > >
> > > Signed-off-by: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
> > > ---
> > >   mm/numa_emulation.c | 44 +++++++++++++++++++++++++++++---------------
> > >   1 file changed, 29 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/mm/numa_emulation.c b/mm/numa_emulation.c
> > > index 2a335b3dd46a..882c349c2a0f 100644
> > > --- a/mm/numa_emulation.c
> > > +++ b/mm/numa_emulation.c
> > > @@ -76,6 +76,34 @@ static int __init emu_setup_memblk(struct numa_meminfo *ei,
> > >       return 0;
> > >   }
> > >
> > > +static void __init __calc_split_params(u64 addr, u64 max_addr,
> > > +             int nr_nodes, u64 *psize, int *pbig)
> > > +{
> > > +     u64 size, usable_size;
> > > +     int big;
> > > +
> > > +     /* total usable memory (skip holes) */
> > > +     usable_size  = max_addr - addr - mem_hole_size(addr, max_addr);
> > > +
> > > +     /*
> > > +      * Calculate target node size.  x86_32 freaks on __udivdi3() so do
> > > +      * the division in ulong number of pages and convert back.
> > > +      */
> > > +     size = PFN_PHYS((unsigned long)(usable_size >> PAGE_SHIFT) / nr_nodes);
> > > +
> > > +     /*
> > > +      * Calculate the number of big nodes that can be allocated as a result
> > > +      * of consolidating the remainder.
> > > +      */
> > > +     big = ((size & (FAKE_NODE_MIN_SIZE - 1UL)) * nr_nodes) / FAKE_NODE_MIN_SIZE;
> > > +
> > > +     /* Align the base size down to the minimum granularity */
> > > +     size = ALIGN_DOWN(size, FAKE_NODE_MIN_SIZE);
> > > +
> > > +     *psize = size;
> > > +     *pbig  = big;
> >
> > Having to return simple type values through pointers is usually the
> > start of proplems.Whenever possible you shouldn't returning simple types
> > via pointers.
> Thanks Christophe for your comments. Can you please help me understand what kind of
> problems can we run into so I can be mindful of this going forward?
> >
> > Your function is void, it could return size instead.
> Sure, it can be done.
> >
> > And big seems independant, could be returned by another function.
> Had included big in this function as it was calculated before we align the size to
> FAKE_NODE_MIN_SIZE. If we move the calculation of big to a separate function,
> it would compute the value after the alignment of size, which would always render
> big as zero, wouldn't it?
>
> And if I move the calculation of big to a separate function which takes in the precomputed
> size value as input and call it within the new helper, then I would still have to return big
> from this new helper, won't I?
>
> Please let me know if I am missing something.
Hi Christophe

Can you please help me to understand here?
> >
> > > +}
> > > +
> > >   /*
> > >    * Sets up nr_nodes fake nodes interleaved over physical nodes ranging from addr
> > >    * to max_addr.
> > > @@ -100,21 +128,7 @@ static int __init split_nodes_interleave(struct numa_meminfo *ei,
> > >               nr_nodes = MAX_NUMNODES;
> > >       }
> > >
> > > -     /*
> > > -      * Calculate target node size.  x86_32 freaks on __udivdi3() so do
> > > -      * the division in ulong number of pages and convert back.
> > > -      */
> > > -     size = max_addr - addr - mem_hole_size(addr, max_addr);
> > > -     size = PFN_PHYS((unsigned long)(size >> PAGE_SHIFT) / nr_nodes);
> > > -
> > > -     /*
> > > -      * Calculate the number of big nodes that can be allocated as a result
> > > -      * of consolidating the remainder.
> > > -      */
> > > -     big = ((size & (FAKE_NODE_MIN_SIZE - 1UL)) * nr_nodes) /
> > > -             FAKE_NODE_MIN_SIZE;
> > > -
> > > -     size = ALIGN_DOWN(size, FAKE_NODE_MIN_SIZE);
> > > +     __calc_split_params(addr, max_addr, nr_nodes, &size, &big);
> > >       if (!size) {
> > >               pr_err("Not enough memory for each node.  "
> > >                       "NUMA emulation disabled.\n");
> > >
> >
Thanks and Regards
Pratyush

Re: [PATCH 0/2] (no cover subject)

[Anshuman Khandual]

On 26/09/25 4:04 PM, pratyush.brahma@oss.qualcomm.com wrote:
> mm/numa_emulation: Code refactoring to improve modularity and
> readability

This should have been the cover letter subject line.
> 
> This series intends to improve the code readability by using
> existing macros instead of hardcoded values for size and improves
> the modularity by moving the size calculation for emulated blocks
> to a separate function.
> 
> Signed-off-by: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>

In general, a signed of statement is not required for the cover letter.
> ---
> Pratyush Brahma (2):
>       mm/numa_emulation: Refactor NUMA emulation size handling to use kernel macros
>       mm/numa_emulation: Move the size calculation in split_nodes_interleave() to a separate function
> 
>  mm/numa_emulation.c | 47 ++++++++++++++++++++++++++++++-----------------
>  1 file changed, 30 insertions(+), 17 deletions(-)
> ---
> base-commit: 4ff71af020ae59ae2d83b174646fc2ad9fcd4dc4
> change-id: 20250926-numa-emu-6e6c27bd8f6f
> 
> Best regards,
> --  
> Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>

Ditto.

Seems like portions of this cover letter has been all jumbled up.