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: H. Peter Anvin on 20 Jul 2010 16:30 On 07/20/2010 11:50 AM, Robert Richter wrote: > > 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. > Probably, yes. -hpa -- 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 21 Jul 2010 12:20 On 20.07.10 16:17:40, Cyrill Gorcunov wrote: > 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. This in smpboot.c makes it clear: void __cpuinit smp_store_cpu_info(int id) { struct cpuinfo_x86 *c = &cpu_data(id); copy_cpuinfo_x86(c, &boot_cpu_data); c->cpu_index = id; if (id != 0) identify_secondary_cpu(c); } So boot cpu id is always 0. Also note, as Hans Peter already pointed out, this for CONFIG_SMP: &cpu_data(0) != &boot_cpu_data The data in boot_cpu_data is (partly) already available after early_cpu_init(). It is later copied to the &cpu_data() structures. So boot_cpu_data should be used for init code. Also, to make the test obviously, instead of testing (cpu == boot_cpu_id) I rather tend to use an is_boot_cpu() macro as you suggested in your earlier mail. -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 21 Jul 2010 12:40 On 07/20/2010 01:17 PM, Cyrill Gorcunov wrote: > > 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. > I think the style (!smp_processor_id()) is already in use in other places, but we should be consistent in style; if you want to introduce a new style I certainly agree that (is_boot_cpu()) is pretty clear but it should be introduced universally. -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: Cyrill Gorcunov on 21 Jul 2010 12:40 On Wed, Jul 21, 2010 at 06:16:54PM +0200, Robert Richter wrote: > On 20.07.10 16:17:40, Cyrill Gorcunov wrote: > > > 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. > > This in smpboot.c makes it clear: > > void __cpuinit smp_store_cpu_info(int id) > { > struct cpuinfo_x86 *c = &cpu_data(id); > > copy_cpuinfo_x86(c, &boot_cpu_data); > c->cpu_index = id; > if (id != 0) > identify_secondary_cpu(c); > } > > So boot cpu id is always 0. yeah, thanks! > > Also note, as Hans Peter already pointed out, this for CONFIG_SMP: > > &cpu_data(0) != &boot_cpu_data > > The data in boot_cpu_data is (partly) already available after > early_cpu_init(). It is later copied to the &cpu_data() structures. So > boot_cpu_data should be used for init code. > > Also, to make the test obviously, instead of testing (cpu == > boot_cpu_id) I rather tend to use an is_boot_cpu() macro as you > suggested in your earlier mail. > > -Robert > > -- > Advanced Micro Devices, Inc. > Operating System Research Center > ok, thanks Robert! -- 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: Cyrill Gorcunov on 21 Jul 2010 13:00
On Wed, Jul 21, 2010 at 09:32:35AM -0700, H. Peter Anvin wrote: > On 07/20/2010 01:17 PM, Cyrill Gorcunov wrote: > > > > 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. > > > > I think the style (!smp_processor_id()) is already in use in other > places, but we should be consistent in style; if you want to introduce a > new style I certainly agree that (is_boot_cpu()) is pretty clear but it > should be introduced universally. > > -hpa > yes, also I bet there will be places with patterns like cpu = smp_processor_id(); if (!cpu) or if (cpu == 0) so every single smp_processor_id and "raw" version as well should be checked. I'll take a look as only get ability. If Robert (or anyone) will like to beat me on this -- I would be only glad ;) -- 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/ |