tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

timers: Silently ignore timers with a NULL function

Tearing down timers which have circular dependencies to other
functionality, e.g. workqueues, where the timer can schedule work and work
can arm timers, is not trivial.

In those cases it is desired to shutdown the timer in a way which prevents
rearming of the timer. The mechanism to do so is to set timer->function to
NULL and use this as an indicator for the timer arming functions to ignore
the (re)arm request.

In preparation for that replace the warnings in the relevant code paths
with checks for timer->function == NULL. If the pointer is NULL, then
discard the rearm request silently.

Add debug_assert_init() instead of the WARN_ON_ONCE(!timer->function)
checks so that debug objects can warn about non-initialized timers.

The warning of debug objects does not warn if timer->function == NULL. It
warns when timer was not initialized using timer_setup[_on_stack]() or via
DEFINE_TIMER(). If developers fail to enable debug objects and then waste
lots of time to figure out why their non-initialized timer is not firing,
they deserve it. Same for initializing a timer with a NULL function.

Co-developed-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
Link: https://lore.kernel.org/all/20220407161745.7d6754b3@gandalf.local.home
Link: https://lore.kernel.org/all/20221110064101.429013735@goodmis.org
Link: https://lore.kernel.org/r/87wn7kdann.ffs@tglx

Thomas Gleixner d02e382c 87bdd932

+52 -5
+52 -5
kernel/time/timer.c
··· 1017 1017 unsigned int idx = UINT_MAX; 1018 1018 int ret = 0; 1019 1019 1020 - BUG_ON(!timer->function); 1020 + debug_assert_init(timer); 1021 1021 1022 1022 /* 1023 1023 * This is a common optimization triggered by the networking code - if ··· 1044 1044 * dequeue/enqueue dance. 1045 1045 */ 1046 1046 base = lock_timer_base(timer, &flags); 1047 + /* 1048 + * Has @timer been shutdown? This needs to be evaluated 1049 + * while holding base lock to prevent a race against the 1050 + * shutdown code. 1051 + */ 1052 + if (!timer->function) 1053 + goto out_unlock; 1054 + 1047 1055 forward_timer_base(base); 1048 1056 1049 1057 if (timer_pending(timer) && (options & MOD_TIMER_REDUCE) && ··· 1078 1070 } 1079 1071 } else { 1080 1072 base = lock_timer_base(timer, &flags); 1073 + /* 1074 + * Has @timer been shutdown? This needs to be evaluated 1075 + * while holding base lock to prevent a race against the 1076 + * shutdown code. 1077 + */ 1078 + if (!timer->function) 1079 + goto out_unlock; 1080 + 1081 1081 forward_timer_base(base); 1082 1082 } 1083 1083 ··· 1144 1128 * mod_timer_pending() is the same for pending timers as mod_timer(), but 1145 1129 * will not activate inactive timers. 1146 1130 * 1131 + * If @timer->function == NULL then the start operation is silently 1132 + * discarded. 1133 + * 1147 1134 * Return: 1148 - * * %0 - The timer was inactive and not modified 1135 + * * %0 - The timer was inactive and not modified or was in 1136 + * shutdown state and the operation was discarded 1149 1137 * * %1 - The timer was active and requeued to expire at @expires 1150 1138 */ 1151 1139 int mod_timer_pending(struct timer_list *timer, unsigned long expires) ··· 1175 1155 * same timer, then mod_timer() is the only safe way to modify the timeout, 1176 1156 * since add_timer() cannot modify an already running timer. 1177 1157 * 1158 + * If @timer->function == NULL then the start operation is silently 1159 + * discarded. In this case the return value is 0 and meaningless. 1160 + * 1178 1161 * Return: 1179 - * * %0 - The timer was inactive and started 1162 + * * %0 - The timer was inactive and started or was in shutdown 1163 + * state and the operation was discarded 1180 1164 * * %1 - The timer was active and requeued to expire at @expires or 1181 1165 * the timer was active and not modified because @expires did 1182 1166 * not change the effective expiry time ··· 1200 1176 * modify an enqueued timer if that would reduce the expiration time. If 1201 1177 * @timer is not enqueued it starts the timer. 1202 1178 * 1179 + * If @timer->function == NULL then the start operation is silently 1180 + * discarded. 1181 + * 1203 1182 * Return: 1204 - * * %0 - The timer was inactive and started 1183 + * * %0 - The timer was inactive and started or was in shutdown 1184 + * state and the operation was discarded 1205 1185 * * %1 - The timer was active and requeued to expire at @expires or 1206 1186 * the timer was active and not modified because @expires 1207 1187 * did not change the effective expiry time such that the ··· 1227 1199 * 1228 1200 * The @timer->expires and @timer->function fields must be set prior 1229 1201 * to calling this function. 1202 + * 1203 + * If @timer->function == NULL then the start operation is silently 1204 + * discarded. 1230 1205 * 1231 1206 * If @timer->expires is already in the past @timer will be queued to 1232 1207 * expire at the next timer tick. ··· 1259 1228 struct timer_base *new_base, *base; 1260 1229 unsigned long flags; 1261 1230 1262 - if (WARN_ON_ONCE(timer_pending(timer) || !timer->function)) 1231 + debug_assert_init(timer); 1232 + 1233 + if (WARN_ON_ONCE(timer_pending(timer))) 1263 1234 return; 1264 1235 1265 1236 new_base = get_timer_cpu_base(timer->flags, cpu); ··· 1272 1239 * wrong base locked. See lock_timer_base(). 1273 1240 */ 1274 1241 base = lock_timer_base(timer, &flags); 1242 + /* 1243 + * Has @timer been shutdown? This needs to be evaluated while 1244 + * holding base lock to prevent a race against the shutdown code. 1245 + */ 1246 + if (!timer->function) 1247 + goto out_unlock; 1248 + 1275 1249 if (base != new_base) { 1276 1250 timer->flags |= TIMER_MIGRATING; 1277 1251 ··· 1292 1252 1293 1253 debug_timer_activate(timer); 1294 1254 internal_add_timer(base, timer); 1255 + out_unlock: 1295 1256 raw_spin_unlock_irqrestore(&base->lock, flags); 1296 1257 } 1297 1258 EXPORT_SYMBOL_GPL(add_timer_on); ··· 1581 1540 detach_timer(timer, true); 1582 1541 1583 1542 fn = timer->function; 1543 + 1544 + if (WARN_ON_ONCE(!fn)) { 1545 + /* Should never happen. Emphasis on should! */ 1546 + base->running_timer = NULL; 1547 + continue; 1548 + } 1584 1549 1585 1550 if (timer->flags & TIMER_IRQSAFE) { 1586 1551 raw_spin_unlock(&base->lock);