Messages in this thread | | | Date | Wed, 08 Jun 2011 14:55:01 +0900 | From | Hidetoshi Seto <> | Subject | Re: Test for CFS Bandwidth Control V6 |
| |
(2011/06/08 11:54), Paul Turner wrote: > On Mon, May 23, 2011 at 5:53 PM, Hidetoshi Seto > <seto.hidetoshi@jp.fujitsu.com> wrote: > >> diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c >> index 3936393..544072f 100644 >> --- a/kernel/sched_fair.c >> +++ b/kernel/sched_fair.c >> @@ -1537,7 +1537,7 @@ static void unthrottle_cfs_rq(struct cfs_rq *cfs_rq) >> walk_tg_tree_from(cfs_rq->tg, tg_unthrottle_down, tg_nop, >> (void *)&udd); >> >> - if (!cfs_rq->load.weight) >> + if (!cfs_rq->h_nr_running) >> return; >> > > Why change here?
I've confused a bit - just curious if by any chance there is throttled cfs_rq that have (load.weight, h_nr_running) = (0, >0).
>> task_delta = cfs_rq->h_nr_running; >> @@ -1843,10 +1843,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags) >> cfs_rq->h_nr_running++; >> >> /* end evaluation on throttled cfs_rq */ >> - if (cfs_rq_throttled(cfs_rq)) { >> - se = NULL; > > Hmm.. yeah this is a casualty of moving the h_nr_running computations > in-line as a part of the previous refactoring within the last > releases. This optimization (setting se = NULL to skip the second > half) obviously won't work properly with detecting whether we made it > to the end of the tree. > (snip) > > How about instead something like the following. We can actually take > advantage of the second loop always executing by deferring the > accounting update on a throttle entity. This keeps the control flow > within dequeue_task_fair linear. > > What do you think of (untested): > > --- a/kernel/sched_fair.c > +++ b/kernel/sched_fair.c > @@ -1744,13 +1744,12 @@ enqueue_task_fair(struct rq *rq, struct > task_struct *p, int flags) > break; > cfs_rq = cfs_rq_of(se); > enqueue_entity(cfs_rq, se, flags); > - cfs_rq->h_nr_running++; > > - /* end evaluation on throttled cfs_rq */ > - if (cfs_rq_throttled(cfs_rq)) { > - se = NULL; > + /* note: ordering with throttle check to perform > h_nr_running accounting on throttled entity below */ > + if (cfs_rq_throttled(cfs_rq)) > break; > - } > + > + cfs_rq->h_nr_running++; > flags = ENQUEUE_WAKEUP; > } > > @@ -1786,13 +1785,12 @@ static void dequeue_task_fair(struct rq *rq, > struct task_struct *p, int flags) > for_each_sched_entity(se) { > cfs_rq = cfs_rq_of(se); > dequeue_entity(cfs_rq, se, flags); > - cfs_rq->h_nr_running--; > > - /* end evaluation on throttled cfs_rq */ > - if (cfs_rq_throttled(cfs_rq)) { > - se = NULL; > + /* note: ordering with throttle check to perform > h_nr_running accounting on throttled entity below */ > + if (cfs_rq_throttled(cfs_rq)) > break; > - } > + > + cfs_rq->h_nr_running--; > /* Don't dequeue parent if it has other entities besides
Looks good if it abides by the nature of scheduler codes ;-)
Thanks, H.Seto
| |