Message-ID: <6d665575-7658-4619-a763-438e0d3aaf2e@sirena.org.uk>
Date:   Mon, 9 Oct 2023 20:43:44 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     Martin Povišer <povik+lin@...ebit.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, asahi@...ts.linux.dev,
        alsa-devel@...a-project.org, Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Tom Rix <trix@...hat.com>, linux-kernel@...r.kernel.org,
        llvm@...ts.linux.dev, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] ASoC: apple: mca: Annotate struct mca_data with
 __counted_by

On Mon, Oct 09, 2023 at 10:17:33AM -0700, Kees Cook wrote:
> On Fri, Oct 06, 2023 at 09:53:49PM +0100, Mark Brown wrote:

> > Please don't send content free pings and please allow a reasonable time
> > for review.  People get busy, go on holiday, attend conferences and so 
> > on so unless there is some reason for urgency (like critical bug fixes)
> > please allow at least a couple of weeks for review.  If there have been
> > review comments then people may be waiting for those to be addressed.

> I'm happy to do whatever you'd like for this kind of thing, but I'm
> annoyed by this likely automated response seems to ask for the things
> that have already happened or generally don't make sense. :P

It's a form letter so not quite automated but sure.  Since it's the same
form letter I send for all these pings it covers a bunch of things that
might not apply in each individual case.

> - It _has_ been 2 weeks.

That's *at least* two weeks.  For a non-urgent change like this I'd
generally go with longer than that, for example I'd originally had these
changes queued for -rc5 to give the driver maintainers a couple of weeks
to look at them (my scripting understands -rcs more than dates so you'll
see more patches going in on Mondays).  

> - Review comments have _not_ required changes.
> - Sending a no-change patch is just as much email as sending a ping.

A no-change patch is directly and readily actionable, a ping typically
requires going and digging out the original mail or sending a reply
asking for a resend.

> - It's not content-free: I'm asking if you're going to take it;
>   patches have gotten lost in the past, so it's a valid question.

That is not something I can meaningfully distinguish from being content
free, it provides no new information.  Something with content would be
for example information about dependencies progressing.

> - I'm not interested in other subsystems, I'm interested in yours. :P

> You've made it clear you don't want me to pick up these kinds of trivial
> patches that would normally go through your tree, so I'm left waiting
> with no indication if you've seen the patch.

Sure, but that seems fairly normal for the kernel - when sending this
sort of stuff myself I'd be leaving it more like a month before I got
particularly worried.  One way or another it seems fairly common for
things to be left for at least a couple of weeks with things like
waiting for review, restrictions on when patches actually get applied
and just people being busy or whatever.

Personally for incoming patches when I'm leaving time for driver
maintainers I tend to go for leaving things for a -rc or two - things
like who's involved, how early it is in the week when the original patch
gets sent and how late in the release cycle we are will factor in there.
More urgent things like fixes will tend to go faster, minor stuff that
just needs to be handled sometime before the next release will tend to
be slower.

I don't send out mails saying that I've reviewed and queued things
before actually applying them since doing that tends to discourage other
people from doing review and I'd rather they did, this means I don't
generally send out entirely positive review comments prior to applying
anything unless I'm actively chasing for feedback from someone.  It can
also be a bit confusing for people if I tell them something is OK then
later run into test issues.

> My normal routine with treewide changes is to pick up trivial stuff that
> has gotten review but the traditional maintainer hasn't responded to
> in 2 weeks.

> Do you want these kinds of patches to be re-sent every 2 weeks if they
> haven't been replied to by you?

No, please leave it longer - that's the main thing here, you're not
leaving adequate time for non-urgent patches like this.  If you leave it
two weeks for maintainer review and I also leave it two weeks for
maintainer review then we will both expire the timers at the same time
and we're going to trample over each other.  For me it will typically be
a bit more or less than two weeks rather than two weeks to the day but
IIRC the time you applied something it was while the patch was actually
running through my CI.

Off the top of my head I'd say wait at least three weeks for this sort
of patch before doing anything and then prefer to do a resend, that's
should avoid most issues.  If you're going to just apply things yourself
I'd suggest waiting for -rc6 or so before doing so (assuming the patches
were initially sent reasonably early), that does seem like a reasonable
backstop so things don't completely miss releases.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists