2022-11-22 23:13:49

by Keller, Jacob E

[permalink] [raw]
Subject: Re: [patch V2 13/17] timers: Split [try_to_]del_timer[_sync]() to prepare for shutdown mode



On 11/22/2022 9:45 AM, Thomas Gleixner wrote:
> +/**
> + * try_to_del_timer_sync - Try to deactivate a timer
> + * @timer: Timer to deactivate
> + *
> + * This function tries to deactivate a timer. On success the timer is not
> + * queued and the timer callback function is not running on any CPU.
> + *
> + * This function does not guarantee that the timer cannot be rearmed right
> + * after dropping the base lock. That needs to be prevented by the calling
> + * code if necessary.
> + *
> + * Return:
> + * * %0 - The timer was not pending
> + * * %1 - The timer was pending and deactivated
> + * * %-1 - The timer callback function is running on a different CPU
> + */
> +int try_to_del_timer_sync(struct timer_list *timer)
> +{
> + return __try_to_del_timer_sync(timer);
> +}
> EXPORT_SYMBOL(try_to_del_timer_sync);
>


Its a bit odd to me that some patches refactor and replace functions
with new variants all under timer_* namespace, but then we've left some
of them available without that.

Any reasoning behind this? I guess "try_*" is pretty clear and unlikely
to get stolen by other code..?

Thanks,
Jake


2022-11-23 17:08:16

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch V2 13/17] timers: Split [try_to_]del_timer[_sync]() to prepare for shutdown mode

On Tue, Nov 22 2022 at 15:04, Jacob Keller wrote:
> On 11/22/2022 9:45 AM, Thomas Gleixner wrote:
>> +int try_to_del_timer_sync(struct timer_list *timer)
>> +{
>> + return __try_to_del_timer_sync(timer);
>> +}
>> EXPORT_SYMBOL(try_to_del_timer_sync);
>>
>
>
> Its a bit odd to me that some patches refactor and replace functions
> with new variants all under timer_* namespace, but then we've left some
> of them available without that.
>
> Any reasoning behind this? I guess "try_*" is pretty clear and unlikely
> to get stolen by other code..?

Kinda. I renamed del_timer*() because that's the ones which we want to
substitute with timer_shutdown*() where possible and reasonable.

A larger timer namespace cleanup is subject to a follow up series.

Thanks,

tglx

2022-11-23 19:03:01

by Keller, Jacob E

[permalink] [raw]
Subject: Re: [patch V2 13/17] timers: Split [try_to_]del_timer[_sync]() to prepare for shutdown mode



On 11/23/2022 9:05 AM, Thomas Gleixner wrote:
> On Tue, Nov 22 2022 at 15:04, Jacob Keller wrote:
>> On 11/22/2022 9:45 AM, Thomas Gleixner wrote:
>>> +int try_to_del_timer_sync(struct timer_list *timer)
>>> +{
>>> + return __try_to_del_timer_sync(timer);
>>> +}
>>> EXPORT_SYMBOL(try_to_del_timer_sync);
>>>
>>
>>
>> Its a bit odd to me that some patches refactor and replace functions
>> with new variants all under timer_* namespace, but then we've left some
>> of them available without that.
>>
>> Any reasoning behind this? I guess "try_*" is pretty clear and unlikely
>> to get stolen by other code..?
>
> Kinda. I renamed del_timer*() because that's the ones which we want to
> substitute with timer_shutdown*() where possible and reasonable.
>
> A larger timer namespace cleanup is subject to a follow up series.
>
> Thanks,
>
> tglx

Yep thats what I figured once I got to the end of the series. Thanks!

Regards,
Jake