Messages in this thread | | | Subject | Re: [RFC][PATCH] new timeofday core subsystem (v. A3) | From | john stultz <> | Date | Mon, 14 Mar 2005 12:04:07 -0800 |
| |
On Mon, 2005-03-14 at 11:51 -0800, Matt Mackall wrote: > On Mon, Mar 14, 2005 at 11:43:21AM -0800, john stultz wrote: > > On Mon, 2005-03-14 at 11:29 -0800, Matt Mackall wrote: > > > 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. > > > > Huh. So if I understand you properly, all timesources should have valid > > read_fnct pointers that return the cycle value, however we'll still > > preserve the type and mmio_ptr so fsyscall/vsyscall bits can use them > > externally? > > Well where we'd read an MMIO address, we'd simply set read_fnct to > generic_timesource_mmio32 or so. And that function just does the read. > So both that function and read_timesource become one-liners and we > drop the conditional branches in the switch.
However the vsyscall/fsyscall bits cannot call in-kernel functions (as they execute in userspace or a sudo-userspace). As it stands now in my design TIMESOURCE_FUNCTION timesources will not be usable for vsyscall/fsyscall implementations, so I'm not sure if that's doable.
I'd be interested you've got a way around that.
thanks -john
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |