Messages in this thread | | | Date | Tue, 22 Jan 2013 17:54:21 -0800 | From | John Stultz <> | Subject | Re: [RFC PATCH 0/5] Add support for S3 non-stop TSC support. |
| |
On 01/22/2013 05:37 PM, Jason Gunthorpe wrote: > On Tue, Jan 22, 2013 at 04:41:58PM -0800, John Stultz wrote: > >> Right but to calculate an suspend interval (since they are likely >> many orders of magnitude larger then the intervals between timer >> interrupts), you need different mult/shift selection. Its splitting >> out the mult/shift management into a per-subsystem level that is the > You are talking about overflow in cyclecounter_cyc2ns and the like > right? The 64 bit cycle_t and the underlying hw counter (eg 64 bit > rdtsc) are not going to overflow.. > > An alternate version of cyclecounter_cyc2ns for use by the suspend > code that handles overflow during the mult/shift operation solves that > problem: > > // Drops some small precision along the way but is simple.. > static inline u64 cyclecounter_cyc2ns_128(const struct cyclecounter *cc, > cycle_t cycles) > { > u64 max = U64_MAX/cc->mult; > u64 num = cycles/max; > u64 result = num * ((max * cc->mult) >> cc->shift); > return result + cyclecounter_cyc2ns(cc, cycles - num*cc->mult); > } > > Or am I missing the issue?
Well, cyclecounters and clocksources are currently different things. There was some hope that cyclecounters would be a simpler base structure that would supersede clocksources, but the complexity of all the variants of clocksources have limited the ability to make such a conversion. At least so far. I hope to eventually clean that up as the potential overlap is obvious - although as the cyclecounters/timecounters code never grew as I expected. But I'm not sure how soon "eventually" will end up being.
But regardless of historical tangents :), you're right, an alternate and slower cyc2ns function could be used to avoid overflow issues.
> >> complicated part. Additionally, there may be cases where the >> timekeeping clocksource is one clocksource and the suspend >> clocksource is another. So I think to properly integrate this sort > Does the difference matter? The clocksource to use is detected at > runtime, if the timekeeping clocksource is no good for suspend time > keeping then it just won't be used. With a distinct > read_persistent_clock API then they are already seperate??
Not sure I'm following you here. I still think the selection of which clocksource to use for suspend timing is problematic (especially if its not the active timekeeping clocksource). So I think instead of complicating the generic timekeeping code with the logic, I'd rather push it off to new read_presistent_clock api.
thanks -john
| |