[GIT,PULL] gfs2: 4.20 fixes
mbox series

Message ID CAHc6FU6P0HdDWGMmsjyArQ746N7RBE3qMVq3aZgkfGjb153yJw@mail.gmail.com
State New, archived
Headers show
Series
  • [GIT,PULL] gfs2: 4.20 fixes
Related show

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git

Message

Andreas Gruenbacher Nov. 15, 2018, noon UTC
Hi Linus,

could you please pull the following gfs2 fixes for 4.20?

Thank you,
Andreas

The following changes since commit 651022382c7f8da46cb4872a545ee1da6d097d2a:

  Linux 4.20-rc1 (2018-11-04 15:37:52 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.20.fixes2

for you to fetch changes up to 77d36cabdf8e7c8d46b3e275a3d7958549de04b0:

  gfs2: Fix iomap buffer head reference counting bug (2018-11-11 12:20:05 +0000)

----------------------------------------------------------------
Fix two bugs leading to leaked buffer head references:

  gfs2: Put bitmap buffers in put_super
  gfs2: Fix iomap buffer head reference counting bug

And one bug leading to significant slow-downs when deleting large files:

  gfs2: Fix metadata read-ahead during truncate (2)

----------------------------------------------------------------
Andreas Gruenbacher (4):
      gfs2: Put bitmap buffers in put_super
      gfs2: Fix metadata read-ahead during truncate (2)
      Merge tag 'v4.20-rc1'
      gfs2: Fix iomap buffer head reference counting bug

 fs/gfs2/bmap.c | 54 +++++++++++++++++++++++++++---------------------------
 fs/gfs2/rgrp.c |  3 ++-
 2 files changed, 29 insertions(+), 28 deletions(-)

Comments

Linus Torvalds Nov. 15, 2018, 5:07 p.m. UTC | #1
On Thu, Nov 15, 2018 at 6:00 AM Andreas Gruenbacher <agruenba@redhat.com> wrote:
>
> could you please pull the following gfs2 fixes for 4.20?

No.

I'm not pulling this useless commit message:

  "Merge tag 'v4.20-rc1'"

with absolutely _zero_ explanation for why that merge was done.

Guys, stop doing this. Because I will stop pulling them.

If you can't be bothered to explain exactly why you're doing a merge,
I can't be bothered to pull the result.

Commit messages are important. They explain why something was done.
Merge commits are to some degree *more* important, because they do odd
things and the reason is not obvious from the code ("Oh, it's a
oneliner obvious fix").

So merge commits without  a reason for them are simply not acceptable.

                Linus
Andreas Gruenbacher Nov. 15, 2018, 6:20 p.m. UTC | #2
On Thu, 15 Nov 2018 at 18:11, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Nov 15, 2018 at 6:00 AM Andreas Gruenbacher <agruenba@redhat.com> wrote:
> >
> > could you please pull the following gfs2 fixes for 4.20?
>
> No.
>
> I'm not pulling this useless commit message:
>
>   "Merge tag 'v4.20-rc1'"
>
> with absolutely _zero_ explanation for why that merge was done.

Sorry for that.

I guess rebasing the for-next branch onto something more recent to
avoid the back-merge in the first place will be best, resulting in a
cleaner history.

> Guys, stop doing this. Because I will stop pulling them.
>
> If you can't be bothered to explain exactly why you're doing a merge,
> I can't be bothered to pull the result.
>
> Commit messages are important. They explain why something was done.
> Merge commits are to some degree *more* important, because they do odd
> things and the reason is not obvious from the code ("Oh, it's a
> oneliner obvious fix").
>
> So merge commits without  a reason for them are simply not acceptable.

Thanks,
Andreas
Linus Torvalds Nov. 15, 2018, 6:23 p.m. UTC | #3
On Thu, Nov 15, 2018 at 12:20 PM Andreas Gruenbacher
<agruenba@redhat.com> wrote:
>
> I guess rebasing the for-next branch onto something more recent to
> avoid the back-merge in the first place will be best, resulting in a
> cleaner history.

Rebases aren't really any better at all.

If you have a real *reason* for a merge, do the merge. But then the
reason should be clearly stated in the merge commit. Not just some
random undocumented merge message. Tell why some other branch was
relevant to your fix and needed to be pulled in.

Better yet, don't do either merges or rebases.

             Linus
Andreas Gruenbacher Nov. 15, 2018, 8:44 p.m. UTC | #4
On Thu, 15 Nov 2018 at 19:23, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Nov 15, 2018 at 12:20 PM Andreas Gruenbacher
> <agruenba@redhat.com> wrote:
> >
> > I guess rebasing the for-next branch onto something more recent to
> > avoid the back-merge in the first place will be best, resulting in a
> > cleaner history.
>
> Rebases aren't really any better at all.
>
> If you have a real *reason* for a merge, do the merge. But then the
> reason should be clearly stated in the merge commit. Not just some
> random undocumented merge message. Tell why some other branch was
> relevant to your fix and needed to be pulled in.
>
> Better yet, don't do either merges or rebases.

Ok, I've changed the merge commit as follows now:

    Merge tag 'v4.20-rc1'

    Pull in the gfs2 fixes that went into v4.19-rc8:

      gfs2: Fix iomap buffered write support for journaled files
      gfs2: Fix iomap buffered write support for journaled files (2)

    Without these two commits, the following fix would cause conflicts.

So merging v4.19-rc8 would have been sufficient. v4.20-rc1 is what I
ended up testing, though.

Are you okay with that now?

Thanks,
Andreas

--

The following changes since commit 651022382c7f8da46cb4872a545ee1da6d097d2a:

  Linux 4.20-rc1 (2018-11-04 15:37:52 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.20.fixes3

for you to fetch changes up to 01ed1606d30966dc8fa255a941b2fc42d4e308a1:

  gfs2: Fix iomap buffer head reference counting bug (2018-11-15 21:31:58 +0100)

----------------------------------------------------------------
Fix two bugs leading to leaked buffer head references:

  gfs2: Put bitmap buffers in put_super
  gfs2: Fix iomap buffer head reference counting bug

And one bug leading to significant slow-downs when deleting large files:

  gfs2: Fix metadata read-ahead during truncate (2)

----------------------------------------------------------------
Andreas Gruenbacher (4):
      gfs2: Put bitmap buffers in put_super
      gfs2: Fix metadata read-ahead during truncate (2)
      Merge tag 'v4.20-rc1'
      gfs2: Fix iomap buffer head reference counting bug

 fs/gfs2/bmap.c | 54 +++++++++++++++++++++++++++---------------------------
 fs/gfs2/rgrp.c |  3 ++-
 2 files changed, 29 insertions(+), 28 deletions(-)
Linus Torvalds Nov. 16, 2018, 5:51 p.m. UTC | #5
On Thu, Nov 15, 2018 at 2:44 PM Andreas Gruenbacher <agruenba@redhat.com> wrote:
>
> Ok, I've changed the merge commit as follows now:
>
>     Merge tag 'v4.20-rc1'
>
>     Pull in the gfs2 fixes that went into v4.19-rc8:
>
>       gfs2: Fix iomap buffered write support for journaled files
>       gfs2: Fix iomap buffered write support for journaled files (2)
>
>     Without these two commits, the following fix would cause conflicts.
>
> So merging v4.19-rc8 would have been sufficient. v4.20-rc1 is what I
> ended up testing, though.
>
> Are you okay with that now?

If the only reason for this was the trivial one-liner conflict, the
real fix is to just not do the merge at all, and not worry about
conflicts.

I handle simple conflicts like this in my sleep. It's actually much
better and clearer if the development trees just do their own
development, and not worry about "Oh, Linus will get a small conflict
due to other changes he has pulled".

So what I did was to actually try to generate the tree I think it
*should* have been, and just merge that with the simple conflict
resolution.

You might want to double-check it - not only because I rewrote your
pull to not have the merge at all, and in the process modified things,
but just to see what I think the right approach would have been.

Now, there *are* reasons to do back-merges, but no, "Oh, there's a
trivial conflict" is not one of them.

                  Linus