Re: [PATCH] Enforce valid shift values in clocksource_register()

[Thomas Gleixner]

On Wed, 26 Nov 2008, John Stultz wrote:
> On Thu, 2008-11-27 at 01:08 +0100, Thomas Gleixner wrote:
> > On Wed, 26 Nov 2008, John Stultz wrote:
> > > +	if (c->shift >= 32) {
> > > +		printk(KERN_WARNING "===============================\n");
> > > +		printk(KERN_WARNING "WARNING: Cannot register %s clocksource\n",
> > > +					c->name);
> > > +		printk(KERN_WARNING "The shift value must be less then 32\n");
> > > +		printk(KERN_WARNING "===============================\n");
> > > +		return -EINVAL;
> > 
> > Just setting the shift value to 31 along with a WARN_ON() should be
> > enough. We don't need to kill the clocksource in that case, as this
> > can be nasty when it happens in the early boot code where we dont have
> > any output.
> 
> Well, we can't tweak the shift value without changing the mult. And that
> seemed a bit overreaching (but might be safe). I'll give it a shot.
> 
> Also early boot we should have the jiffies clocksource around, so unless
> I'm forgetting something, I don't think it has early boot concerns.

Fair enough. I forgot that we do a late handover from jiffies to the
real clocksource. Still a WARN_ON is more prominent and more useful
for developers to get to the root cause. Setting the shift to 31 will
result in wrong timer values but not brick the box. So espcially for
code, which calculates the values this might be worthwhile.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/