Hi,
This patch implements SMP alternatives, i.e. switching at runtime
between different code versions for UP and SMP. The code can patch both
SMP->UP and UP->SMP. The UP->SMP case is useful for CPU hotplug.
With CONFIG_CPU_HOTPLUG enabled the code switches to UP at boot
time and when the number of CPUs goes down to 1, and switches to
SMP when the number of CPUs goes up to 2.
Without CONFIG_CPU_HOTPLUG or on non-SMP-capable systems the code
is patched once at boot time (if needed) and the tables are
released afterwards.
The changes in detail:
* The current alternatives bits are moved to a separate file,
the SMP alternatives code is added there.
* The patch adds some new elf sections to the kernel:
.smp_altinstructions
like .altinstructions, also contains a list
of alt_instr structs.
.smp_altinstr_replacement
like .altinstr_replacement, but also has some space to
save original instruction before replaving it.
.smp_locks
list of pointers to lock prefixes which can be nop'ed
out on UP.
The first two are used to replace more complex instruction
sequences such as spinlocks and semaphores. It would be possible
to deal with the lock prefixes with that as well, but by handling
them as special case the table sizes become much smaller.
* The sections are page-aligned and padded up to page size, so they
can be free if they are not needed.
* Splitted the code to release init pages to a separate function and
use it to release the elf sections if they are unused.
please apply,
Gerd
Gerd Knorr wrote:
>+#ifdef CONFIG_SMP
>+#define alternative_smp(smpinstr, upinstr, args...) \
>+ asm volatile ("661:\n\t" smpinstr "\n662:\n" \
>+ ".section .smp_altinstructions,\"a\"\n" \
>+ " .align 4\n" \
>+ " .long 661b\n" /* label */ \
>+ " .long 663f\n" /* new instruction */ \
>+ " .byte 0x68\n" /* X86_FEATURE_UP */ \
>+ " .byte 662b-661b\n" /* sourcelen */ \
>+ " .byte 664f-663f\n" /* replacementlen */ \
>+ ".previous\n" \
>+ ".section .smp_altinstr_replacement,\"awx\"\n" \
>+ "663:\n\t" upinstr "\n" /* replacement */ \
>+ "664:\n\t.fill 662b-661b,1,0x42\n" /* space for original */ \
>+ ".previous" : args)
>+
>+#define LOCK_PREFIX \
>+ ".section .smp_locks,\"a\"\n" \
>+ " .align 4\n" \
>+ " .long 661f\n" /* address */ \
>+ ".previous\n" \
>+ "661:\n\tlock; "
>+
>+#else /* ! CONFIG_SMP */
>+#define alternative_smp(smpinstr, upinstr, args...) \
>+ asm volatile (upinstr : args)
>+#define LOCK_PREFIX ""
>+#endif
>+
>+#endif /* _I386_ALTERNATIVE_H */
>
>
Overall technically, I like this patch. Philosophically, I agree with
it as well - but might I strongly suggest that you avoid the .section
.previous directives and use the nestable .pushsection, .popsection
instead? We are almost to the complexity point with fault handling and
alternatives that we will need nested section overrides.
Zach
The SMP problems with self modifying code are very real but the hotplug
architecture for CPU already deals with these synchronizations and you
can use the SMP cross calls to 'capture' the other processors. The x86
case of processors with differing feature sets is extremely common on
PII/PIII systems and can also occur on other platforms where features
like SSE3 were introduced in later steppings. Clean alternatives support
for such platforms automatically would be good.
Other than that it looks sane to me (well the CPU hotplug case is silly
but the rest makes sense). The various CPU errata come out ok that I can
see as well because they use rmb/wmb/etc and those don't change
behaviour so still maintain store order against the bus.
> Overall technically, I like this patch. Philosophically, I agree with
> it as well - but might I strongly suggest that you avoid the .section
> .previous directives and use the nestable .pushsection, .popsection
> instead? We are almost to the complexity point with fault handling and
> alternatives that we will need nested section overrides.
Fortunaly we don't need that at the moment. And in case we'll really
need it some day it will not be *that* easy. The pointers in the fixup
section will point to the wrong location because we run the UP code not
in-place but copy it to another address ...
cheers,
Gerd
diff -urN -x 'build-*' -x '*~' -x Make -x scripts linux-2.6.15-rc5/arch/i386/kernel/smpboot.c work-2.6.15-rc5/arch/i386/kernel/smpboot.c
--- linux-2.6.15-rc5/arch/i386/kernel/smpboot.c 2005-12-06 17:00:36.000000000 +0100
+++ work-2.6.15-rc5/arch/i386/kernel/smpboot.c 2005-12-06 17:06:48.000000000 +0100
@@ -1351,6 +1352,9 @@
fixup_irqs(map);
/* It's now safe to remove this processor from the online map */
cpu_clear(cpu, cpu_online_map);
+
+ if (1 == num_online_cpus())
+ alternatives_smp_switch(0);
return 0;
}
Is that really safe there? Ideally you want to do the switch when the
processor going offline is no longer executing kernel code.
On 14 Dec 2005, at 19:35, Zwane Mwaikambo wrote:
> diff -urN -x 'build-*' -x '*~' -x Make -x scripts
> linux-2.6.15-rc5/arch/i386/kernel/smpboot.c
> work-2.6.15-rc5/arch/i386/kernel/smpboot.c
> --- linux-2.6.15-rc5/arch/i386/kernel/smpboot.c 2005-12-06
> 17:00:36.000000000 +0100
> +++ work-2.6.15-rc5/arch/i386/kernel/smpboot.c 2005-12-06
> 17:06:48.000000000 +0100
> @@ -1351,6 +1352,9 @@
> fixup_irqs(map);
> /* It's now safe to remove this processor from the online map */
> cpu_clear(cpu, cpu_online_map);
> +
> + if (1 == num_online_cpus())
> + alternatives_smp_switch(0);
> return 0;
> }
>
> Is that really safe there? Ideally you want to do the switch when the
> processor going offline is no longer executing kernel code.
Perhaps that check belongs at the end of __cpu_die()? That's where it
lives in xen's smpboot.c.
-- Keir
Hello Keir,
On Wed, 14 Dec 2005, Keir Fraser wrote:
> > fixup_irqs(map);
> > /* It's now safe to remove this processor from the online map */
> > cpu_clear(cpu, cpu_online_map);
> > +
> > + if (1 == num_online_cpus())
> > + alternatives_smp_switch(0);
> > return 0;
> > }
> >
> > Is that really safe there? Ideally you want to do the switch when the
> > processor going offline is no longer executing kernel code.
>
> Perhaps that check belongs at the end of __cpu_die()? That's where it lives in
> xen's smpboot.c.
Yes indeed, end of __cpu_die would be perfect for x86 too as that's where
CPU_DEAD acknowledge is checked.
>>> + if (1 == num_online_cpus())
>>> + alternatives_smp_switch(0);
> Yes indeed, end of __cpu_die would be perfect for x86 too as that's where
> CPU_DEAD acknowledge is checked.
Good point, fixed patch attached.
cheers,
Gerd