On Tue, 31 Jan 2012 16:17:19 -0800
Venkatesh Pallipadi <[email protected]> wrote:
> Kernel's notion of possible cpus (from include/linux/cpumask.h)
> * cpu_possible_mask- has bit 'cpu' set iff cpu is populatable
>
> * The cpu_possible_mask is fixed at boot time, as the set of CPU id's
> * that it is possible might ever be plugged in at anytime during the
> * life of that system boot.
>
> #define num_possible_cpus() cpumask_weight(cpu_possible_mask)
>
> and on x86 cpumask_weight() calls hweight64 and hweight64 (on older kernels
> and systems with !X86_FEATURE_POPCNT) or a popcnt based alternative.
>
> i.e, We needlessly go through this mask based calculation everytime
> num_possible_cpus() is called.
>
> The problem is there with cpu_online_mask() as well, which is fixed value at
> boot time in !CONFIG_HOTPLUG_CPU case and should not change that often even
> in HOTPLUG case.
>
> Though most of the callers of these two routines are init time (with few
> exceptions of runtime calls), it is cleaner to use variables
> and not go through this repeated mask based calculation.
Looks good to me.
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -604,9 +604,13 @@ EXPORT_SYMBOL(cpu_all_bits);
> #ifdef CONFIG_INIT_ALL_POSSIBLE
> static DECLARE_BITMAP(cpu_possible_bits, CONFIG_NR_CPUS) __read_mostly
> = CPU_BITS_ALL;
> +unsigned int nr_possible_cpus __read_mostly = CONFIG_NR_CPUS;
> #else
> static DECLARE_BITMAP(cpu_possible_bits, CONFIG_NR_CPUS) __read_mostly;
> +unsigned int nr_possible_cpus __read_mostly;
> #endif
> +EXPORT_SYMBOL(nr_possible_cpus);
What the heck is CONFIG_INIT_ALL_POSSIBLE?
<blames Rusty>
: 1) Some archs (m32, parisc, s390) set possible_map to all 1, so we add a
: CONFIG_INIT_ALL_POSSIBLE for this rather than break them.
Seems strange. Do these architectures really need to initialise
cpu_possible_map at compile-time, when all the other architectures
manage to do it at runtime?
On Wed, 1 Feb 2012 14:01:25 -0800, Andrew Morton <[email protected]> wrote:
> > Though most of the callers of these two routines are init time (with few
> > exceptions of runtime calls), it is cleaner to use variables
> > and not go through this repeated mask based calculation.
>
> Looks good to me.
But, I wonder who's asking num_possible_cpus(). It's not a very useful
thing to know, though some arch's "know" it's contiguous, so can cheat.
Optimizing it seems particularly foolish. We either audit and wean
everyone off who's using it incorrectly, or insist on contiguous CPU
numbers and drop the mask altogether.
> > --- a/kernel/cpu.c
> > +++ b/kernel/cpu.c
> > @@ -604,9 +604,13 @@ EXPORT_SYMBOL(cpu_all_bits);
> > #ifdef CONFIG_INIT_ALL_POSSIBLE
> > static DECLARE_BITMAP(cpu_possible_bits, CONFIG_NR_CPUS) __read_mostly
> > = CPU_BITS_ALL;
> > +unsigned int nr_possible_cpus __read_mostly = CONFIG_NR_CPUS;
> > #else
> > static DECLARE_BITMAP(cpu_possible_bits, CONFIG_NR_CPUS) __read_mostly;
> > +unsigned int nr_possible_cpus __read_mostly;
> > #endif
> > +EXPORT_SYMBOL(nr_possible_cpus);
>
> What the heck is CONFIG_INIT_ALL_POSSIBLE?
>
> <blames Rusty>
>
> : 1) Some archs (m32, parisc, s390) set possible_map to all 1, so we add a
> : CONFIG_INIT_ALL_POSSIBLE for this rather than break them.
>
> Seems strange. Do these architectures really need to initialise
> cpu_possible_map at compile-time, when all the other architectures
> manage to do it at runtime?
IIRC playing with 3 archs boot code seemed like a recipe for disaster.
Feel free to try to fix this in -next though, and see what breaks...
Cheers,
Rusty.
On Fri, 03 Feb 2012 06:33:02 +1030
Rusty Russell <[email protected]> wrote:
> On Wed, 1 Feb 2012 14:01:25 -0800, Andrew Morton <[email protected]> wrote:
> > > Though most of the callers of these two routines are init time (with few
> > > exceptions of runtime calls), it is cleaner to use variables
> > > and not go through this repeated mask based calculation.
> >
> > Looks good to me.
>
> But, I wonder who's asking num_possible_cpus(). It's not a very useful
> thing to know, though some arch's "know" it's contiguous, so can cheat.
>
> Optimizing it seems particularly foolish.
We're fools for optimisations!
> We either audit and wean
> everyone off who's using it incorrectly, or insist on contiguous CPU
> numbers and drop the mask altogether.
drivers/block/nvme.c looks like it's assuming a contiguous map. Maybe
also drivers/scsi/bnx2fc (wtf?). I didn't see anything else outside
arch code.
On Thu, Feb 2, 2012 at 12:19 PM, Andrew Morton
<[email protected]> wrote:
> On Fri, 03 Feb 2012 06:33:02 +1030
> Rusty Russell <[email protected]> wrote:
>
>> On Wed, 1 Feb 2012 14:01:25 -0800, Andrew Morton <[email protected]> wrote:
>> > > Though most of the callers of these two routines are init time (with few
>> > > exceptions of runtime calls), it is cleaner to use variables
>> > > and not go through this repeated mask based calculation.
>> >
>> > Looks good to me.
>>
>> But, I wonder who's asking num_possible_cpus(). ?It's not a very useful
>> thing to know, though some arch's "know" it's contiguous, so can cheat.
>>
>> Optimizing it seems particularly foolish.
>
> We're fools for optimisations!
>
I would think that if we are giving an API people will abuse it sooner
or later :-).
>> We either audit and wean
>> everyone off who's using it incorrectly, or insist on contiguous CPU
>> numbers and drop the mask altogether.
>
> drivers/block/nvme.c looks like it's assuming a contiguous map. ?Maybe
> also drivers/scsi/bnx2fc (wtf?). ?I didn't see anything else outside
> arch code.
>
Yes. I found a bunch of them which seemed obviously wrong. Doing
things like allocating space based on num_possible_cpus() and
accessing the space with get_cpu() or doing cou_online() check etc.
arch/x86/kernel/apic/x2apic_uv_x.c
arch/x86/kernel/cpu/mcheck/mce-inject.c
arch/x86/platform/uv/tlb_uv.c
drivers/scsi/bnx2fc/bnx2fc_io.c
kernel/debug/kdb/kdb_bt.c
I have a patch to fix these obvious ones. But, there are other users
which were not very obvious to me and also I am know of code in older
kernels (code which since have been rewritten) which can get benefit
of this API change.
Thanks,
Venki
On Thu, Feb 2, 2012 at 12:03 PM, Rusty Russell <[email protected]> wrote:
> IIRC playing with 3 archs boot code seemed like a recipe for disaster.
> Feel free to try to fix this in -next though, and see what breaks...
ia64 is what breaks ... well not actually broken ... but some very
weird delays that
show up in different places depending on whether this patch is present.
First linux-next kernel to be blessed with this patch was
next-20120210. Booting it
I see:
[ 7.164233] Switching to clocksource itc
[ 146.077315] pnp: PnP ACPI init
An ugly 138.913 second delay. Digging in the code showed that the bad bits
happened inside stop_machine()
Reverting just this patch makes this big delay disappear:
[ 32.780232] Switching to clocksource itc
[ 32.832100] pnp: PnP ACPI init
but notice that it takes 25 extra seconds to get to this point in the
boot (and while
we expect to save some time by not re-computing num_online_cpus each time we
need it ... this looks to be a lot more than I'd expect!)
-Tony
On Mon, Feb 13, 2012 at 11:54 AM, Tony Luck <[email protected]> wrote:
> On Thu, Feb 2, 2012 at 12:03 PM, Rusty Russell <[email protected]> wrote:
>> IIRC playing with 3 archs boot code seemed like a recipe for disaster.
>> Feel free to try to fix this in -next though, and see what breaks...
>
> ia64 is what breaks ... well not actually broken ... but some very
> weird delays that
> show up in different places depending on whether this patch is present.
>
> First linux-next kernel to be blessed with this patch was
> next-20120210. Booting it
> I see:
> [ ? ?7.164233] Switching to clocksource itc
> [ ?146.077315] pnp: PnP ACPI init
>
> An ugly 138.913 second delay. ?Digging in the code showed that the bad bits
> happened inside stop_machine()
>
> Reverting just this patch makes this big delay disappear:
>
> [ ? 32.780232] Switching to clocksource itc
> [ ? 32.832100] pnp: PnP ACPI init
>
> but notice that it takes 25 extra seconds to get to this point in the
> boot (and while
> we expect to save some time by not re-computing num_online_cpus each time we
> need it ... this looks to be a lot more than I'd expect!)
>
> -Tony
What is NR_CPUS set to here? And how many actual CPUs are there on this machine?
On 02/14/2012 01:24 AM, Tony Luck wrote:
> On Thu, Feb 2, 2012 at 12:03 PM, Rusty Russell <[email protected]> wrote:
>> IIRC playing with 3 archs boot code seemed like a recipe for disaster.
>> Feel free to try to fix this in -next though, and see what breaks...
>
> ia64 is what breaks ... well not actually broken ... but some very
> weird delays that
> show up in different places depending on whether this patch is present.
>
> First linux-next kernel to be blessed with this patch was
> next-20120210. Booting it
> I see:
> [ 7.164233] Switching to clocksource itc
> [ 146.077315] pnp: PnP ACPI init
>
> An ugly 138.913 second delay. Digging in the code showed that the bad bits
> happened inside stop_machine()
>
> Reverting just this patch makes this big delay disappear:
>
> [ 32.780232] Switching to clocksource itc
> [ 32.832100] pnp: PnP ACPI init
>
> but notice that it takes 25 extra seconds to get to this point in the
> boot (and while
> we expect to save some time by not re-computing num_online_cpus each time we
> need it ... this looks to be a lot more than I'd expect!)
>
Oh no!! ia64 directly uses cpu_set() and cpu_clear() on cpu_online_map!!
Grr.. It means num_online_cpus can be different from the actual number of
online cpus because it doesn't go through the set_cpu_online() path.. I haven't
yet pin-pointed the exact problem, but this definitely doesn't look good...
Regards,
Srivatsa S. Bhat
On Mon, Feb 13, 2012 at 12:25 PM, Srivatsa S. Bhat
<[email protected]> wrote:
> On 02/14/2012 01:24 AM, Tony Luck wrote:
>
>> On Thu, Feb 2, 2012 at 12:03 PM, Rusty Russell <[email protected]> wrote:
>>> IIRC playing with 3 archs boot code seemed like a recipe for disaster.
>>> Feel free to try to fix this in -next though, and see what breaks...
>>
>> ia64 is what breaks ... well not actually broken ... but some very
>> weird delays that
>> show up in different places depending on whether this patch is present.
>>
>> First linux-next kernel to be blessed with this patch was
>> next-20120210. Booting it
>> I see:
>> [ ? ?7.164233] Switching to clocksource itc
>> [ ?146.077315] pnp: PnP ACPI init
>>
>> An ugly 138.913 second delay. ?Digging in the code showed that the bad bits
>> happened inside stop_machine()
>>
>> Reverting just this patch makes this big delay disappear:
>>
>> [ ? 32.780232] Switching to clocksource itc
>> [ ? 32.832100] pnp: PnP ACPI init
>>
>> but notice that it takes 25 extra seconds to get to this point in the
>> boot (and while
>> we expect to save some time by not re-computing num_online_cpus each time we
>> need it ... this looks to be a lot more than I'd expect!)
>>
>
>
> Oh no!! ia64 directly uses cpu_set() and cpu_clear() on cpu_online_map!!
> Grr.. It means num_online_cpus can be different from the actual number of
> online cpus because it doesn't go through the set_cpu_online() path.. I haven't
> yet pin-pointed the exact problem, but this definitely doesn't look good...
>
This feels like a minefield in general. ia64, mips and um seems to
have cpu_set and cpu_clear of cpu_online_map and/or cpu_possible_map
in there.
On 02/14/2012 01:55 AM, Srivatsa S. Bhat wrote:
> On 02/14/2012 01:24 AM, Tony Luck wrote:
>
>> On Thu, Feb 2, 2012 at 12:03 PM, Rusty Russell <[email protected]> wrote:
>>> IIRC playing with 3 archs boot code seemed like a recipe for disaster.
>>> Feel free to try to fix this in -next though, and see what breaks...
>>
>> ia64 is what breaks ... well not actually broken ... but some very
>> weird delays that
>> show up in different places depending on whether this patch is present.
>>
>> First linux-next kernel to be blessed with this patch was
>> next-20120210. Booting it
>> I see:
>> [ 7.164233] Switching to clocksource itc
>> [ 146.077315] pnp: PnP ACPI init
>>
>> An ugly 138.913 second delay. Digging in the code showed that the bad bits
>> happened inside stop_machine()
>>
>> Reverting just this patch makes this big delay disappear:
>>
>> [ 32.780232] Switching to clocksource itc
>> [ 32.832100] pnp: PnP ACPI init
>>
>> but notice that it takes 25 extra seconds to get to this point in the
>> boot (and while
>> we expect to save some time by not re-computing num_online_cpus each time we
>> need it ... this looks to be a lot more than I'd expect!)
>>
>
>
> Oh no!! ia64 directly uses cpu_set() and cpu_clear() on cpu_online_map!!
> Grr.. It means num_online_cpus can be different from the actual number of
> online cpus because it doesn't go through the set_cpu_online() path.. I haven't
> yet pin-pointed the exact problem, but this definitely doesn't look good...
>
Hmm.. interesting.. The only calls that ia64 uses which updates the
num_online_cpus macro seem to be init_cpu_online(cpumask_of(0)); Atleast this
is what the mainline code tells me (haven't checked linux-next).
So, if I am not mistaken, is the value of num_online_cpus() always 1 when
Venki's patch is applied?
IOW, what output do you see from the following printk from
arch/ia64/kernel/smpboot.c?
printk(KERN_INFO "Total of %d processors activated (%lu.%02lu BogoMIPS).\n",
(int)num_online_cpus(), bogosum/(500000/HZ), (bogosum/(5000/HZ))%100);
Regards,
Srivatsa S. Bhat
On 02/14/2012 02:13 AM, Venki Pallipadi wrote:
> On Mon, Feb 13, 2012 at 12:25 PM, Srivatsa S. Bhat
> <[email protected]> wrote:
>> On 02/14/2012 01:24 AM, Tony Luck wrote:
>>
>>> On Thu, Feb 2, 2012 at 12:03 PM, Rusty Russell <[email protected]> wrote:
>>>> IIRC playing with 3 archs boot code seemed like a recipe for disaster.
>>>> Feel free to try to fix this in -next though, and see what breaks...
>>>
>>> ia64 is what breaks ... well not actually broken ... but some very
>>> weird delays that
>>> show up in different places depending on whether this patch is present.
>>>
>>> First linux-next kernel to be blessed with this patch was
>>> next-20120210. Booting it
>>> I see:
>>> [ 7.164233] Switching to clocksource itc
>>> [ 146.077315] pnp: PnP ACPI init
>>>
>>> An ugly 138.913 second delay. Digging in the code showed that the bad bits
>>> happened inside stop_machine()
>>>
>>> Reverting just this patch makes this big delay disappear:
>>>
>>> [ 32.780232] Switching to clocksource itc
>>> [ 32.832100] pnp: PnP ACPI init
>>>
>>> but notice that it takes 25 extra seconds to get to this point in the
>>> boot (and while
>>> we expect to save some time by not re-computing num_online_cpus each time we
>>> need it ... this looks to be a lot more than I'd expect!)
>>>
>>
>>
>> Oh no!! ia64 directly uses cpu_set() and cpu_clear() on cpu_online_map!!
>> Grr.. It means num_online_cpus can be different from the actual number of
>> online cpus because it doesn't go through the set_cpu_online() path.. I haven't
>> yet pin-pointed the exact problem, but this definitely doesn't look good...
>>
>
> This feels like a minefield in general. ia64, mips and um seems to
> have cpu_set and cpu_clear of cpu_online_map and/or cpu_possible_map
> in there.
>
Since I had almost never seen code using "cpu_online_map" instead of
"cpu_online_mask", I hadn't checked it while reviewing your patch... :-(
Honestly, it is only now that I realized that there is this other variant too!
Regards,
Srivatsa S. Bhat
On Mon, Feb 13, 2012 at 12:44 PM, Srivatsa S. Bhat
<[email protected]> wrote:
> IOW, what output do you see from the following printk from
> arch/ia64/kernel/smpboot.c?
>
> printk(KERN_INFO "Total of %d processors activated (%lu.%02lu BogoMIPS).\n",
> ? ? ? ? (int)num_online_cpus(), bogosum/(500000/HZ), (bogosum/(5000/HZ))%100);
That is a complicated question - because linux-next also has patches
by Arjan that
change how (when) cpus are brought online. Initially I blamed his
patches and tried
reverting them ... and saw the symptom you are wondering about (message said
"Total of 1 processors", but the BogoMIPs was a number big enough to be all of
them. Thanks to you, I can now understand why.
Fix will be to stop ia64 from messing directly with cpu_online_map?
-Tony
On Mon, 13 Feb 2012 13:57:45 -0800, Tony Luck <[email protected]> wrote:
> On Mon, Feb 13, 2012 at 12:44 PM, Srivatsa S. Bhat
> <[email protected]> wrote:
> > IOW, what output do you see from the following printk from
> > arch/ia64/kernel/smpboot.c?
> >
> > printk(KERN_INFO "Total of %d processors activated (%lu.%02lu BogoMIPS).\n",
> > (int)num_online_cpus(), bogosum/(500000/HZ), (bogosum/(5000/HZ))%100);
>
> That is a complicated question - because linux-next also has patches
> by Arjan that
> change how (when) cpus are brought online. Initially I blamed his
> patches and tried
> reverting them ... and saw the symptom you are wondering about (message said
> "Total of 1 processors", but the BogoMIPs was a number big enough to be all of
> them. Thanks to you, I can now understand why.
>
> Fix will be to stop ia64 from messing directly with cpu_online_map?
Yes, and the other architectures.
We're well within reach of removing cpu_*_map now I think.
Cheers,
Rusty.
On 02/14/2012 02:55 PM, Rusty Russell wrote:
> On Mon, 13 Feb 2012 13:57:45 -0800, Tony Luck <[email protected]> wrote:
>> On Mon, Feb 13, 2012 at 12:44 PM, Srivatsa S. Bhat
>> <[email protected]> wrote:
>>> IOW, what output do you see from the following printk from
>>> arch/ia64/kernel/smpboot.c?
>>>
>>> printk(KERN_INFO "Total of %d processors activated (%lu.%02lu BogoMIPS).\n",
>>> (int)num_online_cpus(), bogosum/(500000/HZ), (bogosum/(5000/HZ))%100);
>>
>> That is a complicated question - because linux-next also has patches
>> by Arjan that
>> change how (when) cpus are brought online. Initially I blamed his
>> patches and tried
>> reverting them ... and saw the symptom you are wondering about (message said
>> "Total of 1 processors", but the BogoMIPs was a number big enough to be all of
>> them. Thanks to you, I can now understand why.
>>
>> Fix will be to stop ia64 from messing directly with cpu_online_map?
>
> Yes, and the other architectures.
>
Right. And we should also ensure that nobody messes directly with
cpu_possible_map as well. I have written up a patch for ia64 (see below).
Sorry, I haven't even compile tested it - I neither have the toolchain nor the
hardware. I hope it works!
But don't expect the above printk statement to print the right value if you are
running a kernel which has the patch posted at https://lkml.org/lkml/2012/1/31/286
applied. That is another patch that can alter boot-up time and many related
things. So, it would be best to test Venki's patch + following fix without having
Arjan's patch applied.
---
From: Srivatsa S. Bhat <[email protected]>
Subject: [PATCH] ia64: Don't use cpu_set()/cpu_clear() over cpu_[online|possible]_map
Directly using cpu_set() and cpu_clear() on cpu_online_map or cpu_possible_map
is strongly discouraged. Use the functions set_cpu_online() and
set_cpu_possible() instead. This also means that the new implementation of
num_[online|possible]_cpus can track all changes to cpu_[online|possible]_mask
and hence give the correct results always.
Reported-by: Tony Luck <[email protected]>
Signed-off-by: Srivatsa S. Bhat <[email protected]>
---
0 files changed, 0 insertions(+), 0 deletions(-)
diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index cd57d73..4d1a550 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -486,7 +486,7 @@ mark_bsp_online (void)
{
#ifdef CONFIG_SMP
/* If we register an early console, allow CPU 0 to printk */
- cpu_set(smp_processor_id(), cpu_online_map);
+ set_cpu_online(smp_processor_id(), true);
#endif
}
diff --git a/arch/ia64/kernel/smp.c b/arch/ia64/kernel/smp.c
index 0bd537b..8551979 100644
--- a/arch/ia64/kernel/smp.c
+++ b/arch/ia64/kernel/smp.c
@@ -77,7 +77,7 @@ stop_this_cpu(void)
/*
* Remove this CPU:
*/
- cpu_clear(smp_processor_id(), cpu_online_map);
+ set_cpu_online(smp_processor_id(), false);
max_xtp();
local_irq_disable();
cpu_halt();
diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
index 5590979..35f6e3c 100644
--- a/arch/ia64/kernel/smpboot.c
+++ b/arch/ia64/kernel/smpboot.c
@@ -401,7 +401,7 @@ smp_callin (void)
/* Setup the per cpu irq handling data structures */
__setup_vector_irq(cpuid);
notify_cpu_starting(cpuid);
- cpu_set(cpuid, cpu_online_map);
+ set_cpu_online(cpuid, true);
per_cpu(cpu_state, cpuid) = CPU_ONLINE;
spin_unlock(&vector_lock);
ipi_call_unlock_irq();
@@ -548,7 +548,7 @@ do_rest:
if (!cpu_isset(cpu, cpu_callin_map)) {
printk(KERN_ERR "Processor 0x%x/0x%x is stuck.\n", cpu, sapicid);
ia64_cpu_to_sapicid[cpu] = -1;
- cpu_clear(cpu, cpu_online_map); /* was set in smp_callin() */
+ set_cpu_online(cpu, false); /* was set in smp_callin() */
return -EINVAL;
}
return 0;
@@ -609,7 +609,7 @@ smp_prepare_cpus (unsigned int max_cpus)
/*
* We have the boot CPU online for sure.
*/
- cpu_set(0, cpu_online_map);
+ set_cpu_online(0, true);
cpu_set(0, cpu_callin_map);
local_cpu_data->loops_per_jiffy = loops_per_jiffy;
@@ -633,7 +633,7 @@ smp_prepare_cpus (unsigned int max_cpus)
void __devinit smp_prepare_boot_cpu(void)
{
- cpu_set(smp_processor_id(), cpu_online_map);
+ set_cpu_online(smp_processor_id(), true);
cpu_set(smp_processor_id(), cpu_callin_map);
set_numa_node(cpu_to_node_map[smp_processor_id()]);
per_cpu(cpu_state, smp_processor_id()) = CPU_ONLINE;
@@ -732,10 +732,10 @@ int __cpu_disable(void)
return -EBUSY;
}
- cpu_clear(cpu, cpu_online_map);
+ set_cpu_online(cpu, false);
if (migrate_platform_irqs(cpu)) {
- cpu_set(cpu, cpu_online_map);
+ set_cpu_online(cpu, true);
return -EBUSY;
}
> Yes, and the other architectures.
Here are the patches for other instances I see in plain git grep.
I have been brave (foolish) enough to send this without any testing. So,
this comes with 'use it at your own risk' tag :-).
Thanks,
Venki
Use set_cpu_possible instead.
Signed-off-by: Venkatesh Pallipadi <[email protected]>
---
arch/hexagon/kernel/smp.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/hexagon/kernel/smp.c b/arch/hexagon/kernel/smp.c
index c871a2c..8962705 100644
--- a/arch/hexagon/kernel/smp.c
+++ b/arch/hexagon/kernel/smp.c
@@ -272,5 +272,5 @@ void smp_start_cpus(void)
int i;
for (i = 0; i < NR_CPUS; i++)
- cpu_set(i, cpu_possible_map);
+ set_cpu_possible(i, true);
}
--
1.7.7.3
Use set_cpu_* and init_cpu_* variants instead.
Signed-off-by: Venkatesh Pallipadi <[email protected]>
---
arch/mips/cavium-octeon/smp.c | 2 +-
arch/mips/kernel/smp.c | 4 ++--
arch/mips/netlogic/xlr/smp.c | 4 ++--
arch/mips/pmc-sierra/yosemite/smp.c | 4 ++--
arch/mips/sgi-ip27/ip27-smp.c | 2 +-
arch/mips/sibyte/bcm1480/smp.c | 5 ++---
arch/mips/sibyte/sb1250/smp.c | 5 ++---
7 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c
index efcfff4..5cce09c 100644
--- a/arch/mips/cavium-octeon/smp.c
+++ b/arch/mips/cavium-octeon/smp.c
@@ -268,7 +268,7 @@ static int octeon_cpu_disable(void)
spin_lock(&smp_reserve_lock);
- cpu_clear(cpu, cpu_online_map);
+ set_cpu_online(cpu, false);
cpu_clear(cpu, cpu_callin_map);
local_irq_disable();
fixup_irqs();
diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index 32c1e95..28777ff 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -148,7 +148,7 @@ static void stop_this_cpu(void *dummy)
/*
* Remove this CPU:
*/
- cpu_clear(smp_processor_id(), cpu_online_map);
+ set_cpu_online(smp_processor_id(), false);
for (;;) {
if (cpu_wait)
(*cpu_wait)(); /* Wait if available. */
@@ -248,7 +248,7 @@ int __cpuinit __cpu_up(unsigned int cpu)
while (!cpu_isset(cpu, cpu_callin_map))
udelay(100);
- cpu_set(cpu, cpu_online_map);
+ set_cpu_online(cpu, true);
return 0;
}
diff --git a/arch/mips/netlogic/xlr/smp.c b/arch/mips/netlogic/xlr/smp.c
index 080284d..8084221 100644
--- a/arch/mips/netlogic/xlr/smp.c
+++ b/arch/mips/netlogic/xlr/smp.c
@@ -154,7 +154,7 @@ void __init nlm_smp_setup(void)
cpu_set(boot_cpu, phys_cpu_present_map);
__cpu_number_map[boot_cpu] = 0;
__cpu_logical_map[0] = boot_cpu;
- cpu_set(0, cpu_possible_map);
+ set_cpu_possible(0, true);
num_cpus = 1;
for (i = 0; i < NR_CPUS; i++) {
@@ -166,7 +166,7 @@ void __init nlm_smp_setup(void)
cpu_set(i, phys_cpu_present_map);
__cpu_number_map[i] = num_cpus;
__cpu_logical_map[num_cpus] = i;
- cpu_set(num_cpus, cpu_possible_map);
+ set_cpu_possible(num_cpus, true);
++num_cpus;
}
}
diff --git a/arch/mips/pmc-sierra/yosemite/smp.c b/arch/mips/pmc-sierra/yosemite/smp.c
index 2608752..b2b23eb 100644
--- a/arch/mips/pmc-sierra/yosemite/smp.c
+++ b/arch/mips/pmc-sierra/yosemite/smp.c
@@ -155,10 +155,10 @@ static void __init yos_smp_setup(void)
{
int i;
- cpus_clear(cpu_possible_map);
+ init_cpu_possible(cpumask_of(0));
for (i = 0; i < 2; i++) {
- cpu_set(i, cpu_possible_map);
+ set_cpu_possible(i, true);
__cpu_number_map[i] = i;
__cpu_logical_map[i] = i;
}
diff --git a/arch/mips/sgi-ip27/ip27-smp.c b/arch/mips/sgi-ip27/ip27-smp.c
index c6851df..735b43b 100644
--- a/arch/mips/sgi-ip27/ip27-smp.c
+++ b/arch/mips/sgi-ip27/ip27-smp.c
@@ -76,7 +76,7 @@ static int do_cpumask(cnodeid_t cnode, nasid_t nasid, int highest)
/* Only let it join in if it's marked enabled */
if ((acpu->cpu_info.flags & KLINFO_ENABLE) &&
(tot_cpus_found != NR_CPUS)) {
- cpu_set(cpuid, cpu_possible_map);
+ set_cpu_possible(cpuid, true);
alloc_cpupda(cpuid, tot_cpus_found);
cpus_found++;
tot_cpus_found++;
diff --git a/arch/mips/sibyte/bcm1480/smp.c b/arch/mips/sibyte/bcm1480/smp.c
index d667875..63d2211 100644
--- a/arch/mips/sibyte/bcm1480/smp.c
+++ b/arch/mips/sibyte/bcm1480/smp.c
@@ -147,14 +147,13 @@ static void __init bcm1480_smp_setup(void)
{
int i, num;
- cpus_clear(cpu_possible_map);
- cpu_set(0, cpu_possible_map);
+ init_cpu_possible(cpumask_of(0));
__cpu_number_map[0] = 0;
__cpu_logical_map[0] = 0;
for (i = 1, num = 0; i < NR_CPUS; i++) {
if (cfe_cpu_stop(i) == 0) {
- cpu_set(i, cpu_possible_map);
+ set_cpu_possible(i, true);
__cpu_number_map[i] = ++num;
__cpu_logical_map[num] = i;
}
diff --git a/arch/mips/sibyte/sb1250/smp.c b/arch/mips/sibyte/sb1250/smp.c
index 38e7f6b..77f0df5 100644
--- a/arch/mips/sibyte/sb1250/smp.c
+++ b/arch/mips/sibyte/sb1250/smp.c
@@ -135,14 +135,13 @@ static void __init sb1250_smp_setup(void)
{
int i, num;
- cpus_clear(cpu_possible_map);
- cpu_set(0, cpu_possible_map);
+ init_cpu_possible(cpumask_of(0));
__cpu_number_map[0] = 0;
__cpu_logical_map[0] = 0;
for (i = 1, num = 0; i < NR_CPUS; i++) {
if (cfe_cpu_stop(i) == 0) {
- cpu_set(i, cpu_possible_map);
+ set_cpu_possible(i, true);
__cpu_number_map[i] = ++num;
__cpu_logical_map[num] = i;
}
--
1.7.7.3
Use init_cpu_online and set_cpu_online instead.
Signed-off-by: Venkatesh Pallipadi <[email protected]>
---
arch/um/kernel/skas/process.c | 2 +-
arch/um/kernel/smp.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/um/kernel/skas/process.c b/arch/um/kernel/skas/process.c
index 2e9852c..5daa0f5 100644
--- a/arch/um/kernel/skas/process.c
+++ b/arch/um/kernel/skas/process.c
@@ -41,7 +41,7 @@ static int __init start_kernel_proc(void *unused)
cpu_tasks[0].pid = pid;
cpu_tasks[0].task = current;
#ifdef CONFIG_SMP
- cpu_online_map = cpumask_of_cpu(0);
+ init_cpu_online(cpumask_of(0));
#endif
start_kernel();
return 0;
diff --git a/arch/um/kernel/smp.c b/arch/um/kernel/smp.c
index 155206a..b5d2ca9 100644
--- a/arch/um/kernel/smp.c
+++ b/arch/um/kernel/smp.c
@@ -76,7 +76,7 @@ static int idle_proc(void *cpup)
cpu_relax();
notify_cpu_starting(cpu);
- cpu_set(cpu, cpu_online_map);
+ set_cpu_online(cpu, true);
default_idle();
return 0;
}
@@ -110,8 +110,8 @@ void smp_prepare_cpus(unsigned int maxcpus)
for (i = 0; i < ncpus; ++i)
set_cpu_possible(i, true);
- cpu_clear(me, cpu_online_map);
- cpu_set(me, cpu_online_map);
+ set_cpu_online(me, false);
+ set_cpu_online(me, true);
cpu_set(me, cpu_callin_map);
err = os_pipe(cpu_data[me].ipi_pipe, 1, 1);
@@ -138,7 +138,7 @@ void smp_prepare_cpus(unsigned int maxcpus)
void smp_prepare_boot_cpu(void)
{
- cpu_set(smp_processor_id(), cpu_online_map);
+ set_cpu_online(smp_processor_id(), true);
}
int __cpu_up(unsigned int cpu)
--
1.7.7.3
On Tue, Feb 14, 2012 at 1:35 PM, Srivatsa S. Bhat
<[email protected]> wrote:
> Right. And we should also ensure that nobody messes directly with
> cpu_possible_map as well. I have written up a patch for ia64 (see below).
> Sorry, I haven't even compile tested it - I neither have the toolchain nor the
> hardware. I hope it works!
Thanks for doing this - compiles and seems to work.
Tested-by: Tony Luck <[email protected]>
Can we get this added to the series - so it gets applied along with
Venki's patch.
> ?0 files changed, 0 insertions(+), 0 deletions(-)
I think your patch generation script needs some attention - I see
arch/ia64/kernel/setup.c | 2 +-
arch/ia64/kernel/smp.c | 2 +-
arch/ia64/kernel/smpboot.c | 12 ++++++------
3 files changed, 8 insertions(+), 8 deletions(-)
> @@ -609,7 +609,7 @@ smp_prepare_cpus (unsigned int max_cpus)
> ? ? ? ?/*
> ? ? ? ? * We have the boot CPU online for sure.
> ? ? ? ? */
> - ? ? ? cpu_set(0, cpu_online_map);
> + ? ? ? set_cpu_online(0, true);
> ? ? ? ?cpu_set(0, cpu_callin_map);
>
> ? ? ? ?local_cpu_data->loops_per_jiffy = loops_per_jiffy;
Generic code has already marked cpu 0 online ... so this one could
just be dropped (and the preceding comment too). Though it does no
harm to set it again.
-Tony
On 02/14/2012 02:49 PM, Venkatesh Pallipadi wrote:
> Use set_cpu_* and init_cpu_* variants instead.
>
> Signed-off-by: Venkatesh Pallipadi<[email protected]>
I came up with the same thing, so...
Acked-by: David Daney <[email protected]>
Ralf: If you too were to Acknowledge the patch, we might get it merged.
David Daney
> ---
> arch/mips/cavium-octeon/smp.c | 2 +-
> arch/mips/kernel/smp.c | 4 ++--
> arch/mips/netlogic/xlr/smp.c | 4 ++--
> arch/mips/pmc-sierra/yosemite/smp.c | 4 ++--
> arch/mips/sgi-ip27/ip27-smp.c | 2 +-
> arch/mips/sibyte/bcm1480/smp.c | 5 ++---
> arch/mips/sibyte/sb1250/smp.c | 5 ++---
> 7 files changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c
> index efcfff4..5cce09c 100644
> --- a/arch/mips/cavium-octeon/smp.c
> +++ b/arch/mips/cavium-octeon/smp.c
> @@ -268,7 +268,7 @@ static int octeon_cpu_disable(void)
>
> spin_lock(&smp_reserve_lock);
>
> - cpu_clear(cpu, cpu_online_map);
> + set_cpu_online(cpu, false);
> cpu_clear(cpu, cpu_callin_map);
> local_irq_disable();
> fixup_irqs();
> diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
> index 32c1e95..28777ff 100644
> --- a/arch/mips/kernel/smp.c
> +++ b/arch/mips/kernel/smp.c
> @@ -148,7 +148,7 @@ static void stop_this_cpu(void *dummy)
> /*
> * Remove this CPU:
> */
> - cpu_clear(smp_processor_id(), cpu_online_map);
> + set_cpu_online(smp_processor_id(), false);
> for (;;) {
> if (cpu_wait)
> (*cpu_wait)(); /* Wait if available. */
> @@ -248,7 +248,7 @@ int __cpuinit __cpu_up(unsigned int cpu)
> while (!cpu_isset(cpu, cpu_callin_map))
> udelay(100);
>
> - cpu_set(cpu, cpu_online_map);
> + set_cpu_online(cpu, true);
>
> return 0;
> }
> diff --git a/arch/mips/netlogic/xlr/smp.c b/arch/mips/netlogic/xlr/smp.c
> index 080284d..8084221 100644
> --- a/arch/mips/netlogic/xlr/smp.c
> +++ b/arch/mips/netlogic/xlr/smp.c
> @@ -154,7 +154,7 @@ void __init nlm_smp_setup(void)
> cpu_set(boot_cpu, phys_cpu_present_map);
> __cpu_number_map[boot_cpu] = 0;
> __cpu_logical_map[0] = boot_cpu;
> - cpu_set(0, cpu_possible_map);
> + set_cpu_possible(0, true);
>
> num_cpus = 1;
> for (i = 0; i< NR_CPUS; i++) {
> @@ -166,7 +166,7 @@ void __init nlm_smp_setup(void)
> cpu_set(i, phys_cpu_present_map);
> __cpu_number_map[i] = num_cpus;
> __cpu_logical_map[num_cpus] = i;
> - cpu_set(num_cpus, cpu_possible_map);
> + set_cpu_possible(num_cpus, true);
> ++num_cpus;
> }
> }
> diff --git a/arch/mips/pmc-sierra/yosemite/smp.c b/arch/mips/pmc-sierra/yosemite/smp.c
> index 2608752..b2b23eb 100644
> --- a/arch/mips/pmc-sierra/yosemite/smp.c
> +++ b/arch/mips/pmc-sierra/yosemite/smp.c
> @@ -155,10 +155,10 @@ static void __init yos_smp_setup(void)
> {
> int i;
>
> - cpus_clear(cpu_possible_map);
> + init_cpu_possible(cpumask_of(0));
>
> for (i = 0; i< 2; i++) {
> - cpu_set(i, cpu_possible_map);
> + set_cpu_possible(i, true);
> __cpu_number_map[i] = i;
> __cpu_logical_map[i] = i;
> }
> diff --git a/arch/mips/sgi-ip27/ip27-smp.c b/arch/mips/sgi-ip27/ip27-smp.c
> index c6851df..735b43b 100644
> --- a/arch/mips/sgi-ip27/ip27-smp.c
> +++ b/arch/mips/sgi-ip27/ip27-smp.c
> @@ -76,7 +76,7 @@ static int do_cpumask(cnodeid_t cnode, nasid_t nasid, int highest)
> /* Only let it join in if it's marked enabled */
> if ((acpu->cpu_info.flags& KLINFO_ENABLE)&&
> (tot_cpus_found != NR_CPUS)) {
> - cpu_set(cpuid, cpu_possible_map);
> + set_cpu_possible(cpuid, true);
> alloc_cpupda(cpuid, tot_cpus_found);
> cpus_found++;
> tot_cpus_found++;
> diff --git a/arch/mips/sibyte/bcm1480/smp.c b/arch/mips/sibyte/bcm1480/smp.c
> index d667875..63d2211 100644
> --- a/arch/mips/sibyte/bcm1480/smp.c
> +++ b/arch/mips/sibyte/bcm1480/smp.c
> @@ -147,14 +147,13 @@ static void __init bcm1480_smp_setup(void)
> {
> int i, num;
>
> - cpus_clear(cpu_possible_map);
> - cpu_set(0, cpu_possible_map);
> + init_cpu_possible(cpumask_of(0));
> __cpu_number_map[0] = 0;
> __cpu_logical_map[0] = 0;
>
> for (i = 1, num = 0; i< NR_CPUS; i++) {
> if (cfe_cpu_stop(i) == 0) {
> - cpu_set(i, cpu_possible_map);
> + set_cpu_possible(i, true);
> __cpu_number_map[i] = ++num;
> __cpu_logical_map[num] = i;
> }
> diff --git a/arch/mips/sibyte/sb1250/smp.c b/arch/mips/sibyte/sb1250/smp.c
> index 38e7f6b..77f0df5 100644
> --- a/arch/mips/sibyte/sb1250/smp.c
> +++ b/arch/mips/sibyte/sb1250/smp.c
> @@ -135,14 +135,13 @@ static void __init sb1250_smp_setup(void)
> {
> int i, num;
>
> - cpus_clear(cpu_possible_map);
> - cpu_set(0, cpu_possible_map);
> + init_cpu_possible(cpumask_of(0));
> __cpu_number_map[0] = 0;
> __cpu_logical_map[0] = 0;
>
> for (i = 1, num = 0; i< NR_CPUS; i++) {
> if (cfe_cpu_stop(i) == 0) {
> - cpu_set(i, cpu_possible_map);
> + set_cpu_possible(i, true);
> __cpu_number_map[i] = ++num;
> __cpu_logical_map[num] = i;
> }