From patchwork Fri Aug 2 20:01:36 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sean Christopherson X-Patchwork-Id: 13751951 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 09B69C3DA4A for ; Fri, 2 Aug 2024 20:03:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Reply-To:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:Cc:To: From:Subject:Message-ID:References:Mime-Version:In-Reply-To:Date: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=ykfKcxdUa9fLrTb96clu+XLU5WSDBheRMe8+PcsjFjM=; b=jmU1+va5YqnysPAGQnxQFwz7Mb EDCYbr7tvOm29S83MUYVuZ++S/Fldx/91cpG7TeX4O8FtFRSImizkGXvWEHvWFT6s6t+r9FTdwFWM J8HMt6BXlYUcmOfIIXkxOaIUs3jEHP8+BE36tfmCAyYvWo900n303nZcBuGrfNFJre05KO55VsVvn GR6RJ0QStURRVa6W6e3uMR10ixmOBV7av+GxCiAUm0ZhhtLRb2x1VXpyWUTO0YLtvCJ4xQo5rdG4b QHIGHdDN1erOS+iMIp8uFV6VZqdPH2iJ3iviHpSHo8U0nEizBTDWVqbG4yNRB8vP1C9ZUb2nKrj2J f3kqdnxA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sZyUd-00000009wX1-2n6j; Fri, 02 Aug 2024 20:03:15 +0000 Received: from mail-yw1-x114a.google.com ([2607:f8b0:4864:20::114a]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sZyTC-00000009w3p-0Gxt for linux-arm-kernel@lists.infradead.org; Fri, 02 Aug 2024 20:01:47 +0000 Received: by mail-yw1-x114a.google.com with SMTP id 00721157ae682-666010fb35cso59121497b3.0 for ; Fri, 02 Aug 2024 13:01:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1722628904; x=1723233704; darn=lists.infradead.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=ykfKcxdUa9fLrTb96clu+XLU5WSDBheRMe8+PcsjFjM=; b=Ll3EQcyo6clxVOWwxGEyU+4v4UFmQx1uuQuB7aUSNMD/Z4yGRloyf7tYkJ9I4YjiHZ G2WzBcgFqZNvPPtDtyqG+bjUDxZQzf9qTk+HeBEryIy77AQADJ3XcDLJZJ6nfyNxVioB wNIrKS6Lf7GRcTMqhWqp3NtVvSH13KBGtme5vQBRSKIur38g5bBskc7TGNF1yM6cNYFd dkYFpnX5XuDuGoXEEs7T4nRDJ0Cs3OfqxQV9Hf1YKgIYTAbBuoLrkM35NdhO3MPwCEBW zf7xhoWmGNFgzF2kPE0vq7wkynTyH97tkDeroQlprcWNNXA8vnJeeEq0ypguzVsQjaRj l23w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722628904; x=1723233704; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ykfKcxdUa9fLrTb96clu+XLU5WSDBheRMe8+PcsjFjM=; b=Fk6RUQa1pLYatc6pJlnbWb1TOxy+OB2brhuJzQKBGrtHuuyb+ltjyljISUJNJy60Gg bXKWpbuFOQX0MicBeHefokevI/tQ4EP3aB0O/YgBlT3rQe6USb0KeMDRm76gdaUhfsEO A9bHcyEPZC45YgahOXvCDa1bKSFeZ+qFWRhxbpi6NkqsUXTcnEYS7U0FRyuVP3dT/kr0 rptvC26p+vGgZHu5Ym5CmPnTh3oVB4SfgRDQEedQXK3Qe/zKDlAwaVbaxLuxzH0EqzZz oK2bi3jaFUD2jS9hGT0NROHbBp1Q+PB6UGZFtid3NmO2FYnhrBpab4wD22F1s2CeXfIu COGw== X-Gm-Message-State: AOJu0Yw2gMlPwbxAmghoGMIyElkrBxEu+EVCTX6855hX5SZqRIP/0Xiv Z+JhLQm7WtG7K3DjKNlGAsBBfzrOyu3qv5jZ5yOBhpIvWq2EAu3BqwSTr/8BPHmFu94Mgorkikh Mag== X-Google-Smtp-Source: AGHT+IGvtD2k+Z8XEDfN/3DgERyf+vS3FmJyCb7y6ktKFZouZ8vRLWbcTwmFsQsX8ntvC2Xbw1LZ/rv2S6Q= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a05:690c:2901:b0:644:c4d6:add0 with SMTP id 00721157ae682-6884f7ffdfcmr402097b3.1.1722628903604; Fri, 02 Aug 2024 13:01:43 -0700 (PDT) Date: Fri, 2 Aug 2024 13:01:36 -0700 In-Reply-To: <20240802200136.329973-1-seanjc@google.com> Mime-Version: 1.0 References: <20240802200136.329973-1-seanjc@google.com> X-Mailer: git-send-email 2.46.0.rc2.264.g509ed76dc8-goog Message-ID: <20240802200136.329973-3-seanjc@google.com> Subject: [PATCH 2/2] KVM: Protect vCPU's "last run PID" with rwlock, not RCU From: Sean Christopherson To: Marc Zyngier , Oliver Upton , Paolo Bonzini Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Sean Christopherson , Steve Rutherford X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240802_130146_126742_A5EC56AD X-CRM114-Status: GOOD ( 18.19 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Sean Christopherson Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org To avoid jitter on KVM_RUN due to synchronize_rcu(), use a rwlock instead of RCU to protect vcpu->pid, a.k.a. the pid of the task last used to a vCPU. When userspace is doing M:N scheduling of tasks to vCPUs, e.g. to run SEV migration helper vCPUs during post-copy, the synchronize_rcu() needed to change the PID associated with the vCPU can stall for hundreds of milliseconds, which is problematic for latency sensitive post-copy operations. In the directed yield path, do not acquire the lock if it's contended, i.e. if the associated PID is changing, as that means the vCPU's task is already running. Reported-by: Steve Rutherford Signed-off-by: Sean Christopherson Reviewed-by: Steve Rutherford --- arch/arm64/include/asm/kvm_host.h | 2 +- include/linux/kvm_host.h | 3 ++- virt/kvm/kvm_main.c | 32 +++++++++++++++++-------------- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index a33f5996ca9f..7199cb014806 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -1115,7 +1115,7 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu, void kvm_arm_halt_guest(struct kvm *kvm); void kvm_arm_resume_guest(struct kvm *kvm); -#define vcpu_has_run_once(vcpu) !!rcu_access_pointer((vcpu)->pid) +#define vcpu_has_run_once(vcpu) (!!READ_ONCE((vcpu)->pid)) #ifndef __KVM_NVHE_HYPERVISOR__ #define kvm_call_hyp_nvhe(f, ...) \ diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 689e8be873a7..d6f4e8b2b44c 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -342,7 +342,8 @@ struct kvm_vcpu { #ifndef __KVM_HAVE_ARCH_WQP struct rcuwait wait; #endif - struct pid __rcu *pid; + struct pid *pid; + rwlock_t pid_lock; int sigset_active; sigset_t sigset; unsigned int halt_poll_ns; diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 91048a7ad3be..fabffd85fa34 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -486,6 +486,7 @@ static void kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id) vcpu->kvm = kvm; vcpu->vcpu_id = id; vcpu->pid = NULL; + rwlock_init(&vcpu->pid_lock); #ifndef __KVM_HAVE_ARCH_WQP rcuwait_init(&vcpu->wait); #endif @@ -513,7 +514,7 @@ static void kvm_vcpu_destroy(struct kvm_vcpu *vcpu) * the vcpu->pid pointer, and at destruction time all file descriptors * are already gone. */ - put_pid(rcu_dereference_protected(vcpu->pid, 1)); + put_pid(vcpu->pid); free_page((unsigned long)vcpu->run); kmem_cache_free(kvm_vcpu_cache, vcpu); @@ -3930,15 +3931,17 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_kick); int kvm_vcpu_yield_to(struct kvm_vcpu *target) { - struct pid *pid; struct task_struct *task = NULL; int ret; - rcu_read_lock(); - pid = rcu_dereference(target->pid); - if (pid) - task = get_pid_task(pid, PIDTYPE_PID); - rcu_read_unlock(); + if (!read_trylock(&target->pid_lock)) + return 0; + + if (target->pid) + task = get_pid_task(target->pid, PIDTYPE_PID); + + read_unlock(&target->pid_lock); + if (!task) return 0; ret = yield_to(task, 1); @@ -4178,9 +4181,9 @@ static int vcpu_get_pid(void *data, u64 *val) { struct kvm_vcpu *vcpu = data; - rcu_read_lock(); - *val = pid_nr(rcu_dereference(vcpu->pid)); - rcu_read_unlock(); + read_lock(&vcpu->pid_lock); + *val = pid_nr(vcpu->pid); + read_unlock(&vcpu->pid_lock); return 0; } @@ -4466,7 +4469,7 @@ static long kvm_vcpu_ioctl(struct file *filp, r = -EINVAL; if (arg) goto out; - oldpid = rcu_access_pointer(vcpu->pid); + oldpid = vcpu->pid; if (unlikely(oldpid != task_pid(current))) { /* The thread running this VCPU changed. */ struct pid *newpid; @@ -4476,9 +4479,10 @@ static long kvm_vcpu_ioctl(struct file *filp, break; newpid = get_task_pid(current, PIDTYPE_PID); - rcu_assign_pointer(vcpu->pid, newpid); - if (oldpid) - synchronize_rcu(); + write_lock(&vcpu->pid_lock); + vcpu->pid = newpid; + write_unlock(&vcpu->pid_lock); + put_pid(oldpid); } vcpu->wants_to_run = !READ_ONCE(vcpu->run->immediate_exit__unsafe);