Re: [Ksummit-discuss] bug-introducing patches

From: Willy Tarreau
Date: Sat May 05 2018 - 00:24:38 EST


On Fri, May 04, 2018 at 07:35:42PM -0400, Theodore Y. Ts'o wrote:
> On Fri, May 04, 2018 at 09:51:14PM +0000, Sasha Levin wrote:
> > I don't have an objection to moving this to it's own tag. It will make
> > my scripts somewhat simpler for sure.
>
> It's not a matter "moving this it's own tag", but creating a new tag
> --- because what is in the docs is a lie. It does not describe what
> we do today. And current practice is the reality, not what is in the
> docs.
>
> As to whether we should create a new tag to support explicit
> dependencies, I'll leave that between you and Greg K-H and the rest of
> the stable maintainers. :-)

Guys, *personally*, I've sometimes been a bit annoyed by the huge amount
of irregular extra headers trying to compensate for horribly vague commit
messages, and I'm pretty sure it pisses off patch authors who don't know
anymore what to put in their description. We need to keep in mind that
authors are humans and not machines, and that natural language remains
the best to explain complex dependencies. I'd prefer to see :

This patch needs to be backported to all stable branches that contain
717d3133 and 207f5b3c (that's 3.10+) or their respective backports but
must be adapted (contact me) if only a backport of 717d3133 is present.

Cc: stable # v3.10+

Rather than horrible stuff like this :

Cc: stable # v3.10+ (717d3133 && 207f5b3c) || WARN_ON(back(717d3133))

Of course it's a bit made up, but not too far from what is being discussed
here, probably only the next step. People will often get complex rules
wrong, both on the producer and on the consumer side. The day we need a
compiler to emit commit messages, we'll have to wonder if we didn't go
too far.

Also I've found the Fixes header pretty useful. It allows patch authors
to mention what is being fixed without necessarily copying stable,
because sometimes you'd rather not see your patch immediately backported
or you think the risks are higher than the bug. And here as well, it's
only suited for simple situations with a single commit ID, complex
desriptions have to be part of the commit message body.

I think that what we have now works pretty well but that some descriptions
lack a bit of detail, especially on the impact of the bug which would help
decide to backport or drop. This is understandable because often the person
fixing a bug documents it for people knowing the same subsystem well. But
when you backport fixes into other kernel versions, you don't know well
how each subsystem works, and guessing the impact of a bug is not always
obvious. Most of the time, authors who add Fixes: and/or Cc: stable take
care of providing enough information, though I'd suspect that sometimes
they're making efforts trying to figure how to place the information
there and possibly try to avoid redundancy by writing a shorter body.

At this point, I'm really not seeing what we're trying to improve or
optimize, and to be honest this discussion worries me a bit, by just
thinking that it could result in annoying changes...

Willy