Re: [External] : Re: [PATCH 1/2] timer: add add_timer_active_cpu()

[Thomas Gleixner <tglx@kernel.org>]

On Mon, Jun 08 2026 at 19:22, Partha Satapathy wrote:
> RDS already checks cpu_online() before calling add_timer_on().
> However, that still races with CPU hotplug and can leave a timer
> queued on a CPU which goes offline before the enqueue completes.

I can't find any invocation of add_timer_on() in the RDS code and the
commit you mentioned does not exist and there is no trace of it on
lore. But Gemini told me it's Oracle private hackery. So what the heck
are you telling me about RDS checking something and having a race
condition?

> The current workaround is to serialize the operation with
> cpus_read_lock(), but doing so in the networking path adds overhead
> to every timer enqueue and increases CPU hotplug latency.

I buy the cpus_read_lock() overhead, but who cares about hotplug latency?

Aside of that if you just want to temporary prevent a CPU from vanishing
while queueing the timer, then preempt_disable() does the trick too and
that's definitely more lightweight than cpus_read_lock().

>> On Thu, Apr 23 2026 at 09:19, Partha Satapathy wrote:

>>>  /**
>>>   * __timer_delete - Internal function: Deactivate a timer
>>>   * @timer:	The timer to be deactivated
>>> @@ -2507,6 +2543,7 @@ int timers_prepare_cpu(unsigned int cpu)
>>>  		base->next_expiry_recalc = false;
>>>  		base->timers_pending = false;
>>>  		base->is_idle = false;
>>> +		base->is_active = true;
>> 
>> That's just wrong. The base is active only when the CPU reaches online state.
>
> Setting base->is_active = true in timers_prepare_cpu() was
> intentional as a small optimization to avoid an additional hotplug
> callback. Timers queued in this window will be serviced as the CPU
> completes bring-up.

It's still intentionally wrong.

> However, to keep the state strictly aligned with CPU online state, I
> can move this activation logic to a dedicated callback under
> CPUHP_AP_ONLINE_DYN instead if you prefer.

What's wrong with checking cpu_online() under the CPU's base lock? When
the lock is held the CPU can't go away.

> The RDS use case treats the target CPU primarily as a locality hint
> rather than a strict execution requirement.
>
> Given your outline above, this may be better addressed by a generic
> placement-hint interface than by strict CPU placement semantics.

Well, a placement hint interface still needs to ensure that the target
is online and does not vanish before the timer or work has been queued,
which means the whole operation needs to be protected against hotplug no
matter what. So you end up with something like this:

       scoped_guard(preempt) {
          if (!cpu_online(cpu))
       	  	cpu = pick_online_cpu();
          add_timer_on(.....);
       }

The simplest variant for pick_online_cpu() is obviously
cpumask_first(onlinemask), but you can go wild there and find the
nearest one.

But making this a generic thing is not necessarily a good idea because
pick_online_cpu() could turn out to be expensive and you might want to
cache the result once your preferred CPU vanished.

Alternatively you implement a hotplug callback for RDS which changes the
preferred CPU before the CPU goes offline. That's cheap and a one time
effort and you don't have to do anything on the timer/workqueue
side. There you can even justify to search for the nearest neighbor or
whatever preference you have.

Thanks,

        tglx