On Fri, Feb 13, 2015 at 10:03 AM, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: > On Friday, February 13, 2015 08:53:38 AM John Stultz wrote: >> On Wed, Feb 11, 2015 at 12:03 PM, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: >> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> >> > >> > Theoretically, ktime_get_mono_fast_ns() may be executed after >> > timekeeping has been suspended (or before it is resumed) which >> > in turn may lead to undefined behavior, for example, when the >> > clocksource read from timekeeping_get_ns() called by it is >> > not accessible at that time. >> >> And the callers of the ktime_get_mono_fast_ns() have to get back a >> value? > > Yes, they do. > >> Or can we return an error on timekeeping_suspended like we do >> w/ __getnstimeofday64()? > > No, we can't. > >> Also, what exactly is the case when the clocksource being read isn't >> accessible? I see this is conditionalized on >> CLOCK_SOURCE_SUSPEND_NONSTOP, so is the concern on resume we read the >> clocksource and its been reset causing a crazy time value? > > The clocksource's ->suspend method may have been called (during suspend) > and depending on what that did we may even crash things theoretically. > > During resume, before the clocksource's ->resume callback, it may just > be undefined behavior (random data etc). > > For system suspend as we have today the window is quite narrow, but after > patch [4/6] from this series suspend-to-idle may suspend timekeeping and > just sit there in idle for extended time (hours even) which broadens the > potential exposure quite a bit. > > Of course, it does that with interrupts disabled, but ktime_get_mono_fast_ns() > is for NMI, so theoretically, if an NMI happens while we're in suspend-to-idle > with timekeeping suspended and the clocksource is not CLOCK_SOURCE_SUSPEND_NONSTOP > and the NMI calls ktime_get_mono_fast_ns(), strange and undesirable things may > happen. Ok.. No objection to the approach then. But maybe could you wrap the new logic in a halt_fast_timekeeper() function? Also is there much value in not halting it for SUSPEND_NONSTOP clocksources? If not, might as well halt it in all cases just to simplify the conditions we have to keep track of in our heads. :) thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
