Re: [PATCH 2/5] PM: runtime: Avoid racy clock checks for autosuspend-retry

[Doug Anderson]

Hi,

On Tue, Sep 29, 2026 at 11:47 AM Brian Norris <briannorris@xxxxxxxxxxxx> wrote:
>
> @@ -242,7 +242,12 @@ static inline bool pm_runtime_has_no_callbacks(struct device *dev)
>   */
>  static inline void pm_runtime_mark_last_busy(struct device *dev)
>  {
> -       WRITE_ONCE(dev->power.last_busy, ktime_get_mono_fast_ns());
> +       u64 now = ktime_get_mono_fast_ns();
> +
> +       if (now == READ_ONCE(dev->power.last_busy))
> +               now++;
> +
> +       WRITE_ONCE(dev->power.last_busy, now);

While we definitely want to fix the problem identified in this patch,
the proposed logic doesn't sit right with me. Let's say that
'power.last_busy" starts out as a given value, let's say "1020". Now,
while the clock hasn't ticked you call pm_runtime_mark_last_busy(). It
detects that 1020 == 1020 so it sets the time to 1021 so it's
different. Now you call pm_runtime_mark_last_busy() again when the
clock hasn't ticked. Now 1020 != 1021, so it goes back to 1020. This
could cause the whole heuristic to fail, can't it?

Maybe I'm just worrying about something that can't happen, but the
logic still seems odd.

It felt to me like we could just have a "bool". We set it to false
before we call runtime_suspend() and we check it after
runtime_suspend() returns. If the bool is set then we know they called
pm_runtime_mark_last_busy(). There shouldn't even be any weird
problems with weakly ordered memory since this boolean should always
be cleared, set, and checked in the same thread, right?

-Doug