Prev: x86, xsave: reduce cpu_has_xsave checks
Next: x86, xsave: do not initialize xsave in fpu_init()
From: Robert Richter on 22 Jul 2010 08:20 On 21.07.10 17:53:31, H. Peter Anvin wrote: > From 55b936c7a359a14d72bcba6c3fceba4cfbe3fedf Mon Sep 17 00:00:00 2001 > From: H. Peter Anvin <hpa(a)linux.intel.com> > Date: Wed, 21 Jul 2010 14:23:10 -0700 > Subject: [PATCH] x86, xsave: Make xstate_enable_boot_cpu() __init, protect on CPU 0 > > xstate_enable_boot_cpu() is, as the name implies, only used on the > boot CPU; furthermore, it invokes alloc_bootmem(), which is __init; > hence it needs to be tagged __init rather than __cpuinit. > > Furthermore, it is *not* safe in the long run to rely on CPU 0 only > coming online during the early boot -- at some point we're going to > support offlining (and re-onlining) the boot CPU, and at that point we > must not call xstate_enable_boot_cpu() again. > > The code is a fair bit more obscure than one would like, because the > __ref overrides aren't quite powerful enough. > > Signed-off-by: H. Peter Anvin <hpa(a)linux.intel.com> > Acked-by: Suresh Siddha <suresh.b.siddha(a)intel.com> > Cc: Robert Richter <robert.richter(a)amd.com> > LKML-Reference: <4C476236.1020302(a)zytor.com> I am fine with your changes. > void __cpuinit xsave_init(void) > { > + static __refdata void (*next_func)(void) = xstate_enable_boot_cpu; > + void (*this_func)(void); > + > if (!cpu_has_xsave) > return; > > - /* > - * Boot processor to setup the FP and extended state context info. > - */ > - if (!smp_processor_id()) > - xstate_enable_boot_cpu(); > - else > - xstate_enable(pcntxt_mask); > + this_func = next_func; > + next_func = xstate_enable; > + this_func(); > } Just wondering why you are using this_func()? Instead, you could simply do: next_func(); next_func = xstate_enable; Do you see races when bringing up multiple cpus in parallel? Thanks. -Robert -- Advanced Micro Devices, Inc. Operating System Research Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 22 Jul 2010 08:30 On 07/22/2010 05:15 AM, Robert Richter wrote: > > Just wondering why you are using this_func()? Instead, you could > simply do: > > next_func(); > next_func = xstate_enable; > > Do you see races when bringing up multiple cpus in parallel? > It allows the compiler to turn it into a tailcall if frame pointers are disabled. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Robert Richter on 22 Jul 2010 09:20 On 22.07.10 08:23:56, H. Peter Anvin wrote: > On 07/22/2010 05:15 AM, Robert Richter wrote: > > > > Just wondering why you are using this_func()? Instead, you could > > simply do: > > > > next_func(); > > next_func = xstate_enable; > > > > Do you see races when bringing up multiple cpus in parallel? > > > > It allows the compiler to turn it into a tailcall if frame pointers are > disabled. Yes, that makes sense. Thanks. -Robert -- Advanced Micro Devices, Inc. Operating System Research Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
First
|
Prev
|
Pages: 1 2 Prev: x86, xsave: reduce cpu_has_xsave checks Next: x86, xsave: do not initialize xsave in fpu_init() |