On Tue, 15 Nov 2022 21:28:49 +0100 (CET)
Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> @@ -1128,6 +1144,9 @@ static inline int
> * mod_timer_pending() is the same for pending timers as mod_timer(), but
> * will not activate inactive timers.
> *
> + * If @timer->function == NULL then the start operation is silently
> + * discarded.
> + *
> * Return:
> * * %0 - The timer was inactive and not modified
> * * %1 - The timer was active and requeued to expire at @expires
> @@ -1154,6 +1173,9 @@ EXPORT_SYMBOL(mod_timer_pending);
> * same timer, then mod_timer() is the only safe way to modify the timeout,
> * since add_timer() cannot modify an already running timer.
> *
> + * If @timer->function == NULL then the start operation is silently
> + * discarded, the return value is 0 and meaningless.
> + *
> * Return:
> * * %0 - The timer was inactive and started
For those that only read the "Return" portion of kernel-doc, perhaps add
here:
"or the timer is in the shutdown state and was not started".
> * * %1 - The timer was active and requeued to expire at @expires or
> @@ -1175,6 +1197,9 @@ EXPORT_SYMBOL(mod_timer);
> * modify an enqueued timer if that would reduce the expiration time. If
> * @timer is not enqueued it starts the timer.
> *
> + * If @timer->function == NULL then the start operation is silently
> + * discarded.
> + *
> * Return:
> * * %0 - The timer was inactive and started
> * * %1 - The timer was active and requeued to expire at @expires or
> @@ -1201,6 +1226,9 @@ EXPORT_SYMBOL(timer_reduce);
> *
> * If @timer->expires is already in the past @timer will be queued to
> * expire at the next timer tick.
> + *
> + * If @timer->function == NULL then the start operation is silently
> + * discarded.
> */
> void add_timer(struct timer_list *timer)
> {
> @@ -1217,13 +1245,18 @@ EXPORT_SYMBOL(add_timer);
> *
> * This can only operate on an inactive timer. Attempts to invoke this on
> * an active timer are rejected with a warning.
> + *
> + * If @timer->function == NULL then the start operation is silently
> + * discarded.
> */
> void add_timer_on(struct timer_list *timer, int cpu)
> {
> struct timer_base *new_base, *base;
> unsigned long flags;
>
> - if (WARN_ON_ONCE(timer_pending(timer) || !timer->function))
> + debug_assert_init(timer);
> +
> + if (WARN_ON_ONCE(timer_pending(timer)))
> return;
>
> new_base = get_timer_cpu_base(timer->flags, cpu);
> @@ -1234,6 +1267,13 @@ void add_timer_on(struct timer_list *tim
> * wrong base locked. See lock_timer_base().
> */
> base = lock_timer_base(timer, &flags);
> + /*
> + * Has @timer been shutdown? This needs to be evaluated while
> + * holding base lock to prevent a race against the shutdown code.
> + */
> + if (!timer->function)
> + goto out_unlock;
> +
> if (base != new_base) {
> timer->flags |= TIMER_MIGRATING;
>
> @@ -1247,6 +1287,7 @@ void add_timer_on(struct timer_list *tim
>
> debug_timer_activate(timer);
> internal_add_timer(base, timer);
> +out_unlock:
> raw_spin_unlock_irqrestore(&base->lock, flags);
> }
> EXPORT_SYMBOL_GPL(add_timer_on);
> @@ -1532,6 +1573,12 @@ static void expire_timers(struct timer_b
>
> fn = timer->function;
>
> + if (WARN_ON_ONCE(!fn)) {
> + /* Should never happen. Emphasis on should! */
> + base->running_timer = NULL;
> + return;
Why return and not continue?
Wont this drop the other timers in the queue?
-- Steve
> + }
> +
> if (timer->flags & TIMER_IRQSAFE) {
> raw_spin_unlock(&base->lock);
> call_timer_fn(timer, fn, baseclk);
