Re: [RFC][PATCH] new timeofday core subsystem (v. A3)

[Matt Mackall <mpm@selenic.com>]

On Mon, Mar 14, 2005 at 10:42:45AM -0800, john stultz wrote:
> 
> > > +static inline cycle_t read_timesource(struct timesource_t* ts)
> > > +{
> > > +	switch (ts->type) {
> > > +	case TIMESOURCE_MMIO_32:
> > > +		return (cycle_t)readl(ts->mmio_ptr);
> > > +	case TIMESOURCE_MMIO_64:
> > > +		return (cycle_t)readq(ts->mmio_ptr);
> > > +	case TIMESOURCE_CYCLES:
> > > +		return (cycle_t)get_cycles();
> > > +	default:/* case: TIMESOURCE_FUNCTION */
> > > +		return ts->read_fnct();
> > > +	}
> > > +}
> > 
> > Wouldn't it be better to change read_fnct to take a timesource * and
> > then change all the other guys to generic_timesource_<foo> helper
> > functions? This does away with the switch and makes it trivial to add
> > new generic sources. Change mmio_ptr to void *private.
> 
> Not sure if I totally understand this, but originally I just had a read
> function, but to allow this framework to function w/ ia64 fsyscalls (and
> likely other arches vsyscalls) we need to pass the raw mmio pointers.
> Thus the timesource type and switch idea was taken from the time
> interpolator code.

Well for vsyscall, we can leave the mmio_ptr and type. But in-kernel,
I think we'd rather always call read_fnct with generic helpers than hit this
switch every time.

> > > +	if (time_suspend_state != TIME_RUNNING) {
> > > +		printk(KERN_INFO "timeofday_suspend_hook: ACK! called while we're suspended!");
> > 
> > Line length. Perhaps BUG_ON instead.
> 
> Eh, its not fatal to BUG_ON seems a bit harsh. I'll fix the line length
> though. 

Well there's a trade-off here. If it's something that should never
happen and you only printk, you may never get a failure report
(especially at KERN_INFO). It's good to be accomodating of external
errors, but catching internal should-never-happen errors is important.

> > Excellent question. 
> 
> Indeed.  Currently jiffies is used as both a interrupt counter and a
> time unit, and I'm trying make it just the former. If I emulate it then
> it stops functioning as a interrupt counter, and if I don't then I'll
> probably break assumptions about jiffies being a time unit. So I'm not
> sure which is the easiest path to go until all the users of jiffies are
> audited for intent. 

Post this as a separate thread. There are various thoughts floating
around on this already.

-- 
Mathematics is the supreme nostalgia of our time.