[RFC PATCH 04/11] Add a function to start/reduce a timer

Thomas Gleixner tglx at linutronix.de
Fri Oct 20 05:20:27 PDT 2017


On Fri, 1 Sep 2017, David Howells wrote:

> Add a function, similar to mod_timer(), that will start a timer it isn't

s/it /if it /

> running and will modify it if it is running and has an expiry time longer
> than the new time.  If the timer is running with an expiry time that's the
> same or sooner, no change is made.
>
> The function looks like:
>
>       int reduce_timer(struct timer_list *timer, unsigned long expires);

Well, yes. But what's the purpose of this function? You explain the what,
but not the why.

> +extern int reduce_timer(struct timer_list *timer, unsigned long expires);

For new timer functions we really should use the timer_xxxx()
convention. The historic naming convention is horrible.

Aside of that timer_reduce() is kinda ugly but I failed to come up with
something reasonable as well.

>  static inline int
> -__mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only)
> +__mod_timer(struct timer_list *timer, unsigned long expires, unsigned int options)
>  {
>  	struct timer_base *base, *new_base;
>  	unsigned int idx = UINT_MAX;
> @@ -938,8 +941,13 @@ __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only)
>  	 * same array bucket then just return:
>  	 */
>  	if (timer_pending(timer)) {
> -		if (timer->expires == expires)
> -			return 1;
> +		if (options & MOD_TIMER_REDUCE) {
> +			if (time_before_eq(timer->expires, expires))
> +				return 1;
> +		} else {
> +			if (timer->expires == expires)
> +				return 1;
> +		}

This hurts the common networking optimzation case. Please keep that check
first:

		if (timer->expires == expires)
			return 1;

		if ((options & MOD_TIMER_REDUCE) &&
		    time_before(timer->expires, expires))
		    	return 1;

Also please check whether it's more efficient code wise to have that option
thing or if an additional 'bool reduce' argument cerates better code.

Thanks,

	tglx



More information about the linux-afs mailing list