x86, xsave: some code cleanups and reworks
in [Kernel]
|
Prev: x86, numa: fix boot without RAM on node0 again
Next: [PATCH] vga16fb: refuse to load in face of other driver controlling primary card
From: Cyrill Gorcunov on 20 Jul 2010 15:30 On Tue, Jul 20, 2010 at 08:50:47PM +0200, Robert Richter wrote: > > This patch series contains some cleanups and reworks I made during > code review and feature implementation for upcoming cpus. > > Most patches refactor the xsave initialization that is very dependent > on fpu initialization. This series starts to decouple this a little > bit as xsave not only supports fpu features. Also this is an attempt > to ease the xsave interface by making some of the functions and > variables static. > > There is also one patch that removes boot_cpu_id variable, which is > not really related to xsave. Maybe this should be applied to another > branch. > > The patches are relative to today's tip/x86/xsave branch. > > (The patches are small for better review and rebasing.) > > -Robert > Hi Robert, I recall there was a thread related to boot_cpu_id and cpu = 0. Unfortunately I can't find it neither in my mbox nor somewhere in net at moment. Ie technically speaking -- yes boot_cpu_id will be 0 but perhaps instead of magic !cpu and friends explicit boot_cpu_id might be better for code reading. It might be is_boot_cpu() macro helper or so as well. Though I don't have strong opinion but for ones who will be reading the code first time it might be confusing :) Agreed? -- Cyrill -- 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 20 Jul 2010 15:50 On 20.07.10 15:27:17, Cyrill Gorcunov wrote: > On Tue, Jul 20, 2010 at 08:50:47PM +0200, Robert Richter wrote: > > > > This patch series contains some cleanups and reworks I made during > > code review and feature implementation for upcoming cpus. > > > > Most patches refactor the xsave initialization that is very dependent > > on fpu initialization. This series starts to decouple this a little > > bit as xsave not only supports fpu features. Also this is an attempt > > to ease the xsave interface by making some of the functions and > > variables static. > > > > There is also one patch that removes boot_cpu_id variable, which is > > not really related to xsave. Maybe this should be applied to another > > branch. > > > > The patches are relative to today's tip/x86/xsave branch. > > > > (The patches are small for better review and rebasing.) > > > > -Robert > > > > Hi Robert, I recall there was a thread related to boot_cpu_id and > cpu = 0. Unfortunately I can't find it neither in my mbox nor somewhere > in net at moment. I found this patch: b3572e3 x86/voyager: fix compile breakage caused by dc1e35c6e95e8923cf1d3510438b63c600fee1e2 indicating that boot cpu id could be different than 0. But either this is broken again, or the issue is gone in a different way. > Ie technically speaking -- yes boot_cpu_id will be 0 > but perhaps instead of magic !cpu and friends explicit boot_cpu_id might > be better for code reading. It might be is_boot_cpu() macro helper or > so as well. > > Though I don't have strong opinion but for ones who will be > reading the code first time it might be confusing :) Agreed? That's true, but once you know... I could make a follow on patch with an is_boot_cpu() macro. Ingo, what do you think? But first question is, is it always !smp_processor_id()? At least current implementation indicates this: void __cpuinit identify_secondary_cpu(struct cpuinfo_x86 *c) { BUG_ON(c == &boot_cpu_data); ... with: #define boot_cpu_data cpu_data[0] .... which is valid for 32 and 64 bit. -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: Cyrill Gorcunov on 20 Jul 2010 16:10 On Tue, Jul 20, 2010 at 09:46:06PM +0200, Robert Richter wrote: > On 20.07.10 15:27:17, Cyrill Gorcunov wrote: > > On Tue, Jul 20, 2010 at 08:50:47PM +0200, Robert Richter wrote: > > > > > > This patch series contains some cleanups and reworks I made during > > > code review and feature implementation for upcoming cpus. > > > > > > Most patches refactor the xsave initialization that is very dependent > > > on fpu initialization. This series starts to decouple this a little > > > bit as xsave not only supports fpu features. Also this is an attempt > > > to ease the xsave interface by making some of the functions and > > > variables static. > > > > > > There is also one patch that removes boot_cpu_id variable, which is > > > not really related to xsave. Maybe this should be applied to another > > > branch. > > > > > > The patches are relative to today's tip/x86/xsave branch. > > > > > > (The patches are small for better review and rebasing.) > > > > > > -Robert > > > > > > > Hi Robert, I recall there was a thread related to boot_cpu_id and > > cpu = 0. Unfortunately I can't find it neither in my mbox nor somewhere > > in net at moment. > > I found this patch: > > b3572e3 x86/voyager: fix compile breakage caused by dc1e35c6e95e8923cf1d3510438b63c600fee1e2 > > indicating that boot cpu id could be different than 0. > yeah, I forgot about voyager indeed but seems this is quite specific to voyager trick > But either this is broken again, or the issue is gone in a different > way. > > > Ie technically speaking -- yes boot_cpu_id will be 0 > > but perhaps instead of magic !cpu and friends explicit boot_cpu_id might > > be better for code reading. It might be is_boot_cpu() macro helper or > > so as well. > > > > Though I don't have strong opinion but for ones who will be > > reading the code first time it might be confusing :) Agreed? > > That's true, but once you know... > yes, but before you know ;) > I could make a follow on patch with an is_boot_cpu() macro. Ingo, what > do you think? > > But first question is, is it always !smp_processor_id()? At least > current implementation indicates this: > I guess so, since it's assigned from boot_cpu_id iirc > void __cpuinit identify_secondary_cpu(struct cpuinfo_x86 *c) > { > BUG_ON(c == &boot_cpu_data); > ... > > with: > > #define boot_cpu_data cpu_data[0] > > ... which is valid for 32 and 64 bit. > I suppose this is just self-protection for "what if something will go wrong and this will be called on non-BP cpu". > -Robert > > -- > Advanced Micro Devices, Inc. > Operating System Research Center > -- Cyrill -- 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: Suresh Siddha on 20 Jul 2010 16:10 On Tue, 2010-07-20 at 12:46 -0700, Robert Richter wrote: > On 20.07.10 15:27:17, Cyrill Gorcunov wrote: > > On Tue, Jul 20, 2010 at 08:50:47PM +0200, Robert Richter wrote: > > > > > > This patch series contains some cleanups and reworks I made during > > > code review and feature implementation for upcoming cpus. > > > > > > Most patches refactor the xsave initialization that is very dependent > > > on fpu initialization. This series starts to decouple this a little > > > bit as xsave not only supports fpu features. Also this is an attempt > > > to ease the xsave interface by making some of the functions and > > > variables static. > > > > > > There is also one patch that removes boot_cpu_id variable, which is > > > not really related to xsave. Maybe this should be applied to another > > > branch. > > > > > > The patches are relative to today's tip/x86/xsave branch. > > > > > > (The patches are small for better review and rebasing.) > > > > > > -Robert > > > > > > > Hi Robert, I recall there was a thread related to boot_cpu_id and > > cpu = 0. Unfortunately I can't find it neither in my mbox nor somewhere > > in net at moment. > > I found this patch: > > b3572e3 x86/voyager: fix compile breakage caused by dc1e35c6e95e8923cf1d3510438b63c600fee1e2 > > indicating that boot cpu id could be different than 0. > > But either this is broken again, or the issue is gone in a different > way. Voyager code was removed from the tree since then. thanks suresh -- 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: Cyrill Gorcunov on 20 Jul 2010 16:20
On Wed, Jul 21, 2010 at 12:07:29AM +0400, Cyrill Gorcunov wrote: .... > > > > But first question is, is it always !smp_processor_id()? At least > > current implementation indicates this: > > > > I guess so, since it's assigned from boot_cpu_id iirc > well, not true, this id is being set in setup_per_cpu_areas() note the snippet if (cpu == boot_cpu_id) switch_to_new_gdt(cpu); but cycle of assignment is done over all possible cpus so smp_processor_id will be = 0 for BP but definitely it's confusing and better to check for BP via explicit cpu == boot_cpu_id I think. Though I might be missing something. > > void __cpuinit identify_secondary_cpu(struct cpuinfo_x86 *c) > > { > > BUG_ON(c == &boot_cpu_data); > > ... > > > > with: > > > > #define boot_cpu_data cpu_data[0] > > > > ... which is valid for 32 and 64 bit. > > -- Cyrill -- 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/ |