1. Forum
  2. Usenet
  3. LINUX.KERNEL

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

    From Thomas Gleixner@21:1/5 to Nicolas Pitre on Mon Oct 2 22:50:05 2017
    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)
  • From Nicolas Pitre@21:1/5 to All on Mon Oct 2 22:50:11 2017
    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