linux-kernel - Re: [PATCH 1/2] KVM: vmx, pmu: accept 0 for absent MSRs when host-initiated

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YpeWPAHNhQQ/lRKF@google.com>
Date:   Wed, 1 Jun 2022 16:39:24 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Like Xu <like.xu.linux@...il.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Subject: Re: [PATCH 1/2] KVM: vmx, pmu: accept 0 for absent MSRs when
 host-initiated

On Wed, Jun 01, 2022, Like Xu wrote:
> On 1/6/2022 2:37 am, Sean Christopherson wrote:
> > Can we just punt this out of kvm/queue until its been properly reviewed?  At the
> > barest of glances, there are multiple flaws that should block this from being
> 
> TBH, our reviewers' attention could not be focused on these patches until the
> day it was ready to be ravaged. "Try to accept" is a good thing, and things need
> to move forward, not simply be abandoned to the side.

I strongly disagree, to put it mildly.  Accepting flawed, buggy code because
reviewers and maintainers are overloaded does not solve anything, it only makes
the problem worse.  More than likely, the submitter(s) has moved on to writing
the next pile of patches, while the same set of people that are trying to review
submissions are left to pick up the pieces.  There are numerous examples of
accepting code without (IMO) proper review and tests costing us dearly in the
long run.

If people want their code to be merged more quickly, then they can do so by
helping address the underlying problems, e.g. write tests that actually try to
break their feature instead of doing the bare minimum, review each others code,
clean up the existing code (and tests!), etc...  There's a reason vPMU features
tend to not get a lot of reviews; KVM doesn't even get the basics right, so there's
not a lot of interest in trying to enable fancy, complex features.

Merging patches/series because they _haven't_ gotten reviews is all kinds of
backwards.  In addition to creating _more_ work for reviewers and maintainers,
it effectively penalizes teams/companies for reviewing each other's code, which
is seriously fubar and again exacerbates the problem of reviewers being overloaded.