Re: [PATCH 2/2] mm/migrate: add CPU hotplug to demotion #ifdef

[Wei Xu]

The initialization of node_demotion doesn't have to depend on
CONFIG_MEMORY_HOTPLUG and CONFIG_HOTPLUG_CPU.  While you are at this,
can you replace cpuhp_setup_state() with cpuhp_setup_state_nocalls()
and also call set_migration_target_nodes() directly in
migrate_on_reclaim_init() outside
CONFIG_MEMORY_HOTPLUG/CONFIG_HOTPLUG_CPU?  Thanks.

Wei

On Fri, Sep 17, 2021 at 3:35 PM Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx> wrote:
>
>
> From: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
>
> Once upon a time, the node demotion updates were driven solely by
> memory hotplug events.  But now, there are  handlers for both CPU
> and memory hotplug.
>
> However, the #ifdef around the code checks only memory hotplug.
> A system that has HOTPLUG_CPU=y but MEMORY_HOTPLUG=n would miss
> CPU hotplug events.
>
> Update the #ifdef around the common code.  Add memory and
> CPU-specific #ifdefs for their handlers.  These memory/CPU
> #ifdefs avoid unused function warnings when their Kconfig option
> is off.
>
> Fixes: 884a6e5d1f93 ("mm/migrate: update node demotion order on hotplug events")
> Signed-off-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Cc: "Huang, Ying" <ying.huang@xxxxxxxxx>
> Cc: Michal Hocko <mhocko@xxxxxxxx>
> Cc: Wei Xu <weixugc@xxxxxxxxxx>
> Cc: Oscar Salvador <osalvador@xxxxxxx>
> Cc: David Rientjes <rientjes@xxxxxxxxxx>
> Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
> Cc: David Hildenbrand <david@xxxxxxxxxx>
> Cc: Greg Thelen <gthelen@xxxxxxxxxx>
> Cc: Yang Shi <yang.shi@xxxxxxxxxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> ---
>
>  b/mm/migrate.c |   46 +++++++++++++++++++++++++---------------------
>  1 file changed, 25 insertions(+), 21 deletions(-)
>
> diff -puN mm/migrate.c~add-cpu-hotplug-config mm/migrate.c
> --- a/mm/migrate.c~add-cpu-hotplug-config       2021-09-16 16:14:01.770140593 -0700
> +++ b/mm/migrate.c      2021-09-17 11:30:19.197027668 -0700
> @@ -3066,7 +3066,7 @@ void migrate_vma_finalize(struct migrate
>  EXPORT_SYMBOL(migrate_vma_finalize);
>  #endif /* CONFIG_DEVICE_PRIVATE */
>
> -#if defined(CONFIG_MEMORY_HOTPLUG)
> +#if defined(CONFIG_MEMORY_HOTPLUG) || defined(CONFIG_HOTPLUG_CPU)
>  /* Disable reclaim-based migration. */
>  static void __disable_all_migrate_targets(void)
>  {
> @@ -3248,25 +3248,7 @@ static void set_migration_target_nodes(v
>         put_online_mems();
>  }
>
> -/*
> - * React to hotplug events that might affect the migration targets
> - * like events that online or offline NUMA nodes.
> - *
> - * The ordering is also currently dependent on which nodes have
> - * CPUs.  That means we need CPU on/offline notification too.
> - */
> -static int migration_online_cpu(unsigned int cpu)
> -{
> -       set_migration_target_nodes();
> -       return 0;
> -}
> -
> -static int migration_offline_cpu(unsigned int cpu)
> -{
> -       set_migration_target_nodes();
> -       return 0;
> -}
> -
> +#if defined(CONFIG_MEMORY_HOTPLUG)
>  /*
>   * This leaves migrate-on-reclaim transiently disabled between
>   * the MEM_GOING_OFFLINE and MEM_OFFLINE events.  This runs
> @@ -3313,6 +3295,27 @@ static int __meminit migrate_on_reclaim_
>
>         return notifier_from_errno(0);
>  }
> +#endif /* CONFIG_MEMORY_HOTPLUG */
> +
> +#ifdef CONFIG_HOTPLUG_CPU
> +/*
> + * React to hotplug events that might affect the migration targets
> + * like events that online or offline NUMA nodes.
> + *
> + * The ordering is also currently dependent on which nodes have
> + * CPUs.  That means we need CPU on/offline notification too.
> + */
> +static int migration_online_cpu(unsigned int cpu)
> +{
> +       set_migration_target_nodes();
> +       return 0;
> +}
> +
> +static int migration_offline_cpu(unsigned int cpu)
> +{
> +       set_migration_target_nodes();
> +       return 0;
> +}
>
>  static int __init migrate_on_reclaim_init(void)
>  {
> @@ -3333,4 +3336,5 @@ static int __init migrate_on_reclaim_ini
>         return 0;
>  }
>  late_initcall(migrate_on_reclaim_init);
> -#endif /* CONFIG_MEMORY_HOTPLUG */
> +#endif /* CONFIG_HOTPLUG_CPU */
> +#endif /* CONFIG_MEMORY_HOTPLUG || CONFIG_HOTPLUG_CPU */
> _