Skip to content

Commit 7349a69

Browse files
committed
iopoll: Do not use timekeeping in read_poll_timeout_atomic()
read_poll_timeout_atomic() uses ktime_get() to implement the timeout feature, just like its non-atomic counterpart. However, there are several issues with this, due to its use in atomic contexts: 1. When called in the s2ram path (as typically done by clock or PM domain drivers), timekeeping may be suspended, triggering the WARN_ON(timekeeping_suspended) in ktime_get(): WARNING: CPU: 0 PID: 654 at kernel/time/timekeeping.c:843 ktime_get+0x28/0x78 Calling ktime_get_mono_fast_ns() instead of ktime_get() would get rid of that warning. However, that would break timeout handling, as (at least on systems with an ARM architectured timer), the time returned by ktime_get_mono_fast_ns() does not advance while timekeeping is suspended. Interestingly, (on the same ARM systems) the time returned by ktime_get() does advance while timekeeping is suspended, despite the warning. 2. Depending on the actual clock source, and especially before a high-resolution clocksource (e.g. the ARM architectured timer) becomes available, time may not advance in atomic contexts, thus breaking timeout handling. Fix this by abandoning the idea that one can rely on timekeeping to implement timeout handling in all atomic contexts, and switch from a global time-based to a locally-estimated timeout handling. In most (all?) cases the timeout condition is exceptional and an error condition, hence any additional delays due to underestimating wall clock time are irrelevant. Signed-off-by: Geert Uytterhoeven <[email protected]> Acked-by: Arnd Bergmann <[email protected]> Reviewed-by: Tony Lindgren <[email protected]> Reviewed-by: Ulf Hansson <[email protected]> Link: https://lore.kernel.org/r/3d2a2f4e553489392d871108797c3be08f88300b.1685692810.git.geert+renesas@glider.be
1 parent b407460 commit 7349a69

File tree

1 file changed

+17
-5
lines changed

1 file changed

+17
-5
lines changed

include/linux/iopoll.h

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,29 +74,41 @@
7474
* Returns 0 on success and -ETIMEDOUT upon a timeout. In either
7575
* case, the last read value at @args is stored in @val.
7676
*
77+
* This macro does not rely on timekeeping. Hence it is safe to call even when
78+
* timekeeping is suspended, at the expense of an underestimation of wall clock
79+
* time, which is rather minimal with a non-zero delay_us.
80+
*
7781
* When available, you'll probably want to use one of the specialized
7882
* macros defined below rather than this macro directly.
7983
*/
8084
#define read_poll_timeout_atomic(op, val, cond, delay_us, timeout_us, \
8185
delay_before_read, args...) \
8286
({ \
8387
u64 __timeout_us = (timeout_us); \
88+
s64 __left_ns = __timeout_us * NSEC_PER_USEC; \
8489
unsigned long __delay_us = (delay_us); \
85-
ktime_t __timeout = ktime_add_us(ktime_get(), __timeout_us); \
86-
if (delay_before_read && __delay_us) \
90+
u64 __delay_ns = __delay_us * NSEC_PER_USEC; \
91+
if (delay_before_read && __delay_us) { \
8792
udelay(__delay_us); \
93+
if (__timeout_us) \
94+
__left_ns -= __delay_ns; \
95+
} \
8896
for (;;) { \
8997
(val) = op(args); \
9098
if (cond) \
9199
break; \
92-
if (__timeout_us && \
93-
ktime_compare(ktime_get(), __timeout) > 0) { \
100+
if (__timeout_us && __left_ns < 0) { \
94101
(val) = op(args); \
95102
break; \
96103
} \
97-
if (__delay_us) \
104+
if (__delay_us) { \
98105
udelay(__delay_us); \
106+
if (__timeout_us) \
107+
__left_ns -= __delay_ns; \
108+
} \
99109
cpu_relax(); \
110+
if (__timeout_us) \
111+
__left_ns--; \
100112
} \
101113
(cond) ? 0 : -ETIMEDOUT; \
102114
})

0 commit comments

Comments
 (0)