Re: [PATCH 1/2] devres: add managed timer shutdown helper

[Guenter Roeck]

On 11/4/26 15:03, Kirtikumar Anandrao Ramchandani wrote:
> Provide a devres action for initialized timers. The action calls
> timer_shutdown_sync(), draining the callback and rejecting later rearming
> before device-managed storage is released.
>
> Use devm_add_action_or_reset() so a failed action registration shuts the
> timer down immediately.
>
> Signed-off-by: Kirtikumar Anandrao Ramchandani <kirtiar15502@xxxxxxxxx>

I see no benefit in this patch. The calling code now calls
devm_timer_autoshutdown() instead of devm_add_action_or_reset().
That does not really make a difference. Sure, we could add similar
code for every instance of devm_add_action_or_reset() in the kernel,
but what would be the point ?

Guenter

> ---
>   include/linux/devm-helpers.h | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
>
> diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
> index 708ca9131..ffe8b68b7 100644
> --- a/include/linux/devm-helpers.h
> +++ b/include/linux/devm-helpers.h
> @@ -24,8 +24,33 @@
>    */
>
>   #include <linux/device.h>
> +#include <linux/timer.h>
>   #include <linux/workqueue.h>
>
> +static inline void devm_timer_shutdown(void *res)
> +{
> +	timer_shutdown_sync(res);
> +}
> +
> +/**
> + * devm_timer_autoshutdown - Shut down an initialized timer on resource release
> + * @dev: Device whose lifetime the timer is bound to
> + * @timer: Initialized timer to shut down
> + *
> + * Register a timer_shutdown_sync() action. Register this action before
> + * arming the timer, and after resources that its callback may access.
> + * The timer is shut down immediately if action registration fails.
> + *
> + * The callback's resources must remain live until shutdown completes. Any
> + * independent user of the timer must stop before the timer's storage is
> + * released. Devres actions run in reverse registration order.
> + */
> +static inline int devm_timer_autoshutdown(struct device *dev,
> +					  struct timer_list *timer)
> +{
> +	return devm_add_action_or_reset(dev, devm_timer_shutdown, timer);
> +}
> +
>   static inline void devm_delayed_work_drop(void *res)
>   {
>   	cancel_delayed_work_sync(res);