Messages in this thread | | | Date | Mon, 16 Jul 2018 22:50:38 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH v4] time: Fix extra sleeptime injection when suspend fails |
| |
On Tue, 17 Jul 2018, Mukesh Ojha wrote: > @@ -102,7 +102,7 @@ static int rtc_resume(struct device *dev) > struct timespec64 sleep_time; > int err; > > - if (timekeeping_rtc_skipresume()) > + if (!timekeeping_rtc_skipresume()) > return 0;
That does not make any sense at all, really.
> /* Flag for if there is a persistent clock on this platform */ > static bool persistent_clock_exists; > @@ -1610,7 +1622,7 @@ static void __timekeeping_inject_sleeptime(struct timekeeper *tk, > */ > bool timekeeping_rtc_skipresume(void) > { > - return sleeptime_injected; > + return suspend_timing_needed;
Just make this !suspend_timing_needed and the function name and its return value still makes sense.
> @@ -1701,13 +1714,13 @@ void timekeeping_resume(void) > tk->tkr_mono.mask); > nsec = mul_u64_u32_shr(cyc_delta, clock->mult, clock->shift); > ts_delta = ns_to_timespec64(nsec); > - sleeptime_injected = true; > + suspend_timing_needed = false; > } else if (timespec64_compare(&ts_new, &timekeeping_suspend_time) > 0) { > ts_delta = timespec64_sub(ts_new, timekeeping_suspend_time); > - sleeptime_injected = true; > + suspend_timing_needed = false; > } > > - if (sleeptime_injected) > + if (!suspend_timing_needed) > __timekeeping_inject_sleeptime(tk, &ts_delta);
This reads odd as well. I'd rather keep a local variable inject_sleeptime or such and set that in the code pathes above.
if (...) { ... inject_sleeptime = true; } else if (...) { ... inject_sleeptime = true; }
if (inject_sleeptime) { suspend_timing_needed = false; __timekeeping_inject_sleeptime(); }
Hmm? Just blindly converting everything results in functional, but nonsensical code. Think about what happens when you look at that stuff 6 month from now...
Thanks,
tglx
| |