[GIT PULL] iomap: bug fixes for 6.6-rc7

[GIT PULL] iomap: bug fixes for 6.6-rc7

[Darrick J. Wong]

Hi Linus,

Please pull this branch with changes for iomap for 6.6-rc7.

As usual, I did a test-merge with the main upstream branch as of a few
minutes ago, and didn't see any conflicts.  Please let me know if you
encounter any problems.

--D

The following changes since commit 684f7e6d28e8087502fc8efdb6c9fe82400479dd:

iomap: Spelling s/preceeding/preceding/g (2023-09-28 09:26:58 -0700)

are available in the Git repository at:

https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git iomap-6.6-fixes-5

for you to fetch changes up to 3ac974796e5d94509b85a403449132ea660127c2:

iomap: fix short copy in iomap_write_iter() (2023-10-19 09:41:36 -0700)

----------------------------------------------------------------
Bug fixes for 6.6-rc6:

* Fix a bug where a writev consisting of a bunch of sub-fsblock writes
where the last buffer address is invalid could lead to an infinite
loop.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>

----------------------------------------------------------------
Jan Stancek (1):
iomap: fix short copy in iomap_write_iter()

fs/iomap/buffered-io.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

Re: [GIT PULL] iomap: bug fixes for 6.6-rc7

[Linus Torvalds]

On Fri, 20 Oct 2023 at 23:27, Darrick J. Wong <djwong@kernel.org> wrote:
>
> Please pull this branch with changes for iomap for 6.6-rc7.
>
> As usual, I did a test-merge with the main upstream branch as of a few
> minutes ago, and didn't see any conflicts.  Please let me know if you
> encounter any problems.

.. and as usual, the branch you point to does not actually exist.

Because you *again* pointed to the wrong tree.

This time I remembered what the mistake was last time, and picked out
the right tree by hand, but *please* just fix your completely broken
scripts or workflow.

> https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git iomap-6.6-fixes-5

No.

It's pub/scm/fs/xfs/xfs-linux, once again.

                 Linus

Re: [GIT PULL] iomap: bug fixes for 6.6-rc7

[Darrick J. Wong]

On Sat, Oct 21, 2023 at 09:46:35AM -0700, Linus Torvalds wrote:
> On Fri, 20 Oct 2023 at 23:27, Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > Please pull this branch with changes for iomap for 6.6-rc7.
> >
> > As usual, I did a test-merge with the main upstream branch as of a few
> > minutes ago, and didn't see any conflicts.  Please let me know if you
> > encounter any problems.
> 
> .. and as usual, the branch you point to does not actually exist.
> 
> Because you *again* pointed to the wrong tree.
> 
> This time I remembered what the mistake was last time, and picked out
> the right tree by hand, but *please* just fix your completely broken
> scripts or workflow.
> 
> > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git iomap-6.6-fixes-5
> 
> No.
> 
> It's pub/scm/fs/xfs/xfs-linux, once again.

Sorry about that.  After reviewing the output of git request-pull, I
have learned that if you provide a $url argument that does not point to
a repo containing $start, it will print a warning to stderr and emit a
garbage pull request to stdout anyway.  No --force required or anything.
Piping stdout to mutt without checking the return code is therefore a
bad idea.

I have now updated my wrapper script to buffer the entire pull request
contents and check the return value before proceeding.

It is a poor workman who blames his tools, so I declare publicly that
you have an idiot for a maintainer.

Christian: Do you have the bandwidth to take over fs/iomap/?

--D

> 
>                  Linus

Re: [GIT PULL] iomap: bug fixes for 6.6-rc7

[Christian Brauner]

On Mon, Oct 23, 2023 at 03:38:10PM -0700, Darrick J. Wong wrote:
> On Sat, Oct 21, 2023 at 09:46:35AM -0700, Linus Torvalds wrote:
> > On Fri, 20 Oct 2023 at 23:27, Darrick J. Wong <djwong@kernel.org> wrote:
> > >
> > > Please pull this branch with changes for iomap for 6.6-rc7.
> > >
> > > As usual, I did a test-merge with the main upstream branch as of a few
> > > minutes ago, and didn't see any conflicts.  Please let me know if you
> > > encounter any problems.
> > 
> > .. and as usual, the branch you point to does not actually exist.
> > 
> > Because you *again* pointed to the wrong tree.
> > 
> > This time I remembered what the mistake was last time, and picked out
> > the right tree by hand, but *please* just fix your completely broken
> > scripts or workflow.
> > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git iomap-6.6-fixes-5
> > 
> > No.
> > 
> > It's pub/scm/fs/xfs/xfs-linux, once again.
> 
> Sorry about that.  After reviewing the output of git request-pull, I
> have learned that if you provide a $url argument that does not point to
> a repo containing $start, it will print a warning to stderr and emit a
> garbage pull request to stdout anyway.  No --force required or anything.
> Piping stdout to mutt without checking the return code is therefore a
> bad idea.
> 
> I have now updated my wrapper script to buffer the entire pull request
> contents and check the return value before proceeding.
> 
> It is a poor workman who blames his tools, so I declare publicly that
> you have an idiot for a maintainer.
> 
> Christian: Do you have the bandwidth to take over fs/iomap/?

If this helps you I will take iomap over but only if you and Christoph
stay around as main reviewers. There's not much point in me pretending I
can meaningfully review fs/iomap/ and I don't have the bandwith even if
I could. So not without clear reviewers.

But, - and I'm sorry if I may overstep bounds a little bit - I think
this self-castigation is really unwarranted. And we all very much know
that you definitely aren't an idiot. And personally I think we shouldn't
give the impression that we expect this sort of repentance when we make
mistakes.

In other words, if the sole reason you're proposing this is an
objectively false belief then I would suggest to reconsider.

Re: [GIT PULL] iomap: bug fixes for 6.6-rc7

[Darrick J. Wong]

On Tue, Oct 24, 2023 at 01:47:17PM +0200, Christian Brauner wrote:
> On Mon, Oct 23, 2023 at 03:38:10PM -0700, Darrick J. Wong wrote:
> > On Sat, Oct 21, 2023 at 09:46:35AM -0700, Linus Torvalds wrote:
> > > On Fri, 20 Oct 2023 at 23:27, Darrick J. Wong <djwong@kernel.org> wrote:
> > > >
> > > > Please pull this branch with changes for iomap for 6.6-rc7.
> > > >
> > > > As usual, I did a test-merge with the main upstream branch as of a few
> > > > minutes ago, and didn't see any conflicts.  Please let me know if you
> > > > encounter any problems.
> > > 
> > > .. and as usual, the branch you point to does not actually exist.
> > > 
> > > Because you *again* pointed to the wrong tree.
> > > 
> > > This time I remembered what the mistake was last time, and picked out
> > > the right tree by hand, but *please* just fix your completely broken
> > > scripts or workflow.
> > > 
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git iomap-6.6-fixes-5
> > > 
> > > No.
> > > 
> > > It's pub/scm/fs/xfs/xfs-linux, once again.
> > 
> > Sorry about that.  After reviewing the output of git request-pull, I
> > have learned that if you provide a $url argument that does not point to
> > a repo containing $start, it will print a warning to stderr and emit a
> > garbage pull request to stdout anyway.  No --force required or anything.
> > Piping stdout to mutt without checking the return code is therefore a
> > bad idea.
> > 
> > I have now updated my wrapper script to buffer the entire pull request
> > contents and check the return value before proceeding.
> > 
> > It is a poor workman who blames his tools, so I declare publicly that
> > you have an idiot for a maintainer.
> > 
> > Christian: Do you have the bandwidth to take over fs/iomap/?
> 
> If this helps you I will take iomap over but only if you and Christoph
> stay around as main reviewers.

I can't speak for Christoph, but I am very much willing to continue
developing patches for fs/iomap, running the QA farm to make sure it's
working properly, and reviewing everyone else's patches.  Same as I do
now.

What I would like to concentrate on in the future are:

(a) improving documentation and cleanups that other fs maintainers have
    been asking for and I haven't had time to work on

(b) helping interested fs maintainers port their fs to iomap for better
    performance

(c) figuring out how to integrate smoothly with things like fsverity and
    fscrypt

(d) not stepping on *your* toes every time you want to change something
    in the vfs only to have it collide with iomap changes that you
    didn't see

Similar to what we just did with XFS, I propose breaking up the iomap
Maintainer role into pieces that are more manageable by a single person.
As RM, all you'd have to do is integrate reviewed patches and pull
requests into one of your work branches.  That gives you final say over
what goes in and how it goes in, instead of letting branches collide in
for-next without warning.

You can still forward on the review requests and bug reports to me.
That part isn't changing.  I've enjoyed working with you and hope
that'll continue well into the future. :)

> There's not much point in me pretending I
> can meaningfully review fs/iomap/ and I don't have the bandwith even if
> I could. So not without clear reviewers.

I hope the above assuades your concerns/fears!

> But, - and I'm sorry if I may overstep bounds a little bit - I think
> this self-castigation is really unwarranted. And we all very much know
> that you definitely aren't an idiot. And personally I think we shouldn't
> give the impression that we expect this sort of repentance when we make
> mistakes.
> 
> In other words, if the sole reason you're proposing this is an
> objectively false belief then I would suggest to reconsider.

Quite the opposite, these are changes that I've been wanting to make for
months. :)

--D

Re: [GIT PULL] iomap: bug fixes for 6.6-rc7

[Christian Brauner]

On Wed, Oct 25, 2023 at 08:13:25PM -0700, Darrick J. Wong wrote:
> On Tue, Oct 24, 2023 at 01:47:17PM +0200, Christian Brauner wrote:
> > On Mon, Oct 23, 2023 at 03:38:10PM -0700, Darrick J. Wong wrote:
> > > On Sat, Oct 21, 2023 at 09:46:35AM -0700, Linus Torvalds wrote:
> > > > On Fri, 20 Oct 2023 at 23:27, Darrick J. Wong <djwong@kernel.org> wrote:
> > > > >
> > > > > Please pull this branch with changes for iomap for 6.6-rc7.
> > > > >
> > > > > As usual, I did a test-merge with the main upstream branch as of a few
> > > > > minutes ago, and didn't see any conflicts.  Please let me know if you
> > > > > encounter any problems.
> > > > 
> > > > .. and as usual, the branch you point to does not actually exist.
> > > > 
> > > > Because you *again* pointed to the wrong tree.
> > > > 
> > > > This time I remembered what the mistake was last time, and picked out
> > > > the right tree by hand, but *please* just fix your completely broken
> > > > scripts or workflow.
> > > > 
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git iomap-6.6-fixes-5
> > > > 
> > > > No.
> > > > 
> > > > It's pub/scm/fs/xfs/xfs-linux, once again.
> > > 
> > > Sorry about that.  After reviewing the output of git request-pull, I
> > > have learned that if you provide a $url argument that does not point to
> > > a repo containing $start, it will print a warning to stderr and emit a
> > > garbage pull request to stdout anyway.  No --force required or anything.
> > > Piping stdout to mutt without checking the return code is therefore a
> > > bad idea.
> > > 
> > > I have now updated my wrapper script to buffer the entire pull request
> > > contents and check the return value before proceeding.
> > > 
> > > It is a poor workman who blames his tools, so I declare publicly that
> > > you have an idiot for a maintainer.
> > > 
> > > Christian: Do you have the bandwidth to take over fs/iomap/?
> > 
> > If this helps you I will take iomap over but only if you and Christoph
> > stay around as main reviewers.
> 
> I can't speak for Christoph, but I am very much willing to continue
> developing patches for fs/iomap, running the QA farm to make sure it's
> working properly, and reviewing everyone else's patches.  Same as I do
> now.
> 
> What I would like to concentrate on in the future are:
> 
> (a) improving documentation and cleanups that other fs maintainers have
>     been asking for and I haven't had time to work on
> 
> (b) helping interested fs maintainers port their fs to iomap for better
>     performance
> 
> (c) figuring out how to integrate smoothly with things like fsverity and
>     fscrypt
> 
> (d) not stepping on *your* toes every time you want to change something
>     in the vfs only to have it collide with iomap changes that you
>     didn't see
> 
> Similar to what we just did with XFS, I propose breaking up the iomap
> Maintainer role into pieces that are more manageable by a single person.

Sounds good.

> As RM, all you'd have to do is integrate reviewed patches and pull
> requests into one of your work branches.  That gives you final say over
> what goes in and how it goes in, instead of letting branches collide in
> for-next without warning.
> 
> You can still forward on the review requests and bug reports to me.

Ok, cool.

> That part isn't changing.  I've enjoyed working with you and hope
> that'll continue well into the future. :)

Thanks. That's good to hear and right back at you.

> 
> > There's not much point in me pretending I
> > can meaningfully review fs/iomap/ and I don't have the bandwith even if
> > I could. So not without clear reviewers.
> 
> I hope the above assuades your concerns/fears!

Yes.

> 
> > But, - and I'm sorry if I may overstep bounds a little bit - I think
> > this self-castigation is really unwarranted. And we all very much know
> > that you definitely aren't an idiot. And personally I think we shouldn't
> > give the impression that we expect this sort of repentance when we make
> > mistakes.
> > 
> > In other words, if the sole reason you're proposing this is an
> > objectively false belief then I would suggest to reconsider.
> 
> Quite the opposite, these are changes that I've been wanting to make for
> months. :)

In that case I would propose you send a patch to Linus for MAINTAINERS
updating the tree and the entries for iomap. I believe it's customary
for the current maintainer to do this.

Thanks!
Christian

Re: [GIT PULL] iomap: bug fixes for 6.6-rc7

[Darrick J. Wong]

On Thu, Oct 26, 2023 at 01:54:29PM +0200, Christian Brauner wrote:
> On Wed, Oct 25, 2023 at 08:13:25PM -0700, Darrick J. Wong wrote:
> > On Tue, Oct 24, 2023 at 01:47:17PM +0200, Christian Brauner wrote:
> > > On Mon, Oct 23, 2023 at 03:38:10PM -0700, Darrick J. Wong wrote:
> > > > On Sat, Oct 21, 2023 at 09:46:35AM -0700, Linus Torvalds wrote:
> > > > > On Fri, 20 Oct 2023 at 23:27, Darrick J. Wong <djwong@kernel.org> wrote:
> > > > > >
> > > > > > Please pull this branch with changes for iomap for 6.6-rc7.
> > > > > >
> > > > > > As usual, I did a test-merge with the main upstream branch as of a few
> > > > > > minutes ago, and didn't see any conflicts.  Please let me know if you
> > > > > > encounter any problems.
> > > > > 
> > > > > .. and as usual, the branch you point to does not actually exist.
> > > > > 
> > > > > Because you *again* pointed to the wrong tree.
> > > > > 
> > > > > This time I remembered what the mistake was last time, and picked out
> > > > > the right tree by hand, but *please* just fix your completely broken
> > > > > scripts or workflow.
> > > > > 
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git iomap-6.6-fixes-5
> > > > > 
> > > > > No.
> > > > > 
> > > > > It's pub/scm/fs/xfs/xfs-linux, once again.
> > > > 
> > > > Sorry about that.  After reviewing the output of git request-pull, I
> > > > have learned that if you provide a $url argument that does not point to
> > > > a repo containing $start, it will print a warning to stderr and emit a
> > > > garbage pull request to stdout anyway.  No --force required or anything.
> > > > Piping stdout to mutt without checking the return code is therefore a
> > > > bad idea.
> > > > 
> > > > I have now updated my wrapper script to buffer the entire pull request
> > > > contents and check the return value before proceeding.
> > > > 
> > > > It is a poor workman who blames his tools, so I declare publicly that
> > > > you have an idiot for a maintainer.
> > > > 
> > > > Christian: Do you have the bandwidth to take over fs/iomap/?
> > > 
> > > If this helps you I will take iomap over but only if you and Christoph
> > > stay around as main reviewers.
> > 
> > I can't speak for Christoph, but I am very much willing to continue
> > developing patches for fs/iomap, running the QA farm to make sure it's
> > working properly, and reviewing everyone else's patches.  Same as I do
> > now.
> > 
> > What I would like to concentrate on in the future are:
> > 
> > (a) improving documentation and cleanups that other fs maintainers have
> >     been asking for and I haven't had time to work on
> > 
> > (b) helping interested fs maintainers port their fs to iomap for better
> >     performance
> > 
> > (c) figuring out how to integrate smoothly with things like fsverity and
> >     fscrypt
> > 
> > (d) not stepping on *your* toes every time you want to change something
> >     in the vfs only to have it collide with iomap changes that you
> >     didn't see
> > 
> > Similar to what we just did with XFS, I propose breaking up the iomap
> > Maintainer role into pieces that are more manageable by a single person.
> 
> Sounds good.
> 
> > As RM, all you'd have to do is integrate reviewed patches and pull
> > requests into one of your work branches.  That gives you final say over
> > what goes in and how it goes in, instead of letting branches collide in
> > for-next without warning.
> > 
> > You can still forward on the review requests and bug reports to me.
> 
> Ok, cool.
> 
> > That part isn't changing.  I've enjoyed working with you and hope
> > that'll continue well into the future. :)
> 
> Thanks. That's good to hear and right back at you.

Thank you!  I enjoyed reading that! :)

> > 
> > > There's not much point in me pretending I
> > > can meaningfully review fs/iomap/ and I don't have the bandwith even if
> > > I could. So not without clear reviewers.
> > 
> > I hope the above assuades your concerns/fears!
> 
> Yes.
> 
> > 
> > > But, - and I'm sorry if I may overstep bounds a little bit - I think
> > > this self-castigation is really unwarranted. And we all very much know
> > > that you definitely aren't an idiot. And personally I think we shouldn't
> > > give the impression that we expect this sort of repentance when we make
> > > mistakes.
> > > 
> > > In other words, if the sole reason you're proposing this is an
> > > objectively false belief then I would suggest to reconsider.
> > 
> > Quite the opposite, these are changes that I've been wanting to make for
> > months. :)
> 
> In that case I would propose you send a patch to Linus for MAINTAINERS
> updating the tree and the entries for iomap. I believe it's customary
> for the current maintainer to do this.

Will do.  I'll change the M: entry to you, and add myself as an R:.

--D

> 
> Thanks!
> Christian

Re: [GIT PULL] iomap: bug fixes for 6.6-rc7

[Linus Torvalds]

On Wed, 25 Oct 2023 at 17:13, Darrick J. Wong <djwong@kernel.org> wrote:
>
> Similar to what we just did with XFS, I propose breaking up the iomap
> Maintainer role into pieces that are more manageable by a single person.
> As RM, all you'd have to do is integrate reviewed patches and pull
> requests into one of your work branches.  That gives you final say over
> what goes in and how it goes in, instead of letting branches collide in
> for-next without warning.

I _think_ what you are saying is that you'd like to avoid being both a
maintainer and a developer.

Now, I'm probably hugely biased and going by personal experience, but
I do think that doing double duty is the worst of both worlds, and
pointlessly stressful.

As a maintainer, you have to worry about the big picture (things like
release timing, even if it's just a "is this a fix for this release,
or should it get queued for the next one") but also code-related
things like "we have two different things going on, let's sort them
out separately". Christian had that kind of issue just a couple of
days ago with the btrfs tree.

But then, as a developer, those are distractions and just add stress
and worry, and distract from whatever you're working on. As a
developer, the last thing you want to worry about is something else
than the actual technical issue you're trying to solve.

And obviously, there's a lot of overlap. A maintainer needs to be
_able_ to be a developer just to make good choices. And the whole
"maintainer vs developer" doesn't have to be two different people,
somebody might shift from one to the other simply because maybe they
enjoy both roles. Just not at the same time, all the time, having both
things putting different stress on you.

You can *kind* of see the difference in our git tree if you do

    git rev-list --count --author=XYZ --no-merges --since=1.year HEAD

to see "code authorship" (aka developer), vs

    git rev-list --count --committer=XYZ --since=1.year HEAD

which shows some kind of approximation of "maintainership". Obviously
there is overlap (potentially a lot of it) and the above isn't some
absolute thing, but you can see some patterns.

I personally wish we had more people who are maintainers _without_
having to worry too much about developing new code.  One of the issues
that keeps coming up is that companies don't always seem to appreciate
maintainership (which is a bit strange - the same companies may then
_love_ appreciateing managers, which is something very different but
has some of the same flavour to it).

And btw, I don't want to make that "I wish we had more maintainers" be
a value judgement. It's not that maintainers are somehow more
important than developers. I just think they are two fairly different
roles, and I think one person doing both puts unnecessary stress on
that person.

             Linus

Re: [GIT PULL] iomap: bug fixes for 6.6-rc7

[Jeff Layton]

On Thu, 2023-10-26 at 08:10 -1000, Linus Torvalds wrote:
> On Wed, 25 Oct 2023 at 17:13, Darrick J. Wong <djwong@kernel.org> wrote:
> > 
> > Similar to what we just did with XFS, I propose breaking up the iomap
> > Maintainer role into pieces that are more manageable by a single person.
> > As RM, all you'd have to do is integrate reviewed patches and pull
> > requests into one of your work branches.  That gives you final say over
> > what goes in and how it goes in, instead of letting branches collide in
> > for-next without warning.
> 
> I _think_ what you are saying is that you'd like to avoid being both a
> maintainer and a developer.
> 
> Now, I'm probably hugely biased and going by personal experience, but
> I do think that doing double duty is the worst of both worlds, and
> pointlessly stressful.
> 
> As a maintainer, you have to worry about the big picture (things like
> release timing, even if it's just a "is this a fix for this release,
> or should it get queued for the next one") but also code-related
> things like "we have two different things going on, let's sort them
> out separately". Christian had that kind of issue just a couple of
> days ago with the btrfs tree.
> 
> But then, as a developer, those are distractions and just add stress
> and worry, and distract from whatever you're working on. As a
> developer, the last thing you want to worry about is something else
> than the actual technical issue you're trying to solve.
> 
> And obviously, there's a lot of overlap. A maintainer needs to be
> _able_ to be a developer just to make good choices. And the whole
> "maintainer vs developer" doesn't have to be two different people,
> somebody might shift from one to the other simply because maybe they
> enjoy both roles. Just not at the same time, all the time, having both
> things putting different stress on you.
> 
> You can *kind* of see the difference in our git tree if you do
> 
>     git rev-list --count --author=XYZ --no-merges --since=1.year HEAD
> 
> to see "code authorship" (aka developer), vs
> 
>     git rev-list --count --committer=XYZ --since=1.year HEAD
> 
> which shows some kind of approximation of "maintainership". Obviously
> there is overlap (potentially a lot of it) and the above isn't some
> absolute thing, but you can see some patterns.
> 
> I personally wish we had more people who are maintainers _without_
> having to worry too much about developing new code.  One of the issues
> that keeps coming up is that companies don't always seem to appreciate
> maintainership (which is a bit strange - the same companies may then
> _love_ appreciateing managers, which is something very different but
> has some of the same flavour to it).
> 
> And btw, I don't want to make that "I wish we had more maintainers" be
> a value judgement. It's not that maintainers are somehow more
> important than developers. I just think they are two fairly different
> roles, and I think one person doing both puts unnecessary stress on
> that person.
> 

Personally, I wouldn't want to give up doing development work
altogether. I don't mind doing some maintainership duties, and helping
out other maintainers where I can, but maintainership takes a lot more
time than most people realize. You're sort of obligated to review
everything that comes across your plate.

I know Trond and Anna have been successful alternating the handling of
pull requests and linux-next. I think it makes a lot of sense to have
maintainership teams, at least for larger subsystems. We sort of have
them informally now, but maybe we should be aiming to have a group of
people who are all nominally maintainers on larger subsystems. They can
then rotate between themselves on some schedule (every year? or every 5
releases? Whatever works...).

That provides some extra incentive for other team members to help out
with reviews, and it may make it simpler for someone else to step in
when the current maintainer needs to step away.

It does rely on a high degree of trust between team members, but most of
us seem to get along.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [GIT PULL] iomap: bug fixes for 6.6-rc7

[Christian Brauner]

On Thu, Oct 26, 2023 at 08:10:01AM -1000, Linus Torvalds wrote:
> On Wed, 25 Oct 2023 at 17:13, Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > Similar to what we just did with XFS, I propose breaking up the iomap
> > Maintainer role into pieces that are more manageable by a single person.
> > As RM, all you'd have to do is integrate reviewed patches and pull
> > requests into one of your work branches.  That gives you final say over
> > what goes in and how it goes in, instead of letting branches collide in
> > for-next without warning.
> 
> I _think_ what you are saying is that you'd like to avoid being both a
> maintainer and a developer.
> 
> Now, I'm probably hugely biased and going by personal experience, but
> I do think that doing double duty is the worst of both worlds, and
> pointlessly stressful.
> 
> As a maintainer, you have to worry about the big picture (things like
> release timing, even if it's just a "is this a fix for this release,
> or should it get queued for the next one") but also code-related
> things like "we have two different things going on, let's sort them
> out separately". Christian had that kind of issue just a couple of
> days ago with the btrfs tree.
> 
> But then, as a developer, those are distractions and just add stress
> and worry, and distract from whatever you're working on. As a
> developer, the last thing you want to worry about is something else
> than the actual technical issue you're trying to solve.
> 
> And obviously, there's a lot of overlap. A maintainer needs to be
> _able_ to be a developer just to make good choices. And the whole
> "maintainer vs developer" doesn't have to be two different people,
> somebody might shift from one to the other simply because maybe they
> enjoy both roles. Just not at the same time, all the time, having both
> things putting different stress on you.
> 
> You can *kind* of see the difference in our git tree if you do
> 
>     git rev-list --count --author=XYZ --no-merges --since=1.year HEAD
> 
> to see "code authorship" (aka developer), vs
> 
>     git rev-list --count --committer=XYZ --since=1.year HEAD
> 
> which shows some kind of approximation of "maintainership". Obviously
> there is overlap (potentially a lot of it) and the above isn't some
> absolute thing, but you can see some patterns.
> 
> I personally wish we had more people who are maintainers _without_
> having to worry too much about developing new code.  One of the issues
> that keeps coming up is that companies don't always seem to appreciate
> maintainership (which is a bit strange - the same companies may then
> _love_ appreciateing managers, which is something very different but
> has some of the same flavour to it).
> 
> And btw, I don't want to make that "I wish we had more maintainers" be
> a value judgement. It's not that maintainers are somehow more
> important than developers. I just think they are two fairly different
> roles, and I think one person doing both puts unnecessary stress on
> that person.

I think most of us enjoy both. Personally, I always try to carve out
time where I can work on stuff that I find interesting. It helps to stay
sane and it helps with providing good reviews. It also helps being
reminded how easy it is to code up bugs.

When there's an interesting series that morphes into larger subsystem
massaging that's always great because that's a chance to collaborate and
different people can take on different parts. And it usually means one
does get a few patches in as well.

In a smaller subsystem it's probably ok to be main developer, reviewer,
and developer at the same time. But in a busy subsystem that trias isn't
sustainable. Let alone scaling that to multiple subsystems. 

One of the critical parts is review. Good reviews are often insanely
expensive and they are very much a factor in burning people out. If one
only ever reviews and the load never ends that's going to fsck with you
in the long run.

A reviewer must be technically great, have design, context,
maintainability, and wider context and future of the subsystem in mind,
and not be afraid to have code changed or removed that they wrote (That
last part really matters.), and also be ok with being publicly wrong in
front of a whole bunch of smart people (Which I want to stress is
absolutely fine and almost unavoidable.).

But once you have a couple of good reviewers in a subsystem that you can
really rely on that if they give an ack/rvb it's solid enough (not
necessarily bug free even) that's really good. It not just takes away
pressure it also means that it's possible for other
reviewers/maintainers to go develop stuff themselves.

Re: [GIT PULL] iomap: bug fixes for 6.6-rc7

[Linus Torvalds]

On Fri, 27 Oct 2023 at 08:46, Christian Brauner <brauner@kernel.org> wrote:
>
> One of the critical parts is review. Good reviews are often insanely
> expensive and they are very much a factor in burning people out. If one
> only ever reviews and the load never ends that's going to fsck with you
> in the long run.

I absolutely despise the review requirement that several companies
have. I very much understand why it happens, but I think it's actively
detrimental to the workflow.

It's not just that reviewing is hard, the review requirement tends to
be a serialization point where now you as a developer are waiting for
others to review it, and those others are not nearly as motivated to
do so or are easily going to be nitpicking about the non-critical
things.

So it's not just the reviewers that get burned out, I think the
process ends up being horrific for developers too, and easily leads to
the "let's send out version 17 of this patch based on previous
review". At which point everybody is completely fed up with the whole
process.

And if it doesn't get to version 17, it's because the reviewers too
have gotten so fed up that by version three they go "whatever, I've
seen this before, they fixed the obvious thing I noticed, I'll mark it
reviewed".

The other dynamic with reviews is that you end up getting
review-cliques, either due to company pressure or just a very natural
"you review mine, I review yours" back-scratching.

Don't get me wrong - it can work, and it can even work well, but I
think the times it works really well is when people have gotten so
used to each others, and know each other's quirks and workflows and
they just work well together. But that also means that some people are
having a much easier time getting reviews, because they are part of
that "this group works well together" crowd.

Maybe it's a necessary evil. I certainly do *not* think the "lone
developer goes his own way" model works all that well. But the reason
I said that I wish we had more maintainers, is that I think we would
often be better off with not a "review process" back-and-forth. but a
_pipeline_ through a few levels of maintainers.  Not the "hold things
up and send it back to the developer" kind of thing, but "Oh, this
looks fine, I'll just send it on - possibly with the fixes I think are
needed".

So I think a pipeline of "Signed-off-by" (or just merges) might be
something to strive for as at least a partial replacement for reviews.

Sure, you might get Acked-by's or Reviewed-by's or Tested-by's along
the way *too*, or - when people are just merging directly through git
- you'd just get a merge commit with commentary and perhaps extra
stuff on top.

Back when we started doing the whole "Signed-off-by" - for legal
reasons, not development reasons - the big requirement for me was that
"it needs to work as a pipeline, not as some kind of back-and-forth
that holds up development". And I think the whole sign-off chain has
worked really well, and we've never needed to use it for the original
legal purposes (and hopefully just by virtue of it existing, we never
will), but it has been a huge success from a development standpoint.
When something goes wrong, I think it's been great to have that whole
chain of how it got merged, and in fact one of my least favorite parts
of git ended up being how we never made it easy to see the merge chain
after it's been committed (you can technically get it with "git
name-rev", but it sure is not obvious).

I dunno. Maybe I'm just dreaming. But the complaints about reviews -
or lack of them - do tend to come up a lot, and I feel like the whole
review process is a very big part of the problem.

                 Linus

Re: [GIT PULL] iomap: bug fixes for 6.6-rc7

[Christian Brauner]

On Fri, Oct 27, 2023 at 01:30:00PM -1000, Linus Torvalds wrote:
> On Fri, 27 Oct 2023 at 08:46, Christian Brauner <brauner@kernel.org> wrote:
> >
> > One of the critical parts is review. Good reviews are often insanely
> > expensive and they are very much a factor in burning people out. If one
> > only ever reviews and the load never ends that's going to fsck with you
> > in the long run.
> 
> I absolutely despise the review requirement that several companies
> have. I very much understand why it happens, but I think it's actively
> detrimental to the workflow.
> 
> It's not just that reviewing is hard, the review requirement tends to
> be a serialization point where now you as a developer are waiting for
> others to review it, and those others are not nearly as motivated to
> do so or are easily going to be nitpicking about the non-critical
> things.
> 
> So it's not just the reviewers that get burned out, I think the
> process ends up being horrific for developers too, and easily leads to
> the "let's send out version 17 of this patch based on previous
> review". At which point everybody is completely fed up with the whole
> process.
> 
> And if it doesn't get to version 17, it's because the reviewers too
> have gotten so fed up that by version three they go "whatever, I've
> seen this before, they fixed the obvious thing I noticed, I'll mark it
> reviewed".

You're talking about companies having review requirements for their
internal project stuff, right? I haven't heard that there's any sort of
review requirement with respect to the upstream kernel from companies.

Sure, if you make it a requirement then it sucks because it's just
another chore.

I think we lack more reviewers for a multitude of reasons but to me one
of the biggest is that we don't encourage the people to do it and
there's no inherent reward to it. Sure, you get that RVB or ACK in the
changelog but whatever. That neither gets you a job nor does it get you
on LWN. So why bother.

So often our reviewers are the people who have some sort of sense of
ownership with respect to the subsystem. That's mostly the maintainers.

You occassionally get the author of a patch series that sticks around
and reviews stuff that they wrote. But that's not enough and then we're
back to the problem that you can't effectively be maintainer, reviewer,
and main developer.

And yes, nitpicky review isn't really helpful and the goal shouldn't be
to push a series to pointless version numbers before it can be merged.

> 
> The other dynamic with reviews is that you end up getting
> review-cliques, either due to company pressure or just a very natural
> "you review mine, I review yours" back-scratching.
> 
> Don't get me wrong - it can work, and it can even work well, but I
> think the times it works really well is when people have gotten so
> used to each others, and know each other's quirks and workflows and
> they just work well together. But that also means that some people are
> having a much easier time getting reviews, because they are part of
> that "this group works well together" crowd.

Yes, we certainly see that happening. And I really think that's overall
a good thing. It shouldn't become an in-group out-group thing obviously
so that needs to be carefully handled. But long-term trust between core
developers and maintainers is key to subsystem health imho. And such
quick review bounces are a sign of that. I don't think we actively see
useless reviews that are just "scratch my back". The people who review
often do it with sufficient fervor (for better or worse sometimes).

That said, yes, it needs to be fair. But it's just natural that you feel
more likely to throw a ACK or RVB to something someone did you know
usually does good work and you know will come back to help you put out
the fire they started.

> 
> Maybe it's a necessary evil. I certainly do *not* think the "lone

I think it's just a natural development and it's on use to make sure it
doesn't become some sort of group thing where it's five people from the
same company that push their stuff upstream.

> developer goes his own way" model works all that well. But the reason

Yeah, so I do think that this is actually the bigger problem long term.
Lone-wolf models are terrible. But I didn't grow up with lone-wolf
projects so I have no strong attachment to such models in the first
place. Maybe I judge that more harshly simply because of that.

> I said that I wish we had more maintainers, is that I think we would
> often be better off with not a "review process" back-and-forth. but a
> _pipeline_ through a few levels of maintainers.  Not the "hold things
> up and send it back to the developer" kind of thing, but "Oh, this
> looks fine, I'll just send it on - possibly with the fixes I think are
> needed".
> 
> So I think a pipeline of "Signed-off-by" (or just merges) might be
> something to strive for as at least a partial replacement for reviews.
> 
> Sure, you might get Acked-by's or Reviewed-by's or Tested-by's along
> the way *too*, or - when people are just merging directly through git
> - you'd just get a merge commit with commentary and perhaps extra
> stuff on top.

And we kinda do that in some subsystems rather consequently. Networking
does it at least and BPF does it with their subtrees and then BPF and
its subtrees bubble up into networking and then into mainline.

And that model seems to work well, yes. And it takes pressure of because
it formalizes the whole maintenance thing a lot more.

Idk if you've seen that but what we do for new stuff that gets added to
vfs is that we have maintainership entries ala BPF so e.g.,
FILESYSTEMS [EXPORTFS] - that patch is still in -next and the PR for
that won't get sent before next week and then it lists the maintainers.
But it's all part of vfs.git and they just bubble up the patches. We
could just do that via sub merges even.

> 
> Back when we started doing the whole "Signed-off-by" - for legal
> reasons, not development reasons - the big requirement for me was that
> "it needs to work as a pipeline, not as some kind of back-and-forth
> that holds up development". And I think the whole sign-off chain has
> worked really well, and we've never needed to use it for the original
> legal purposes (and hopefully just by virtue of it existing, we never
> will), but it has been a huge success from a development standpoint.
> When something goes wrong, I think it's been great to have that whole
> chain of how it got merged, and in fact one of my least favorite parts
> of git ended up being how we never made it easy to see the merge chain
> after it's been committed (you can technically get it with "git
> name-rev", but it sure is not obvious).
> 
> I dunno. Maybe I'm just dreaming. But the complaints about reviews -
> or lack of them - do tend to come up a lot, and I feel like the whole
> review process is a very big part of the problem.

Yeah, certainly.

Re: [GIT PULL] iomap: bug fixes for 6.6-rc7

[pr-tracker-bot]

The pull request you sent on Fri, 20 Oct 2023 23:27:44 -0700:

> https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git iomap-6.6-fixes-5

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

Thank you!

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