Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230511235917.639770-1-seanjc@google.com> <20230511235917.639770-6-seanjc@google.com>
 <ZF7IRQZo8g7Lg46V@google.com> <ZF7JW2huc2MjXZFA@google.com>
In-Reply-To: <ZF7JW2huc2MjXZFA@google.com>
From:   David Matlack <dmatlack@google.com>
Date:   Fri, 12 May 2023 16:24:49 -0700
Message-ID: <CALzav=dRtju+-NaGiy7ok3chB=535bxNOqm8c+vOMCa9QKWHsQ@mail.gmail.com>
Subject: Re: [PATCH 5/9] KVM: x86/mmu: Convert "runtime" WARN_ON() assertions
 to WARN_ON_ONCE()
To:     Sean Christopherson <seanjc@google.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, Mingwei Zhang <mizhang@google.com>,
        Jim Mattson <jmattson@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, May 12, 2023 at 4:18=E2=80=AFPM Sean Christopherson <seanjc@google.=
com> wrote:
>
> On Fri, May 12, 2023, David Matlack wrote:
> > On Thu, May 11, 2023 at 04:59:13PM -0700, Sean Christopherson wrote:
> > > Convert all "runtime" assertions, i.e. assertions that can be trigger=
ed
> > > while running vCPUs, from WARN_ON() to WARN_ON_ONCE().  Every WARN in=
 the
> > > MMU that is tied to running vCPUs, i.e. not contained to loading and
> > > initializing KVM, is likely to fire _a lot_ when it does trigger.  E.=
g. if
> > > KVM ends up with a bug that causes a root to be invalidated before th=
e
> > > page fault handler is invoked, pretty much _every_ page fault VM-Exit
> > > triggers the WARN.
> > >
> > > If a WARN is triggered frequently, the resulting spam usually causes =
a lot
> > > of damage of its own, e.g. consumes resources to log the WARN and pol=
lutes
> > > the kernel log, often to the point where other useful information can=
 be
> > > lost.  In many case, the damage caused by the spam is actually worse =
than
> > > the bug itself, e.g. KVM can almost always recover from an unexpected=
ly
> > > invalid root.
> > >
> > > On the flip side, warning every time is rarely helpful for debug and
> > > triage, i.e. a single splat is usually sufficient to point a debugger=
 in
> > > the right direction, and automated testing, e.g. syzkaller, typically=
 runs
> > > with warn_on_panic=3D1, i.e. will never get past the first WARN anywa=
ys.
> >
> > On the topic of syzkaller, we should get them to test with
> > CONFIG_KVM_PROVE_MMU once it's available.
>
> +1
>
> > > Lastly, when an assertions fails multiple times, the stack traces in =
KVM
> > > are almost always identical, i.e. the full splat only needs to be cap=
tured
> > > once.  And _if_ there is value in captruing information about the fai=
led
> > > assert, a ratelimited printk() is sufficient and less likely to rack =
up a
> > > large amount of collateral damage.
> >
> > These are all good arguments and I think they apply to KVM_MMU_WARN_ON(=
)
> > as well. Should we convert that to _ONCE() too?
>
> Already done in this patch :-)  I didn't call it out because that warn al=
so falls
> under the "runtime assertions" umbrella.

Doh! Indeed. I was expecting to see KVM_MMU_WARN_ON() change to
KVM_MMU_WARN_ON_ONCE().