[3/3] mach-u300: cleanup clockevent code

Commit Message

Thomas Gleixner June 1, 2011, 10:03 a.m. UTC

On Wed, 1 Jun 2011, Russell King - ARM Linux wrote:

> On Wed, Jun 01, 2011 at 10:34:46AM +0200, Thomas Gleixner wrote:
> > But yeah, we can make it conditional for SMP and simply set
> > cpumask_of(0) in the UP case.
> 
> Or cpumask_of(smp_processor_id()) which would also cover the non-CPU0 boot
> cases (provided its called on the boot CPU.)

Something like the below should work for both UP and SMP

Comments

Stephen Boyd June 2, 2011, 1:51 a.m. UTC | #1

On 06/01/2011 03:03 AM, Thomas Gleixner wrote:
> Something like the below should work for both UP and SMP
>
> diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
> index c027d4f..739801b 100644
> --- a/kernel/time/clockevents.c
> +++ b/kernel/time/clockevents.c
> @@ -173,6 +173,12 @@ static void clockevents_notify_released(void)
>  	}
>  }
>  
> +#ifdef CONFIG_SMP
> +# define CPU_MASK_CHECK(x)		WARN_ON(!x)
> +#else
> +# define CPU_MASK_CHECK(x)		(!x)
> +#endif
> +
>  /**
>   * clockevents_register_device - register a clock event device
>   * @dev:	device to register
> @@ -182,7 +188,8 @@ void clockevents_register_device(struct clock_event_device *dev)
>  	unsigned long flags;
>  
>  	BUG_ON(dev->mode != CLOCK_EVT_MODE_UNUSED);
> -	BUG_ON(!dev->cpumask);
> +	if (CPU_MASK_CHECK(dev->cpumask))
> +		dev->cpumask = cpumask_of(smp_processor_id());
>  
>  	raw_spin_lock_irqsave(&clockevents_lock, flags);

Won't this print a big WARNING on SMP_ON_UP=y and is_smp() == false
kernels? Is there a generic cross-architecture way to check for SMP at
runtime?

Also, I don't understand the original motivation for this change. The
assignment to cpumask was there in the u300 timer code already so the
commit text from Linus is a bit misleading/confusing.

Thomas Gleixner June 2, 2011, 9:35 a.m. UTC | #2

On Wed, 1 Jun 2011, Stephen Boyd wrote:
> On 06/01/2011 03:03 AM, Thomas Gleixner wrote:
> > Something like the below should work for both UP and SMP
> >
> > diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
> > index c027d4f..739801b 100644
> > --- a/kernel/time/clockevents.c
> > +++ b/kernel/time/clockevents.c
> > @@ -173,6 +173,12 @@ static void clockevents_notify_released(void)
> >  	}
> >  }
> >  
> > +#ifdef CONFIG_SMP
> > +# define CPU_MASK_CHECK(x)		WARN_ON(!x)
> > +#else
> > +# define CPU_MASK_CHECK(x)		(!x)
> > +#endif
> > +
> >  /**
> >   * clockevents_register_device - register a clock event device
> >   * @dev:	device to register
> > @@ -182,7 +188,8 @@ void clockevents_register_device(struct clock_event_device *dev)
> >  	unsigned long flags;
> >  
> >  	BUG_ON(dev->mode != CLOCK_EVT_MODE_UNUSED);
> > -	BUG_ON(!dev->cpumask);
> > +	if (CPU_MASK_CHECK(dev->cpumask))
> > +		dev->cpumask = cpumask_of(smp_processor_id());
> >  
> >  	raw_spin_lock_irqsave(&clockevents_lock, flags);
> 
> Won't this print a big WARNING on SMP_ON_UP=y and is_smp() == false
> kernels? Is there a generic cross-architecture way to check for SMP at

Need to check that.

> runtime?
> 
> Also, I don't understand the original motivation for this change. The
> assignment to cpumask was there in the u300 timer code already so the
> commit text from Linus is a bit misleading/confusing.

The point is that we can remove these assignements for UP all over the
place and just do it in the core code.

Patch

diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index c027d4f..739801b 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -173,6 +173,12 @@  static void clockevents_notify_released(void)
 	}
 }
 
+#ifdef CONFIG_SMP
+# define CPU_MASK_CHECK(x)		WARN_ON(!x)
+#else
+# define CPU_MASK_CHECK(x)		(!x)
+#endif
+
 /**
  * clockevents_register_device - register a clock event device
  * @dev:	device to register
@@ -182,7 +188,8 @@  void clockevents_register_device(struct clock_event_device *dev)
 	unsigned long flags;
 
 	BUG_ON(dev->mode != CLOCK_EVT_MODE_UNUSED);
-	BUG_ON(!dev->cpumask);
+	if (CPU_MASK_CHECK(dev->cpumask))
+		dev->cpumask = cpumask_of(smp_processor_id());
 
 	raw_spin_lock_irqsave(&clockevents_lock, flags);