[PATCH/RFC] cpuhp code and data usage on UP systems

[PATCH/RFC] cpuhp code and data usage on UP systems

[Nicolas Pitre]

On !SMP systems, we end up with the following array definition:

static struct cpuhp_step cpuhp_ap_states[] = {
        [CPUHP_ONLINE] = {
                .name                   = "online",
                .startup.single         = NULL,
                .teardown.single        = NULL,
        },
};

where CPUHP_ONLINE = 187. That means up to 99.5% of this array is unused 
but still allocated in the kernel's .data section i.e. 3760 bytes.

The same issue exists with cpuhp_bp_states being 1720 bytes.

The goal of this patch is mainly about illustrating the issue and 
reducing .data usage. I have no clear idea when this stuff is really 
needed besides standard CPU hotplug. Not knowing exactly what I'm doing 
here, I made it conditional on CONFIG_SMP for now. There might be a case 
for moving that code to a separate file (cpuhp.c maybe?) and omitting it 
from the kernel when unneeded.

Comments?

 include/linux/cpu.h        |  4 ++++
 include/linux/cpuhotplug.h | 27 +++++++++++++++++++--------
 kernel/cpu.c               | 26 ++++++++++++++++----------
 kernel/power/Kconfig       |  3 +++
 4 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index acf5308fad..46756fb980 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -35,6 +35,8 @@
 
 #include "smpboot.h"
 
+#ifdef CONFIG_CPUHP
+
 /**
  * cpuhp_cpu_state - Per cpu hotplug state storage
  * @state:	The current cpu state
@@ -1036,8 +1038,6 @@ core_initcall(cpu_hotplug_pm_sync_init);
 
 #endif /* CONFIG_PM_SLEEP_SMP */
 
-int __boot_cpu_id;
-
 #endif /* CONFIG_SMP */
 
 /* Boot processor state steps */
@@ -1709,6 +1709,16 @@ device_initcall(cpuhp_sysfs_init);
 #endif
 
 /*
+ * Must be called _AFTER_ setting up the per_cpu areas
+ */
+void __init boot_cpu_state_init(void)
+{
+	per_cpu_ptr(&cpuhp_state, smp_processor_id())->state = CPUHP_ONLINE;
+}
+
+#endif /* CONFIG_CPUHP */
+
+/*
  * cpu_bit_bitmap[] is a special, "compressed" data structure that
  * represents all NR_CPUS bits binary values of 1<<nr.
  *
@@ -1768,6 +1778,10 @@ void init_cpu_online(const struct cpumask *src)
 	cpumask_copy(&__cpu_online_mask, src);
 }
 
+#ifdef CONFIG_SMP
+int __boot_cpu_id;
+#endif
+
 /*
  * Activate the first processor.
  */
@@ -1785,11 +1799,3 @@ void __init boot_cpu_init(void)
 	__boot_cpu_id = cpu;
 #endif
 }
-
-/*
- * Must be called _AFTER_ setting up the per_cpu areas
- */
-void __init boot_cpu_state_init(void)
-{
-	per_cpu_ptr(&cpuhp_state, smp_processor_id())->state = CPUHP_ONLINE;
-}
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index ca73bc1563..4130ede0f9 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -29,7 +29,11 @@ struct cpu {
 };
 
 extern void boot_cpu_init(void);
+#ifdef CONFIG_CPUHP
 extern void boot_cpu_state_init(void);
+#else
+static inline void boot_cpu_state_init(void) { }
+#endif
 extern void cpu_init(void);
 extern void trap_init(void);
 
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index f24bfb2b9a..c51e2980a7 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -150,15 +150,23 @@ enum cpuhp_state {
 	CPUHP_ONLINE,
 };
 
+#ifdef CONFIG_CPUHP
+#define ___P(proto, def_retcode) extern proto;
+#else
+#define ___P(proto, def_retcode) static inline proto { return def_retcode }
+#endif
+
+___P(
 int __cpuhp_setup_state(enum cpuhp_state state,	const char *name, bool invoke,
 			int (*startup)(unsigned int cpu),
-			int (*teardown)(unsigned int cpu), bool multi_instance);
-
+			int (*teardown)(unsigned int cpu), bool multi_instance), 0; )
+___P(
 int __cpuhp_setup_state_cpuslocked(enum cpuhp_state state, const char *name,
 				   bool invoke,
 				   int (*startup)(unsigned int cpu),
 				   int (*teardown)(unsigned int cpu),
-				   bool multi_instance);
+				   bool multi_instance), 0; )
+
 /**
  * cpuhp_setup_state - Setup hotplug state callbacks with calling the callbacks
  * @state:	The state for which the calls are installed
@@ -239,10 +247,12 @@ static inline int cpuhp_setup_state_multi(enum cpuhp_state state,
 				   (void *) teardown, true);
 }
 
+___P(
 int __cpuhp_state_add_instance(enum cpuhp_state state, struct hlist_node *node,
-			       bool invoke);
+			       bool invoke), 0; )
+___P(
 int __cpuhp_state_add_instance_cpuslocked(enum cpuhp_state state,
-					  struct hlist_node *node, bool invoke);
+					  struct hlist_node *node, bool invoke), 0; )
 
 /**
  * cpuhp_state_add_instance - Add an instance for a state and invoke startup
@@ -282,8 +292,8 @@ cpuhp_state_add_instance_nocalls_cpuslocked(enum cpuhp_state state,
 	return __cpuhp_state_add_instance_cpuslocked(state, node, false);
 }
 
-void __cpuhp_remove_state(enum cpuhp_state state, bool invoke);
-void __cpuhp_remove_state_cpuslocked(enum cpuhp_state state, bool invoke);
+___P( void __cpuhp_remove_state(enum cpuhp_state state, bool invoke), ; )
+___P( void __cpuhp_remove_state_cpuslocked(enum cpuhp_state state, bool invoke), ; )
 
 /**
  * cpuhp_remove_state - Remove hotplug state callbacks and invoke the teardown
@@ -325,8 +335,9 @@ static inline void cpuhp_remove_multi_state(enum cpuhp_state state)
 	__cpuhp_remove_state(state, false);
 }
 
+___P(
 int __cpuhp_state_remove_instance(enum cpuhp_state state,
-				  struct hlist_node *node, bool invoke);
+				  struct hlist_node *node, bool invoke), 0; )
 
 /**
  * cpuhp_state_remove_instance - Remove hotplug instance from state and invoke
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index e8517b63eb..28acf47105 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -311,3 +311,6 @@ config PM_GENERIC_DOMAINS_OF
 
 config CPU_PM
 	bool
+
+config CPUHP
+	def_bool SMP

Re: [PATCH/RFC] cpuhp code and data usage on UP systems

[Thomas Gleixner]

On Mon, 2 Oct 2017, Nicolas Pitre wrote:
> 
> On !SMP systems, we end up with the following array definition:
> 
> static struct cpuhp_step cpuhp_ap_states[] = {
>         [CPUHP_ONLINE] = {
>                 .name                   = "online",
>                 .startup.single         = NULL,
>                 .teardown.single        = NULL,
>         },
> };
> 
> where CPUHP_ONLINE = 187. That means up to 99.5% of this array is unused 
> but still allocated in the kernel's .data section i.e. 3760 bytes.
> 
> The same issue exists with cpuhp_bp_states being 1720 bytes.
> 
> The goal of this patch is mainly about illustrating the issue and 
> reducing .data usage. I have no clear idea when this stuff is really 
> needed besides standard CPU hotplug. Not knowing exactly what I'm doing 
> here, I made it conditional on CONFIG_SMP for now. There might be a case 
> for moving that code to a separate file (cpuhp.c maybe?) and omitting it 
> from the kernel when unneeded.
> 
> Comments?

Sure.

> +#ifdef CONFIG_CPUHP
> +#define ___P(proto, def_retcode) extern proto;
> +#else
> +#define ___P(proto, def_retcode) static inline proto { return def_retcode }
> +#endif
> +
> +___P(
>  int __cpuhp_setup_state(enum cpuhp_state state,	const char *name, bool invoke,
>  			int (*startup)(unsigned int cpu),
> -			int (*teardown)(unsigned int cpu), bool multi_instance);
> -
> +			int (*teardown)(unsigned int cpu), bool multi_instance), 0; )

What exactly is invoking the callbacks of states in the proper context and
manages instances on registration? Ditto for teardwon in case of modules.

Thanks,

	tglx