lkml.org 
[lkml]   [2011]   [May]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [patch 04/15] sched: validate CFS quota hierarchies
    On Tue, May 10, 2011 at 12:20 AM, Hidetoshi Seto
    <seto.hidetoshi@jp.fujitsu.com> wrote:
    > Description typos + one bug.
    >
    > (2011/05/03 18:28), Paul Turner wrote:
    >> Add constraints validation for CFS bandwidth hierachies.
    >
    >                                               hierarchies
    >
    >>
    >> Validate that:
    >>    sum(child bandwidth) <= parent_bandwidth
    >>
    >> In a quota limited hierarchy, an unconstrainted entity
    >
    >                                   unconstrained
    >
    >> (e.g. bandwidth==RUNTIME_INF) inherits the bandwidth of its parent.
    >>
    >> Since bandwidth periods may be non-uniform we normalize to the maximum allowed
    >> period, 1 second.
    >>
    >> This behavior may be disabled (allowing child bandwidth to exceed parent) via
    >> kernel.sched_cfs_bandwidth_consistent=0
    >>
    >> Signed-off-by: Paul Turner <pjt@google.com>
    >>
    >> ---
    > (snip)
    >> +/*
    >> + * normalize group quota/period to be quota/max_period
    >> + * note: units are usecs
    >> + */
    >> +static u64 normalize_cfs_quota(struct task_group *tg,
    >> +                            struct cfs_schedulable_data *d)
    >> +{
    >> +     u64 quota, period;
    >> +
    >> +     if (tg == d->tg) {
    >> +             period = d->period;
    >> +             quota = d->quota;
    >> +     } else {
    >> +             period = tg_get_cfs_period(tg);
    >> +             quota = tg_get_cfs_quota(tg);
    >> +     }
    >> +
    >> +     if (quota == RUNTIME_INF)
    >> +             return RUNTIME_INF;
    >> +
    >> +     return to_ratio(period, quota);
    >> +}
    >
    > Since tg_get_cfs_quota() doesn't return RUNTIME_INF but -1,
    > this function needs a fix like following.
    >
    > For fixed version, feel free to add:
    >
    > Reviewed-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
    >
    > Thanks,
    > H.Seto
    >
    > ---
    >  kernel/sched.c |    7 ++++---
    >  1 files changed, 4 insertions(+), 3 deletions(-)
    >
    > diff --git a/kernel/sched.c b/kernel/sched.c
    > index d2562aa..f171ba5 100644
    > --- a/kernel/sched.c
    > +++ b/kernel/sched.c
    > @@ -9465,16 +9465,17 @@ static u64 normalize_cfs_quota(struct task_group *tg,
    >        u64 quota, period;
    >
    >        if (tg == d->tg) {
    > +               if (d->quota == RUNTIME_INF)
    > +                       return RUNTIME_INF;
    >                period = d->period;
    >                quota = d->quota;
    >        } else {
    > +               if (tg_cfs_bandwidth(tg)->quota == RUNTIME_INF)
    > +                       return RUNTIME_INF;
    >                period = tg_get_cfs_period(tg);
    >                quota = tg_get_cfs_quota(tg);
    >        }
    >

    Good catch!

    Just modifying:
    +if (quota == RUNTIME_INF || quota == -1)
    +                       return RUNTIME_INF;
    Seems simpler.

    Although really there's no reason for tg_get_cfs_runtime (and
    sched_group_rt_runtime from which it's cloned) not to be returning
    RUNTIME_INF and then doing the conversion within the cgroupfs handler.

    Fixing both is probably a better clean-up.

    > -       if (quota == RUNTIME_INF)
    > -               return RUNTIME_INF;
    > -
    >        return to_ratio(period, quota);
    >  }
    >
    >
    >
    --
    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:47    [from the cache]
    ©2003-2014 Jasper Spaans. Advertise on this site