2011-05-13 03:15:15

by Stephen Rothwell

[permalink] [raw]
Subject: linux-next: manual merge of the tip tree with the arm tree

Hi all,

Today's linux-next merge of the tip tree got a conflict in
arch/x86/kernel/i8253.c between commit 3490f584b9ba ("clocksource:
convert x86 to generic i8253 clocksource") from the arm tree and
commitb01cc1b0eae0 ("x86: Convert remaining x86 clocksources to
clocksource_register_hz/khz") from the tip tree.

The former seems to supercede the latter, so I used the former.
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/


Attachments:
(No filename) (482.00 B)
(No filename) (490.00 B)
Download all attachments

2011-05-13 08:07:25

by Ingo Molnar

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree


* Stephen Rothwell <[email protected]> wrote:

> Hi all,
>
> Today's linux-next merge of the tip tree got a conflict in
> arch/x86/kernel/i8253.c between commit 3490f584b9ba ("clocksource: convert
> x86 to generic i8253 clocksource") from the arm tree and commit b01cc1b0eae0
> ("x86: Convert remaining x86 clocksources to clocksource_register_hz/khz")
> from the tip tree.
>
> The former seems to supercede the latter, so I used the former.

Russell, how the heck did this commit:

commit 3490f584b9ba5a0b6f63832fbc9c5ec72506697b
Author: Russell King <[email protected]>
AuthorDate: Sun May 8 18:55:19 2011 +0100
Commit: Russell King <[email protected]>
CommitDate: Tue May 10 08:20:54 2011 +0100

clocksource: convert x86 to generic i8253 clocksource

which has such a clearly x86 diffstat:

arch/x86/Kconfig | 1 +
arch/x86/include/asm/i8253.h | 2 +
arch/x86/kernel/i8253.c | 79 +-----------------------------------------
3 files changed, 4 insertions(+), 78 deletions(-)

end up in the ARM tree without an ack from an x86 maintainer??

Not only did it conflict with a 2 months old commit, but it also broke the
linux-next x86 build.

I see the commit has an ack from John but that feedback is not visible in the
lkml thread of this patch nor did John really realize the conflict nor the
build breakage. The patch was still in the to-be-reviewed queue of our patches.

Nor was it tested properly. The patch looks sane but your workflow sucks.
Please revert it and use a proper Git workflow to change arch/x86/ details ...

Thanks,

Ingo

2011-05-13 08:38:51

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree

On Fri, May 13, 2011 at 10:06:34AM +0200, Ingo Molnar wrote:
>
> * Stephen Rothwell <[email protected]> wrote:
>
> > Hi all,
> >
> > Today's linux-next merge of the tip tree got a conflict in
> > arch/x86/kernel/i8253.c between commit 3490f584b9ba ("clocksource: convert
> > x86 to generic i8253 clocksource") from the arm tree and commit b01cc1b0eae0
> > ("x86: Convert remaining x86 clocksources to clocksource_register_hz/khz")
> > from the tip tree.
> >
> > The former seems to supercede the latter, so I used the former.
>
> Russell, how the heck did this commit:
>
> commit 3490f584b9ba5a0b6f63832fbc9c5ec72506697b
> Author: Russell King <[email protected]>
> AuthorDate: Sun May 8 18:55:19 2011 +0100
> Commit: Russell King <[email protected]>
> CommitDate: Tue May 10 08:20:54 2011 +0100
>
> clocksource: convert x86 to generic i8253 clocksource
>
> which has such a clearly x86 diffstat:
>
> arch/x86/Kconfig | 1 +
> arch/x86/include/asm/i8253.h | 2 +
> arch/x86/kernel/i8253.c | 79 +-----------------------------------------
> 3 files changed, 4 insertions(+), 78 deletions(-)
>
> end up in the ARM tree without an ack from an x86 maintainer??

The "no response" means two things: either that people are busy, or people
don't care about the patch. There is a patch from David Martin modifying
linux/elf.h adding one line to it which has not had any response. Should
we assume the silence means that people are busy? If we did that, nothing
would ever happen.

> I see the commit has an ack from John but that feedback is not visible in the
> lkml thread of this patch nor did John really realize the conflict nor the

I have no idea why John's ack is not visible, especially as it was sent
to lkml _and_ explicitly copied to you.

> build breakage. The patch was still in the to-be-reviewed queue of our patches.
>
> Nor was it tested properly. The patch looks sane but your workflow sucks.
> Please revert it and use a proper Git workflow to change arch/x86/ details ...

I don't think so. I created a patch. I posted it to relevant people.
I got an ack. So I put it into linux-next for further testing rather
than having it sitting around here getting zero testing.

That's the _proper_ thing to do. linux-next found some problems, so
let's sort them out - great, that's what linux-next is there for. Let's
sort them out instead of assigning blame.

And hey, it found a problem, and the problem has now been fixed. Which
is great, and that should be visible to linux-next soon.

As for merge conflicts, they happen. They get sorted. It's no big deal.
Again, that's what linux-next is there to find and allow people to
_discuss_ how to resolve them. It's not about avoiding all conflicts
no matter what or blaming people when conflicts happen.

Lastly, I have absolutely no problem about pulling the x86 bits out of
this series if they cause a conflict or don't get an ack. I operate a
flexible approach to my git tree for stuff like this which allows stuff
to be dropped or updated as necessary.

2011-05-13 08:48:00

by Thomas Gleixner

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree

On Fri, 13 May 2011, Russell King - ARM Linux wrote:

> On Fri, May 13, 2011 at 10:06:34AM +0200, Ingo Molnar wrote:
> >
> > * Stephen Rothwell <[email protected]> wrote:
> >
> > > Hi all,
> > >
> > > Today's linux-next merge of the tip tree got a conflict in
> > > arch/x86/kernel/i8253.c between commit 3490f584b9ba ("clocksource: convert
> > > x86 to generic i8253 clocksource") from the arm tree and commit b01cc1b0eae0
> > > ("x86: Convert remaining x86 clocksources to clocksource_register_hz/khz")
> > > from the tip tree.
> > >
> > > The former seems to supercede the latter, so I used the former.
> >
> > Russell, how the heck did this commit:
> >
> > commit 3490f584b9ba5a0b6f63832fbc9c5ec72506697b
> > Author: Russell King <[email protected]>
> > AuthorDate: Sun May 8 18:55:19 2011 +0100
> > Commit: Russell King <[email protected]>
> > CommitDate: Tue May 10 08:20:54 2011 +0100
> >
> > clocksource: convert x86 to generic i8253 clocksource
> >
> > which has such a clearly x86 diffstat:
> >
> > arch/x86/Kconfig | 1 +
> > arch/x86/include/asm/i8253.h | 2 +
> > arch/x86/kernel/i8253.c | 79 +-----------------------------------------
> > 3 files changed, 4 insertions(+), 78 deletions(-)
> >
> > end up in the ARM tree without an ack from an x86 maintainer??

Acked-by-me!

> The "no response" means two things: either that people are busy, or people
> don't care about the patch. There is a patch from David Martin modifying
> linux/elf.h adding one line to it which has not had any response. Should
> we assume the silence means that people are busy? If we did that, nothing
> would ever happen.
>
> > I see the commit has an ack from John but that feedback is not visible in the
> > lkml thread of this patch nor did John really realize the conflict nor the
>
> I have no idea why John's ack is not visible, especially as it was sent
> to lkml _and_ explicitly copied to you.
>
> > build breakage. The patch was still in the to-be-reviewed queue of our patches.
> >
> > Nor was it tested properly. The patch looks sane but your workflow sucks.
> > Please revert it and use a proper Git workflow to change arch/x86/ details ...
>
> I don't think so. I created a patch. I posted it to relevant people.
> I got an ack. So I put it into linux-next for further testing rather
> than having it sitting around here getting zero testing.
>
> That's the _proper_ thing to do. linux-next found some problems, so
> let's sort them out - great, that's what linux-next is there for. Let's
> sort them out instead of assigning blame.
>
> And hey, it found a problem, and the problem has now been fixed. Which
> is great, and that should be visible to linux-next soon.
>
> As for merge conflicts, they happen. They get sorted. It's no big deal.
> Again, that's what linux-next is there to find and allow people to
> _discuss_ how to resolve them. It's not about avoiding all conflicts
> no matter what or blaming people when conflicts happen.
>
> Lastly, I have absolutely no problem about pulling the x86 bits out of
> this series if they cause a conflict or don't get an ack. I operate a
> flexible approach to my git tree for stuff like this which allows stuff
> to be dropped or updated as necessary.
>

2011-05-13 09:27:14

by Ingo Molnar

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree


* Russell King - ARM Linux <[email protected]> wrote:

> On Fri, May 13, 2011 at 10:06:34AM +0200, Ingo Molnar wrote:
> >
> > * Stephen Rothwell <[email protected]> wrote:
> >
> > > Hi all,
> > >
> > > Today's linux-next merge of the tip tree got a conflict in
> > > arch/x86/kernel/i8253.c between commit 3490f584b9ba ("clocksource: convert
> > > x86 to generic i8253 clocksource") from the arm tree and commit b01cc1b0eae0
> > > ("x86: Convert remaining x86 clocksources to clocksource_register_hz/khz")
> > > from the tip tree.
> > >
> > > The former seems to supercede the latter, so I used the former.
> >
> > Russell, how the heck did this commit:
> >
> > commit 3490f584b9ba5a0b6f63832fbc9c5ec72506697b
> > Author: Russell King <[email protected]>
> > AuthorDate: Sun May 8 18:55:19 2011 +0100
> > Commit: Russell King <[email protected]>
> > CommitDate: Tue May 10 08:20:54 2011 +0100
> >
> > clocksource: convert x86 to generic i8253 clocksource
> >
> > which has such a clearly x86 diffstat:
> >
> > arch/x86/Kconfig | 1 +
> > arch/x86/include/asm/i8253.h | 2 +
> > arch/x86/kernel/i8253.c | 79 +-----------------------------------------
> > 3 files changed, 4 insertions(+), 78 deletions(-)
> >
> > end up in the ARM tree without an ack from an x86 maintainer??
>
> The "no response" means two things: either that people are busy, [...]

Had you asked us before committing it one day after it was posted, or had you
*noticed* that those files are not in your tree and are already modified in
linux-next, you'd have gotten a response like:

--------------->

Yes, the change looks good to me as well in principle. Thomas is busy this
week, he is on a conference.

Guys if you want to do this outside of the x86 tree please test this series
thoroughly on x86 as well before pushing it out anywhere permanent or pushing
it into linux-next.

Also note that there's also some pending changes in the x86 tree modifying
arch/x86/kernel/i8253.c:

b01cc1b0eae0: x86: Convert remaining x86 clocksources to clocksource_register_hz/khz

If you want to do any further change to this file you need to pull in this
change first or need to resolve the conflict in some other way.

Thanks,

Ingo

2011-05-13 10:04:36

by Thomas Gleixner

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree

On Fri, 13 May 2011, Ingo Molnar wrote:
> * Russell King - ARM Linux <[email protected]> wrote:
>
> > On Fri, May 13, 2011 at 10:06:34AM +0200, Ingo Molnar wrote:
> > >
> > > * Stephen Rothwell <[email protected]> wrote:
> > >
> > > > Hi all,
> > > >
> > > > Today's linux-next merge of the tip tree got a conflict in
> > > > arch/x86/kernel/i8253.c between commit 3490f584b9ba ("clocksource: convert
> > > > x86 to generic i8253 clocksource") from the arm tree and commit b01cc1b0eae0
> > > > ("x86: Convert remaining x86 clocksources to clocksource_register_hz/khz")
> > > > from the tip tree.
> > > >
> > > > The former seems to supercede the latter, so I used the former.
> > >
> > > Russell, how the heck did this commit:
> > >
> > > commit 3490f584b9ba5a0b6f63832fbc9c5ec72506697b
> > > Author: Russell King <[email protected]>
> > > AuthorDate: Sun May 8 18:55:19 2011 +0100
> > > Commit: Russell King <[email protected]>
> > > CommitDate: Tue May 10 08:20:54 2011 +0100
> > >
> > > clocksource: convert x86 to generic i8253 clocksource
> > >
> > > which has such a clearly x86 diffstat:
> > >
> > > arch/x86/Kconfig | 1 +
> > > arch/x86/include/asm/i8253.h | 2 +
> > > arch/x86/kernel/i8253.c | 79 +-----------------------------------------
> > > 3 files changed, 4 insertions(+), 78 deletions(-)
> > >
> > > end up in the ARM tree without an ack from an x86 maintainer??
> >
> > The "no response" means two things: either that people are busy, [...]
>
> Had you asked us before committing it one day after it was posted, or had you
> *noticed* that those files are not in your tree and are already modified in
> linux-next, you'd have gotten a response like:
>
> --------------->
>
> Yes, the change looks good to me as well in principle. Thomas is busy this
> week, he is on a conference.

I saw it and missed it to reply, sorry.

> Guys if you want to do this outside of the x86 tree please test this series
> thoroughly on x86 as well before pushing it out anywhere permanent or pushing
> it into linux-next.
>
> Also note that there's also some pending changes in the x86 tree modifying
> arch/x86/kernel/i8253.c:
>
> b01cc1b0eae0: x86: Convert remaining x86 clocksources to clocksource_register_hz/khz
>
> If you want to do any further change to this file you need to pull in this
> change first or need to resolve the conflict in some other way.

If it makes things easier I can pull it through the tip/timers/.... as
well. Just let me know.

Thanks,

tglx

2011-05-13 17:26:25

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree

On Fri, May 13, 2011 at 11:26:46AM +0200, Ingo Molnar wrote:
> Also note that there's also some pending changes in the x86 tree modifying
> arch/x86/kernel/i8253.c:
>
> b01cc1b0eae0: x86: Convert remaining x86 clocksources to clocksource_register_hz/khz
>
> If you want to do any further change to this file you need to pull in this
> change first or need to resolve the conflict in some other way.

I have been told by Linus on more than one occasion that Linus wants to
see exactly these kinds of merge conflicts, as it helps him track what's
going on.

He's made that point again recently during discussions with the ARM
community, and he's gone further to explicitly state that he does _not_
want these kinds of merge conflicts to be hidden from him.

So I really don't see what the issue here is - if Linus is happy _and_
wants to resolve trivial merge conflicts then there's no problem here.
It's probably a good thing that Linus does see the conflict so he gets
to know that at least some ARM folk _are_ looking at common things
across all architectures.

2011-05-13 21:37:54

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree

On Fri, May 13, 2011 at 11:26:46AM +0200, Ingo Molnar wrote:
> Had you asked us before committing it one day after it was posted, or had you
> *noticed* that those files are not in your tree and are already modified in
> linux-next, you'd have gotten a response like:

Please also don't read anything into the commit date - it merely shows
when the last update happened.

My workflow for patch series involves keeping them in git right from the
start. So actually they've been in git since _before_ they were posted.
In fact, the emails which I send out for any patch series are always
generated from the git commits.

So, all my patches live in git _first_ before being mailed out.

2011-05-16 07:22:11

by Ingo Molnar

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree


* Russell King - ARM Linux <[email protected]> wrote:

> On Fri, May 13, 2011 at 11:26:46AM +0200, Ingo Molnar wrote:
> > Also note that there's also some pending changes in the x86 tree modifying
> > arch/x86/kernel/i8253.c:
> >
> > b01cc1b0eae0: x86: Convert remaining x86 clocksources to clocksource_register_hz/khz
> >
> > If you want to do any further change to this file you need to pull in this
> > change first or need to resolve the conflict in some other way.
>
> I have been told by Linus on more than one occasion that Linus wants to
> see exactly these kinds of merge conflicts, as it helps him track what's
> going on.

He has told us on more than one occasion that he wants to see *easy* conflicts,
where he can see benign interaction between properly maintained trees.

Here he would look and would only determine what i have already determined:
that the workflow of applying this patch sucked. That is definitely not the
kind of conflict Linus wants to see in the merge window ...

You applied the patch without talking to the maintainers who are running the
affected tree. You talked to one of the developers which is fine, and i will
generally ack it in hindsight if you do a fine job of sorting out the details -
but here you touched an under-modification file without even realizing it. So
you messed up which fact i will keep pointing out and i will keep asking you to
fix your workflow, so that similar mistakes wont happen in the future.

Really, Russell, you sometimes need to accept blame and you need to admit when
you messed up instead of writing countless mails trying to save face and
wriggle out of it. I mess up all the time and my hand does not rot away from
writing this. Really, write this down: "You are right, I messed this up a bit,
lets fix it instead of wasting time on emails."

Lets resolve the conflict and move on, okay?

Thanks,

Ingo

2011-05-16 07:32:10

by Ingo Molnar

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree


* Russell King - ARM Linux <[email protected]> wrote:

> On Fri, May 13, 2011 at 11:26:46AM +0200, Ingo Molnar wrote:
> > Had you asked us before committing it one day after it was posted, or had you
> > *noticed* that those files are not in your tree and are already modified in
> > linux-next, you'd have gotten a response like:
>
> Please also don't read anything into the commit date - it merely shows
> when the last update happened.
>
> My workflow for patch series involves keeping them in git right from the
> start. So actually they've been in git since _before_ they were posted. In
> fact, the emails which I send out for any patch series are always generated
> from the git commits.
>
> So, all my patches live in git _first_ before being mailed out.

It is not a problem at all if you commit it to some non-permanent development
branch of your own - we all do it.

The commit date i pointed out was of the *final* commit, which got into
linux-next. That showed a timestamp of just a day after the patch was sent
out: presumably you rebased it to add John's Acked-by.

The step where your workflow failed was to take upon yourself to maintain a
file you do not normally maintain *and* messing up doing that:

- you did not ask the maintainers who maintain it (which is fine as long as
you do not mess up)

- you did not realize that the file you modified is already modified in that
tree, almost two months ago (it's not that hard to fetch linux-next once
every week or so)

- you did not even notify them that you committed something so when the bug
happened in linux-next they had no idea what was going on

Had you done any of those steps differently we'd have a better outcome.

It's not a big problem all and we can resolve it, but you need to stop
pretending that your workflow was just fine - it sucked here.

Thanks,

Ingo

2011-05-16 07:43:42

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree

On Mon, May 16, 2011 at 09:31:44AM +0200, Ingo Molnar wrote:
>
> * Russell King - ARM Linux <[email protected]> wrote:
>
> > On Fri, May 13, 2011 at 11:26:46AM +0200, Ingo Molnar wrote:
> > > Had you asked us before committing it one day after it was posted, or had you
> > > *noticed* that those files are not in your tree and are already modified in
> > > linux-next, you'd have gotten a response like:
> >
> > Please also don't read anything into the commit date - it merely shows
> > when the last update happened.
> >
> > My workflow for patch series involves keeping them in git right from the
> > start. So actually they've been in git since _before_ they were posted. In
> > fact, the emails which I send out for any patch series are always generated
> > from the git commits.
> >
> > So, all my patches live in git _first_ before being mailed out.
>
> It is not a problem at all if you commit it to some non-permanent development
> branch of your own - we all do it.

Clearly you're not listening, so no point discussing this further.

2011-05-16 09:18:13

by Ingo Molnar

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree


* Russell King - ARM Linux <[email protected]> wrote:

> On Mon, May 16, 2011 at 09:31:44AM +0200, Ingo Molnar wrote:
> >
> > * Russell King - ARM Linux <[email protected]> wrote:
> >
> > > On Fri, May 13, 2011 at 11:26:46AM +0200, Ingo Molnar wrote:
> > > >
> > > > Had you asked us before committing it one day after it was posted, or
> > > > had you *noticed* that those files are not in your tree and are already
> > > > modified in linux-next, you'd have gotten a response like:
> > >
> > > Please also don't read anything into the commit date - it merely shows
> > > when the last update happened.
> > >
> > > My workflow for patch series involves keeping them in git right from the
> > > start. So actually they've been in git since _before_ they were posted.
> > > In fact, the emails which I send out for any patch series are always
> > > generated from the git commits.
> > >
> > > So, all my patches live in git _first_ before being mailed out.
> >
> > It is not a problem at all if you commit it to some non-permanent
> > development branch of your own - we all do it.
>
> Clearly you're not listening, so no point discussing this further.

Since in the sentence you quote i only repeated what you said above (that you
keep commits in Git from before when they are posted: i do that too for
development) i have trouble following your line of thought of how you could
possibly have concluded that i'm "not listening".

I am very much listening, i just do not agree with what you are saying: i think
it's pretty clear that details of your Git workflow are broken and need to be
improved - as demonstrated by the conflict, build breakage and discussion in
this thread.

Anyway, since you unilaterally stopped discussing this topic and since there's
no acknowledgement from you that you'll fix your workflow it appears i have no
choice but to ask you to refrain from modifying any arch/x86/ code in the
future, without an explicit *prior* ack from one of the x86 maintainers.

When you do that they will be able to guide you through when various files are
proper to modify in a separate branch and what to do if there are already
changes in flight.

Thanks,

Ingo

2011-05-16 09:20:17

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree

On Mon, May 16, 2011 at 11:17:44AM +0200, Ingo Molnar wrote:
> Since in the sentence you quote i only repeated what you said above (that you
> keep commits in Git from before when they are posted: i do that too for
> development) i have trouble following your line of thought of how you could
> possibly have concluded that i'm "not listening".

I'm saying you're not listening because I've described the workflow, I've
told you that what appeared in linux-next was still subject to change,
and yet _you) persist in telling me that what I put in there was the
"final" commit. That doesn't make it seem like you're listening.

2011-05-16 09:41:31

by Ingo Molnar

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree


* Russell King - ARM Linux <[email protected]> wrote:

> On Mon, May 16, 2011 at 11:17:44AM +0200, Ingo Molnar wrote:
> > Since in the sentence you quote i only repeated what you said above (that you
> > keep commits in Git from before when they are posted: i do that too for
> > development) i have trouble following your line of thought of how you could
> > possibly have concluded that i'm "not listening".
>
> I'm saying you're not listening because I've described the workflow, [...]

And i've said that your workflow is broken for this particular case and you
have not reacted to my various descriptions of how your workflow is broken in
this particular case.

> [...] I've told you that what appeared in linux-next was still subject to
> change, [...]

That's not a proper Git workflow: linux-next is *not* a playing ground to break
arbitrarily in an *intentional* fashion like that.

linux-next has enough trouble sorting out the *unintentional* breakages and
spurious conflicts!

Consider linux-next conflicts as a canary for workflow problems. It works very
well in that regard.

The thing is, if sfr has trouble sorting out the conflict we caused here, while
he does a dozen conflict resolutions a week 365 days a year, consider the
workflow problematic by definition ...

So please get your workflow in shape as i suggested:

- When you seriously modify or move files that other maintainers maintain in
their trees then you first need to wait for the opinion of those
maintainers (and not assume lack of ack after 24 hours means acceptance), or
at least you need to *check* linux-next/master whether those files are truly
quiet in this particular cycle ...

It's only common-sense and not hard to do at all!

That way you can avoid most breakages and conflicts both of technical and
social nature *before* pushing things to linux-next. As you are doing things
now you are driving blind in essence, that way you are *asking* for trouble and
conflicts down the road, and that is sad and i cannot just accept it silently.

I think you are too used to being able to do anything within the ARM Git space
and getting away with it if you mess up?

If you have a workflow that seriously modifies other trees without realizing
that those trees have in-flight changes then you have a broken workflow, simple
as that. And yes, i myself messed up such things in the past as well and
modified my workflow to handle such things better.

> [...] and yet _you) persist in telling me that what I put in there was the
> "final" commit. [...]

Yes, because you pushed it out for others to see and it showed up in
linux-next?

Thanks,

Ingo

2011-05-16 10:08:37

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree

On Mon, May 16, 2011 at 11:40:56AM +0200, Ingo Molnar wrote:
> maintainers (and not assume lack of ack after 24 hours means acceptance), or

Wrong, 72 to 96 hours. Sunday to Wednesday/Thursday.

2011-05-16 11:07:25

by Ingo Molnar

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree


* Russell King - ARM Linux <[email protected]> wrote:

> On Mon, May 16, 2011 at 11:40:56AM +0200, Ingo Molnar wrote:
> > maintainers (and not assume lack of ack after 24 hours means acceptance), or
>
> Wrong, 72 to 96 hours. Sunday to Wednesday/Thursday.

Not that this is really material (the argument is pretty much the same even had
you waited 3 days), but you are already wrong about the 'Sunday' part, because
you posted it to lkml on *Monday* 13:27 GMT:

Message-ID: <[email protected]>
Mon, 9 May 2011 09:27:52 -0400

How hard can it be for you to look up the dates of the events before you accuse
others of not listening?

Then you committed/amdended it on Tuesday 7:20 GMT:

commit 3490f584b9ba5a0b6f63832fbc9c5ec72506697b
Author: Russell King <[email protected]>
AuthorDate: Sun May 8 18:55:19 2011 +0100
Commit: Russell King <[email protected]>
CommitDate: Tue May 10 08:20:54 2011 +0100

which is a mere 18 hours after it was mailed to lkml - and then you pushed it
out to linux-next some time after that, probably on the next day, Wednesday,
right?

It does not matter one little bit that you'd have been 'ready to rebase' once
more had some objection come in that short 2 days time window from Monday to
Wednesday, or any of the dates after that.

What i'm saying for the fourth time is that what you did here is not a proper
Git workflow: we only push bits out into permanent branches (and expose them to
conflicts, etc.) once they are final, and we only do that after making sure
that maintainers who maintain the trees of the affected files are fine with it
and make sure that there are no conflicts.

Or, failing all that, if you are such a superhero kernel hacker who never makes
any mistakes and never apologizes and can freely ignore well-established Git
workflow best practices you should *at least* make sure you do not mess up and
make sure the file you modify still builds on the architecture you are
modifying:

$ git checkout 3490f584b9ba # clocksource: convert x86 to generic i8253 clocksource
$ make defconfig
$ make -j2 bzImage

arch/x86/kernel/i8253.c: In function ‘init_pit_clocksource’:
arch/x86/kernel/i8253.c:133: error: implicit declaration of function ‘clocksource_pit_init’
make[2]: *** [arch/x86/kernel/i8253.o] Error 1
make[1]: *** [arch/x86/kernel] Error 2
make: *** [arch/x86] Error 2
make: *** Waiting for unfinished jobs....

Or is that too much to ask for?

Thanks,

Ingo

2011-05-16 11:38:23

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree

On Mon, May 16, 2011 at 01:06:52PM +0200, Ingo Molnar wrote:
>
> * Russell King - ARM Linux <[email protected]> wrote:
>
> > On Mon, May 16, 2011 at 11:40:56AM +0200, Ingo Molnar wrote:
> > > maintainers (and not assume lack of ack after 24 hours means acceptance), or
> >
> > Wrong, 72 to 96 hours. Sunday to Wednesday/Thursday.
>
> Not that this is really material (the argument is pretty much the same even had
> you waited 3 days), but you are already wrong about the 'Sunday' part, because
> you posted it to lkml on *Monday* 13:27 GMT:
>
> Message-ID: <[email protected]>
> Mon, 9 May 2011 09:27:52 -0400

Sigh. So you're only looking at the _second_ posting of them, not the first.
Here's the message, minus the patch.

| Date: Sun, 8 May 2011 19:24:07 +0100
| From: Russell King - ARM Linux <[email protected]>
| To: Ralf Baechle <[email protected]>, John Stultz <[email protected]>,
| "H. Peter Anvin" <[email protected]>, Ingo Molnar <[email protected]>
| Subject: i8253 clocksource consolidation
| Message-ID: <[email protected]>
|
| Ralf, John, Ingo, hpa,
|
| Below you'll find a work in progress patch. We have three i8253 PIT
| clocksource implementations in the kernel tree, which are all very
| similar. It seems pointless to have three copies of the code knocking
| about, so this patch creates a common version in drivers/clocksource.
|
| This is a combined patch; I have it broken out into a series locally,
| and follows on from other consolidation work which I've been doing with
| ARM clocksources.
|
| This patch build-tests cleanly on ARM for NetWinder (one of the footbridge
| platforms which uses an i8253.)
|
| So, do we think moving this to drivers/clocksource in this way is a
| good idea? Any other comments? If feedback is positive, I'll rebase
| them onto mainline, add the necessary cc's, and send them out properly.

To which I had this _single_ response:

| Date: Mon, 9 May 2011 14:07:35 +0100
| From: Ralf Baechle <[email protected]>
| To: Russell King - ARM Linux <[email protected]>
| Cc: John Stultz <[email protected]>, "H. Peter Anvin" <[email protected]>,
| Ingo Molnar <[email protected]>
|
| I like this patch; it's been long overdue and and while atm the patch
| does not seem to apply it seems to be mostly right.
|
| Ralf

And "it doesn't seem to apply" is precisely what I expected with the patch
because of the conflicts with _other_ work which was in progress, and as I
was asking in the original message about the _idea_, that is perfectly
acceptable.

> How hard can it be for you to look up the dates of the events before you
> accuse others of not listening?

I think I've just proven above that it was Sunday, not Monday.

> Then you committed/amdended it on Tuesday 7:20 GMT:
>
> commit 3490f584b9ba5a0b6f63832fbc9c5ec72506697b
> Author: Russell King <[email protected]>
> AuthorDate: Sun May 8 18:55:19 2011 +0100
> Commit: Russell King <[email protected]>
> CommitDate: Tue May 10 08:20:54 2011 +0100

Yes. Committed on _Sunday_ before I sent out the _first_ message which
I've included above. Then tweaked and after Ralf's response, the series
was then posted on _Monday_ in full. Then John responded with his ack,
which caused the amendment on _Tuesday_ morning.

And then _36_ further hours passed before the branch was merged into
for-next on _Wednesday_ _evening_. So, Sunday evening to Wednesday
evening. Three times 24 is 72 hours, which is what I corrected you to.

> which is a mere 18 hours after it was mailed to lkml - and then you pushed it
> out to linux-next some time after that, probably on the next day, Wednesday,
> right?

Are you _seriously_ trying to tell me that you have a problem with a commit
dated 18 hours after being mailed out? If so, you're being rediculous here.

Obviously I shouldn't have added John's ack, which was sent during Monday
nighttime to the commit so quickly, but instead waited a week before doing
so. Had I done that you wouldn't be complaining about "24 hours" or "18
hours".

> It does not matter one little bit that you'd have been 'ready to rebase' once
> more had some objection come in that short 2 days time window from Monday to
> Wednesday, or any of the dates after that.

Well, stop making such a big deal about "24 hours" or "18 hours" then,
but start realizing that the commit date is actually a total
_irrelevance_ to the time that it appeared in linux-next.

Your continual waving of that point, and reduction in time period, just
shows that you're trying to make this a _political_ issue, not a technical
or social one, which again is born out by the amount of people _you_ added
to this thread.

> What i'm saying for the fourth time is that what you did here is not a proper
> Git workflow: we only push bits out into permanent branches (and expose them to
> conflicts, etc.) once they are final, and we only do that after making sure
> that maintainers who maintain the trees of the affected files are fine with it
> and make sure that there are no conflicts.

Well, the fact that I messed up the function name was unfortunate and
should've been caught locally, which I appologize for. I would have
thought that much was obvious, but since you seem to believe that I
_intentionally_ broke the x86 build.

Anyway, the issue has been resolved _properly_ over the weekend, off-list,
between Thomas and myself, in a way that results in no conflicts being
exposed in any tree.

2011-05-16 18:47:31

by John Stultz

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree

On Mon, 2011-05-16 at 12:37 +0100, Russell King - ARM Linux wrote:
> Obviously I shouldn't have added John's ack, which was sent during Monday
> nighttime to the commit so quickly, but instead waited a week before doing
> so. Had I done that you wouldn't be complaining about "24 hours" or "18
> hours".

Yea, so its unfair for Russell to be taking all the heat on this one.

I acked it, and I should have caught this issue. Part of the reason for
that is because my base for testing things is usually linus' tree, and
not tip.

Also the clocksource_register_hz/khz patches in tip (which also includes
Russell's "clocksource name const" patch) have been taking forever to
get upstream. They are low priority non-critical cleanups at this point,
so that's ok, but the original pull request for those was in Feb. And
the slower those sorts of things take to get upstream, the bigger the
window is for a collision.

So I'll try to be more proactive about grabbing and queuing such fixes
against the appropriate tip branch so that we can catch these issues,
and ideally the related patches properly move through their proper
maintainer tree.

At the same time, having recently done some similar consolidation work
in the RTC tree, as well as in the past the various clocksource work,
splitting up such changes across a number of different maintainers can
be a painfully slow process. This case isn't so bad because it was only
three spots, but I can understand if you don't see your patches pulled
into various maintainer trees quickly, you might use your normal
upstream route to get them in. Maybe it was a little too quick this
time, but I'm somewhat sympathetic to it.


thanks
-john







2011-05-17 11:56:44

by Ingo Molnar

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree


* Russell King - ARM Linux <[email protected]> wrote:

> On Mon, May 16, 2011 at 01:06:52PM +0200, Ingo Molnar wrote:
> >
> > * Russell King - ARM Linux <[email protected]> wrote:
> >
> > > On Mon, May 16, 2011 at 11:40:56AM +0200, Ingo Molnar wrote:
> > > > maintainers (and not assume lack of ack after 24 hours means acceptance), or
> > >
> > > Wrong, 72 to 96 hours. Sunday to Wednesday/Thursday.
> >
> > Not that this is really material (the argument is pretty much the same even had
> > you waited 3 days), but you are already wrong about the 'Sunday' part, because
> > you posted it to lkml on *Monday* 13:27 GMT:
> >
> > Message-ID: <[email protected]>
> > Mon, 9 May 2011 09:27:52 -0400
>
> Sigh. So you're only looking at the _second_ posting of them, not the first.
>
> Here's the message, minus the patch.
>
> | Date: Sun, 8 May 2011 19:24:07 +0100
> | From: Russell King - ARM Linux <[email protected]>
> | To: Ralf Baechle <[email protected]>, John Stultz <[email protected]>,
> | "H. Peter Anvin" <[email protected]>, Ingo Molnar <[email protected]>
> | Subject: i8253 clocksource consolidation
> | Message-ID: <[email protected]>
> |
> | Ralf, John, Ingo, hpa,

I did not get that mail, at all. It's not in my spam mbox either.

This pretty much explains why you assumed us informed and should explain to you
why i was surprised by your way of handling the patch :-)

Thanks,

Ingo

2011-05-17 18:29:46

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: linux-next: manual merge of the tip tree with the arm tree

On Tue, May 17, 2011 at 01:56:14PM +0200, Ingo Molnar wrote:
> * Russell King - ARM Linux <[email protected]> wrote:
>
> > On Mon, May 16, 2011 at 01:06:52PM +0200, Ingo Molnar wrote:
> > >
> > > * Russell King - ARM Linux <[email protected]> wrote:
> > >
> > > > On Mon, May 16, 2011 at 11:40:56AM +0200, Ingo Molnar wrote:
> > > > > maintainers (and not assume lack of ack after 24 hours means acceptance), or
> > > >
> > > > Wrong, 72 to 96 hours. Sunday to Wednesday/Thursday.
> > >
> > > Not that this is really material (the argument is pretty much the same even had
> > > you waited 3 days), but you are already wrong about the 'Sunday' part, because
> > > you posted it to lkml on *Monday* 13:27 GMT:
> > >
> > > Message-ID: <[email protected]>
> > > Mon, 9 May 2011 09:27:52 -0400
> >
> > Sigh. So you're only looking at the _second_ posting of them, not the first.
> >
> > Here's the message, minus the patch.
> >
> > | Date: Sun, 8 May 2011 19:24:07 +0100
> > | From: Russell King - ARM Linux <[email protected]>
> > | To: Ralf Baechle <[email protected]>, John Stultz <[email protected]>,
> > | "H. Peter Anvin" <[email protected]>, Ingo Molnar <[email protected]>
> > | Subject: i8253 clocksource consolidation
> > | Message-ID: <[email protected]>
> > |
> > | Ralf, John, Ingo, hpa,
>
> I did not get that mail, at all. It's not in my spam mbox either.
>
> This pretty much explains why you assumed us informed and should explain to you
> why i was surprised by your way of handling the patch :-)

I never got a bounce either. Here's my outgoing MTA log lines for that
message:

2011-05-08 19:24:10 1QJ8eH-0004Rc-GR <= [email protected]
H=n2100.arm.linux.org.uk [2002:4e20:1eda:1:214:fdff:fe10:4f86]
I=[2002:4e20:1eda:1:a00:2bff:fe95:1d7b]:25 P=esmtpsa
X=TLSv1:AES256-SHA:256 A=cram:n2100.arm.linux.org.uk S=16326
[email protected]
T="i8253 clocksource consolidation" for [email protected]
[email protected] [email protected] [email protected]
2011-05-08 19:24:54 1QJ8eH-0004Rc-GR => [email protected] R=verp_dnslookup
T=verp_smtp S=16866 H=mx1.redhat.com [209.132.183.28] C="250 2.0.0
p48IOnv1008509 Message accepted for delivery"

So, mx1.redhat.com accepted it... In theory, whoever has access to
mx1.redhat.com's logs should be able to trace what happened to the
message...