Re: [PATCH v4] time: Fix extra sleeptime injection when suspend fails

[Mukesh Ojha]

On 7/17/2018 2:20 AM, Thomas Gleixner wrote:

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();
        }

Will do suggested change and send in v5.

Thanks.

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