From: H. Peter Anvin on
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
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
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
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
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/