[lkml]   [2009]   [Jul]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    Subject[patch 2/3] net: sanitize hrtimer usage in sched_cbq
    The usage of hrtimer in cbq_ovl_delay() is less than obvious and in
    two aspects wrong.

    The intention of the code is to arm an hrtimer with the expiry time
    X. If the timer is already armed the code needs to check whether the
    expiry time X is earlier than the expiry time of the armed timer and
    either keep the active timer or rearm it to X. If the timer is not
    armed it needs to schedule it to X. That's not what the code does.

    It calls hrtimer_try_to_cancel() unconditionally and checks the return
    value. If the return value is non zero then it compares the expiry
    time of the timer with the new expiry time and picks the earlier
    one. The return value check does not take into account that the timer
    might run its callback (expressed by a return value of -1). In that
    case the expiry time of the timer is probably earlier than the new
    expiry time so it rearms the already expired timer with the expiry
    value in the past.

    If the timer is not active (hrtimer_try_to_cancel() returns 0) it does
    not set the new expiry time X but instead restarts the timer with the
    expiry time which was active when the timer fired last. That's in the
    past as well.

    Change the code to check whether the timer is enqueued. If it is
    enqueued then the expiry time of the timer is checked against the new
    expiry time and it only calls hrtimer_start when the new expiry time
    is earlier than the already armed timer. If the timer is not active
    then arm it unconditionally with the new expiry time.

    Signed-off-by: Thomas Gleixner <>
    Cc: Patrick McHardy <>
    net/sched/sch_cbq.c | 21 ++++++++++++++-------
    1 file changed, 14 insertions(+), 7 deletions(-)

    Index: linux-2.6/net/sched/sch_cbq.c
    --- linux-2.6.orig/net/sched/sch_cbq.c
    +++ linux-2.6/net/sched/sch_cbq.c
    @@ -494,7 +494,6 @@ static void cbq_ovl_delay(struct cbq_cla

    if (!cl->delayed) {
    psched_time_t sched = q->now;
    - ktime_t expires;

    delay += cl->offtime;
    if (cl->avgidle < 0)
    @@ -504,6 +503,7 @@ static void cbq_ovl_delay(struct cbq_cla
    cl->undertime = q->now + delay;

    if (delay > 0) {
    + ktime_t expires, actexp;
    unsigned long flags;

    spin_lock_irqsave(&q->lock, flags);
    @@ -514,12 +514,19 @@ static void cbq_ovl_delay(struct cbq_cla

    expires = ktime_set(0, 0);
    expires = ktime_add_ns(expires, PSCHED_TICKS2NS(sched));
    - if (hrtimer_try_to_cancel(&q->delay_timer) &&
    - ktime_to_ns(ktime_sub(
    - hrtimer_get_expires(&q->delay_timer),
    - expires)) > 0)
    - hrtimer_set_expires(&q->delay_timer, expires);
    - hrtimer_restart(&q->delay_timer);
    + /*
    + * If the timer is queued check whether the
    + * new expiry time is earlier than the current
    + * one.
    + */
    + if (hrtimer_is_queued(&q->delay_timer)) {
    + actexp = hrtimer_get_expires(&q->delay_timer);
    + if (expires.tv64 >= actexp.tv64)
    + expires.tv64 = 0;
    + }
    + if (expires.tv64)
    + hrtimer_start(&q->delay_timer, expires,
    cl->delayed = 1;
    spin_unlock_irqrestore(&q->lock, flags);

     \ /
      Last update: 2009-07-10 00:03    [W:0.023 / U:131.444 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site