Messages in this thread Patch in this message | | | Date | Thu, 26 Nov 2020 10:07:10 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH -tip 02/32] sched: Introduce sched_class::pick_task() |
| |
On Wed, Nov 25, 2020 at 05:28:36PM +0100, Vincent Guittot wrote: > On Wed, 18 Nov 2020 at 00:20, Joel Fernandes (Google)
> > +#ifdef CONFIG_SMP > > +static struct task_struct *pick_task_fair(struct rq *rq) > > +{ > > + struct cfs_rq *cfs_rq = &rq->cfs; > > + struct sched_entity *se; > > + > > + if (!cfs_rq->nr_running) > > + return NULL; > > + > > + do { > > + struct sched_entity *curr = cfs_rq->curr; > > + > > + se = pick_next_entity(cfs_rq, NULL); > > Calling pick_next_entity clears buddies. This is fine without > coresched because the se will be the next one. But calling > pick_task_fair doesn't mean that the se will be used
Urgh, nice one :/
> > + > > + if (curr) { > > + if (se && curr->on_rq) > > + update_curr(cfs_rq); > > + > > Shouldn't you check if cfs_rq is throttled ?
Hmm,... I suppose we do.
> > + if (!se || entity_before(curr, se)) > > + se = curr; > > + } > > + > > + cfs_rq = group_cfs_rq(se); > > + } while (cfs_rq); > > + > > + return task_of(se); > > +} > > +#endif
Something like so then?
--- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -4354,6 +4354,8 @@ check_preempt_tick(struct cfs_rq *cfs_rq static void set_next_entity(struct cfs_rq *cfs_rq, struct sched_entity *se) { + clear_buddies(cfs_rq, se); + /* 'current' is not kept within the tree. */ if (se->on_rq) { /* @@ -4440,8 +4442,6 @@ pick_next_entity(struct cfs_rq *cfs_rq, se = cfs_rq->last; } - clear_buddies(cfs_rq, se); - return se; } @@ -6982,20 +6982,29 @@ static void check_preempt_wakeup(struct #ifdef CONFIG_SMP static struct task_struct *pick_task_fair(struct rq *rq) { - struct cfs_rq *cfs_rq = &rq->cfs; struct sched_entity *se; - + struct cfs_rq *cfs_rq; + +again: + cfs_rq = &rq->cfs; if (!cfs_rq->nr_running) return NULL; do { struct sched_entity *curr = cfs_rq->curr; - if (curr && curr->on_rq) - update_curr(cfs_rq); + /* When we pick for a remote RQ, we'll not have done put_prev_entity() */ + if (curr) { + if (curr->on_rq) + update_curr(cfs_rq); + else + curr = NULL; - se = pick_next_entity(cfs_rq, curr); + if (unlikely(check_cfs_rq_runtime(cfs_rq))) + goto again; + } + se = pick_next_entity(cfs_rq, curr); cfs_rq = group_cfs_rq(se); } while (cfs_rq);
| |