Received-SPF: pass (google.com: domain of linux-kernel+bounces-180031-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Wed, 15 May 2024 07:57:41 -0700
From: "Paul E. McKenney" <paulmck@kernel.org>
To: Leonardo Bras <leobras@redhat.com>
Cc: Sean Christopherson <seanjc@google.com>,
	Frederic Weisbecker <frederic@kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Marcelo Tosatti <mtosatti@redhat.com>, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org
Subject: Re: [RFC PATCH 1/1] kvm: Note an RCU quiescent state on guest exit
Message-ID: <17ebd54d-a058-4bc8-bd65-a175d73b6d1a@paulmck-laptop>
Reply-To: paulmck@kernel.org
References: <20240511020557.1198200-1-leobras@redhat.com>
 <ZkJsvTH3Nye-TGVa@google.com>
 <CAJ6HWG7pgMu7sAUPykFPtsDfq5Kfh1WecRcgN5wpKQj_EyrbJA@mail.gmail.com>
 <68c39823-6b1d-4368-bd1e-a521ade8889b@paulmck-laptop>
 <ZkQ97QcEw34aYOB1@LeoBras>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ZkQ97QcEw34aYOB1@LeoBras>

On Wed, May 15, 2024 at 01:45:33AM -0300, Leonardo Bras wrote:
> On Tue, May 14, 2024 at 03:54:16PM -0700, Paul E. McKenney wrote:
> > On Mon, May 13, 2024 at 06:47:13PM -0300, Leonardo Bras Soares Passos wrote:
> > > On Mon, May 13, 2024 at 4:40 PM Sean Christopherson <seanjc@google.com> wrote:
> > > >
> > > > On Fri, May 10, 2024, Leonardo Bras wrote:
> > > > > As of today, KVM notes a quiescent state only in guest entry, which is good
> > > > > as it avoids the guest being interrupted for current RCU operations.
> > > > >
> > > > > While the guest vcpu runs, it can be interrupted by a timer IRQ that will
> > > > > check for any RCU operations waiting for this CPU. In case there are any of
> > > > > such, it invokes rcu_core() in order to sched-out the current thread and
> > > > > note a quiescent state.
> > > > >
> > > > > This occasional schedule work will introduce tens of microsseconds of
> > > > > latency, which is really bad for vcpus running latency-sensitive
> > > > > applications, such as real-time workloads.
> > > > >
> > > > > So, note a quiescent state in guest exit, so the interrupted guests is able
> > > > > to deal with any pending RCU operations before being required to invoke
> > > > > rcu_core(), and thus avoid the overhead of related scheduler work.
> > > >
> > > > Are there any downsides to this?  E.g. extra latency or anything?  KVM will note
> > > > a context switch on the next VM-Enter, so even if there is extra latency or
> > > > something, KVM will eventually take the hit in the common case no matter what.
> > > > But I know some setups are sensitive to handling select VM-Exits as soon as possible.
> > > >
> > > > I ask mainly because it seems like a no brainer to me to have both VM-Entry and
> > > > VM-Exit note the context switch, which begs the question of why KVM isn't already
> > > > doing that.  I assume it was just oversight when commit 126a6a542446 ("kvm,rcu,nohz:
> > > > use RCU extended quiescent state when running KVM guest") handled the VM-Entry
> > > > case?
> > > 
> > > I don't know, by the lore I see it happening in guest entry since the
> > > first time it was introduced at
> > > https://lore.kernel.org/all/1423167832-17609-5-git-send-email-riel@redhat.com/
> > > 
> > > Noting a quiescent state is cheap, but it may cost a few accesses to
> > > possibly non-local cachelines. (Not an expert in this, Paul please let
> > > me know if I got it wrong).
> > 
> > Yes, it is cheap, especially if interrupts are already disabled.
> > (As in the scheduler asks RCU to do the same amount of work on its
> > context-switch fastpath.)
> 
> Thanks!
> 
> > 
> > > I don't have a historic context on why it was just implemented on
> > > guest_entry, but it would make sense when we don't worry about latency
> > > to take the entry-only approach:
> > > - It saves the overhead of calling rcu_virt_note_context_switch()
> > > twice per guest entry in the loop
> > > - KVM will probably run guest entry soon after guest exit (in loop),
> > > so there is no need to run it twice
> > > - Eventually running rcu_core() may be cheaper than noting quiescent
> > > state every guest entry/exit cycle
> > > 
> > > Upsides of the new strategy:
> > > - Noting a quiescent state in guest exit avoids calling rcu_core() if
> > > there was a grace period request while guest was running, and timer
> > > interrupt hits the cpu.
> > > - If the loop re-enter quickly there is a high chance that guest
> > > entry's rcu_virt_note_context_switch() will be fast (local cacheline)
> > > as there is low probability of a grace period request happening
> > > between exit & re-entry.
> > > - It allows us to use the rcu patience strategy to avoid rcu_core()
> > > running if any grace period request happens between guest exit and
> > > guest re-entry, which is very important for low latency workloads
> > > running on guests as it reduces maximum latency in long runs.
> > > 
> > > What do you think?
> > 
> > Try both on the workload of interest with appropriate tracing and
> > see what happens?  The hardware's opinion overrides mine.  ;-)
> 
> That's a great approach!
> 
> But in this case I think noting a quiescent state in guest exit is 
> necessary to avoid a scenario in which a VM takes longer than RCU 
> patience, and it ends up running rcuc in a nohz_full cpu, even if guest 
> exit was quite brief. 
> 
> IIUC Sean's question is more on the tone of "Why KVM does not note a 
> quiescent state in guest exit already, if it does in guest entry", and I 
> just came with a few arguments to try finding a possible rationale, since 
> I could find no discussion on that topic in the lore for the original 
> commit.

Understood, and maybe trying it would answer that question quickly.
Don't get me wrong, just because it appears to work in a few tests doesn't
mean that it really works, but if it visibly blows up, that answers the
question quite quickly and easily.  ;-)

But yes, if it appears to work, there must be a full investigation into
whether or not the change really is safe.

							Thanx, Paul

> Since noting a quiescent state in guest exit is cheap enough, avoids rcuc 
> schedules when grace period starts during guest execution, and enables a 
> much more rational usage of RCU patience, it's a safe to assume it's a 
> better way of dealing with RCU compared to current implementation.
> 
> Sean, what do you think?
> 
> Thanks!
> Leo
> 
> > 
> > 							Thanx, Paul
> > 
>