Re: [PATCH 1/1] liveupdate: kho: calculate per-node scratch sizes before allocation

[Sourabh Jain <sourabhjain@linux.ibm.com>]

Hello George,

On 12/09/26 12:03, Sourabh Jain wrote:
> Hello George,
>
> On 04/09/26 08:21, George Guo wrote:
>> From: George Guo <guodongtai@kylinos.cn>
>>
>> The default percentage-based policy sizes scratch areas from the current
>> kernel's MEMBLOCK_RSRV_KERN footprint. This is a reasonable heuristic 
>> for
>> predicting the early memory demand of the next kernel.
>>
>> scratch_size_update() calculates the lowmem and global sizes before 
>> either
>> area is allocated. However, kho_reserve_scratch() calculates each 
>> per-node
>> size only after allocating the lowmem and global areas. Since memblock
>> allocations are marked MEMBLOCK_RSRV_KERN, the per-node calculation
>> includes those newly allocated scratch areas and scales them again.
>
> I may be missing something here, but doesn't 
> memblock_reserved_kern_size()
> check the node ID of the region before checking the region flag 
> (MEMBLOCK_RSRV_KERN)?
>
> Code snippet from memblock_reserved_kern_size()
> ```
> if (nid == memblock_get_region_node(r) || !numa_valid_node(nid))
>     if (r->flags & MEMBLOCK_RSRV_KERN)
>         total += size;
> ```
>
> For a valid nid, my understanding is that the global and lowmem scratch
> areas should not be counted because they are allocated with NUMA_NO_NODE
> (-1). So, ideally, these regions should be excluded when calculating the
> reserved memory for a specific node ID.
>
> Based on this, I am not sure that marking the lowmem and global areas as
> MEMBLOCK_RSRV_KERN is what causes the per-node size calculation to be
> inflated. I am looking into the code further to better understand the
> actual cause of the issue that this patch is trying to address.

I added some prints in kho_reserve_scratch() and found that the per-node 
size
calculation is not impacted by the lowmem and global scratch memory 
allocations.

KHO: Before low and global scratch allocations
KHO: low size = 899 KB
KHO: global size = 137 MB
KHO: Per node 2 = 80 MB

KHO: After low and global scratch allocations
KHO: low size = 312195 KB
KHO: global size = 441 MB
KHO: Per node 2 = 80 MB

KHO: After per node allocation
KHO: low size = 394115 KB
KHO: global size = 521 MB
KHO: Per node 2 = 240 MB

I only had one NUMA node (nid=2), and the per-NUMA
allocation before and after the lowmem and global scratch
memory allocations remained the same at 80 MB.

The experiment was done on the PowerPC architecture.

I am wondering how the per-NUMA allocation in your setup is
getting inflated due to the lowmem and global scratch memory
reservations.

Feel free to use the change below to print the similar states
in your setup.

index 9260e601c..b5bd9617b 100644
--- a/kernel/liveupdate/kexec_handover.c
+++ b/kernel/liveupdate/kexec_handover.c
@@ -670,6 +670,24 @@ static phys_addr_t __init scratch_size_node(int nid)
         return round_up(size, SCRATCH_ALIGNMENT_BYTES);
  }

+static void __init scratch_size_print(char *s)
+{
+       int nid;
+       phys_addr_t size;
+
+       pr_info("%s\n", s);
+       size = memblock_reserved_kern_size(ARCH_LOW_ADDRESS_LIMIT, 
NUMA_NO_NODE);
+       pr_info("low size = %llu KB\n", (unsigned long long)(size >> 10));
+
+       size = memblock_reserved_kern_size(MEMBLOCK_ALLOC_ANYWHERE, 
NUMA_NO_NODE);
+       pr_info("global size = %llu MB\n", (unsigned long long)(size >> 
20));
+
+       for_each_node_state(nid, N_MEMORY) {
+               size = scratch_size_node(nid);
+               pr_info("Per node %d = %llu MB\n", nid, (unsigned long 
long)(size >> 20));
+       }
+}
+
  /**
   * kho_reserve_scratch - Reserve a contiguous chunk of memory for kexec
   *
@@ -698,6 +716,7 @@ static void __init kho_reserve_scratch(void)
                 goto err_disable_kho;
         }

+       scratch_size_print("Before low and global scratch allocations");
         /*
          * reserve scratch area in low memory for lowmem allocations in the
          * next kernel
@@ -730,6 +749,8 @@ static void __init kho_reserve_scratch(void)
          * Loop over nodes that have both memory and are online. Skip
          * memoryless nodes, as we can not allocate scratch areas there.
          */
+
+       scratch_size_print("After low and global scratch allocations");
         for_each_node_state(nid, N_MEMORY) {
                 size = scratch_size_node(nid);
                 addr = memblock_alloc_range_nid(size, 
SCRATCH_ALIGNMENT_BYTES,
@@ -744,6 +765,7 @@ static void __init kho_reserve_scratch(void)
                 kho_scratch[i].size = size;
                 i++;
         }
+       scratch_size_print("After per node allocation");

         return;


>
> With that said, I wonder if this fix might be more of a stop-gap 
> solution.
> As mentioned above, since NUMA_NO_NODE (-1) is used for the lowmem and
> global allocations, my understanding is that these areas ideally should
> not be included when calculating the size for a specific node ID.
>
> I could be missing something in my understanding, so I would appreciate
> your thoughts on these observations.
>
> - Sourabh Jain
>
>> Fixes: 3dc92c311498 ("kexec: add Kexec HandOver (KHO) generation 
>> helpers")
>> Reported-by: Kexin Liu <liukexin@kylinos.cn>
>> Co-developed-by: Kexin Liu <liukexin@kylinos.cn>
>> Signed-off-by: Kexin Liu <liukexin@kylinos.cn>
>> Signed-off-by: George Guo <guodongtai@kylinos.cn>
>> ---
>>   kernel/liveupdate/kexec_handover.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/liveupdate/kexec_handover.c 
>> b/kernel/liveupdate/kexec_handover.c
>> index 7c4d86daf86d..39f489a258d9 100644
>> --- a/kernel/liveupdate/kexec_handover.c
>> +++ b/kernel/liveupdate/kexec_handover.c
>> @@ -847,6 +847,17 @@ static void __init kho_reserve_scratch(void)
>>           goto err_disable_kho;
>>       }
>>   +    /*
>> +     * Calculate the per-node sizes before reserving any scratch areas.
>> +     * memblock allocations are marked MEMBLOCK_RSRV_KERN, so 
>> calculating
>> +     * them later would count the lowmem and global scratch areas as 
>> kernel
>> +     * allocations and scale them again.
>> +     */
>> +    i = 2;
>> +    for_each_node_state(nid, N_MEMORY)
>> +        kho_scratch[i++].size = scratch_size_node(nid);
>> +    i = 0;
>> +
>>       /*
>>        * reserve scratch area in low memory for lowmem allocations in 
>> the
>>        * next kernel
>> @@ -880,7 +891,7 @@ static void __init kho_reserve_scratch(void)
>>        * memoryless nodes, as we can not allocate scratch areas there.
>>        */
>>       for_each_node_state(nid, N_MEMORY) {
>> -        size = scratch_size_node(nid);
>> +        size = kho_scratch[i].size;
>>           addr = memblock_alloc_range_nid(size, SCRATCH_ALIGNMENT_BYTES,
>>                           0, MEMBLOCK_ALLOC_ACCESSIBLE,
>>                           nid, true);
>