All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Matlack <dmatlack@google.com>
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>
Subject: Re: [PATCH 5/9] KVM: x86/mmu: Convert "runtime" WARN_ON() assertions to WARN_ON_ONCE()
Date: Fri, 12 May 2023 16:14:13 -0700	[thread overview]
Message-ID: <ZF7IRQZo8g7Lg46V@google.com> (raw)
In-Reply-To: <20230511235917.639770-6-seanjc@google.com>

On Thu, May 11, 2023 at 04:59:13PM -0700, Sean Christopherson wrote:
> Convert all "runtime" assertions, i.e. assertions that can be triggered
> 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 the
> 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 pollutes
> 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 unexpectedly
> 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=1, i.e. will never get past the first WARN anyways.

On the topic of syzkaller, we should get them to test with
CONFIG_KVM_PROVE_MMU once it's available.

> 
> 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 captured
> once.  And _if_ there is value in captruing information about the failed
> 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?

  reply	other threads:[~2023-05-12 23:14 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-11 23:59 [PATCH 0/9] KVM: x86/mmu: Clean up MMU_DEBUG and BUG/WARN usage Sean Christopherson
2023-05-11 23:59 ` [PATCH 1/9] KVM: x86/mmu: Delete pgprintk() and all its usage Sean Christopherson
2023-05-11 23:59 ` [PATCH 2/9] KVM: x86/mmu: Delete rmap_printk() " Sean Christopherson
2023-05-11 23:59 ` [PATCH 3/9] KVM: x86/mmu: Delete the "dbg" module param Sean Christopherson
2023-05-11 23:59 ` [PATCH 4/9] KVM: x86/mmu: Rename MMU_WARN_ON() to KVM_MMU_WARN_ON() Sean Christopherson
2023-05-12 23:23   ` David Matlack
2023-05-12 23:30     ` Sean Christopherson
2023-05-12 23:35       ` David Matlack
2023-05-11 23:59 ` [PATCH 5/9] KVM: x86/mmu: Convert "runtime" WARN_ON() assertions to WARN_ON_ONCE() Sean Christopherson
2023-05-12 23:14   ` David Matlack [this message]
2023-05-12 23:18     ` Sean Christopherson
2023-05-12 23:24       ` David Matlack
2023-05-11 23:59 ` [PATCH 6/9] KVM: x86/mmu: Bug the VM if a vCPU ends up in long mode without PAE enabled Sean Christopherson
2023-05-12 23:33   ` David Matlack
2023-05-12 23:40     ` Sean Christopherson
2023-05-11 23:59 ` [PATCH 7/9] KVM: x86/mmu: Replace MMU_DEBUG with proper KVM_PROVE_MMU Kconfig Sean Christopherson
2023-05-11 23:59 ` [PATCH 8/9] KVM: x86/mmu: Plumb "struct kvm" all the way to pte_list_remove() Sean Christopherson
2023-05-11 23:59 ` [PATCH 9/9] KVM: x86/mmu: BUG() in rmap helpers iff CONFIG_BUG_ON_DATA_CORRUPTION=y Sean Christopherson
2023-05-18 19:05   ` Mingwei Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZF7IRQZo8g7Lg46V@google.com \
    --to=dmatlack@google.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mizhang@google.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.