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

On Tue, 18 Aug 2026 13:59:30 +0200
Andreas Hindborg <[email protected]> wrote:

> "FUJITA Tomonori" <[email protected]> writes:
> 
>> On Tue, 18 Aug 2026 11:02:27 +0200
>> Andreas Hindborg <[email protected]> wrote:
>>
>>>> perf and CFS bandwidth have a flag as well as a lock. The flag is "do
>>>> not arm while armed", which is the same rule the types enforce
>>>> here. rtc and the softlockup watchdog look like they cancel first and
>>>> then start instead. None of them arms a timer that is active, so I
>>>> would rather the abstraction did not allow it either. Does that seem
>>>> reasonable?
>>>
>>> I am fine with preventing starting a timer that is Started or Running,
>>> but I am not liking the `UniqueArc` requirement.
>>>
>>> I have a use case in `rnull` where I have to start a timer behind an
>>> `Arc` with no way to obtain a `UniqueArc`, so I would prefer if that use
>>> case keeps on working. Without this, I would have to allocate a box and
>>> put it behind a lock, leading to double indirection.
>>
>> Before the UniqueArc requirement, I would like to check which timer
>> you have in mind? The bandwidth timer, the per-command timer, or
>> something else? The two seem to need different things, so I would
>> rather not guess.
> 
> The bandwidth timer. It is started from an `Arc<NullBlkDevice>:
> HasHrTimer`. If we make the suggested change to `ArcTimerHandle`, I
> think I would need to change the `NullBlkDevice::bandwidth_timer` from
> an embedded `HrTimer` to a `SpinLock<Box<HrTimer>>` or something like
> that.
> 
> Maybe this is fine. I don't think it will affect performance for `rnull`
> - this is already a throttled path. But it gives slightly more
> convoluted code in the caller by reducing the way we can use the API.

I think we can allow creating HrTimerArc<T> from Arc<T>, so that UniqueArc is
not required. ListArc does the same with AtomicTracker, an atomic bool in the
object that records whether a ListArc exists:

    HrTimerArc::try_from_arc(Arc<T>) -> Result<HrTimerArc<T>, Arc<T>>

It fails when there is already another HrTimerArc for the object.

> For the completion timer, the change you propose would work fine I
> think. I would just start the timer via the unique request reference
> rather than the shared one. This is probably a better way to do it
> anyway.

Agreed.