lkml.org 
[lkml]   [2017]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [RFC PATCH 04/11] Add a function to start/reduce a timer
    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

    \
     
     \ /
      Last update: 2017-10-22 17:39    [W:4.133 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site