Received-SPF: pass (google.com: domain of linux-kernel+bounces-177759-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Mon, 13 May 2024 08:31:59 -0700
In-Reply-To: <22def35f-5b8d-4424-a03b-c90e9174a14d@redhat.com>
Precedence: bulk
Mime-Version: 1.0
References: <20240110214723.695930-1-seanjc@google.com> <22def35f-5b8d-4424-a03b-c90e9174a14d@redhat.com>
Message-ID: <ZkIyXCBCLeOaAzN4@google.com>
Subject: Re: [PATCH] sched/core: Drop spinlocks on contention iff kernel is preemptible
From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>, 
	Juri Lelli <juri.lelli@redhat.com>, Vincent Guittot <vincent.guittot@linaro.org>, 
	linux-kernel@vger.kernel.org, Valentin Schneider <valentin.schneider@arm.com>, 
	Marco Elver <elver@google.com>, Frederic Weisbecker <frederic@kernel.org>, 
	David Matlack <dmatlack@google.com>
Content-Type: text/plain; charset="us-ascii"

On Mon, May 13, 2024, Paolo Bonzini wrote:
> On 1/10/24 22:47, Sean Christopherson wrote:
> > Use preempt_model_preemptible() to detect a preemptible kernel when
> > deciding whether or not to reschedule in order to drop a contended
> > spinlock or rwlock.  Because PREEMPT_DYNAMIC selects PREEMPTION, kernels
> > built with PREEMPT_DYNAMIC=y will yield contended locks even if the live
> > preemption model is "none" or "voluntary".  In short, make kernels with
> > dynamically selected models behave the same as kernels with statically
> > selected models.
> 
> Peter, looks like this patch fell through the cracks.  Could this be applied
> for 6.10?
> 
> There is a slightly confusing line in the commit message below, so that it
> reads more like an RFC; but the patch fixes a CONFIG_PREEMPT_DYNAMIC
> regression wrt static preemption models and has no functional change for
> !CONFIG_PREEMPT_DYNAMIC.
> 
> > Somewhat counter-intuitively, NOT yielding a lock can provide better
> > latency for the relevant tasks/processes.  E.g. KVM x86's mmu_lock, a
> > rwlock, is often contended between an invalidation event (takes mmu_lock
> > for write) and a vCPU servicing a guest page fault (takes mmu_lock for
> > read).  For _some_ setups, letting the invalidation task complete even
> > if there is mmu_lock contention provides lower latency for *all* tasks,
> > i.e. the invalidation completes sooner *and* the vCPU services the guest
> > page fault sooner.
> > 
> > But even KVM's mmu_lock behavior isn't uniform, e.g. the "best" behavior
> > can vary depending on the host VMM, the guest workload, the number of
> > vCPUs, the number of pCPUs in the host, why there is lock contention, etc.
> > 
> > In other words, simply deleting the CONFIG_PREEMPTION guard (or doing the
>                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> This should be "deleting the preempt_model_preemptible() guard" given that
> the patch does delete CONFIG_PREEMPTION, and only leaves
> preempt_model_preemptible() in place.

Note, this version won't apply cleanly, v2[*] handles the code movement and still
applies on Linus' tree.

[*] https://lore.kernel.org/all/20240312193911.1796717-1-seanjc@google.com