[PATCH v4] api: timer: cancellation event

[TuomasTaipale]

Draft for cancellation event for periodic timers. Alternative to #1969.

v2:

• Reworded (now backward incompatible and won't compile until implementation and tests updated)

v4:

• Further reworded, ownership flow clarified, introduced a new function for cancel finalization and ownership transfer, ack signature reverted back to original

[JannePeltonen]

This seems fine, but there appears to be an inconsistency in the existing description of odp_timer_free().

The text for odp_timer_free() says "The timer must be inactive when calling this function", which is fine. But it also says: "Similarly for an active periodic timer, the application must cancel it (odp_timer_periodic_cancel()) and receive the last event from the timer (odp_timer_periodic_ack()) before freeing it". The latter sentence, which tries to explain what "inactive" means, does not require acking the last event or other remaining events.

[JannePeltonen]

Maybe changing "timeout event" to just "event" in various places is not needed now that the last event must also be a timeout event.

[JannePeltonen]

I am not sure the tmo_ev output parameter is very good here. It is only needed in the non-time-critical timer cancellation sequence but not in the vast majority of ack calls. It might add a slight overhead in the normal ack calls.

An application should be able to remember what timeout event it provided in the timer start routine, but if not it can also put the event handle in the user pointer of the last event or in the memory area pointed to be the user pointer. Or there could event be an ODP API function to return the timeout event when given a timer or cancelled timer (but that does not seem so good either).

[JannePeltonen]

I think this clarifies the ownership of the timeout event and maybe makes it more ok to have the same event appear more than once in the timer queue in corner cases. But there is an inconsistency in the API that odp_timer_periodic_ack() spec says that the event being acked is consumed in certain cases. But if the ownership remains in the timer, the application does not own the event and the ack call cannot consume it. I think the ack speck needs to be updated for the clarified ownership model.

[JannePeltonen]

The (existing) text says " Each event from a periodic timer must be acknowledged with this call" but it also says that the same event can appear in the application queue more than once. This makes is somewhat ambiguous whether a "duplicate" event must be acked multiple times or just once. I know the intention is to call the ack for each time a timeout event is received from the queue, even if the same event is received multiple times, but text is not as clear as it could be.

[shijith]

Same comment as @JannePeltonen . Do we need to return tmo_ev for the last event ? Since application manages the lifecycle of both events, it should store the event pointers for cancel. Plus the driver has ownership of the events only till cancel and _ack of cancel_ev comes after that.

[TuomasTaipale]

With the current periodic timer API, ownership of the timeout event has been slightly fuzzy. Several workers and the implementation could have valid access to the same event simultaneously.

With v4, this should be now more clear: implementation is now specified to own the timeout and cancel events beginning from successful timer start call until cancellation finalization (application is allowed to call some read-only functions for the timeout in between). There's a new function odp_timer_periodic_cancel_finalize() to be called by the application after timer cancel and remaining event acking, which returns ownership of the events back to application (meaning implementation needs to be able to return the events passed when starting the timer).