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

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

[Kirtikumar Anandrao Ramchandani]

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@gmail.com>
---
 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);

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@gmail.com>

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);

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

[Hans de Goede]

Hi,

On 5-Nov-26 12:23 AM, Guenter Roeck wrote:
> 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@gmail.com>
> 
> 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 ?

I agree.

If you do this, this should be a replacement for timer_setup()
which then does the setup + adds the automatic shutdown-sync on dev
resource cleanup.

Similar to how include/linux/devm-helpers.h has devm_work_autocancel()
which calls INIT_WORK(w, worker) and sets-up a cancel_work_sync()
call on dev resource cleanup.

Regards,

Hans



>> ---
>>   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);
>