Skip to content

Commit

Permalink
x86/smp: Don't ever patch back to UP if we unplug cpus
Browse files Browse the repository at this point in the history
We still patch SMP instructions to UP variants if we boot with a
single CPU, but not at any other time.  In particular, not if we
unplug CPUs to return to a single cpu.

Paul McKenney points out:

 mean offline overhead is 6251/48=130.2 milliseconds.

 If I remove the alternatives_smp_switch() from the offline
 path [...] the mean offline overhead is 550/42=13.1 milliseconds

Basically, we're never going to get those 120ms back, and the
code is pretty messy.

We get rid of:

 1) The "smp-alt-once" boot option. It's actually "smp-alt-boot", the
    documentation is wrong. It's now the default.

 2) The skip_smp_alternatives flag used by suspend.

 3) arch_disable_nonboot_cpus_begin() and arch_disable_nonboot_cpus_end()
    which were only used to set this one flag.

Signed-off-by: Rusty Russell <[email protected]>
Cc: Paul McKenney <[email protected]>
Cc: Suresh Siddha <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
  • Loading branch information
rustyrussell authored and Ingo Molnar committed Aug 23, 2012
1 parent 23dcfa6 commit 816afe4
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 119 deletions.
3 changes: 0 additions & 3 deletions Documentation/kernel-parameters.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2638,9 +2638,6 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
smart2= [HW]
Format: <io1>[,<io2>[,...,<io8>]]

smp-alt-once [X86-32,SMP] On a hotplug CPU system, only
attempt to substitute SMP alternatives once at boot.

smsc-ircc2.nopnp [HW] Don't use PNP to discover SMC devices
smsc-ircc2.ircc_cfg= [HW] Device configuration I/O port
smsc-ircc2.ircc_sir= [HW] SIR base I/O port
Expand Down
4 changes: 2 additions & 2 deletions arch/x86/include/asm/alternative.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,15 @@ extern void alternatives_smp_module_add(struct module *mod, char *name,
void *locks, void *locks_end,
void *text, void *text_end);
extern void alternatives_smp_module_del(struct module *mod);
extern void alternatives_smp_switch(int smp);
extern void alternatives_enable_smp(void);
extern int alternatives_text_reserved(void *start, void *end);
extern bool skip_smp_alternatives;
#else
static inline void alternatives_smp_module_add(struct module *mod, char *name,
void *locks, void *locks_end,
void *text, void *text_end) {}
static inline void alternatives_smp_module_del(struct module *mod) {}
static inline void alternatives_smp_switch(int smp) {}
static inline void alternatives_enable_smp(void) {}
static inline int alternatives_text_reserved(void *start, void *end)
{
return 0;
Expand Down
107 changes: 26 additions & 81 deletions arch/x86/kernel/alternative.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,6 @@

#define MAX_PATCH_LEN (255-1)

#ifdef CONFIG_HOTPLUG_CPU
static int smp_alt_once;

static int __init bootonly(char *str)
{
smp_alt_once = 1;
return 1;
}
__setup("smp-alt-boot", bootonly);
#else
#define smp_alt_once 1
#endif

static int __initdata_or_module debug_alternative;

static int __init debug_alt(char *str)
Expand Down Expand Up @@ -326,9 +313,6 @@ static void alternatives_smp_unlock(const s32 *start, const s32 *end,
{
const s32 *poff;

if (noreplace_smp)
return;

mutex_lock(&text_mutex);
for (poff = start; poff < end; poff++) {
u8 *ptr = (u8 *)poff + *poff;
Expand Down Expand Up @@ -359,7 +343,7 @@ struct smp_alt_module {
};
static LIST_HEAD(smp_alt_modules);
static DEFINE_MUTEX(smp_alt);
static int smp_mode = 1; /* protected by smp_alt */
static bool uniproc_patched = false; /* protected by smp_alt */

void __init_or_module alternatives_smp_module_add(struct module *mod,
char *name,
Expand All @@ -368,19 +352,18 @@ void __init_or_module alternatives_smp_module_add(struct module *mod,
{
struct smp_alt_module *smp;

if (noreplace_smp)
return;
mutex_lock(&smp_alt);
if (!uniproc_patched)
goto unlock;

if (smp_alt_once) {
if (boot_cpu_has(X86_FEATURE_UP))
alternatives_smp_unlock(locks, locks_end,
text, text_end);
return;
}
if (num_possible_cpus() == 1)
/* Don't bother remembering, we'll never have to undo it. */
goto smp_unlock;

smp = kzalloc(sizeof(*smp), GFP_KERNEL);
if (NULL == smp)
return; /* we'll run the (safe but slow) SMP code then ... */
/* we'll run the (safe but slow) SMP code then ... */
goto unlock;

smp->mod = mod;
smp->name = name;
Expand All @@ -392,36 +375,29 @@ void __init_or_module alternatives_smp_module_add(struct module *mod,
__func__, smp->locks, smp->locks_end,
smp->text, smp->text_end, smp->name);

mutex_lock(&smp_alt);
list_add_tail(&smp->next, &smp_alt_modules);
if (boot_cpu_has(X86_FEATURE_UP))
alternatives_smp_unlock(smp->locks, smp->locks_end,
smp->text, smp->text_end);
smp_unlock:
alternatives_smp_unlock(locks, locks_end, text, text_end);
unlock:
mutex_unlock(&smp_alt);
}

void __init_or_module alternatives_smp_module_del(struct module *mod)
{
struct smp_alt_module *item;

if (smp_alt_once || noreplace_smp)
return;

mutex_lock(&smp_alt);
list_for_each_entry(item, &smp_alt_modules, next) {
if (mod != item->mod)
continue;
list_del(&item->next);
mutex_unlock(&smp_alt);
DPRINTK("%s: %s\n", __func__, item->name);
kfree(item);
return;
break;
}
mutex_unlock(&smp_alt);
}

bool skip_smp_alternatives;
void alternatives_smp_switch(int smp)
void alternatives_enable_smp(void)
{
struct smp_alt_module *mod;

Expand All @@ -436,34 +412,21 @@ void alternatives_smp_switch(int smp)
pr_info("lockdep: fixing up alternatives\n");
#endif

if (noreplace_smp || smp_alt_once || skip_smp_alternatives)
return;
BUG_ON(!smp && (num_online_cpus() > 1));
/* Why bother if there are no other CPUs? */
BUG_ON(num_possible_cpus() == 1);

mutex_lock(&smp_alt);

/*
* Avoid unnecessary switches because it forces JIT based VMs to
* throw away all cached translations, which can be quite costly.
*/
if (smp == smp_mode) {
/* nothing */
} else if (smp) {
if (uniproc_patched) {
pr_info("switching to SMP code\n");
BUG_ON(num_online_cpus() != 1);
clear_cpu_cap(&boot_cpu_data, X86_FEATURE_UP);
clear_cpu_cap(&cpu_data(0), X86_FEATURE_UP);
list_for_each_entry(mod, &smp_alt_modules, next)
alternatives_smp_lock(mod->locks, mod->locks_end,
mod->text, mod->text_end);
} else {
pr_info("switching to UP code\n");
set_cpu_cap(&boot_cpu_data, X86_FEATURE_UP);
set_cpu_cap(&cpu_data(0), X86_FEATURE_UP);
list_for_each_entry(mod, &smp_alt_modules, next)
alternatives_smp_unlock(mod->locks, mod->locks_end,
mod->text, mod->text_end);
uniproc_patched = false;
}
smp_mode = smp;
mutex_unlock(&smp_alt);
}

Expand Down Expand Up @@ -540,40 +503,22 @@ void __init alternative_instructions(void)

apply_alternatives(__alt_instructions, __alt_instructions_end);

/* switch to patch-once-at-boottime-only mode and free the
* tables in case we know the number of CPUs will never ever
* change */
#ifdef CONFIG_HOTPLUG_CPU
if (num_possible_cpus() < 2)
smp_alt_once = 1;
#endif

#ifdef CONFIG_SMP
if (smp_alt_once) {
if (1 == num_possible_cpus()) {
pr_info("switching to UP code\n");
set_cpu_cap(&boot_cpu_data, X86_FEATURE_UP);
set_cpu_cap(&cpu_data(0), X86_FEATURE_UP);

alternatives_smp_unlock(__smp_locks, __smp_locks_end,
_text, _etext);
}
} else {
/* Patch to UP if other cpus not imminent. */
if (!noreplace_smp && (num_present_cpus() == 1 || setup_max_cpus <= 1)) {
uniproc_patched = true;
alternatives_smp_module_add(NULL, "core kernel",
__smp_locks, __smp_locks_end,
_text, _etext);

/* Only switch to UP mode if we don't immediately boot others */
if (num_present_cpus() == 1 || setup_max_cpus <= 1)
alternatives_smp_switch(0);
}
#endif
apply_paravirt(__parainstructions, __parainstructions_end);

if (smp_alt_once)
if (!uniproc_patched || num_possible_cpus() == 1)
free_init_pages("SMP alternatives",
(unsigned long)__smp_locks,
(unsigned long)__smp_locks_end);
#endif

apply_paravirt(__parainstructions, __parainstructions_end);

restart_nmi();
}
Expand Down
20 changes: 2 additions & 18 deletions arch/x86/kernel/smpboot.c
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,8 @@ static int __cpuinit do_boot_cpu(int apicid, int cpu, struct task_struct *idle)
unsigned long boot_error = 0;
int timeout;

alternatives_smp_switch(1);
/* Just in case we booted with a single CPU. */
alternatives_enable_smp();

idle->thread.sp = (unsigned long) (((struct pt_regs *)
(THREAD_SIZE + task_stack_page(idle))) - 1);
Expand Down Expand Up @@ -1053,20 +1054,6 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
preempt_enable();
}

void arch_disable_nonboot_cpus_begin(void)
{
/*
* Avoid the smp alternatives switch during the disable_nonboot_cpus().
* In the suspend path, we will be back in the SMP mode shortly anyways.
*/
skip_smp_alternatives = true;
}

void arch_disable_nonboot_cpus_end(void)
{
skip_smp_alternatives = false;
}

void arch_enable_nonboot_cpus_begin(void)
{
set_mtrr_aps_delayed_init();
Expand Down Expand Up @@ -1256,9 +1243,6 @@ void native_cpu_die(unsigned int cpu)
if (per_cpu(cpu_state, cpu) == CPU_DEAD) {
if (system_state == SYSTEM_RUNNING)
pr_info("CPU %u is now offline\n", cpu);

if (1 == num_online_cpus())
alternatives_smp_switch(0);
return;
}
msleep(100);
Expand Down
6 changes: 2 additions & 4 deletions arch/x86/xen/smp.c
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,8 @@ static int __cpuinit xen_cpu_up(unsigned int cpu, struct task_struct *idle)
return rc;

if (num_online_cpus() == 1)
alternatives_smp_switch(1);
/* Just in case we booted with a single CPU. */
alternatives_enable_smp();

rc = xen_smp_intr_init(cpu);
if (rc)
Expand Down Expand Up @@ -424,9 +425,6 @@ static void xen_cpu_die(unsigned int cpu)
unbind_from_irqhandler(per_cpu(xen_irq_work, cpu), NULL);
xen_uninit_lock_cpu(cpu);
xen_teardown_timer(cpu);

if (num_online_cpus() == 1)
alternatives_smp_switch(0);
}

static void __cpuinit xen_play_dead(void) /* used only with HOTPLUG_CPU */
Expand Down
11 changes: 0 additions & 11 deletions kernel/cpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -439,14 +439,6 @@ EXPORT_SYMBOL_GPL(cpu_up);
#ifdef CONFIG_PM_SLEEP_SMP
static cpumask_var_t frozen_cpus;

void __weak arch_disable_nonboot_cpus_begin(void)
{
}

void __weak arch_disable_nonboot_cpus_end(void)
{
}

int disable_nonboot_cpus(void)
{
int cpu, first_cpu, error = 0;
Expand All @@ -458,7 +450,6 @@ int disable_nonboot_cpus(void)
* with the userspace trying to use the CPU hotplug at the same time
*/
cpumask_clear(frozen_cpus);
arch_disable_nonboot_cpus_begin();

printk("Disabling non-boot CPUs ...\n");
for_each_online_cpu(cpu) {
Expand All @@ -474,8 +465,6 @@ int disable_nonboot_cpus(void)
}
}

arch_disable_nonboot_cpus_end();

if (!error) {
BUG_ON(num_online_cpus() > 1);
/* Make sure the CPUs won't be enabled by someone else */
Expand Down

0 comments on commit 816afe4

Please sign in to comment.