lkml.org 
[lkml]   [2011]   [May]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [patch 13/15] sched: add exports tracking cfs bandwidth control statistics
    On Wed, May 11, 2011 at 12:56 AM, Hidetoshi Seto
    <seto.hidetoshi@jp.fujitsu.com> wrote:
    > Oops, I found an issue here.
    >
    > (2011/05/03 18:28), Paul Turner wrote:
    >> @@ -1628,6 +1631,12 @@ retry:
    >>               raw_spin_unlock(&cfs_b->lock);
    >>               goto retry;
    >>       }
    >> +
    >> +     /* update throttled stats */
    >> +     cfs_b->nr_periods += overrun;
    >> +     if (throttled)
    >> +             cfs_b->nr_throttled += overrun;
    >> +
    >>       cfs_b->runtime = runtime;
    >>       cfs_b->idle = idle;
    >>  out_unlock:
    >
    > Quoting from patch 09/15:
    >
    > +       if (!throttled || quota == RUNTIME_INF)
    > +               goto out;
    > +       idle = 0;
    > +
    > +retry:
    > +       runtime = distribute_cfs_runtime(cfs_b, runtime, runtime_expires);
    > +
    > +       raw_spin_lock(&cfs_b->lock);
    > +       /* new new bandwidth may have been set */
    > +       if (unlikely(runtime_expires != cfs_b->runtime_expires))
    > +               goto out_unlock;
    > +       /*
    > +        * make sure no-one was throttled while we were handing out the new
    > +        * runtime.
    > +        */
    > +       if (runtime > 0 && !list_empty(&cfs_b->throttled_cfs_rq)) {
    > +               raw_spin_unlock(&cfs_b->lock);
    > +               goto retry;
    > +       }
    > +       cfs_b->runtime = runtime;
    > +       cfs_b->idle = idle;
    > +out_unlock:
    > +       raw_spin_unlock(&cfs_b->lock);
    > +out:
    >
    > Since we skip distributing runtime (by "goto out") when !throttled,
    > the new block inserted by this patch is passed only when throttled.
    > So I see that nr_periods and nr_throttled look the same.
    >
    > Maybe we should move this block up like followings.
    >

    Yes, makes sense, incorporated -- thanks!

    > Thanks,
    > H.Seto
    >
    > ---
    > --- a/kernel/sched_fair.c
    > +++ b/kernel/sched_fair.c
    > @@ -1620,6 +1620,12 @@ static int do_sched_cfs_period_timer(struct cfs_bandwidth *cfs_b, int overrun)
    >                idle = cfs_b->idle;
    >                cfs_b->idle = 1;
    >        }
    > +
    > +       /* update throttled stats */
    > +       cfs_b->nr_periods += overrun;
    > +       if (throttled)
    > +               cfs_b->nr_throttled += overrun;
    > +
    >        raw_spin_unlock(&cfs_b->lock);
    >
    >        if (!throttled || quota == RUNTIME_INF)
    > @@ -1642,11 +1648,6 @@ retry:
    >                goto retry;
    >        }
    >
    > -       /* update throttled stats */
    > -       cfs_b->nr_periods += overrun;
    > -       if (throttled)
    > -               cfs_b->nr_throttled += overrun;
    > -
    >        cfs_b->runtime = runtime;
    >        cfs_b->idle = idle;
    >  out_unlock:
    >
    >
    >
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-05-11 18:17    [W:3.177 / U:0.948 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site