Re: [PATCH v2 2/2] driver/base: Remove unused functions

[Donet Tom <donettom@linux.ibm.com>]

On 4/29/25 2:51 AM, David Hildenbrand wrote:
> On 28.04.25 19:03, Donet Tom wrote:
>
> Nit: I'd call this patch
>
>     "driver/base: remove register_mem_block_under_node_early()"
>
> And then just naturally remove get_nid_for_pfn() with it, as it is the 
> last user and it resides in the same file.

Sure, I will change it.


>
>> The functions register_mem_block_under_node_early and get_nid_for_pfn
>> are not used, as register_memory_blocks_under_node_early is now used
>> to register memory blocks during early boot. Therefore, these unused
>> functions have been removed.
>>
>> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
>> ---
>>   drivers/base/node.c | 54 +--------------------------------------------
>>   1 file changed, 1 insertion(+), 53 deletions(-)
>>
>> diff --git a/drivers/base/node.c b/drivers/base/node.c
>> index 4869333d366d..59ec507fc97d 100644
>> --- a/drivers/base/node.c
>> +++ b/drivers/base/node.c
>> @@ -748,15 +748,6 @@ int unregister_cpu_under_node(unsigned int cpu, 
>> unsigned int nid)
>>   }
>>     #ifdef CONFIG_MEMORY_HOTPLUG
>> -static int __ref get_nid_for_pfn(unsigned long pfn)
>> -{
>> -#ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
>> -    if (system_state < SYSTEM_RUNNING)
>> -        return early_pfn_to_nid(pfn);
>> -#endif
>> -    return pfn_to_nid(pfn);
>> -}
>> -
>>   static void do_register_memory_block_under_node(int nid,
>>                           struct memory_block *mem_blk,
>>                           enum meminit_context context)
>> @@ -783,46 +774,6 @@ static void 
>> do_register_memory_block_under_node(int nid,
>>                       ret);
>>   }
>>   -/* register memory section under specified node if it spans that 
>> node */
>> -static int register_mem_block_under_node_early(struct memory_block 
>> *mem_blk,
>> -                           void *arg)
>> -{
>> -    unsigned long memory_block_pfns = memory_block_size_bytes() / 
>> PAGE_SIZE;
>> -    unsigned long start_pfn = 
>> section_nr_to_pfn(mem_blk->start_section_nr);
>> -    unsigned long end_pfn = start_pfn + memory_block_pfns - 1;
>> -    int nid = *(int *)arg;
>> -    unsigned long pfn;
>> -
>> -    for (pfn = start_pfn; pfn <= end_pfn; pfn++) {
>> -        int page_nid;
>> -
>> -        /*
>> -         * memory block could have several absent sections from start.
>> -         * skip pfn range from absent section
>> -         */
>> -        if (!pfn_in_present_section(pfn)) {
>> -            pfn = round_down(pfn + PAGES_PER_SECTION,
>> -                     PAGES_PER_SECTION) - 1;
>> -            continue;
>> -        }
>> -
>> -        /*
>> -         * We need to check if page belongs to nid only at the boot
>> -         * case because node's ranges can be interleaved.
>> -         */
>> -        page_nid = get_nid_for_pfn(pfn);
>> -        if (page_nid < 0)
>> -            continue;
>> -        if (page_nid != nid)
>> -            continue;
>> -
>> -        do_register_memory_block_under_node(nid, mem_blk, 
>> MEMINIT_EARLY);
>> -        return 0;
>> -    }
>> -    /* mem section does not span the specified node */
>> -    return 0;
>> -}
>> -
>>   /*
>>    * During hotplug we know that all pages in the memory block belong 
>> to the same
>>    * node.
>> @@ -895,10 +846,7 @@ void register_memory_blocks_under_node(int nid, 
>> unsigned long start_pfn,
>>   {
>>       walk_memory_blocks_func_t func;
>>   -    if (context == MEMINIT_HOTPLUG)
>> -        func = register_mem_block_under_node_hotplug;
>> -    else
>> -        func = register_mem_block_under_node_early;
>> +    func = register_mem_block_under_node_hotplug;
>>         walk_memory_blocks(PFN_PHYS(start_pfn), PFN_PHYS(end_pfn - 
>> start_pfn),
>>                  (void *)&nid, func);
>
> You can pass func directly here and avoid the temporary variable.

ok. I will change it.

>
> Very nice!
>
> Acked-by: David Hildenbrand <david@redhat.com>

Thanks David

I will address the review comments and submit the next version.