Re: [PATCH 0/4] Fix forward()/expires() racing with concurrent arming

On Fri, 14 Aug 2026 01:54:25 +0100
"Gary Guo" <[email protected]> wrote:

>>> But further complicating the type system to prevent concurrent restart sounds
>>> like a bad approach to me.
>>
>> My intent is the opposite: I think this makes the design simpler.
>>
>> All four implementations of start() already take self by value. For
>> Pin<Box<T, A>> and Pin<&mut T> that means what it says -- the box is moved
>> into the handle, the exclusive borrow is consumed -- so "no arming while
>> armed" is already the design there. For Arc<T> and Pin<&T> the same
>> signature meant nothing, because Clone and Copy let you build another
>> pointer and call start() again.
>>
>> So the contract depended on which pointer type you picked, and the module
>> documentation had to spell that out: "When a type implements both
>> HrTimerPointer and Clone, it is possible to issue the start operation
>> while the timer is in the started state." After the series there is one
>> rule for all four types, and that paragraph is gone together with the
>> restart operation it described.
> 
> Let's ignore the implementation detail of all various Rust pointers. It is
> something that I plan to overhaul and doesn't matter to the core issue here.
> 
> The change you're making is to remove the ability to concurrently start a timer
> in Rust. So if you have a timer might be running, you'd need to first cancel it
> before you can arm it again.
> 
> I do think it is conceptually cleaner -- however given this is explicitly added
> in
> https://lore.kernel.org/all/[email protected]/
> and the pattern is what perf core uses; so I wouldn't just dismiss the existence
> of this pattern. Perhaps cancelling before restarting is considered too
> expensive and has to be avoided?

The pattern perf core uses is "no arming while armed".

While perf_mux_hrtimer_handler() returns HRTIMER_RESTART -- while the
timer is active -- perf_mux_hrtimer_restart() does nothing. Only once
the handler has cleared cpc->hrtimer_active and returned
HRTIMER_NORESTART does perf_mux_hrtimer_restart() arm it again.

That flag was added precisely to implement "no arming while armed", in
4cfafd3082af ("sched,perf: Fix periodic timers"):

    We do not want to race such that the handler has already decided
    to stop, but the (external) restart sees the timer still active and we
    end up with a 'lost' timer.

    The problem with the current code is that the re-start can come before
    the callback does the forward, at which point the forward from the
    callback will WARN about forwarding an enqueued timer.


With cpc->hrtimer_active in place, neither of the two conditions that
5de2755c8c8b touches is reachable in perf's usage.

v1 is missing the ability to restart a stopped timer: once the
callback has returned NoRestart, the handle owns the right to arm and
never gives it back. I'll add it in v2, including a non-blocking
variant built on hrtimer_try_to_cancel(), so that a caller which
cannot sleep can re-arm the way perf does. That makes perf's model
expressible in the Rust abstraction.