lkml.org 
[lkml]   [2011]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [patch 13/15] sched: expire slack quota using generation counters
    From
    Date
    On Tue, 2011-03-22 at 20:03 -0700, Paul Turner wrote:

    Argh, this patch is terrible for the reason that it changes the whole
    accounting just introduced and me having to re-open all the previous
    patches to look up hth stuff worked before.

    > @@ -436,8 +438,10 @@ void init_cfs_bandwidth(struct cfs_bandw
    > raw_spin_lock_init(&cfs_b->lock);
    > cfs_b->quota = cfs_b->runtime = quota;
    > cfs_b->period = ns_to_ktime(period);
    > + cfs_b->quota_generation = 0;
    > INIT_LIST_HEAD(&cfs_b->throttled_cfs_rq);
    >
    > +
    > hrtimer_init(&cfs_b->period_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
    > cfs_b->period_timer.function = sched_cfs_period_timer;

    We're in desperate need of more whitespace there? :-)

    > @@ -9333,6 +9337,8 @@ static int tg_set_cfs_bandwidth(struct t
    > raw_spin_lock_irq(&cfs_b->lock);
    > cfs_b->period = ns_to_ktime(period);
    > cfs_b->runtime = cfs_b->quota = quota;
    > +
    > + cfs_bump_quota_generation(cfs_b);
    > raw_spin_unlock_irq(&cfs_b->lock);
    >
    > for_each_possible_cpu(i) {
    > Index: tip/kernel/sched_fair.c
    > ===================================================================
    > --- tip.orig/kernel/sched_fair.c
    > +++ tip/kernel/sched_fair.c
    > @@ -1331,11 +1331,25 @@ static void check_cfs_rq_quota(struct cf
    > resched_task(rq_of(cfs_rq)->curr);
    > }
    >
    > +static void cfs_bump_quota_generation(struct cfs_bandwidth *cfs_b)
    > +{
    > + cfs_b->quota_generation++;
    > + smp_mb();
    > +}

    Memory barriers come in pairs and with a comment, you fail on both
    counts.

    > +
    > +static inline int cfs_rq_quota_current(struct cfs_rq *cfs_rq)
    > +{
    > + struct cfs_bandwidth *cfs_b = tg_cfs_bandwidth(cfs_rq->tg);
    > +
    > + return cfs_rq->quota_generation == cfs_b->quota_generation;
    > +}
    > +
    > static void request_cfs_rq_quota(struct cfs_rq *cfs_rq)
    > {
    > struct task_group *tg = cfs_rq->tg;
    > struct cfs_bandwidth *cfs_b = tg_cfs_bandwidth(tg);
    > u64 amount = 0, min_amount;
    > + int generation;

    Not initialized,

    > min_amount = sched_cfs_bandwidth_slice() + (-cfs_rq->quota_remaining);
    >
    > @@ -1347,10 +1361,18 @@ static void request_cfs_rq_quota(struct
    > } else {
    > amount = min_amount;
    > }
    > + generation = cfs_b->quota_generation;
    > raw_spin_unlock(&cfs_b->lock);
    > }

    and since there's an if there, one can fail it, leaving generation
    uninitialized,

    >
    > + /* a deficit should be carried forwards, surplus should be dropped */
    > +
    > + if (generation != cfs_rq->quota_generation &&
    > + cfs_rq->quota_remaining > 0)
    > + cfs_rq->quota_remaining = 0;
    > +
    > cfs_rq->quota_remaining += amount;
    > + cfs_rq->quota_generation = generation;
    > }

    Resulting in uninitialized usage right there.



    \
     
     \ /
      Last update: 2011-04-05 15:33    [W:4.956 / U:0.600 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site