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 1B1E7C3DA41 for ; Wed, 10 Jul 2024 23:11:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7F4156B008A; Wed, 10 Jul 2024 19:11:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7A4886B008C; Wed, 10 Jul 2024 19:11:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 644716B0092; Wed, 10 Jul 2024 19:11:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 462CC6B008A for ; Wed, 10 Jul 2024 19:11:10 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id EFE40A0309 for ; Wed, 10 Jul 2024 23:11:09 +0000 (UTC) X-FDA: 82325390658.09.C5A97D7 Received: from mail-qt1-f170.google.com (mail-qt1-f170.google.com [209.85.160.170]) by imf15.hostedemail.com (Postfix) with ESMTP id 342B3A0029 for ; Wed, 10 Jul 2024 23:11:08 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=SeX8Hovl; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf15.hostedemail.com: domain of jthoughton@google.com designates 209.85.160.170 as permitted sender) smtp.mailfrom=jthoughton@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720653041; a=rsa-sha256; cv=none; b=RDzgAA3IiX1n+BpT+fj8G/ltxzXbYDh82Y/HVS64mncgLUlex+RrhCgTsFZuH3MI9XbOKf 2RlAkvtvsXNrO5vdC2WLlbX4peIpjTsgAt7bPikd2Ql6gJMWUeccl9j5uXe7XZHQe2oDHW +WV6n0SXOdOeP1N8foQUGTyu433Z0k4= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=SeX8Hovl; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf15.hostedemail.com: domain of jthoughton@google.com designates 209.85.160.170 as permitted sender) smtp.mailfrom=jthoughton@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720653041; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=MJTeSm6eMBTbHXnqICN2W1jCeLI7nzO2Ktbu8vr4Dfo=; b=S2016WEwQlq22+pDxh03sf/pkyQEdPm7kVmKPWWAa1z1otlhlhHMFXJ53lLcXWm+yRbAEb ePo1VhzjgPpYh1oAmxiz1QFQxcqu346AEprtB7orwhyMv4iHHjSNSqvj19H3B/BItaRQae 9BYf0pt4WqoDkLCMIBcxxrUywLYL++E= Received: by mail-qt1-f170.google.com with SMTP id d75a77b69052e-447f8aa87bfso165751cf.0 for ; Wed, 10 Jul 2024 16:11:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1720653067; x=1721257867; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=MJTeSm6eMBTbHXnqICN2W1jCeLI7nzO2Ktbu8vr4Dfo=; b=SeX8Hovliu3/1ZoGuvvSA5D8ClfXTlvseldF8qw96jCVeRLWfXahdLD5+80y5MV0Dc 2XJa3qoqjNGKlS6R9pKTpgv+cmGE0iaZc0AQVLWCYYaDvJe0cWlKWdDghGaMviMNvXzQ n3Fzqu4k0qFqpv6L/le93grfWIm3cE7xaB2X1UuMq4Q4CPO/H+fPmU4dmBhzEvnU6V0i Qn0GWN3PbILNiQxa4UAYJP9SMNneck+n6vBY2dbTqUQ+Ft9giHZ+oudCiL/Zdq3bK0A7 ws64ctfIs5176IU/WcdHt6uiUUYtcHq/V7TM1uYtsR0UXpmRrpbAE1SK9w1xWmvWZ3SR Lycg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720653067; x=1721257867; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=MJTeSm6eMBTbHXnqICN2W1jCeLI7nzO2Ktbu8vr4Dfo=; b=ly+LmisQxBBIPpO80mF/BkK1Y3KkbTvgQ8ujy2RTpGl58VimpqiFNz42qEeSfN7Uvw QWc86CUmXtlb1ks4KlDGt77OI++JHOU+qlN0g6qv20tg2l/R83kSThZk4SRkcCmFPZmp GH/TyZpj0mDi7gKv2rT0kNJn67v3k3X8QKJ9Tl2n04bONC0JaP6AFzR5WLooD1a7y/4D OjZ9HrYR6+53CYvE4ERu5bXkjQRSHvq8SQ688duF+lAgZ8tcfF/EScwaUc4QLmn2Czzf 8PFT5P0TbNEZrO3C7nqMGhprcKCAVOw4NTchyTnw1WbvxOAc9EbvBdXfB6oEDnnnxQgu 59FA== X-Forwarded-Encrypted: i=1; AJvYcCVY+o+rW+/IOhIXADy4uw4WAHC5It8NaFTJQgQU1jO3rmEar57rFlJddWOMALF4cdDaXIa/AsNiMevZcxlEOuaXEl0= X-Gm-Message-State: AOJu0YxHUJifFBdBIn02gJ8rLsg09bBYH/FpOw7yw5NthnGLvVHsA4Uw KK75R/d5AkCZPm1JRHB4agIoHmfidlRfsFLp7dN1Q1u4XPd9G4kdmc4BicX6cHL7QsjbfJ2hAju KCKGcvEvEqLeoEfnkdKJh9PttDH1Aw83Mi5vc X-Google-Smtp-Source: AGHT+IGzszVF+IxKrY/ClY51Y2kjfgE37Xj8trnZtH5JYTor6nkkszob6If5ud2bc91ZsUHmQ4RjHdSOlc0bNNlnZxM= X-Received: by 2002:ac8:7cba:0:b0:447:f958:ab83 with SMTP id d75a77b69052e-44d35b2b31emr557951cf.21.1720653067221; Wed, 10 Jul 2024 16:11:07 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: James Houghton Date: Wed, 10 Jul 2024 16:10:29 -0700 Message-ID: Subject: Re: [PATCH v5 4/9] mm: Add test_clear_young_fast_only MMU notifier To: Sean Christopherson Cc: Yu Zhao , Andrew Morton , Paolo Bonzini , Ankit Agrawal , Axel Rasmussen , Catalin Marinas , David Matlack , David Rientjes , James Morse , Jonathan Corbet , Marc Zyngier , Oliver Upton , Raghavendra Rao Ananta , Ryan Roberts , Shaoqin Huang , Suzuki K Poulose , Wei Xu , Will Deacon , 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 342B3A0029 X-Stat-Signature: angdn1zzzqjjoqdr1hhfqqgnou14pska X-Rspam-User: X-HE-Tag: 1720653068-555416 X-HE-Meta: U2FsdGVkX1/30ajQ+RbOeJRVLAFKIHma2o5RQI49uYsVeFV/Y94eqmjnoI966Wo3cLtHeXWhmj6d2cWMQ8lehq3zLRYje1a6BbbkoBkye+2pUtO2QJaYPOKOgUojJVb5Eeg5lQN+32FMHOm6AnZlxxBQPI8JYaghRg3R4ZJFfddNiVmk2qTyUlKE3OWSyTLDf+9QWGG2OyosOw0/AnCmptqM2EVM6fk3A0+tazJIMtIQuA1/J6dNi9kWMQeMQkkmN7xyAujQr/mBJWIKaLBKkL+ET4ZwVU6yKcIjrqiLkYVgoDFZFp0r1Nkf4rPOmSWgMxHG9IsxTNw6Q5olJqx/zA9s5ZuuaJRebAIWt5EkuvRYypok2P9wc00Uo12DH+qBmmib8t6YftuGyJ+00tIf7BFULKNOZSvVWrS7HmNA4CapJ5XrR1dS0Hb2i1DhKoFvtYArEdpQAypKvH8/TAd85l8auPqQuPD7neWxzflGORM7hR6Y7//JkG/VHDUmYAm20U4THoLusMkroCtEl9c2JRoMMbuR9mm59YbM0/zJb7+7k9cMyDE9ps8NqdFIxmJSDIuhNR/s+0/DNt9uoNsNiCNrh/WzT56jOrlkenqwMmUE1J65VW7E6XyUSgxDzAGr+BKhjTMJ3kGg7qE6PspkAlQy21PjksWh2j+P266b93kOrQlxQcgTtcFP9Uw6TdRS2teuWiLGSW99VZslbcl4JjADAgRNZbMSOz7oSfmlrFI3IShyT4zx+bQ1G+njwzb4igFJheefdZEa097vqJ1WdS4zEJGRTsDyoPu9ATq4bPDXZIsCwmxIL5xejd/RuHjLGrOndApQ+6xLkOYHRdXqCf/3UYwoQhn1M1OjAjVusuwldYFpHdT/f1bDtlmkSwCTChMr3wN1E5ffG4qq99ZUwbWTTWgNmH828Kg/zMwf7BPiJXnaw+9rar7b9zVkqDGjeHlO+p6630SdBNsGyEH QjqwvQzO qHanj6f2vi9hqLhqLABvKfc2rP8nZSUXc4pI8sJ6TuWwTWTgUHuxP+SpTBVkIg/ctHZ46UQm8csBnY0mWtqM5uzmrG6jbLuAbjuk5KF+UxXClZGzt/vOGH5/a35j0QUFura1J56f5S08hPfYnAKlhEaTNNDXZUWFTVtVjNoqCRYld7f+kbvqFseqEDjgeMWzcIgxBb7JaLNwOzc6yDy/p93zS0indXr5q0UZQ+2yROkyWPwZkJq3OBL+GJoX4YPkZyfKhcSbDBBBXoFpCIW2w71M16h6X7DB2+0dI/T6yY1xiSEQYig6CD3C0+IgFRGzE46eR 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 Tue, Jul 9, 2024 at 10:49=E2=80=AFAM Sean Christopherson wrote: > > On Mon, Jul 08, 2024, James Houghton wrote: > > On Fri, Jun 28, 2024 at 7:38=E2=80=AFPM James Houghton wrote: > > > > > > On Mon, Jun 17, 2024 at 11:37=E2=80=AFAM Sean Christopherson wrote: > > I still don't think we should get rid of the WAS_FAST stuff. > > I do :-) > > > The assumption that the L1 VM will almost never share pages between L2 > > VMs is questionable. The real question becomes: do we care to have > > accurate age information for this case? I think so. > > I think you're conflating two different things. WAS_FAST isn't about acc= uracy, > it's about supporting lookaround in conditionally fast secondary MMUs. > > Accuracy only comes into play when we're talking about the last-minute ch= eck, > which, IIUC, has nothing to do with WAS_FAST because any potential lookar= ound has > already been performed. Sorry, I thought you meant: have the MMU notifier only ever be lockless (when tdp_mmu_enabled), and just return a potentially wrong result in the unlikely case that L1 is sharing pages between L2s. I think it's totally fine to just drop WAS_FAST. So then we can either do look-around (1) always, or (2) only when there is a secondary MMU with has_fast_aging. (2) is pretty simple, I'll just do that. We can add some shadow MMU lockless support later to make the look-around not as useless for the nested TDP case. > > It's not completely trivial to get the lockless walking of the shadow > > MMU rmaps correct either (please see the patch I attached here[1]). > > Heh, it's not correct. Invoking synchronize_rcu() in kvm_mmu_commit_zap_= page() > is illegal, as mmu_lock (rwlock) is held and synchronize_rcu() might_slee= p(). > > For kvm_test_age_rmap_fast(), KVM can blindly read READ_ONCE(*sptep). KV= M might > read garbage, but that would be an _extremely_ rare scenario, and reporti= ng a > zapped page as being young is acceptable in that 1 in a billion situation= . > > For kvm_age_rmap_fast(), i.e. where KVM needs to write, I'm pretty sure K= VM can > handle that by rechecking the rmap and using CMPXCHG to write the SPTE. = If the > rmap is unchanged, then the old SPTE value is guaranteed to be valid, in = the sense > that its value most definitely came from a KVM shadow page table. Ah, dr= at, that > won't work, because very theoretically, the page table could be freed, re= allocated, > and rewritten with the exact same value by something other than KVM. Hrm= . > > Looking more closely, I think we can go straight to supporting rmap walks= outside > of mmu_lock. There will still be a "lock", but it will be a *very* rudim= entary > lock, akin to the TDP MMU's REMOVED_SPTE approach. Bit 0 of rmap_head->v= al is > used to indicate "many", while bits 63:3/31:2 on 64-bit/32-bit KVM hold t= he > pointer (to a SPTE or a list). That means bit 1 is available for shenani= gans. > > If we use bit 1 to lock the rmap, then the fast mmu_notifier can safely w= alk the > entire rmap chain. And with a reader/write scheme, the rmap walks that a= re > performed under mmu_lock don't need to lock the rmap, which means flows l= ike > kvm_mmu_zap_collapsible_spte() don't need to be modified to avoid recursi= ve > self-deadlock. Lastly, the locking can be conditioned on the rmap being = valid, > i.e. having at least one SPTE. That way the common case of a gfn not hav= ing any > rmaps is a glorified nop. > > Adding the locking isn't actually all that difficult, with the *huge* cav= eat that > the below patch is compile-tested only. The vast majority of the churn i= s to make > it so existing code ignores the new KVM_RMAP_LOCKED bit. This is very interesting, thanks for laying out how this could be done. I don't want to hold this series up on getting the details of the shadow MMU lockless walk exactly right. :) > I don't know that we should pursue such an approach in this series unless= we have > to. E.g. if we can avoid WAS_FAST or don't have to carry too much interm= ediate > complexity, then it'd probably be better to land the TDP MMU support firs= t and > then add nested TDP support later. Agreed! > At the very least, it does make me more confident that a fast walk of the= rmaps > is very doable (at least for nested TDP), i.e. makes me even more steadfa= st > against adding WAS_FAST. > > > And the WAS_FAST functionality isn't even that complex to begin with. > > I agree the raw code isn't terribly complex, but it's not trivial either.= And the > concept and *behavior* is complex, which is just as much of a maintenance= burden > as the code itself. E.g. it requires knowing that KVM has multiple MMUs = buried > behind a single mmu_notifier, and that a "hit" on the fast MMU will trigg= er > lookaround on the fast MMU, but not the slow MMU. Understanding and desc= ribing > the implications of that behavior isn't easy. E.g. if GFN=3DX is young i= n the TDP > MMU, but X+1..X+N are young only in the shadow MMU, is doing lookaround a= nd making > decisions based purely on the TDP MMU state the "right" behavior? > > I also really don't like bleeding KVM details into the mmu_nofitier APIs.= The > need for WAS_FAST is 100% a KVM limitation. AFAIK, no other secondary MM= U has > multiple MMU implementations active behind a single notifier, and other t= han lack > of support, nothing fundamentally prevents a fast query in the shadow MMU= . Makes sense. So in v6, I will make the following changes: 1. Drop the WAS_FAST complexity. 2. Add a function like mm_has_fast_aging_notifiers(), use that to determine if we should be doing look-around. 3. Maybe change the notifier calls slightly[1], still need to check perform= ance. Does that sound good to you? Thanks! [1]: https://lore.kernel.org/linux-mm/CAOUHufb2f_EwHY5LQ59k7Nh7aS1-ZbOKtkoy= sb8BtxRNRFMypQ@mail.gmail.com/