Date:   Thu, 2 Jun 2022 10:12:41 +0800
From:   Like Xu <like.xu.linux@...il.com>
To:     Sean Christopherson <seanjc@...gle.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

Thanks for your sincerity, always.

On 2/6/2022 12:39 am, Sean Christopherson wrote:
> 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.

I actually agree and understand the situation of maintainers/reviewers.
No one wants to maintain flawed code, especially in this community
where the majority of previous contributors disappeared after the code
was merged in. The existing heavy maintenance burden is already visible.

Thus we may have a maintainer/reviewers scalability issue. Due to missing
trust, competence or mastery of rules, most of the patches sent to the list
have no one to point out their flaws. I have privately received many complaints
about the indifference of our community, which is distressing.

To improve that, I propose to add "let's try to accept" before "queued, thanks".

Obviously, "try to accept" is not a 100% commitment and it will fail with high
probability, but such a stance (along with standard clarifications and requirements)
from reviewers and maintainers will make the contributors more concerned,
attract potential volunteers, and focus the efforts of our nominated reviewers.

Such moves include explicit acceptance or rejection, a "try to accept" response
from some key persons (even if it ends up being a failure), or a separate git 
branch,
but please, don't leave a lasting silence, especially for those big series.

Similar moves will increase transparency in decision making to reward and
attract a steady stream of high quality trusted contributors to do more and more
for our community and their employers (if any).

> 
> 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.

I'd like know more about "KVM doesn't even get the basics right" on vPMU. :D

> 
> 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.

Powered by blists - more mailing lists