Messages in this thread | | | Subject | Re: [PATCH 5/8] sched,cfs: use explicit cfs_rq of parent se helper | From | Rik van Riel <> | Date | Thu, 20 Jun 2019 12:29:22 -0400 |
| |
On Thu, 2019-06-20 at 18:23 +0200, Dietmar Eggemann wrote: > On 6/12/19 9:32 PM, Rik van Riel wrote: > > Use an explicit "cfs_rq of parent sched_entity" helper in a few > > strategic places, where cfs_rq_of(se) may no longer point at the > > right runqueue once we flatten the hierarchical cgroup runqueues. > > > > No functional change. > > > > Signed-off-by: Rik van Riel <riel@surriel.com> > > --- > > kernel/sched/fair.c | 17 +++++++++++++---- > > 1 file changed, 13 insertions(+), 4 deletions(-) > > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > > index dcc521d251e3..c6ede2ecc935 100644 > > --- a/kernel/sched/fair.c > > +++ b/kernel/sched/fair.c > > @@ -275,6 +275,15 @@ static inline struct cfs_rq > > *group_cfs_rq(struct sched_entity *grp) > > return grp->my_q; > > } > > > > +/* runqueue owned by the parent entity */ > > +static inline struct cfs_rq *group_cfs_rq_of_parent(struct > > sched_entity *se) > > +{ > > + if (se->parent) > > + return group_cfs_rq(se->parent); > > + > > + return &cfs_rq_of(se)->rq->cfs; > > +} > > + > > static inline bool list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq) > > { > > struct rq *rq = rq_of(cfs_rq); > > @@ -3298,7 +3307,7 @@ static inline int > > propagate_entity_load_avg(struct sched_entity *se) > > > > gcfs_rq->propagate = 0; > > > > - cfs_rq = cfs_rq_of(se); > > + cfs_rq = group_cfs_rq_of_parent(se); > > > > add_tg_cfs_propagate(cfs_rq, gcfs_rq->prop_runnable_sum); > > > > @@ -7779,7 +7788,7 @@ static void update_cfs_rq_h_load(struct > > cfs_rq *cfs_rq) > > > > WRITE_ONCE(cfs_rq->h_load_next, NULL); > > for_each_sched_entity(se) { > > - cfs_rq = cfs_rq_of(se); > > + cfs_rq = group_cfs_rq_of_parent(se); > > Why do you change this here? task_se_h_load() calls > update_cfs_rq_h_load() with cfs_rq = group_cfs_rq_of_parent(se) > because > the task might not be on the cfs_rq yet.
Because patch 6 points cfs_rq_of(se) at the CPU's top level cfs_rq for every task se ...
... but I guess since I have not changed where the cfs_rq_of points for cgroup sched_entities, this change is not necessary at this time, and I should be able to go without it, in this function.
> But inside update_cfs_rq_h_load() the first se is derived from > cfs_rq->tg->se[cpu_of(rq)] so in the first for_each_sched_entity() > loop > we should still start with group_cfs_rq() (se->my_q) ? > > The system doesn't barf with these two WARN_ON's in. > > @@ -7663,12 +7673,17 @@ static void update_cfs_rq_h_load(struct > cfs_rq > *cfs_rq) > unsigned long now = jiffies; > unsigned long load; > > + WARN_ON(se && (se != group_cfs_rq(se)->tg->se[cpu_of(rq)])); > + > if (cfs_rq->last_h_load_update == now) > return; > > WRITE_ONCE(cfs_rq->h_load_next, NULL); > for_each_sched_entity(se) { > cfs_rq = group_cfs_rq_of_parent(se); > + > + WARN_ON(se != group_cfs_rq(se)->tg->se[cpu_of(rq)]); > + > WRITE_ONCE(cfs_rq->h_load_next, se); > if (cfs_rq->last_h_load_update == now) > break; > > > [...] > > -- All Rights Reversed. [unhandled content-type:application/pgp-signature] | |