GFS2: Pull Request

GFS2: Pull Request

[Andreas Gruenbacher]

Hi Linus,

please consider pulling the following changes for the GFS2 file system.

There was a conflict with commit 2b070cfe582b ("block: remove the i
argument to bio_for_each_segment_all") on Jens's block layer changes
which you've already merged. I've resolved that by merging those block
layer changes; please let me know if you want this done differently.

Thanks,
Andreas

The following changes since commit b4b52b881cf08e13d110eac811d4becc0775abbf:

  Merge tag 'Wimplicit-fallthrough-5.2-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux
(2019-05-07 12:48:10 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-for-5.2

for you to fetch changes up to dd665ce42728aa985ec4c7002ffe8690cde74c54:

  Merge tag 'for-5.2/block-20190507' of
git://git.kernel.dk/linux-block (2019-05-08 10:30:57 +0200)

----------------------------------------------------------------
We've got the following patches ready for this merge window:

"gfs2: Fix loop in gfs2_rbm_find (v2)"

  A rework of a fix we ended up reverting in 5.0 because of an iozone
  performance regression.

"gfs2: read journal in large chunks" and
"gfs2: fix race between gfs2_freeze_func and unmount"

  An improved version of a commit we also ended up reverting in 5.0
  because of a regression in xfstest generic/311.  It turns out that the
  journal changes were mostly innocent and that unfreeze didn't wait for
  the freeze to complete, which caused the filesystem to be unmounted
  before it was actually idle.

"gfs2: Fix occasional glock use-after-free"
"gfs2: Fix iomap write page reclaim deadlock"
"gfs2: Fix lru_count going negative"

  Fixes for various problems reported and partially fixed by Citrix
  engineers.  Thank you very much.

"gfs2: clean_journal improperly set sd_log_flush_head"

  Another fix from Bob.

A few other minor cleanups.

----------------------------------------------------------------
Abhi Das (2):
      gfs2: fix race between gfs2_freeze_func and unmount
      gfs2: read journal in large chunks

Andreas Gruenbacher (8):
      gfs2: Fix loop in gfs2_rbm_find (v2)
      gfs2: Fix occasional glock use-after-free
      gfs2: Remove misleading comments in gfs2_evict_inode
      gfs2: Remove unnecessary extern declarations
      gfs2: Rename sd_log_le_{revoke,ordered}
      gfs2: Rename gfs2_trans_{add_unrevoke => remove_revoke}
      gfs2: Fix iomap write page reclaim deadlock
      Merge tag 'for-5.2/block-20190507' of git://git.kernel.dk/linux-block

Bob Peterson (2):
      gfs2: clean_journal improperly set sd_log_flush_head
      gfs2: Replace gl_revokes with a GLF flag

Ross Lagerwall (1):
      gfs2: Fix lru_count going negative

 fs/gfs2/aops.c       |  14 ++-
 fs/gfs2/bmap.c       | 118 ++++++++++++++---------
 fs/gfs2/bmap.h       |   1 +
 fs/gfs2/dir.c        |   2 +-
 fs/gfs2/glock.c      |  25 +++--
 fs/gfs2/glops.c      |   3 +-
 fs/gfs2/incore.h     |   9 +-
 fs/gfs2/log.c        |  47 ++++++----
 fs/gfs2/log.h        |   5 +-
 fs/gfs2/lops.c       | 260 ++++++++++++++++++++++++++++++++++++++++++++++-----
 fs/gfs2/lops.h       |  11 +--
 fs/gfs2/main.c       |   1 -
 fs/gfs2/ops_fstype.c |   7 +-
 fs/gfs2/recovery.c   | 135 ++------------------------
 fs/gfs2/recovery.h   |   4 +-
 fs/gfs2/rgrp.c       |  56 ++++++-----
 fs/gfs2/super.c      |  20 ++--
 fs/gfs2/trans.c      |   4 +-
 fs/gfs2/trans.h      |   2 +-
 fs/gfs2/xattr.c      |   6 +-
 20 files changed, 437 insertions(+), 293 deletions(-)

Re: GFS2: Pull Request

[Linus Torvalds]

On Wed, May 8, 2019 at 4:49 AM Andreas Gruenbacher <agruenba@redhat.com> wrote:
>
> There was a conflict with commit 2b070cfe582b ("block: remove the i
> argument to bio_for_each_segment_all") on Jens's block layer changes
> which you've already merged. I've resolved that by merging those block
> layer changes; please let me know if you want this done differently.

PLEASE.

I say this to somebody pretty much every single merge window: don't do
merges for me.

You are actually just hurting, not helping. I want to know what the
conflicts are, not by being told after-the-fact, but by just seeing
them and resolving them.

Yes, I like being _warned_ ahead of time - partly just as a heads up
to me, but partly also to show that the maintainers are aware of the
notifications from linux-next, and that linux-next is working as
intended, and people aren't just ignoring what it reports.

But I do *NOT* want to see maintainers cross-merging each others trees.

It can cause nasty problems, ranging from simply mis-merges to causing
me to not pull a tree at all because one side of the development
effort had done something wrong.

And yes, mis-merges happen - and they happen to me too. It's fairly
rare, but it can be subtle and painful when it does happen.

But (a) I do a _lot_ of merges, so I'm pretty good at them, and (b) if
_I_ do the merge, at least I know about the conflict and am not as
taken by surprise by possible problems due to a mis-merge.

And that kind of thing is really really important to me as an upstream
maintainer. I *need* to know when different subtrees step on each
others toes.

As a result, even when there's a conflict and a merge is perfectly
fine, I want to know about it and see it, and I want to have the
option to pull the maintainer trees in different orders (or not pull
one tree at all), which means that maintainers *MUST NOT* do
cross-tree merges. See?

And I don't want to see back-merges (ie merges from my upstream tree,
as opposed to merges between different maintainer trees) either, even
as a "let me help Linus, he's already merged the other tree, I'll do
the merge for him". That's not helping, that's just hiding the issue.

Now, very very occasionally I will hit a merge that is so hard that I
will go "Hmm, I really need the involved parties to sort this out".
Honestly, I can't remember the last time that happened, but it _has_
happened.

Much more commonly, I'll do the merge, but ask for verification,
saying "ok, this looked more subtle than I like, and I can't test any
of it, so can you check out my merge". Even that isn't all that
common, but it definitely happens.

There is _one_ very special kind of merge that I like maintainers
doing: the "test merge".

That test merge wouldn't be sent to me in the pull request as the
primary signed pull, but it's useful for the maintainer to do to do a
final *check* before doing the pull request, so that you as a
maintainer know what's going on, and perhaps to warn me about
conflicts.

If you do a test merge, and you think the test merge was complex, you
might then point to your resolution in the pull request as a "this is
how I would do it". But you should not make that merge be *the* pull
request.

One additional advantage of a test merge is that it actually gives a
"more correct" diffstat for the pull request. Particularly if the pull
request is for something with complex history (ie you as a maintainer
have sub-maintainers, and have pulled other peoples work), a
test-merge can get a much better diffstat. I don't _require_ that
better diffstat, though - I can see how you got the "odd" diffstat if
you don't do a test merge - but it's can be a "quality of pull
request" thing.

See what I'm saying? You would ask me to pull the un-merged state, but
then say "I noticed a few merge conflicts when I did my test merge,
and this is what I did" kind of aside.

                     Linus

Re: GFS2: Pull Request

[Linus Torvalds]

On Wed, May 8, 2019 at 10:55 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> See what I'm saying? You would ask me to pull the un-merged state, but
> then say "I noticed a few merge conflicts when I did my test merge,
> and this is what I did" kind of aside.

Side note: this is more important if you know of a merge issue that
doesn't cause a conflict, and that I won't see in my simple build
tests.

For example, in this case, the merge issue doesn't cause a conflict
(because it's a totally new user of bio_for_each_segment_all() and the
syntax changed in another branch), but I see it trivially when I do a
test build, since the compiler spews out lots of warnings, and so I
can trivially fix it up (and you _mentioning_ the issue gives me the
heads up that you knew about it and what it's all about).

But if it's other architectures, or only happens under special config
options etc, I might not have seen the merge issue at all. And then
it's really good if the maintainer talks about it and shows that yes,
the maintainer knows what he's doing.

Now I'm in the situation where I have actually done the merge the way
I *like* doing them, and without your superfluous merge commit. But if
I use my merge, I'll lose the signature from your tag, because you
signed *your* merge that I didn't actually want to use at all.

See? Your "helpful" merge actually caused me extra work, and made me
have to pick one of two *worse* situations than if you had just tagged
your own development tree. Either my tree has a extra pointless merge
commit, or my tree lacks your signature on your work.

I'll just undo my pull, and delay it all in the hope that you'll just
send me a new signed tag of the non-merged state.

                   Linus

Re: GFS2: Pull Request

[Andreas Gruenbacher]

Linus,

On Wed, 8 May 2019 at 20:06, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, May 8, 2019 at 10:55 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > See what I'm saying? You would ask me to pull the un-merged state, but
> > then say "I noticed a few merge conflicts when I did my test merge,
> > and this is what I did" kind of aside.
>
> Side note: this is more important if you know of a merge issue that
> doesn't cause a conflict, and that I won't see in my simple build
> tests.
>
> For example, in this case, the merge issue doesn't cause a conflict
> (because it's a totally new user of bio_for_each_segment_all() and the
> syntax changed in another branch), but I see it trivially when I do a
> test build, since the compiler spews out lots of warnings, and so I
> can trivially fix it up (and you _mentioning_ the issue gives me the
> heads up that you knew about it and what it's all about).
>
> But if it's other architectures, or only happens under special config
> options etc, I might not have seen the merge issue at all. And then
> it's really good if the maintainer talks about it and shows that yes,
> the maintainer knows what he's doing.
>
> Now I'm in the situation where I have actually done the merge the way
> I *like* doing them, and without your superfluous merge commit. But if
> I use my merge, I'll lose the signature from your tag, because you
> signed *your* merge that I didn't actually want to use at all.
>
> See? Your "helpful" merge actually caused me extra work, and made me
> have to pick one of two *worse* situations than if you had just tagged
> your own development tree. Either my tree has a extra pointless merge
> commit, or my tree lacks your signature on your work.

Ok, got it.

Would it make sense to describe how to deal with merge conflicts in
Documentation/maintainer/pull-requests.rst to stop people from getting
this wrong over and over again?

Thanks,
Andreas

Re: GFS2: Pull Request

[Linus Torvalds]

On Wed, May 8, 2019 at 1:17 PM Andreas Gruenbacher <agruenba@redhat.com> wrote:
>
> Would it make sense to describe how to deal with merge conflicts in
> Documentation/maintainer/pull-requests.rst to stop people from getting
> this wrong over and over again?

Probably. I do think it got written up at some point (lwn or something
like that), but it's possible it never got made into an actual
documentation file..

Anybody want to try to massage that email into the appropriate doc file?

              Linus

Re: GFS2: Pull Request

[Jonathan Corbet]

On Wed, 8 May 2019 22:17:12 +0200
Andreas Gruenbacher <agruenba@redhat.com> wrote:

> Would it make sense to describe how to deal with merge conflicts in
> Documentation/maintainer/pull-requests.rst to stop people from getting
> this wrong over and over again?

I think this certainly belongs in the maintainer manual, but probably not
in pull-requests.rst.  There are a lot of things about repository
management that seem to trip up even experienced maintainers; pre-pull
merges is just one of those.  I would love to see a proper guide on when
and how to do merges in general.

CCing Dan, who has ambitions for the maintainer manual as well, just in
case he has anything in mind here.

Thanks,

jon

Re: GFS2: Pull Request

[Linus Torvalds]

On Wed, May 8, 2019 at 1:58 PM Jonathan Corbet <corbet@lwn.net> wrote:
>
> I think this certainly belongs in the maintainer manual, but probably not
> in pull-requests.rst.  There are a lot of things about repository
> management that seem to trip up even experienced maintainers; pre-pull
> merges is just one of those.  I would love to see a proper guide on when
> and how to do merges in general.

We had another pull request issue today, about a situation that I
definitely know you've written about in the past, because I linked to
lwn in my email:

   https://lore.kernel.org/lkml/CAHk-=wiKoePP_9CM0fn_Vv1bYom7iB5N=ULaLLz7yOST3K+k5g@mail.gmail.com/

and while I suspect people don't actually read documentation
(_particularly_ maintainers that have already been around for a long
time but still do this), maybe that part could be in the same
documentation?

                 Linus

Re: GFS2: Pull Request

[Jonathan Corbet]

On Wed, 8 May 2019 14:05:35 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, May 8, 2019 at 1:58 PM Jonathan Corbet <corbet@lwn.net> wrote:
> >
> > I think this certainly belongs in the maintainer manual, but probably not
> > in pull-requests.rst.  There are a lot of things about repository
> > management that seem to trip up even experienced maintainers; pre-pull
> > merges is just one of those.  I would love to see a proper guide on when
> > and how to do merges in general.  
> 
> We had another pull request issue today, about a situation that I
> definitely know you've written about in the past, because I linked to
> lwn in my email:
> 
>    https://lore.kernel.org/lkml/CAHk-=wiKoePP_9CM0fn_Vv1bYom7iB5N=ULaLLz7yOST3K+k5g@mail.gmail.com/

Sigh, I never can escape my past sins...

> and while I suspect people don't actually read documentation
> (_particularly_ maintainers that have already been around for a long
> time but still do this), maybe that part could be in the same
> documentation?

Something derived from that would make sense, yes.  I'll see if I can't
pull something together, unless by some delightful miracle somebody beats
me to it.

jon

cross-merges with MFD tree (was: Re: GFS2: Pull Request)

[Jacek Anaszewski]

Hi Linus,

On 5/8/19 7:55 PM, Linus Torvalds wrote:
> On Wed, May 8, 2019 at 4:49 AM Andreas Gruenbacher <agruenba@redhat.com> wrote:
>>
>> There was a conflict with commit 2b070cfe582b ("block: remove the i
>> argument to bio_for_each_segment_all") on Jens's block layer changes
>> which you've already merged. I've resolved that by merging those block
>> layer changes; please let me know if you want this done differently.
> 
> PLEASE.
> 
> I say this to somebody pretty much every single merge window: don't do
> merges for me.
> 
> You are actually just hurting, not helping. I want to know what the
> conflicts are, not by being told after-the-fact, but by just seeing
> them and resolving them.
> 
> Yes, I like being _warned_ ahead of time - partly just as a heads up
> to me, but partly also to show that the maintainers are aware of the
> notifications from linux-next, and that linux-next is working as
> intended, and people aren't just ignoring what it reports.
> 
> But I do *NOT* want to see maintainers cross-merging each others trees.

I would like to clarify if this applies to immutable integration
branches that are usually created for MFD subsystem. That subsystem
is somehow specific since changes made to MFD drivers are often a part
of bigger patch sets that add drivers of MFD cells to the other
subsystems.

Like in my area of interest an addition of a driver for LED cell
of MFD device must be followed by addition of a corresponding entry to
struct mfd_cell array in the related MFD driver.

And sometimes even another subsystem is involved, like e.g. regulator
framework in case of recent extension of ti-lmu driver.

So far you haven't complained about this specific workflow, but I'd like
to make sure how you see it.

-- 
Best regards,
Jacek Anaszewski

Re: cross-merges with MFD tree (was: Re: GFS2: Pull Request)

[Lee Jones]

On Tue, 28 May 2019, Jacek Anaszewski wrote:

> Hi Linus,
> 
> On 5/8/19 7:55 PM, Linus Torvalds wrote:
> > On Wed, May 8, 2019 at 4:49 AM Andreas Gruenbacher <agruenba@redhat.com> wrote:
> > > 
> > > There was a conflict with commit 2b070cfe582b ("block: remove the i
> > > argument to bio_for_each_segment_all") on Jens's block layer changes
> > > which you've already merged. I've resolved that by merging those block
> > > layer changes; please let me know if you want this done differently.
> > 
> > PLEASE.
> > 
> > I say this to somebody pretty much every single merge window: don't do
> > merges for me.
> > 
> > You are actually just hurting, not helping. I want to know what the
> > conflicts are, not by being told after-the-fact, but by just seeing
> > them and resolving them.
> > 
> > Yes, I like being _warned_ ahead of time - partly just as a heads up
> > to me, but partly also to show that the maintainers are aware of the
> > notifications from linux-next, and that linux-next is working as
> > intended, and people aren't just ignoring what it reports.
> > 
> > But I do *NOT* want to see maintainers cross-merging each others trees.
> 
> I would like to clarify if this applies to immutable integration
> branches that are usually created for MFD subsystem. That subsystem
> is somehow specific since changes made to MFD drivers are often a part
> of bigger patch sets that add drivers of MFD cells to the other
> subsystems.
> 
> Like in my area of interest an addition of a driver for LED cell
> of MFD device must be followed by addition of a corresponding entry to
> struct mfd_cell array in the related MFD driver.
> 
> And sometimes even another subsystem is involved, like e.g. regulator
> framework in case of recent extension of ti-lmu driver.
> 
> So far you haven't complained about this specific workflow, but I'd like
> to make sure how you see it.

After you bought this conversation to my attention last week, I took
the initiative and spoke to a few senior maintainers (Mark Brown,
Linus Walleij, etc) for their views.  I choose these guys because they
are commonly on the receiving end of my Pull Requests when we need to
do this.

Due to its inherent nature, MFD usually finds itself interacting with
other subsystems in ways which deal with both physical merge conflicts
and build dependencies.  For some time now the vast majority of the
other maintainers I work with and I have believed, and still do, that
the best way to mitigate these issues is to produce small, succinct,
immutable topic branches.  These branches only usually contain a few 
patches and serve to solve a specific potential issue - usually
build-time problems, but also have the added bonus of preventing merge
conflicts and keeping us out of the forefront of Linus' mind (and out
of trouble!)

We've been doing this for some time and have interacted with many
subsystems (ACPI, ASoC, ARM-SoC, Clocksource, Excon, GPIO, Hwmon, I2C,
IIO, Input, IRQChip, LED, PWM, Media, Net, Pinctrl, Platform, Power,
Regulator, RTC, USB, Video, Watchdog, etc, etc) over the years [0].

In my mind, small, immutable topic branches is still the cleanest way
to deal with the issues facing MFD currently.  And is not what Linus
is detailing in his recent mail(s) on the subject.  We are not "doing
merges on his behalf", we are ensuring; buildable, bisectable,
error-free branches/history that will also merge cleanly.

[0] `git log --oneline --grep ib-mfd mainline/master`

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog