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@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 ?

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