[GIT PULL] virtio: last minute fixup

[GIT PULL] virtio: last minute fixup

[Michael S. Tsirkin]

The following changes since commit 1c80cf031e0204fde471558ee40183695773ce13:

  vdpa: mlx5: synchronize driver status with CVQ (2022-03-30 04:18:14 -0400)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

for you to fetch changes up to 7ff960a6fe399fdcbca6159063684671ae57eee9:

  virtio: fix virtio transitional ids (2022-05-10 07:22:28 -0400)

----------------------------------------------------------------
virtio: last minute fixup

A last minute fixup of the transitional ID numbers.
Important to get these right - if users start to depend on the
wrong ones they are very hard to fix.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Shunsuke Mie (1):
      virtio: fix virtio transitional ids

 include/uapi/linux/virtio_ids.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Re: [GIT PULL] virtio: last minute fixup

[Linus Torvalds]

On Tue, May 10, 2022 at 5:24 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> A last minute fixup of the transitional ID numbers.
> Important to get these right - if users start to depend on the
> wrong ones they are very hard to fix.

Hmm. I've pulled this, but those numbers aren't exactly "new".

They've been that way since 5.14, so what makes you think people
haven't already started depending on them?

And - once again - I want to complain about the "Link:" in that commit.

It points to a completely useless patch submission. It doesn't point
to anything useful at all.

I think it's a disease that likely comes from "b4", and people decided
that "hey, I can use the -l parameter to add that Link: field", and it
looks better that way.

And then they add it all the time, whether it makes any sense or not.

I've mainly noticed it with the -tip tree, but maybe that's just
because I've happened to look at it.

I really hate those worthless links that basically add zero actual
information to the commit.

The "Link" field is for _useful_ links. Not "let's add a link just
because we can".

                           Linus

Re: [GIT PULL] virtio: last minute fixup

[Nathan Chancellor]

On Tue, May 10, 2022 at 11:23:11AM -0700, Linus Torvalds wrote:
> On Tue, May 10, 2022 at 5:24 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > A last minute fixup of the transitional ID numbers.
> > Important to get these right - if users start to depend on the
> > wrong ones they are very hard to fix.
> 
> Hmm. I've pulled this, but those numbers aren't exactly "new".
> 
> They've been that way since 5.14, so what makes you think people
> haven't already started depending on them?
> 
> And - once again - I want to complain about the "Link:" in that commit.
> 
> It points to a completely useless patch submission. It doesn't point
> to anything useful at all.
> 
> I think it's a disease that likely comes from "b4", and people decided
> that "hey, I can use the -l parameter to add that Link: field", and it
> looks better that way.
> 
> And then they add it all the time, whether it makes any sense or not.
> 
> I've mainly noticed it with the -tip tree, but maybe that's just
> because I've happened to look at it.
> 
> I really hate those worthless links that basically add zero actual
> information to the commit.
> 
> The "Link" field is for _useful_ links. Not "let's add a link just
> because we can".

For what it's worth, as someone who is frequently tracking down and
reporting issues, a link to the mailing list post in the commit message
makes it much easier to get these reports into the right hands, as the
original posting is going to have all relevant parties in one location
and it will usually have all the context necessary to triage the
problem. While lore.kernel.org has made it much easier to find patch
postings with the "all" list and the search syntax that public-inbox
offers, it is simpler to just import the thread with 'b4 mbox' using the
link directly.

However, I do agree that it should be easier for people to tell whether
or not the link is additional context or information or just a link to
the original patch posting on the mailing list. Perhaps there should be
a new tag like "Archived-at:", "Posted-at:", or "Submitted-at:" that
makes this clearer?

Cheers,
Nathan

Re: [GIT PULL] virtio: last minute fixup

[Linus Torvalds]

On Tue, May 10, 2022 at 4:12 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> For what it's worth, as someone who is frequently tracking down and
> reporting issues, a link to the mailing list post in the commit message
> makes it much easier to get these reports into the right hands, as the
> original posting is going to have all relevant parties in one location
> and it will usually have all the context necessary to triage the
> problem.

Honestly, I think such a thing would be trivial to automate with
something like just a patch-id lookup, rather than a "Link:".

And such a lookup model ("where was this patch posted") would work for
<i>any</i> patch (and often also find previous unmodified versions of
it when it has been posted multiple times).

I suspect that most of the building blocks of such automation
effectively already exists, since I think the lore infrastructure
already integrates with patchwork, and patchwork already has a "look
up by patch id".

Wouldn't it be cool if you had some webby interface to just go from
commit SHA1 to patch ID to a lore.kernel.org lookup of where said
patch was done?

Of course, I personally tend to just search by the commit contents
instead, which works just about as well. If the first line of the
commit isn't very unique, add a "f:author" to the search.

IOW, I really don't find much value in the "Link to original
submission", because that thing is *already* trivial to find, and the
lore search is actually better in many ways (it also tends to find
people *reporting* that commit, which is often what you really want -
the reason you're doing the search is that there's something going on
with it).

My argument here really is that "find where this commit was posted" is

 (a) not generally the most interesting thing

 (b) doesn't even need that "Link:" line.

but what *is* interesting, and where the "Link:" line is very useful,
is finding where the original problem that *caused* that patch to be
posted in the first place.

Yes, obviously you can find that original problem by searching too if
the commit message has enough other information.

For example, if there is an oops quoted in the commit message, I have
personally searched for parts of that kind of information to find the
original report and discussion.

So that whole "searching is often an option" is true for pretty much
_any_ Link:, but I think that for the whole "original submission" it's
so mindless and can be automated that it really doesn't add much real
value at all.

                Linus

Re: [GIT PULL] virtio: last minute fixup

[Michael S. Tsirkin]

On Tue, May 10, 2022 at 04:50:47PM -0700, Linus Torvalds wrote:
> On Tue, May 10, 2022 at 4:12 PM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > For what it's worth, as someone who is frequently tracking down and
> > reporting issues, a link to the mailing list post in the commit message
> > makes it much easier to get these reports into the right hands, as the
> > original posting is going to have all relevant parties in one location
> > and it will usually have all the context necessary to triage the
> > problem.
> 
> Honestly, I think such a thing would be trivial to automate with
> something like just a patch-id lookup, rather than a "Link:".
> 
> And such a lookup model ("where was this patch posted") would work for
> <i>any</i> patch (and often also find previous unmodified versions of
> it when it has been posted multiple times).
> 
> I suspect that most of the building blocks of such automation
> effectively already exists, since I think the lore infrastructure
> already integrates with patchwork, and patchwork already has a "look
> up by patch id".
> 
> Wouldn't it be cool if you had some webby interface to just go from
> commit SHA1 to patch ID to a lore.kernel.org lookup of where said
> patch was done?

Yes, that would be cool!

> Of course, I personally tend to just search by the commit contents
> instead, which works just about as well. If the first line of the
> commit isn't very unique, add a "f:author" to the search.
>
> IOW, I really don't find much value in the "Link to original
> submission", because that thing is *already* trivial to find, and the
> lore search is actually better in many ways (it also tends to find
> people *reporting* that commit, which is often what you really want -
> the reason you're doing the search is that there's something going on
> with it).
> 
> My argument here really is that "find where this commit was posted" is
> 
>  (a) not generally the most interesting thing
> 
>  (b) doesn't even need that "Link:" line.
> 
> but what *is* interesting, and where the "Link:" line is very useful,
> is finding where the original problem that *caused* that patch to be
> posted in the first place.
> 
> Yes, obviously you can find that original problem by searching too if
> the commit message has enough other information.
> 
> For example, if there is an oops quoted in the commit message, I have
> personally searched for parts of that kind of information to find the
> original report and discussion.
> 
> So that whole "searching is often an option" is true for pretty much
> _any_ Link:, but I think that for the whole "original submission" it's
> so mindless and can be automated that it really doesn't add much real
> value at all.
> 
>                 Linus

For me a problematic use-case is multiple versions of the patchset.
So I have a tree and I apply a patchset, start testing etc. Meanwhile author
posts another version. At that point I want to know which version
did I apply. Since people put that within [] in the subject, it
gets stripped off.

Thinking about it some more, how about sticking a link to the *cover
letter* in the commit, instead?  That would serve an extra useful purpose of
being able to figure out which patches are part of the same patchset.
And maybe Change "Link:" to "Patchset:" or "Cover-letter:"?

-- 
MST

Re: [GIT PULL] virtio: last minute fixup

[Konstantin Ryabitsev]

On Tue, May 10, 2022 at 04:50:47PM -0700, Linus Torvalds wrote:
> > For what it's worth, as someone who is frequently tracking down and
> > reporting issues, a link to the mailing list post in the commit message
> > makes it much easier to get these reports into the right hands, as the
> > original posting is going to have all relevant parties in one location
> > and it will usually have all the context necessary to triage the
> > problem.
> 
> Honestly, I think such a thing would be trivial to automate with
> something like just a patch-id lookup, rather than a "Link:".

I'm not sure that's quite reliable, and I'm speaking from experience of
running git-patchwork-bot, which attempts to match commits to patches.
Patch-id has these important disadvantages:

1. git-patch-id can be fragile: if the maintainer changes things like add
   curly braces, rename a variable, or edit a code comment for clarity, the
   patch-id stops matching. This happens routinely with git-patchwork-bot,
   and patchwork uses an even laxer algorithm than git-patch-id. In fact, I
   had to hack git-patchwork-bot to fall back on Link: tags to match by
   message-id to address some of the maintainers' complaints.

2. git-patch-id doesn't include author/date/commit message: which can actually
   be important for establishing provenance and attribution and can confuse
   automation. E.g. an author submits a patch as part of a large series, gets
   told to break it apart, then submits it as part of a different series.
   Automated processes trying to match commits to submissions won't be able to
   tell from which series the commit came from.

Cregit folks (cregit.linuxsources.org) have encountered all of these and I
know from talking to them that they are quite happy to have a way to match
commit provenance to the exact messages in the archives.

> Wouldn't it be cool if you had some webby interface to just go from
> commit SHA1 to patch ID to a lore.kernel.org lookup of where said
> patch was done?

Yes, it's https://cregit.linuxsources.org/ and it's... okay. :) It certainly
doesn't manage to match all commits to patches despite having access to all of
lore.kernel.org archives.

> My argument here really is that "find where this commit was posted" is
> 
>  (a) not generally the most interesting thing
> 
>  (b) doesn't even need that "Link:" line.
> 
> but what *is* interesting, and where the "Link:" line is very useful,
> is finding where the original problem that *caused* that patch to be
> posted in the first place.

I think the disconnect here is that you're approaching this from the
perspective of a human being, while what many want is a dumb and reliable way
to match commits to ML submissions, which will allow improving unattended
automation.

> So that whole "searching is often an option" is true for pretty much
> _any_ Link:, but I think that for the whole "original submission" it's
> so mindless and can be automated that it really doesn't add much real
> value at all.

Believe me, I've tried, and I really, really like having a fool-proof way to
match commits directly to the exact ML submissions. :( Even a 99%-reliable
fuzzy matching algorithm has enough of a failure rate that causes maintainers
to get annoyed -- I have many "git-patchwork-bot missed this commit"
complaints in the queue to prove this.

I think we should simply disambiguate the trailer added by tooling like b4.
Instead of using Link:, it can go back to using Message-Id, which is already
standard with git -- it's trivial for git.kernel.org to link them to
lore.kernel.org.

Before:

    Signed-off-by: Main Tainer <main.tainer@linux.dev>
    Link: https://lore.kernel.org/r/CAHk-=wgAk3NEJ2PHtb0jXzCUOGytiHLq=rzjkFKfpiuH-SROgA@mail.gmail.com

After:

    Signed-off-by: Main Tainer <main.tainer@linux.dev>
    Message-Id: <CAHk-=wgAk3NEJ2PHtb0jXzCUOGytiHLq=rzjkFKfpiuH-SROgA@mail.gmail.com>

This would allow people to still use Link: for things like linking to actual
ML discussions. I know this pollutes commits a bit, but I would argue that
this is a worthwhile trade-off that allows us to improve our automation and
better scale maintainers.

-K

Re: [GIT PULL] virtio: last minute fixup

[Michael Ellerman]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:
...
>
> I think we should simply disambiguate the trailer added by tooling like b4.
> Instead of using Link:, it can go back to using Message-Id, which is already
> standard with git -- it's trivial for git.kernel.org to link them to
> lore.kernel.org.

But my mailer, editor and terminal don't know what to do with a Message-Id.

Whereas they can all open an https link.

Making people paste message ids into lore to see the original submission
is not a win. People make enough fun of us already for still using email
to submit patches, let's not make their job any easier :)

> Before:
>
>     Signed-off-by: Main Tainer <main.tainer@linux.dev>
>     Link: https://lore.kernel.org/r/CAHk-=wgAk3NEJ2PHtb0jXzCUOGytiHLq=rzjkFKfpiuH-SROgA@mail.gmail.com
>
> After:
>
>     Signed-off-by: Main Tainer <main.tainer@linux.dev>
>     Message-Id: <CAHk-=wgAk3NEJ2PHtb0jXzCUOGytiHLq=rzjkFKfpiuH-SROgA@mail.gmail.com>
>
> This would allow people to still use Link: for things like linking to actual
> ML discussions. I know this pollutes commits a bit, but I would argue that
> this is a worthwhile trade-off that allows us to improve our automation and
> better scale maintainers.

I went back through the history and I'm pretty sure that the original use
for "Link:" was to link to the original submission, done by tip-bot
starting back in 2011:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f994d99cf140dbb637e49882891c89b3fd84becd

Prior to that there were no "Link:" tags, only "BugLink:".

But if people want to reclaim "Link:" for generic links then fine, but
let's still use a https link, just give it a different name.
eg. "PatchLink:", or "Submitted:" etc.

cheers

Re: [GIT PULL] virtio: last minute fixup

[Konstantin Ryabitsev]

On Wed, May 11, 2022 at 11:40:59PM +1000, Michael Ellerman wrote:
> > I think we should simply disambiguate the trailer added by tooling like b4.
> > Instead of using Link:, it can go back to using Message-Id, which is already
> > standard with git -- it's trivial for git.kernel.org to link them to
> > lore.kernel.org.
> 
> But my mailer, editor and terminal don't know what to do with a Message-Id.
> 
> Whereas they can all open an https link.
> 
> Making people paste message ids into lore to see the original submission
> is not a win. People make enough fun of us already for still using email
> to submit patches, let's not make their job any easier :)

Okay, I'm fine with using a dedicated trailer for this purpose, perhaps an
"Archived-At"? That's a real header that was proposed by IETF for similar
purposes. E.g.:

    Archived-at: https://lore.kernel.org/r/CAHk-=wgAk3NEJ2PHtb0jXzCUOGytiHLq=rzjkFKfpiuH-SROgA@mail.gmail.com

-K

Re: [GIT PULL] virtio: last minute fixup

[Theodore Ts'o]

On Wed, May 11, 2022 at 12:31:16PM -0400, Konstantin Ryabitsev wrote:
> > But my mailer, editor and terminal don't know what to do with a Message-Id.
> > 
> > Whereas they can all open an https link.
> > 
> > Making people paste message ids into lore to see the original submission
> > is not a win. People make enough fun of us already for still using email
> > to submit patches, let's not make their job any easier :)
> 
> Okay, I'm fine with using a dedicated trailer for this purpose, perhaps an
> "Archived-At"? That's a real header that was proposed by IETF for similar
> purposes. E.g.:
> 
>     Archived-at: https://lore.kernel.org/r/CAHk-=wgAk3NEJ2PHtb0jXzCUOGytiHLq=rzjkFKfpiuH-SROgA@mail.gmail.com
>

I'd suggest is "Patch-Link".  Then we can also have "Bug-Link:",
"Test-Link:", etc.

"Patch-Link" is a tad bit shorter "Archived-at", and ultimately, it's
not actually not the patch which is being archived.  It's the fact
that it's a pointer to the patch review which is of most interest.

					- Ted

Re: [GIT PULL] virtio: last minute fixup

[Dave Taht]

On Wed, May 11, 2022 at 2:54 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:

> but what *is* interesting, and where the "Link:" line is very useful,
> is finding where the original problem that *caused* that patch to be
> posted in the first place.

More generally, inside and outside the linux universe, a search engine
that searched for all the *closed bugs* and their symptoms, in the
world would often be helpful. There is such a long deployment tail and
in modern bug trackers the closed bugs tend to vanish, even though the
problem might still exist in the field for another decade or more.


-- 
FQ World Domination pending: https://blog.cerowrt.org/post/state_of_fq_codel/
Dave Täht CEO, TekLibre, LLC

Re: [GIT PULL] virtio: last minute fixup

[Michael S. Tsirkin]

On Tue, May 10, 2022 at 11:23:11AM -0700, Linus Torvalds wrote:
> On Tue, May 10, 2022 at 5:24 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > A last minute fixup of the transitional ID numbers.
> > Important to get these right - if users start to depend on the
> > wrong ones they are very hard to fix.
> 
> Hmm. I've pulled this, but those numbers aren't exactly "new".
> 
> They've been that way since 5.14, so what makes you think people
> haven't already started depending on them?

Yes they have been in the header but they are not used by *Linux* yet.
My worry is for when we start using them and then someone backports
the patches without backporting the macro fix.
Maybe we should just drop these until there's a user, but I am
a bit wary of a step like this so late in the cycle.

> And - once again - I want to complain about the "Link:" in that commit.
> 
> It points to a completely useless patch submission. It doesn't point
> to anything useful at all.
> 
> I think it's a disease that likely comes from "b4", and people decided
> that "hey, I can use the -l parameter to add that Link: field", and it
> looks better that way.
> 
> And then they add it all the time, whether it makes any sense or not.
> 
> I've mainly noticed it with the -tip tree, but maybe that's just
> because I've happened to look at it.
> 
> I really hate those worthless links that basically add zero actual
> information to the commit.
> 
> The "Link" field is for _useful_ links. Not "let's add a link just
> because we can".
> 
>                            Linus


OK I will stop doing this.
I thought they are handy for when there are several versions of the
patch. It helps me make sure I applied the latest one. Saving the
message ID of the original mail in some other way would also be ok.
Any suggestions for a better way to do this?

-- 
MST

Re: [GIT PULL] virtio: last minute fixup

[Michael Ellerman]

Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Tue, May 10, 2022 at 5:24 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> A last minute fixup of the transitional ID numbers.
>> Important to get these right - if users start to depend on the
>> wrong ones they are very hard to fix.
>
> Hmm. I've pulled this, but those numbers aren't exactly "new".
>
> They've been that way since 5.14, so what makes you think people
> haven't already started depending on them?
>
> And - once again - I want to complain about the "Link:" in that commit.
>
> It points to a completely useless patch submission. It doesn't point
> to anything useful at all.
>
> I think it's a disease that likely comes from "b4", and people decided
> that "hey, I can use the -l parameter to add that Link: field", and it
> looks better that way.

Some folks have been doing it since the early 2010s.

But I think it really took off after the Change-Id discussion a few
years back:

  https://lore.kernel.org/all/CAHk-=whFbgy4RXG11c_=S7O-248oWmwB_aZOcWzWMVh3w7=RCw@mail.gmail.com/

Which I read as you endorsing Link: tags :)

> And then they add it all the time, whether it makes any sense or not.

For me it always makes sense, because it means I can easily go from a
commit back to the original submission. That's useful for automating
various things like replies and patchwork status updates.

It allows me to find the exact patch I applied, even if what I committed
is slightly different (due to fuzz or editing), which would be harder
with a search based approach.

It gives us a way to essentially augment the change log after the fact,
by replying to the original patch with things we didn't know at the time
of commit - eg. this patch was reverted because it caused a bug, etc.

If you follow the Link: and there's nothing useful there explaining
what motivated the change then that's a bug in the patch submission, not
the Link: tag.

Really important information should be in the change log itself, but the
space below the "---" is perfect for added context that would be too
verbose for the committed change log. And anyone can reply to the
original submission to add even more useful information.

cheers

Re: [GIT PULL] virtio: last minute fixup

[Linus Torvalds]

On Wed, May 11, 2022 at 3:12 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Which I read as you endorsing Link: tags :)

I absolutely adore "Link:" tags. They've been great.

But they've been great for links that are *usedful*.

They are wonderful when they link to the original problem.

They are *really* wonderful when they link to some long discussion
about how to solve the problem.

They are completely useless when they link to "this is the patch
submission of the SAME DAMN PATCH THAT THE COMMIT IS".

See the difference?

The two first links add actual new information.

That last link adds absolutely nothing. It's a link to the same email
that was just applied.

                   Linus

Re: [GIT PULL] virtio: last minute fixup

[Michael Ellerman]

Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Wed, May 11, 2022 at 3:12 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Which I read as you endorsing Link: tags :)
>
> I absolutely adore "Link:" tags. They've been great.
>
> But they've been great for links that are *usedful*.
>
> They are wonderful when they link to the original problem.
>
> They are *really* wonderful when they link to some long discussion
> about how to solve the problem.
>
> They are completely useless when they link to "this is the patch
> submission of the SAME DAMN PATCH THAT THE COMMIT IS".

Folks wanted to add Change-Id: tags to every commit.

You said we didn't need to, because we have the Link: to the original
patch submission, which includes the Message-Id and therefore is a
de facto change id.

Links to other random places don't serve that function.

cheers

Re: [GIT PULL] virtio: last minute fixup

[Linus Torvalds]

On Thu, May 12, 2022 at 6:30 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Links to other random places don't serve that function.

What "function"?

This is my argument. Those Link: things need to have a *reason*.

Saying "they are a change ID" is not a reason. That's just a random
word-salad. You need to have an active reason that you can explain,
not just say "look, I want to add a message ID to every commit".

Here's the thing. There's a difference between "data" and "information".

We should add information to the commits, not random data.

And most definitely not just random data that can be trivially
auto-generated after-the-fact.

                Linus

Re: [GIT PULL] virtio: last minute fixup

[Linus Torvalds]

On Thu, May 12, 2022 at 10:10 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> And most definitely not just random data that can be trivially
> auto-generated after-the-fact.

Put another way: when people asked for change ID's and I said "we have
links", I by no means meant that "you can just add random worthless
links to commits".

For example, if you have a (public-facing) Gerrit system that tracks a
patch before it gets committed, BY ALL MEANS add a link to that as the
"change ID" that you tracked in Gerrit.

That's a Link: that actually adds *information*. It shows some real
history to the commit, and shows who approved it and when, and gives
you all the Gerrit background.

But a link to the email on lkml that just contains the patch and the
same commentary that was introduced into the commit? Useless garbage.
It adds no actual information.

THAT is my argument. Why do people think I'm arguing against the Link:
tag? No. I'm arguing against adding links with no relevant new
information behind them.

I don't argue against links to lore. Not at all. If those links are
about the background that caused the patch, they are great. Maybe they
are to a long thread about the original problem and how to solve it.
Thats WONDERFUL.

But here's the deal: when I look at a commit that I wonder "why is it
doing this, it seems wrong" (possibly after there's been a bug report
about it, but possibly just because I'm reviewing it as part of doing
the pull), and I see a "Link:" tag, and it just points back to the
SAME DAMN DATA that I already have in the commit, then that Link: tag
not only wasn't helpful, it was ACTIVELY DETRIMENTAL and made me waste
time and just get irritated.

And if you waste my time with useless links, why would you expect me
to be supportive of that behavior?

                      Linus

Re: [GIT PULL] virtio: last minute fixup

[Eric W. Biederman]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Thu, May 12, 2022 at 10:10 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> And most definitely not just random data that can be trivially
>> auto-generated after-the-fact.
>
> Put another way: when people asked for change ID's and I said "we have
> links", I by no means meant that "you can just add random worthless
> links to commits".
>
> For example, if you have a (public-facing) Gerrit system that tracks a
> patch before it gets committed, BY ALL MEANS add a link to that as the
> "change ID" that you tracked in Gerrit.
>
> That's a Link: that actually adds *information*. It shows some real
> history to the commit, and shows who approved it and when, and gives
> you all the Gerrit background.
>
> But a link to the email on lkml that just contains the patch and the
> same commentary that was introduced into the commit? Useless garbage.
> It adds no actual information.
>
> THAT is my argument. Why do people think I'm arguing against the Link:
> tag? No. I'm arguing against adding links with no relevant new
> information behind them.
>
> I don't argue against links to lore. Not at all. If those links are
> about the background that caused the patch, they are great. Maybe they
> are to a long thread about the original problem and how to solve it.
> Thats WONDERFUL.
>
> But here's the deal: when I look at a commit that I wonder "why is it
> doing this, it seems wrong" (possibly after there's been a bug report
> about it, but possibly just because I'm reviewing it as part of doing
> the pull), and I see a "Link:" tag, and it just points back to the
> SAME DAMN DATA that I already have in the commit, then that Link: tag
> not only wasn't helpful, it was ACTIVELY DETRIMENTAL and made me waste
> time and just get irritated.
>
> And if you waste my time with useless links, why would you expect me
> to be supportive of that behavior?

You know.  I have exactly the same reaction about your proposal to
remove the Link tag.

I hate it when v1, v2, v3, v4, etc are not part of the same thread.

I find it very useful to go directly to the patch submission by
following a single url and see the whole entire conversation right
there.

I don't relish the need to instead perform a search and waste my time
filtering through similar submissions just to find the thread where
things happened and what people were thinking.

It is human and messy and unstructured so we probably need a search
engine to find parts of historical conversations, but gosh darn search
engines can take a lot of work to get useful results out of.


As for finding the original problem that can be very hard.  I recently
had someone report a problem in code that had not changed in a decade or
so that had just appeared.  They had just happened to run ltp on a big
enough machine where a poorly written test stressed the hardware on a
large enough machine in just the right way that things started falling
over.

It took asking several times to find that out.


So sure let's aim at getting more and better information in commits and
in the urls that we place in commits.  But let's not throw the baby out
with the bath water and stop doing the part we can automate, because
we have done such a good job of automating the indexing that we can
usually find it with a simple search.

Let's instead aim to keep the conversation connected, and the threads
not broken so that following the url that is the easy thing to create
gives us much more information.

Eric

Re: [GIT PULL] virtio: last minute fixup

[Jakub Kicinski]

On Fri, 13 May 2022 09:14:46 -0500 Eric W. Biederman wrote:
> I hate it when v1, v2, v3, v4, etc are not part of the same thread.
> 
> I find it very useful to go directly to the patch submission by
> following a single url and see the whole entire conversation right
> there.

I was gonna mention the --in-reply-to posting as a potential way around
the problem as well. But it comes with its own downsides - the threads
get huge and hard to keep track of at least in my patch flow.

Re: [GIT PULL] virtio: last minute fixup

[Michael S. Tsirkin]

On Thu, May 12, 2022 at 10:10:34AM -0700, Linus Torvalds wrote:
> On Thu, May 12, 2022 at 6:30 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> > Links to other random places don't serve that function.
> 
> What "function"?
> 
> This is my argument. Those Link: things need to have a *reason*.
> 
> Saying "they are a change ID" is not a reason. That's just a random
> word-salad. You need to have an active reason that you can explain,
> not just say "look, I want to add a message ID to every commit".

So I want to go to my inbox and compare the patch as received with what
is in my tree.  What did I change? And I tweak both the patch content
when applying and the subject so these are not good indicators.  Is this
at all convincing?

-- 
MST

Re: [GIT PULL] virtio: last minute fixup

[Jörg Rödel]

On Tue, May 10, 2022 at 11:23:11AM -0700, Linus Torvalds wrote:
> And - once again - I want to complain about the "Link:" in that commit.

I have to say that for me (probably for others as well) those Link tags
pointing to the patch submission have quite some value:

	1) First of all it is an easy proof that the patch was actually
	   submitted somewhere for public review before it went into a
	   maintainers tree.

	2) The patch submission is often the entry point to the
	   discussion which lead to this patch. From that email I can
	   see what was discussed and often there is even a link to
	   previous versions and the discussions that happened there. It
	   helps to better understand how a patch came to be the way it
	   is. I know this should ideally be part of the commit message,
	   but in reality this is what I also use the link tag for.

	3) When backporting a patch to a downstream kernel it often
	   helps a lot to see the whole patch-set the change was
	   submitted in, especially when it comes to fixes. With the
	   Link: tag the whole submission thread is easy to find.

I can stop adding them to patches if you want, but as I said, I think
there is some value in them which make me want to keep them.

Regards,

	Joerg

Re: [GIT PULL] virtio: last minute fixup

[Michael S. Tsirkin]

On Wed, May 11, 2022 at 02:24:23PM +0200, Jörg Rödel wrote:
> On Tue, May 10, 2022 at 11:23:11AM -0700, Linus Torvalds wrote:
> > And - once again - I want to complain about the "Link:" in that commit.
> 
> I have to say that for me (probably for others as well) those Link tags
> pointing to the patch submission have quite some value:
> 
> 	1) First of all it is an easy proof that the patch was actually
> 	   submitted somewhere for public review before it went into a
> 	   maintainers tree.
> 
> 	2) The patch submission is often the entry point to the
> 	   discussion which lead to this patch. From that email I can
> 	   see what was discussed and often there is even a link to
> 	   previous versions and the discussions that happened there. It
> 	   helps to better understand how a patch came to be the way it
> 	   is. I know this should ideally be part of the commit message,
> 	   but in reality this is what I also use the link tag for.
> 
> 	3) When backporting a patch to a downstream kernel it often
> 	   helps a lot to see the whole patch-set the change was
> 	   submitted in, especially when it comes to fixes. With the
> 	   Link: tag the whole submission thread is easy to find.
> 
> I can stop adding them to patches if you want, but as I said, I think
> there is some value in them which make me want to keep them.
> 
> Regards,
> 
> 	Joerg

Yea, me too ... Linus, will it be less problematic if it's a different
tag, other than Link? What if it's Message-Id: <foo@bar>? Still a
problem?


-- 
MST

Re: [GIT PULL] virtio: last minute fixup

[pr-tracker-bot]

The pull request you sent on Tue, 10 May 2022 08:23:51 -0400:

> https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/feb9c5e19e913b53cb536a7aa7c9f20107bb51ec

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html