Re: [MAINTAINERS SUMMIT] Maintainer burnout

Vegard Nossum
2023-08-21T19:24:14Z

On 8/19/23 08:45, Leon Romanovsky wrote:
> It is worth to try to get honest feedback from active developers/contributors/vendors
> what is their "real" excuse for not doing code review.
>
> I saw in this thread about "have no time to do code review" answers and we
> all can relate to it, but IMHO it is just an excuse and not the real reason.
> Especially for a people who are employed by big corporations to do their
> upstream work.

Hi,

For some drive-by or would-be reviewers, at least, I think part of the
problem is perverse or misaligned incentives.

If you write code and your patches are accepted in the kernel, it counts
towards your commit count, which is a metric that people look at, for
better or worse (probably worse).

When you review a patch and you find some problem with it, the patch
will NOT get accepted in the kernel (at least not in that form), and
your name will NOT appear in the git log. So in a way, in order for
your contribution to get recorded, you have to give the patch a
passing grade -OR- you are now on the hook to keep reviewing every
following iteration of the patch until it's in a state where you're
completely sure it doesn't have any problems.

(Of course, if you just rubber-stamp your Reviewed-by: on everything
then you are bound to be found out sooner or later -- or at the very
least seen as an unreliable reviewer.)

But let's assume you don't give out your Reviewed-by: without having
REALLY checked the patch thoroughly. Even then, mistakes can slip in.
In a way, being a reviewer and missing something critical is even
worse than being the author and missing something critical. Is it even
worth putting your Reviewed-by: on something if you're not 100% sure
this patch is not going to cause an issue? Are people going to trust
you in the future if you make a mistake in your review?

Let's say you're completely sure you found an issue with the patch. Why
not just stay silent, hope that nobody catches it, and then submit your
own patch later to fix it? That will get your name in the log. Even
worse, if it's a security issue you can maybe write an exploit for it
and either get a bounty from Google or sell it for serious $$$ to
various malicious actors. [Note that I'm not saying most people would do
this; I don't know. I am just using it as an example to show that the
incentives are disproportionate.]

The incentives that remain (as far as I can tell) are:

1) you get familiar with a specific part of the kernel, and
2) you get goodwill and recognition from other kernel developers.

Both assume a certain volume of reviews to pay off in any significant
way. Even then, depending on your ultimate goals, doing reviews is
typically a low-ROI activity for individuals compared to things like
actually writing code and submitting patches.

If the community truly values reviews, then I think it's necessary to
find better mechanisms for recognizing and rewarding reviews.

Can we maybe adjust the standards of the Reviewed-by: tag to mean that
the person has looked at the patch (or an earlier version of it) and
provided comments that show they actually put some effort into it?
For example, if a patch is changing a function (as patches often do),
the reviewer should add a line saying "error paths in foo() lgtm"
and not just tack on their Reviewed-by: line.

This adjustment would make it harder to just slap a Reviewed-by: on a
patch, but it would also make it easier to get your name in the
changelog provided that you actually put the effort in.

Maybe release notes/shortlogs could also include reviewer stats as a
bonus. I'm aware of the regular LWN articles covering reviewer stats,
but again, I think that only includes reviews for patches that actually
passed the review and made it into the kernel.

I suspect that review duty often falls on maintainers because they know
their own subsystem the best (and maybe also because they are the
reviewer of last resort). I think that reviews are a great way to learn
more about a subsystem and we should encourage newcomers and non-experts
to contribute their reviews (and get credit for it) even if those
reviews are not as complete as a maintainer's or subsystem expert's
review would be. And we can encourage that by nudging the incentives
in the right direction.

Vegard