From mboxrd@z Thu Jan 1 00:00:00 1970 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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id EDCE2C3DA5D for ; Thu, 25 Jul 2024 18:17:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 86F4A6B0083; Thu, 25 Jul 2024 14:17:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7F7CD6B0088; Thu, 25 Jul 2024 14:17:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 697EA6B008C; Thu, 25 Jul 2024 14:17:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 48C946B0083 for ; Thu, 25 Jul 2024 14:17:51 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id F3B31410D7 for ; Thu, 25 Jul 2024 18:17:50 +0000 (UTC) X-FDA: 82379083500.25.F9EC43D Received: from mail-pf1-f181.google.com (mail-pf1-f181.google.com [209.85.210.181]) by imf21.hostedemail.com (Postfix) with ESMTP id 189901C000D for ; Thu, 25 Jul 2024 18:17:48 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=UW3NCyYG; spf=pass (imf21.hostedemail.com: domain of dmatlack@google.com designates 209.85.210.181 as permitted sender) smtp.mailfrom=dmatlack@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721931467; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=ZCU/39n/MEh1qJPI6jK5LfqbsxyFwdIdQzHQHHv611A=; b=ZRgYGpLRTJRFJ453rFYZZrf4zlNfXx69rb7RDj0GdmD1II98/XFYxum7/FVsXhUjGyYM6r ecKvcePBGRYql781IGdAHLfm0FAGwkk7l5TQbfl2RZ0tSo/vRHoY64AjgRwct2ZDp/deQR GJnoXXz+Ha80J0S7tZm1WZ5SlV5O/fc= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=UW3NCyYG; spf=pass (imf21.hostedemail.com: domain of dmatlack@google.com designates 209.85.210.181 as permitted sender) smtp.mailfrom=dmatlack@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721931467; a=rsa-sha256; cv=none; b=XVcZ8iJMTVXCr0M7maBBPjKmyXGOYAXbkWDqdX04YAY2tssVqmkCMRuo1WXzumtgQ7b2+9 H3qJ2d3JQvvMIL5c7br5yME5UUTlpNleqQv3Xpwqu24vKjjlTa0Xv8mc0auyd+DU8PZ1cG qkGTnuLY8nkxmSlAxyyqx+xnICDh9/0= Received: by mail-pf1-f181.google.com with SMTP id d2e1a72fcca58-70d19c525b5so124121b3a.2 for ; Thu, 25 Jul 2024 11:17:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1721931468; x=1722536268; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=ZCU/39n/MEh1qJPI6jK5LfqbsxyFwdIdQzHQHHv611A=; b=UW3NCyYGxYdH0fR0zcJBo4czCrEgdFgYukCdLA2JQYeSHvCXFJiuQfDLUs+38P9i8Y KK6dXPvWuA6ovY+uIAshgduF/TMVd92x+jRqZ6d/la8eaHxQBzIAOfJ50LHOCQ18OOXm 0TCOk2WfV3C6DyncUEcCmtUfi5e2boGsQ6hYfFKEeayzCbkRcq/pcF6OgsZwcYGGmNoi uZfayJ6bfU0ODsffs7F+gCbr1/lFD9KLyHPauoQiuheh4sZohn1Q2zrJgB6+mDDRASnJ N4Yq+/9B44QWTmzi+OQrondnI4Odix77xlAF9lzZ2WE/svcOCKz1RoSA3vOFdkap+t7m +R8g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721931468; x=1722536268; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ZCU/39n/MEh1qJPI6jK5LfqbsxyFwdIdQzHQHHv611A=; b=slwCYIhgbltpLWFcEoddBF2yxdznT1OQ6llIk5SMt73PqSe83vSnqWNkfWzaQ1rT3e kwmPmOyHdfQzOcn2JA+EVGI5bRJHjjPczqIxD7vmFdDkt2WpNoaZySxXnNW/wQRxPmqU I7yOWPLJWNZTQcGpv+ayf7LJx7pggPC6hf96CjNqZ9RlQvenLI6nP7nWmr8uHoWRwpnO GBIlVSpNDhLYLVrmc57ZUInebXMas9pT3qI/6Jq9SIayV4uoHsqj86GkFh6kt6XxSZ7A WkNuVt7wXlHzXq26osk0edMme6MgPKLgNWwfMpr7jPQDOjXASgrdY9jikyDtrbT0hI0y EAcA== X-Forwarded-Encrypted: i=1; AJvYcCX1+WikP13DsYF1kO5z88N6fiT1OrqM2oY5mcMSPAeOfLHN9dcgKknH/VV8RlM/9YkcDcXnKLHT2T/DlqYbt4g6er0= X-Gm-Message-State: AOJu0Yx3Qd+oOzwrYoSGNRikVYIhfpF+pN/4BqJaezn63bru/jBJ/pn5 +WVV+adE85rgq5LqYlvWkwATpWwZJD/4BMcTyltdMH+7mfnPy1qEnbG6syKEmQ== X-Google-Smtp-Source: AGHT+IGgJJ2MHUDyfdNf8nuLsnVaCfQ9A96+ttKTHUsGKEKorXbwZR90OZ/UEb7mApStThubbs9AYw== X-Received: by 2002:a05:6a00:2d8c:b0:70e:8070:f9d0 with SMTP id d2e1a72fcca58-70eae8d5c8amr3531836b3a.9.1721931466986; Thu, 25 Jul 2024 11:17:46 -0700 (PDT) Received: from google.com (61.139.125.34.bc.googleusercontent.com. [34.125.139.61]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7a9f884c245sm1490390a12.50.2024.07.25.11.17.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Jul 2024 11:17:46 -0700 (PDT) Date: Thu, 25 Jul 2024 11:17:41 -0700 From: David Matlack To: James Houghton Cc: Andrew Morton , Paolo Bonzini , Ankit Agrawal , Axel Rasmussen , Catalin Marinas , David Rientjes , James Morse , Jason Gunthorpe , Jonathan Corbet , Marc Zyngier , Oliver Upton , Raghavendra Rao Ananta , Ryan Roberts , Sean Christopherson , Shaoqin Huang , Suzuki K Poulose , Wei Xu , Will Deacon , Yu Zhao , Zenghui Yu , kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v6 08/11] KVM: x86: Optimize kvm_{test_,}age_gfn a little bit Message-ID: References: <20240724011037.3671523-1-jthoughton@google.com> <20240724011037.3671523-9-jthoughton@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240724011037.3671523-9-jthoughton@google.com> X-Rspam-User: X-Stat-Signature: xf37ob648oodanx1xq7kojbponibhgke X-Rspamd-Queue-Id: 189901C000D X-Rspamd-Server: rspam11 X-HE-Tag: 1721931468-937575 X-HE-Meta: U2FsdGVkX19GOwdj9EfBNpgTQo3dXcJaqBumZfpYnZb5M1FE6CeWT8izW9Xwm01g+fEe9cv3iZy8yGXQgToj5ia+S+BYD9+Jq/2WUKSNMMS8QBaLsLkQQdicUDxIn5XMUqjEo84uIIJyoW7GAl8GokG81VsYL1vDWrWT4PIPUWgHuWtjb2/O3e5BXI1RdYO44DFa3MabUdBkHQ7EV/1BNmMQDTiCIw02CkJdT7wx+kAD21FSC5M4yzAENbASHwXSfFz7FYPAwQU4imGmsYty7neIdmG/SAiN4oh5pClWBgOuFUARh0kMjbuzoZP46xWgmikzZAqN7sM/XH8FZSTSBObAt9IyA67aLm2YNj+0+HrT9w0iQurQdm+xSIx7ZG8tfSHS0QAB2igmDQSqP2AGw9BXiJJtgOry+pOsxdw/BF9X/NAPvCNifOYbWdlZqhrFYF0buPmXjVC7eYUzFO+MCwXI7MrEuxb0XRUxquXtwWnZRAN27N33M/FpedUZhO2WqtsB8Q81IaXZkzeHobcAuwwCJj0NafDQ86f8JYAnz2SvhInu7JaT5VYlnAfnynjg1xvyYlgs/whq7YLh1BIdoq6iz7US1URA1p4xeFtHh3Bf44wNWEncHC+cU23X+FZQOAZ492ldbeLQRQL24xWsW/KE7qZfTmq8CInt3v9IkOo8HvRkH9qYHxMveTHNKjkTZVSKkZ6sOBuaVOSFK3A2Sgk05y5kLtrenpGQ+ia65Jojb4T49UqZpOICHWgl/yKmAtCnqHybbgl4YLQRAeXYVLsS9IFjzxHNZk0xbZnbMyeaAw5ivrgWAFcir7uaVBn7EBDs0CJkUUvojosUq3cQQ53OnCq2B73ikVWuBbHShN8xTF+fbW8S6Kj9RJnznxFrIVXIKD3cRKxnawjsSGxCGAZk3zhlNuwFr5a8NE2RXqA0rp8sP4ktOJzkAOlmPeNb5FpvQLh25dAe7xe4gaW ciVVl/Zn oJNVbKGxbcPhnsVAXpzLRxwBphF2H/UBrswsFzegWYHWpI5r38j/4cqc2GL8kDwg7Mc12SkzN09ubDeg2gFZk8YzXf2oi7ch3F8nUhyzE0Eze2KjskMQWe6DMLf0SJlAzOA78QaogGoBr+RPqvxwLi9YaLMdPtpCtbCVv6EaKB2BEZhc9nKH2JDo/7/Mc1bgdybxlAnOoUMCh17J8VOBjxwnAcgHI1NoRBvwEz72r7UNzSYNqCm2Bcrfdf9xW2p6oIo0xHHwZGE/7JYjPUSuqUIY93VK7kHxC1rXWHRO2qnpeu9yvO/w8NTbe3UYiKoNiojb/N5PVyLrz4Qe4ZJhggpVU2Xc4gpy5wWscQph0rv8q8acq2hOFjCqSNA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2024-07-24 01:10 AM, James Houghton wrote: > Optimize both kvm_age_gfn and kvm_test_age_gfn's interaction with the nit: Use () when referring to functions. > shadow MMU by, rather than checking if our memslot has rmaps, check if > there are any indirect_shadow_pages at all. What is optimized by checking indirect_shadow_pages instead of have_rmaps and what's the benefit? Smells like a premature optimization. > > Also, for kvm_test_age_gfn, reorder the TDP MMU check to be first. If we > find that the range is young, we do not need to check the shadow MMU. This should be a separate commit since it's a logically distinct change and no dependency on the other change in this commit (other than both touch the same function). Splitting the commits up will also make it easier to write more specific short logs (instead of "optimize a little bit" :) Also, the commit re-orders kvm_age_gfn() as well but the commit message only mentions kvm_test_age_gfn(). No objection to keeping the two functions consistent but it should be called out in the commit message. > > Signed-off-by: James Houghton > --- > arch/x86/kvm/mmu/mmu.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 7b93ce8f0680..919d59385f89 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -1629,19 +1629,24 @@ static void rmap_add(struct kvm_vcpu *vcpu, const struct kvm_memory_slot *slot, > __rmap_add(vcpu->kvm, cache, slot, spte, gfn, access); > } > > +static bool kvm_has_shadow_mmu_sptes(struct kvm *kvm) > +{ > + return !tdp_mmu_enabled || READ_ONCE(kvm->arch.indirect_shadow_pages); > +} > + > bool kvm_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range) > { > bool young = false; > > - if (kvm_memslots_have_rmaps(kvm)) { > + if (tdp_mmu_enabled) > + young |= kvm_tdp_mmu_age_gfn_range(kvm, range); > + > + if (kvm_has_shadow_mmu_sptes(kvm)) { > write_lock(&kvm->mmu_lock); > young = kvm_handle_gfn_range(kvm, range, kvm_age_rmap); > write_unlock(&kvm->mmu_lock); > } > > - if (tdp_mmu_enabled) > - young |= kvm_tdp_mmu_age_gfn_range(kvm, range); > - > return young; > } > > @@ -1649,15 +1654,15 @@ bool kvm_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range) > { > bool young = false; > > - if (kvm_memslots_have_rmaps(kvm)) { > + if (tdp_mmu_enabled) > + young |= kvm_tdp_mmu_test_age_gfn(kvm, range); > + > + if (!young && kvm_has_shadow_mmu_sptes(kvm)) { nit: A short comment here might be helpful to explain why young is checked. > write_lock(&kvm->mmu_lock); > young = kvm_handle_gfn_range(kvm, range, kvm_test_age_rmap); > write_unlock(&kvm->mmu_lock); > } > > - if (tdp_mmu_enabled) > - young |= kvm_tdp_mmu_test_age_gfn(kvm, range); > - > return young; > } > > -- > 2.46.0.rc1.232.g9752f9e123-goog >