On Wed 27-11-19 18:41:26, David Hildenbrand wrote:
> Since commit d84f2f5a7552 ("drivers/base/node.c: simplify
> unregister_memory_block_under_nodes()") we only have a single user of
> get_nid_for_pfn(). Let's just inline that code and get rid of
> get_nid_for_pfn().
>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
> Cc: Michal Hocko <mhocko@xxxxxxxxxx>
> Cc: Oscar Salvador <osalvador@xxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
I am not really sure this is an improvement. The code is ugly as hell
and open coding it just makes register_mem_sect_under_node harder to
read.
If anything get_nid_for_pfn deserves a comment why
CONFIG_DEFERRED_STRUCT_PAGE_INIT calls for special case as
early_pfn_to_nid is not bound to that config (it is defined when
CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID || CONFIG_HAVE_MEMBLOCK_NODE_MAP
> ---
> drivers/base/node.c | 23 +++++++----------------
> 1 file changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 98a31bafc8a2..735073fd2926 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -744,17 +744,6 @@ int unregister_cpu_under_node(unsigned int cpu, unsigned int nid)
> }
>
> #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> -static int __ref get_nid_for_pfn(unsigned long pfn)
> -{
> - if (!pfn_valid_within(pfn))
> - return -1;
> -#ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
> - if (system_state < SYSTEM_RUNNING)
> - return early_pfn_to_nid(pfn);
> -#endif
> - return pfn_to_nid(pfn);
> -}
> -
> /* register memory section under specified node if it spans that node */
> static int register_mem_sect_under_node(struct memory_block *mem_blk,
> void *arg)
> @@ -766,8 +755,6 @@ static int register_mem_sect_under_node(struct memory_block *mem_blk,
> 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
> @@ -784,11 +771,15 @@ static int register_mem_sect_under_node(struct memory_block *mem_blk,
> * block belong to the same node.
> */
> if (system_state == SYSTEM_BOOTING) {
> - page_nid = get_nid_for_pfn(pfn);
> - if (page_nid < 0)
> + if (!pfn_valid_within(pfn))
> continue;
> - if (page_nid != nid)
> +#ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
> + if (early_pfn_to_nid(pfn) != nid)
> continue;
> +#else
> + if (pfn_to_nid(pfn) != nid)
> + continue;
> +#endif
> }
>
> /*
> --
> 2.21.0
--
Michal Hocko
SUSE Labs
