Avoid checking for cpu gone when CONFIG_HOTPLUG_CPU not defined

Avoid checking for cpu gone when CONFIG_HOTPLUG_CPU not defined

[Keshavamurthy, Anil S]

Subject: Avoid checking for cpu gone when CONFIG_HOTPLUG_CPU not defined

Avoid checking for cpu gone in mm hot path when
CONFIG_HOTPLUG_CPU is not defined.

Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>

---
 arch/i386/kernel/smp.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Index: work/arch/i386/kernel/smp.c
===================================================================
--- work.orig/arch/i386/kernel/smp.c
+++ work/arch/i386/kernel/smp.c
@@ -365,10 +365,12 @@ static void flush_tlb_others(cpumask_t c
 	BUG_ON(cpu_isset(smp_processor_id(), cpumask));
 	BUG_ON(!mm);
 
+#ifdef CONFIG_HOTPLUG_CPU
 	/* If a CPU which we ran on has gone down, OK. */
 	cpus_and(cpumask, cpumask, cpu_online_map);
-	if (cpus_empty(cpumask))
+	if (unlikely(cpus_empty(cpumask)))
 		return;
+#endif
 
 	/*
 	 * i'm not happy about this global shared spinlock in the

Re: Avoid checking for cpu gone when CONFIG_HOTPLUG_CPU not defined

[Andrew Morton]

On Fri, 6 Apr 2007 14:41:50 -0700 "Keshavamurthy, Anil S" <anil.s.keshavamurthy@intel.com> wrote:

> Subject: Avoid checking for cpu gone when CONFIG_HOTPLUG_CPU not defined
> 
> Avoid checking for cpu gone in mm hot path when
> CONFIG_HOTPLUG_CPU is not defined.
> 
> Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> 
> ---
>  arch/i386/kernel/smp.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> Index: work/arch/i386/kernel/smp.c
> ===================================================================
> --- work.orig/arch/i386/kernel/smp.c
> +++ work/arch/i386/kernel/smp.c
> @@ -365,10 +365,12 @@ static void flush_tlb_others(cpumask_t c
>  	BUG_ON(cpu_isset(smp_processor_id(), cpumask));
>  	BUG_ON(!mm);
>  
> +#ifdef CONFIG_HOTPLUG_CPU
>  	/* If a CPU which we ran on has gone down, OK. */
>  	cpus_and(cpumask, cpumask, cpu_online_map);
> -	if (cpus_empty(cpumask))
> +	if (unlikely(cpus_empty(cpumask)))
>  		return;
> +#endif
>  
>  	/*
>  	 * i'm not happy about this global shared spinlock in the

Fair enough.

The code you're touching in with the original CPU-hotplug-for-i386 patches.

x86_64 doesn't do it.  It handles tlb flushing differently anyway.  But I
suspect that x86_64 is just buggy, unless all callers of flush_tlb_others()
have taken care to disable preemption prior to their calculation of the
passed-in cpumask.

Shudder.  Gautham, this is code which we can cheerfully delete when we get
the freezer stuff done.  Fortunately, Anil's patch will make it nice and
easy to find again.

Re: Avoid checking for cpu gone when CONFIG_HOTPLUG_CPU not defined

[Gautham R Shenoy]

On Fri, Apr 06, 2007 at 11:25:00PM -0700, Andrew Morton wrote:
> On Fri, 6 Apr 2007 14:41:50 -0700 "Keshavamurthy, Anil S" <anil.s.keshavamurthy@intel.com> wrote:
> 
> > Subject: Avoid checking for cpu gone when CONFIG_HOTPLUG_CPU not defined
> > 
> > Avoid checking for cpu gone in mm hot path when
> > CONFIG_HOTPLUG_CPU is not defined.
> > 
> > Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> > 
> > ---
> >  arch/i386/kernel/smp.c |    4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > Index: work/arch/i386/kernel/smp.c
> > ===================================================================
> > --- work.orig/arch/i386/kernel/smp.c
> > +++ work/arch/i386/kernel/smp.c
> > @@ -365,10 +365,12 @@ static void flush_tlb_others(cpumask_t c
> >  	BUG_ON(cpu_isset(smp_processor_id(), cpumask));
> >  	BUG_ON(!mm);
> >  
> > +#ifdef CONFIG_HOTPLUG_CPU
> >  	/* If a CPU which we ran on has gone down, OK. */
> >  	cpus_and(cpumask, cpumask, cpu_online_map);
> > -	if (cpus_empty(cpumask))
> > +	if (unlikely(cpus_empty(cpumask)))
> >  		return;
> > +#endif
> >  
> >  	/*
> >  	 * i'm not happy about this global shared spinlock in the
> 
> Fair enough.
> 
> The code you're touching in with the original CPU-hotplug-for-i386 patches.
> 
> x86_64 doesn't do it.  It handles tlb flushing differently anyway.  But I
> suspect that x86_64 is just buggy, unless all callers of flush_tlb_others()
> have taken care to disable preemption prior to their calculation of the
> passed-in cpumask.
> 
> Shudder.  Gautham, this is code which we can cheerfully delete when we get
> the freezer stuff done.  Fortunately, Anil's patch will make it nice and
> easy to find again.

Ok, I will make a note of this one.

If the IO-test results are good, I hope to post the patchset sometime
this week.


Thanks and Regards
gautham.

-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"