Re: [Ksummit-discuss] [MAINTAINERS SUMMIT] Pull network and Patch Acceptance Consistency

Mauro Carvalho Chehab
2019-06-14T15:11:48Z

Em Fri, 14 Jun 2019 15:58:07 +0200
Greg KH <greg@kroah.com> escreveu:

> On Fri, Jun 14, 2019 at 10:24:24AM -0300, Mauro Carvalho Chehab wrote:
> > Em Fri, 14 Jun 2019 13:12:22 +0300
> > Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:
> >
> > > Hi Mauro,
> > >
> > > On Thu, Jun 13, 2019 at 10:59:16AM -0300, Mauro Carvalho Chehab wrote:
> > > > Em Thu, 06 Jun 2019 19:24:35 +0300 James Bottomley escreveu:
> > > >
> > > > > [splitting issues to shorten replies]
> > > > > On Thu, 2019-06-06 at 17:58 +0200, Greg KH wrote:
> > > > >> On Thu, Jun 06, 2019 at 06:48:36PM +0300, James Bottomley wrote:
> > > > >>> This is probably best done as two separate topics
> > > > >>>
> > > > >>> 1) Pull network: The pull depth is effectively how many pulls your
> > > > >>> tree does before it goes to Linus, so pull depth 0 is sent straight
> > > > >>> to Linus, pull depth 1 is sent to a maintainer who sends to Linus
> > > > >>> and so on. We've previously spent time discussing how increasing
> > > > >>> the pull depth of the network would reduce the amount of time Linus
> > > > >>> spends handling pull requests. However, in the areas I play, like
> > > > >>> security, we seem to be moving in the opposite direction
> > > > >>> (encouraging people to go from pull depth 1 to pull depth 0). If
> > > > >>> we're deciding to move to a flat tree model, where everything is
> > > > >>> depth 0, that's fine, I just think we could do with making a formal
> > > > >>> decision on it so we don't waste energy encouraging greater tree
> > > > >>> depth.
> > > > >>
> > > > >> That depth "change" was due to the perceived problems that having a
> > > > >> deeper pull depth was causing. To sort that out, Linus asked for
> > > > >> things to go directly to him.
> > > > >
> > > > > This seems to go beyond problems with one tree and is becoming a trend.
> > > > >
> > > > >> It seems like the real issue is the problem with that subsystem
> > > > >> collection point, and the fact that the depth changed is a sign that
> > > > >> our model works well (i.e. everyone can be routed around.)
> > > > >
> > > > > I'm not really interested in calling out "problem" maintainers, or
> > > > > indeed having another "my patch collection method is better than yours"
> > > > > type discussion. What I was fishing for is whether the general
> > > > > impression that greater tree depth is worth striving for is actually
> > > > > correct, or we should all give up now and simply accept that the
> > > > > current flat tree is the best we can do, and, indeed is the model that
> > > > > works best for Linus. I get the impression this may be the case, but I
> > > > > think making sure by having an actual discussion among the interested
> > > > > parties who will be at the kernel summit, would be useful.
> > > >
> > > > On media, we came from a "depth 1" model, moving toward a "depth 2" level:
> > > >
> > > > patch author -> media/driver maintainer -> subsystem maintainer -> Linus
> > >
> > > I'd like to use this opportunity to ask again for pull requests to be
> > > pulled instead of cherry-picked.
> >
> > There are other forums for discussing internal media maintainership,
> > like the weekly meetings we have and our own mailing lists.
>
> You all have weekly meetings? That's crazy...

Yep, every week we do a meeting, usually taking about 1 hour via irc,
on this channel:

https://linuxtv.org/irc/irclogger_logs//media-maint

>
> Anyway, I'll reiterate Laurent here, keeping things as a pull instead of
> cherry-picking does make things a lot easier for contributors. I know
> I'm guilty of it as well as a maintainer, but that's only until I start
> trusting the submitter. Once that happens, pulling is _much_ easier as
> a maintainer instead of individual patches for the usual reason that
> linux-next has already verified that the sub-tree works properly before
> I merge it in.
>
> Try it, it might make your load be reduced, it has for me.

If you think this is relevant to a broader audience, let me reply with
a long answer about that. I prepared it and intended to reply to our
internal media maintainer's ML (added as c/c).

Yet, I still think that this is media maintainer's dirty laundry
and should be discussed elsewhere ;-)

---

Laurent,

I already explained a few times, including during the last Media Summit,
but it seems you missed the point.

As shown on our stats:
https://linuxtv.org/patchwork_stats.php

We're receiving about 400 to 1000 patches per month, meaning 18 to 45
patches per working days (22 days/month). From those, we accept about
100 to 300 patches per month (4.5 to 13.6 patches per day).

Currently, I review all accepted patches.

I have bandwidth to review 4.5 to 13.6 patches per day, not without a lot
of personal efforts. For that, I use part of my spare time, as I have other
duties, plus I develop patches myself. So, in order to be able to handle
those, I typically work almost non-stop starting at 6am and sometimes
going up to 10pm. Also, when there are too much stuff pending (like on
busy months), I handle patches also during weekends.

However, 45 patches/day (225 patches per week) is a lot for me to
review. I can't commit to handle such amount of patches.

That's why I review patches after a first review from the other
media maintainers. The way I identify the patches I should review is
when I receive pull requests.

We could do a different workflow. For example, once a media maintainer
review a patch, it could be delegated to me at patchwork. This would likely
increase the time for merging stuff, as the workflow would change from:

+-------+ +------------------+ +---------------+
| patch | -> | media maintainer | -> | submaintainer |
+-------+ +------------------+ +---------------+

to:

+-------+ +------------------+ +---------------+ +------------------+ +---------------+
| patch | -> | media maintainer | -> | submaintainer | -> | media maintainer | -> | submaintainer |
+-------+ +------------------+ +---------------+ +------------------+ +---------------+

\------------------------v--------------------------/ \---------------------v------------------/
Patchwork Pull Request

The pull request part of the new chain could eventually be (semi-)automated
by some scripting that would just do a checksum sum at the received patches
that were previously reviewed by me. If matches, and if it passes on the
usual checks I run for PR patches, it would push on some tree. Still, it
would take more time than the previous flow.

Also, as also discussed during the media summit, in order to have such
kind of automation, we would need to improve our infrastructure, moving
the tests from a noisy/heated server I have over my desk to some VM
inside the cloud, once we get funds for it.

In any case, a discussion that affects the patch latency and our internal
procedures within the media subsystem is something that should be discussed
with other media mantainers, and not at KS.

-

That's said, one day I may not be able to review all accepted patches.
When this day comes, I'll just apply the pull requests I receive.

-

Finally, if you're so interested on improving our maintenance model,
I beg you: please handle the patches delegated to you:

https://patchwork.linuxtv.org/project/linux-media/list/?series=&submitter=&state=&q=&archive=&delegate=2510

As we agreed on our media meetings, I handled about ~60 patches that
were waiting for your review since 2017 a couple of weeks ago -
basically the ones that are not touching the drivers you currently
maintain, but there are still 23 patches sent between 2013-2018
over there, plus the 48 patches sent in 2019.

Thanks,
Mauro