From: Sean Christopherson <seanjc@google.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
mingo@redhat.com, juri.lelli@redhat.com,
vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
vschneid@redhat.com, linux-kernel@vger.kernel.org,
kprateek.nayak@amd.com, wuyun.abel@bytedance.com,
youssefesmat@chromium.org, tglx@linutronix.de, efault@gmx.de,
kvm@vger.kernel.org
Subject: Re: [PATCH 17/24] sched/fair: Implement delayed dequeue
Date: Thu, 10 Oct 2024 11:23:52 -0700 [thread overview]
Message-ID: <ZwgbuA5rggErT7ev@google.com> (raw)
In-Reply-To: <20241010091843.GK33184@noisy.programming.kicks-ass.net>
On Thu, Oct 10, 2024, Peter Zijlstra wrote:
> On Thu, Oct 10, 2024 at 10:19:40AM +0200, Peter Zijlstra wrote:
> > On Wed, Oct 09, 2024 at 07:49:54PM -0700, Sean Christopherson wrote:
> >
> > > TL;DR: Code that checks task_struct.on_rq may be broken by this commit.
> >
> > Correct, and while I did look at quite a few, I did miss KVM used it,
> > damn.
> >
> > > Peter,
> > >
> > > Any thoughts on how best to handle this? The below hack-a-fix resolves the issue,
> > > but it's obviously not appropriate. KVM uses vcpu->preempted for more than just
> > > posted interrupts, so KVM needs equivalent functionality to current->on-rq as it
> > > was before this commit.
> > >
> > > @@ -6387,7 +6390,7 @@ static void kvm_sched_out(struct preempt_notifier *pn,
> > >
> > > WRITE_ONCE(vcpu->scheduled_out, true);
> > >
> > > - if (current->on_rq && vcpu->wants_to_run) {
> > > + if (se_runnable(¤t->se) && vcpu->wants_to_run) {
> > > WRITE_ONCE(vcpu->preempted, true);
> > > WRITE_ONCE(vcpu->ready, true);
> > > }
> >
> > se_runnable() isn't quite right, but yes, a helper along those lines is
> > probably best. Let me try and grep more to see if there's others I
> > missed as well :/
>
> How's the below? I remember looking at the freezer thing before and
> deciding it isn't a correctness thing, but given I added the helper, I
> changed it anyway. I've added a bunch of comments and the perf thing is
> similar to KVM, it wants to know about preemptions so that had to change
> too.
Fixes KVM's woes! Thanks!
prev parent reply other threads:[~2024-10-10 18:23 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20240727102732.960974693@infradead.org>
[not found] ` <20240727105030.226163742@infradead.org>
[not found] ` <CGME20240828223802eucas1p16755f4531ed0611dc4871649746ea774@eucas1p1.samsung.com>
[not found] ` <5618d029-769a-4690-a581-2df8939f26a9@samsung.com>
2024-10-10 2:49 ` [PATCH 17/24] sched/fair: Implement delayed dequeue Sean Christopherson
2024-10-10 7:57 ` Mike Galbraith
2024-10-10 16:18 ` Sean Christopherson
2024-10-10 17:12 ` Mike Galbraith
2024-10-10 8:19 ` Peter Zijlstra
2024-10-10 9:18 ` Peter Zijlstra
2024-10-10 18:23 ` Sean Christopherson [this message]
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=ZwgbuA5rggErT7ev@google.com \
--to=seanjc@google.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=efault@gmx.de \
--cc=juri.lelli@redhat.com \
--cc=kprateek.nayak@amd.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
--cc=wuyun.abel@bytedance.com \
--cc=youssefesmat@chromium.org \
/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