From: Josh Don <joshdon@google.com>
To: Hao Luo <haoluo@google.com>
Cc: Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Dietmar Eggemann <dietmar.eggemann@arm.com>,
Steven Rostedt <rostedt@goodmis.org>,
Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
Daniel Bristot de Oliveira <bristot@redhat.com>,
Joel Fernandes <joel@joelfernandes.org>,
Vineeth Pillai <vineethrp@gmail.com>,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] sched/core: forced idle accounting
Date: Mon, 11 Oct 2021 17:31:41 -0700 [thread overview]
Message-ID: <CABk29NvPz3hDehsLC3z_Jpstr3BveL_XZBg_kcjqg9ewkH0RfQ@mail.gmail.com> (raw)
In-Reply-To: <CA+khW7g+HbLRWTV8vuo-oQ3ROv_ZnuSNMWA_pVp-BQGqT2d9-g@mail.gmail.com>
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.
next prev parent reply other threads:[~2021-10-12 0:32 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-08 0:08 [PATCH] sched/core: forced idle accounting Josh Don
2021-10-08 21:04 ` Josh Don
2021-10-09 15:54 ` Peter Zijlstra
2021-10-12 0:12 ` Josh Don
2021-10-12 12:27 ` Peter Zijlstra
2021-10-12 19:45 ` Josh Don
2021-10-14 14:24 ` Peter Zijlstra
2021-10-14 23:18 ` Josh Don
2021-10-09 18:11 ` Tao Zhou
2021-10-12 0:14 ` Josh Don
2021-10-11 17:33 ` Hao Luo
2021-10-12 0:31 ` Josh Don [this message]
2021-10-14 17:57 ` Hao Luo
2021-10-14 23:29 ` Josh Don
2021-10-14 23:58 ` Hao Luo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABk29NvPz3hDehsLC3z_Jpstr3BveL_XZBg_kcjqg9ewkH0RfQ@mail.gmail.com \
--to=joshdon@google.com \
--cc=bristot@redhat.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=haoluo@google.com \
--cc=joel@joelfernandes.org \
--cc=juri.lelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=vincent.guittot@linaro.org \
--cc=vineethrp@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).