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

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

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