Messages in this thread | | | From | Josh Don <> | Date | Mon, 11 Oct 2021 17:31:41 -0700 | Subject | Re: [PATCH] sched/core: forced idle accounting |
| |
On Mon, Oct 11, 2021 at 10:33 AM Hao Luo <haoluo@google.com> wrote: > > On Thu, Oct 7, 2021 at 5:08 PM Josh Don <joshdon@google.com> wrote: > > -void sched_core_dequeue(struct rq *rq, struct task_struct *p) > > +void sched_core_dequeue(struct rq *rq, struct task_struct *p, int flags) > > { > > rq->core->core_task_seq++; > > > > - if (!sched_core_enqueued(p)) > > - return; > > + if (sched_core_enqueued(p)) { > > + rb_erase(&p->core_node, &rq->core_tree); > > + RB_CLEAR_NODE(&p->core_node); > > + } > > > > - rb_erase(&p->core_node, &rq->core_tree); > > - RB_CLEAR_NODE(&p->core_node); > > + /* > > + * Migrating the last task off the cpu, with the cpu in forced idle > > + * state. Reschedule to create an accounting edge for forced idle, > > + * and re-examine whether the core is still in forced idle state. > > + */ > > + if (!(flags & DEQUEUE_SAVE) && rq->nr_running == 1 && > > + rq->core->core_forceidle && rq->curr == rq->idle) > > + resched_curr(rq); > > Resched_curr is probably an unwanted side effect of dequeue. Maybe we > could extract the check and resched_curr out into a function, and call > the function outside of sched_core_dequeue(). In that way, the > interface of dequeue doesn't need to change.
This resched is an atypical case; normal load balancing won't steal the last runnable task off a cpu. The main reasons this resched could trigger are: migration due to affinity change, and migration due to sched core doing a cookie_steal. Could bubble this up to deactivate_task(), but seems less brittle to keep this in dequeue() with the check against DEQUEUE_SAVE (since this creates an important accounting edge). Thoughts?
> > /* > > @@ -5765,7 +5782,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) > > for_each_cpu_wrap(i, smt_mask, cpu) { > > rq_i = cpu_rq(i); > > > > - if (i != cpu) > > + if (i != cpu && (rq_i != rq->core || !core_clock_updated)) > > update_rq_clock(rq_i); > > Do you mean (rq_i != rq->core && !core_clock_updated)? I thought > rq->core has core_clock updated always.
rq->clock is updated on entry to pick_next_task(). rq->core is only updated if rq == rq->core, or if we've done the clock update for rq->core above.
| |