Request for linux-next inclusion of the voyager tree

Request for linux-next inclusion of the voyager tree

[James Bottomley]

Could you add this (as a postmerge tree) somewhere after the x86 trees,
please (it depends on the auto-x86-next branch).

I'll be sending the pull request for it to Linus somewhere in the next
merge window (probably towards the end) and if he takes it, linux-next
inclusion for a small number of patches it contains will probably be a
recurring feature.

The tree is at:

git://git.kernel.org/pub/scm/linux/kernel/git/jejb/voyager-2.6.git#master

Thanks,

James

Re: Request for linux-next inclusion of the voyager tree

[Tony Breeds]

On Mon, Jun 08, 2009 at 11:10:23AM -0500, James Bottomley wrote:
> Could you add this (as a postmerge tree) somewhere after the x86 trees,
> please (it depends on the auto-x86-next branch).

I'm sure I've seen to somewhere before, but can you provide any relevant
details for building a voyager kernel (defconfig and crosscompile invocations
is probably all we need.)

Yours Tony

Re: Request for linux-next inclusion of the voyager tree

[James Bottomley]

On Tue, 2009-06-09 at 09:28 +1000, Tony Breeds wrote:
> On Mon, Jun 08, 2009 at 11:10:23AM -0500, James Bottomley wrote:
> > Could you add this (as a postmerge tree) somewhere after the x86 trees,
> > please (it depends on the auto-x86-next branch).
> 
> I'm sure I've seen to somewhere before, but can you provide any relevant
> details for building a voyager kernel (defconfig and crosscompile invocations
> is probably all we need.)

It just uses a standard x86 build environment, so it's all x86 build
tools.  You just answer Y to the "Voyager (NCR)" question; you have to
have also answered Y to "Support non-standard 32-bit SMP architectures".
The resulting kernel should be bootable on any x86 system (well, that
the config supports).

James

Re: Request for linux-next inclusion of the voyager tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]

Hi James,

On Mon, 08 Jun 2009 11:10:23 -0500 James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
>
> Could you add this (as a postmerge tree) somewhere after the x86 trees,
> please (it depends on the auto-x86-next branch).
> 
> I'll be sending the pull request for it to Linus somewhere in the next
> merge window (probably towards the end) and if he takes it, linux-next
> inclusion for a small number of patches it contains will probably be a
> recurring feature.
> 
> The tree is at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/jejb/voyager-2.6.git#master

I added it near the end for today (but see my other emails for the
results).  I will move it up just after the x86 tree tomorrow and hope
for better results.

(You have seen the following before, this is just for "form")

What I tell everyone: all patches/commits in the tree/series must
have been:

	posted to a relevant mailing list
	reviewed
	unit tested
	destined for the next merge window (or the current release)

*before* they are included.  The linux-next tree is for integration
testing and to lower the impact of conflicts between subsystems in the
next merge window.

Basically, this should be just what you would send to Linus (or ask him
to fetch).  It is allowed to be rebased if you deem it necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Request for linux-next inclusion of the voyager tree

[James Bottomley]

On Tue, 2009-06-09 at 19:45 +1000, Stephen Rothwell wrote:
> Hi James,
> 
> On Mon, 08 Jun 2009 11:10:23 -0500 James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> >
> > Could you add this (as a postmerge tree) somewhere after the x86 trees,
> > please (it depends on the auto-x86-next branch).
> > 
> > I'll be sending the pull request for it to Linus somewhere in the next
> > merge window (probably towards the end) and if he takes it, linux-next
> > inclusion for a small number of patches it contains will probably be a
> > recurring feature.
> > 
> > The tree is at:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/jejb/voyager-2.6.git#master
> 
> I added it near the end for today (but see my other emails for the
> results).  I will move it up just after the x86 tree tomorrow and hope
> for better results.
> 
> (You have seen the following before, this is just for "form")
> 
> What I tell everyone: all patches/commits in the tree/series must
> have been:
> 
> 	posted to a relevant mailing list
> 	reviewed
> 	unit tested
> 	destined for the next merge window (or the current release)

Check on all of those.

> *before* they are included.  The linux-next tree is for integration
> testing and to lower the impact of conflicts between subsystems in the
> next merge window.
> 
> Basically, this should be just what you would send to Linus (or ask him
> to fetch).  It is allowed to be rebased if you deem it necessary.

Right ... what I really need is wider build testing of the pieces I
can't check easily here (especially random configuration testing).

I'm just trying to set up a build env for the 64 bit failure now.

Thanks,

James

Re: Request for linux-next inclusion of the voyager tree

[Ingo Molnar]

* James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> Could you add this (as a postmerge tree) somewhere after the x86 
> trees, please (it depends on the auto-x86-next branch).
> 
> I'll be sending the pull request for it to Linus somewhere in the 
> next merge window (probably towards the end) and if he takes it, 
> linux-next inclusion for a small number of patches it contains 
> will probably be a recurring feature.

Sigh.

This code has been NAK-ed by the x86 maintainers:

 - Due to the absurd irrelevance of Voyager/x86/Linux hardware

 - Due to the thousands of lines of of code it adds to arch/x86
   to support a 486/P5 era piece of hardware

 - and due to its negative track record of:

    v2.6.27.0:   Voyager was broken - it did not even build. (!)
    v2.6.28.0:   Voyager was broken - it did not even build. (!)
    v2.6.29-rc5: Voyager was broken - it did not even build. (!)

   [ ... which was the point when we yanked it from the x86 devel 
     tree. Then you suddenly found interest in it again. But it was 
     too little, too late. Voyager is irrelevant and we've really 
     got better things to do than to worry about ancient, completely 
     irrelevant hardware. ]

And you were very much aware of its controversial nature and you 
were aware of the NAK, still you sent this mail to Stephen without 
Cc:-ing the x86 maintainers or without Cc:-ing lkml.

You did this on one of the last days of the development window - 
generally the most impossibly busy days for upstream maintainers who 
prepare for the next merge window.

I've Cc:-ed Linus - he might want to overrule our judgement and pull 
this from you directly or tell us to pull it - but this should be 
done above board, not below the radar on the last day of the 
development window.

I made it quite clear to you why i object to this code, didnt I? See 
the (long) thread at:

    http://lkml.org/lkml/2009/4/15/299

James, it also would have been really honest from you to Cc: us to 
this mail. I am not pushing SCSI changes into linux-next either, 
against your NAKs, behind your back. I'd have expected the same of 
you.

So i strongly object against this tree being included in linux-next.

Thanks,

	Ingo

Re: Request for linux-next inclusion of the voyager tree

[James Bottomley]

On Tue, 2009-06-09 at 22:21 +0200, Ingo Molnar wrote:
> * James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> 
> > Could you add this (as a postmerge tree) somewhere after the x86 
> > trees, please (it depends on the auto-x86-next branch).
> > 
> > I'll be sending the pull request for it to Linus somewhere in the 
> > next merge window (probably towards the end) and if he takes it, 
> > linux-next inclusion for a small number of patches it contains 
> > will probably be a recurring feature.
> 
> Sigh.
> 
> This code has been NAK-ed by the x86 maintainers:
> 
>  - Due to the absurd irrelevance of Voyager/x86/Linux hardware
> 
>  - Due to the thousands of lines of of code it adds to arch/x86
>    to support a 486/P5 era piece of hardware
> 
>  - and due to its negative track record of:
> 
>     v2.6.27.0:   Voyager was broken - it did not even build. (!)
>     v2.6.28.0:   Voyager was broken - it did not even build. (!)
>     v2.6.29-rc5: Voyager was broken - it did not even build. (!)
> 
>    [ ... which was the point when we yanked it from the x86 devel 
>      tree. Then you suddenly found interest in it again. But it was 
>      too little, too late. Voyager is irrelevant and we've really 
>      got better things to do than to worry about ancient, completely 
>      irrelevant hardware. ]
> 
> And you were very much aware of its controversial nature and you 
> were aware of the NAK, still you sent this mail to Stephen without 
> Cc:-ing the x86 maintainers or without Cc:-ing lkml.
> 
> You did this on one of the last days of the development window - 
> generally the most impossibly busy days for upstream maintainers who 
> prepare for the next merge window.
> 
> I've Cc:-ed Linus - he might want to overrule our judgement and pull 
> this from you directly or tell us to pull it - but this should be 
> done above board, not below the radar on the last day of the 
> development window.
> 
> I made it quite clear to you why i object to this code, didnt I? See 
> the (long) thread at:
> 
>     http://lkml.org/lkml/2009/4/15/299
> 
> James, it also would have been really honest from you to Cc: us to 
> this mail. I am not pushing SCSI changes into linux-next either, 
> against your NAKs, behind your back. I'd have expected the same of 
> you.
> 
> So i strongly object against this tree being included in linux-next.

Your actual statement was that I should take this to Linus, which I
will.  However, procedurally, I need it through linux-next first before
I can send a pull request, otherwise I'm asking for code which hasn't
been through the usual integration testing to be included, which is
dangerous.

The whole purpose of linux-next is integration testing for code on
upstream track ... whether it actually gets upstream is a different
argument.  You've already made your personal objections to voyager
clear ... you'll likely do so again at the pull request.  However, have
the courtesy to follow the usual process instead of trying to politicise
linux-next into being a pre rejection gate for code you don't like.

James

Re: Request for linux-next inclusion of the voyager tree

[Ingo Molnar]

* James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> On Tue, 2009-06-09 at 22:21 +0200, Ingo Molnar wrote:
> > * James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> > 
> > > Could you add this (as a postmerge tree) somewhere after the x86 
> > > trees, please (it depends on the auto-x86-next branch).
> > > 
> > > I'll be sending the pull request for it to Linus somewhere in the 
> > > next merge window (probably towards the end) and if he takes it, 
> > > linux-next inclusion for a small number of patches it contains 
> > > will probably be a recurring feature.
> > 
> > Sigh.
> > 
> > This code has been NAK-ed by the x86 maintainers:
> > 
> >  - Due to the absurd irrelevance of Voyager/x86/Linux hardware
> > 
> >  - Due to the thousands of lines of of code it adds to arch/x86
> >    to support a 486/P5 era piece of hardware
> > 
> >  - and due to its negative track record of:
> > 
> >     v2.6.27.0:   Voyager was broken - it did not even build. (!)
> >     v2.6.28.0:   Voyager was broken - it did not even build. (!)
> >     v2.6.29-rc5: Voyager was broken - it did not even build. (!)
> > 
> >    [ ... which was the point when we yanked it from the x86 devel 
> >      tree. Then you suddenly found interest in it again. But it was 
> >      too little, too late. Voyager is irrelevant and we've really 
> >      got better things to do than to worry about ancient, completely 
> >      irrelevant hardware. ]
> > 
> > And you were very much aware of its controversial nature and you 
> > were aware of the NAK, still you sent this mail to Stephen without 
> > Cc:-ing the x86 maintainers or without Cc:-ing lkml.
> > 
> > You did this on one of the last days of the development window - 
> > generally the most impossibly busy days for upstream maintainers who 
> > prepare for the next merge window.
> > 
> > I've Cc:-ed Linus - he might want to overrule our judgement and pull 
> > this from you directly or tell us to pull it - but this should be 
> > done above board, not below the radar on the last day of the 
> > development window.
> > 
> > I made it quite clear to you why i object to this code, didnt I? See 
> > the (long) thread at:
> > 
> >     http://lkml.org/lkml/2009/4/15/299
> > 
> > James, it also would have been really honest from you to Cc: us to 
> > this mail. I am not pushing SCSI changes into linux-next either, 
> > against your NAKs, behind your back. I'd have expected the same of 
> > you.
> > 
> > So i strongly object against this tree being included in linux-next.
> 
> Your actual statement was that I should take this to Linus, which 
> I will. [...]

My statement was:

" [...] Meanwhile if James does not want to maintain it out of tree
  he can ask Linus to overrule our NAK, but i'd like to ask him to
  quote this NAK paragraph from me in his mail to Linus. "

  http://lkml.org/lkml/2009/4/19/181

> However, procedurally, I need it through linux-next first before I 
> can send a pull request, otherwise I'm asking for code which 
> hasn't been through the usual integration testing to be included, 
> which is dangerous.

How about the old fashioned way of interpreting my words directly, 
not twisting them like a laywer? Wasnt it clear from my objections 
that i ... object?

Firstly, it is a misunderstanding (at best) on your part that 
patches that are sent to Linus must "procedurally" go through 
linux-next. The stuff from maintainers, the stuff that is not 
controversial, that bit should go into linux-next. The 90% stuff.

The 10% stuff that is disputed and controversial (like kmemcheck 
which was excluded from linux-next for a few weeks in this cycle due 
to an objection, or like perfcounters which was disputed and is thus 
not included) it doesnt and shouldnt go into linux-next. It can be 
sent to Linus on its own right (declaring its controversial nature), 
or if you can convince Andrew to put it into linux-mm.

Does this make it harder to merge controversial bits? Absolutely. 
That's one of the points really. Does this make it impossible to 
merge controversial bits, if the maintainer is on crack? Not at all 
- it just raises the bar.

linux-next is not there to include all random stuff intended by all 
kernel developers on the planet to be sent to Linus, IMHO. It is 
there to _help Linus_ to merge the 90% bulk more easily, to handle 
conflicts between the routine stuff that nobody objects to and to 
test them together. It is there to free up time to handle the _10%_.

And i think linux-next is currently largely fulfilling that purpose 
and role of detecting and excluding/resolving conflicts early on - 
both technical conflicts and conceptual/maintenance conflicts.

If there are long arguments about linux-next that is _bad_. It 
should be a routine medium of friendly conflict resolution and "oh, 
I didnt really mean to push that patch, sorry" type of 
cross-maintainer realizations and cooperation - heavy arguments are 
for other forums such as lkml.

Read the _name_ of linux-next. It's the stuff that is intended to be 
in Linux by maintainers in the next merge window, on a best effort 
basis. Surprises, trickeries and friction are bad.

	Ingo

Re: Request for linux-next inclusion of the voyager tree

[Alan Cox]

> This code has been NAK-ed by the x86 maintainers:
> 
>  - Due to the absurd irrelevance of Voyager/x86/Linux hardware
> 
>  - Due to the thousands of lines of of code it adds to arch/x86
>    to support a 486/P5 era piece of hardware
> 
>  - and due to its negative track record of:
> 
>     v2.6.27.0:   Voyager was broken - it did not even build. (!)
>     v2.6.28.0:   Voyager was broken - it did not even build. (!)
>     v2.6.29-rc5: Voyager was broken - it did not even build. (!)

So Ingo you are arguing "It didn't work in some releases so we want to
make it continue not to work by trying to keep the fixes out" ?

Sounds pretty dumb to me.

Getting this in -next has to be a good thing given the fact you and James
don't agree on it, because thats the one place any merge collisions can
be fixed and the combined bleeding edge x86 and voyager mods can be seen
together and refined.

I don't think ignoring voyager is useful - separating it out more
probably is. Not-quite-PC hardware is on the way back, and OLPC is just
the beginning so we *need* the mechanisms to support such platforms.

Alan

Re: Request for linux-next inclusion of the voyager tree

[Ingo Molnar]

* Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > This code has been NAK-ed by the x86 maintainers:
> > 
> >  - Due to the absurd irrelevance of Voyager/x86/Linux hardware
> > 
> >  - Due to the thousands of lines of of code it adds to arch/x86
> >    to support a 486/P5 era piece of hardware
> > 
> >  - and due to its negative track record of:
> > 
> >     v2.6.27.0:   Voyager was broken - it did not even build. (!)
> >     v2.6.28.0:   Voyager was broken - it did not even build. (!)
> >     v2.6.29-rc5: Voyager was broken - it did not even build. (!)
> 
> So Ingo you are arguing "It didn't work in some releases so we 
> want to make it continue not to work by trying to keep the fixes 
> out" ?

No. This code is not in Linux right now, and that i see no reason to 
put it back, for the (many) reasons outlined.

	Ingo

Re: Request for linux-next inclusion of the voyager tree

[Linus Torvalds]

On Wed, 10 Jun 2009, Ingo Molnar wrote:

> 
> * Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> > > This code has been NAK-ed by the x86 maintainers:
> > > 
> > >  - Due to the absurd irrelevance of Voyager/x86/Linux hardware
> > > 
> > >  - Due to the thousands of lines of of code it adds to arch/x86
> > >    to support a 486/P5 era piece of hardware
> > > 
> > >  - and due to its negative track record of:
> > > 
> > >     v2.6.27.0:   Voyager was broken - it did not even build. (!)
> > >     v2.6.28.0:   Voyager was broken - it did not even build. (!)
> > >     v2.6.29-rc5: Voyager was broken - it did not even build. (!)
> > 
> > So Ingo you are arguing "It didn't work in some releases so we 
> > want to make it continue not to work by trying to keep the fixes 
> > out" ?
> 
> No. This code is not in Linux right now, and that i see no reason to 
> put it back, for the (many) reasons outlined.

Ingo, "absurd irrelevance" is not a reason. If it was, we'd lose about 
half our filesystems etc.

Neither is "thousands of lines of code", or "it hasn't always worked". 
Again, if it was, then we'd have to get rid of just about all drivers out 
there.

So give some real reasons. "It's a maintenance nightmare because it does 
xyz" might be a reason. But then we really need to see the "xyz" part too.

Alan is definitely right that we're likely to see more of the "non-PC" 
platforms as x86 tries to do embedded.

			Linus

Re: Request for linux-next inclusion of the voyager tree

[Ingo Molnar]

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

> On Wed, 10 Jun 2009, Ingo Molnar wrote:
> 
> > 
> > * Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > 
> > > > This code has been NAK-ed by the x86 maintainers:
> > > > 
> > > >  - Due to the absurd irrelevance of Voyager/x86/Linux hardware
> > > > 
> > > >  - Due to the thousands of lines of of code it adds to arch/x86
> > > >    to support a 486/P5 era piece of hardware
> > > > 
> > > >  - and due to its negative track record of:
> > > > 
> > > >     v2.6.27.0:   Voyager was broken - it did not even build. (!)
> > > >     v2.6.28.0:   Voyager was broken - it did not even build. (!)
> > > >     v2.6.29-rc5: Voyager was broken - it did not even build. (!)
> > > 
> > > So Ingo you are arguing "It didn't work in some releases so we 
> > > want to make it continue not to work by trying to keep the fixes 
> > > out" ?
> > 
> > No. This code is not in Linux right now, and that i see no reason to 
> > put it back, for the (many) reasons outlined.
> 
> Ingo, "absurd irrelevance" is not a reason. If it was, we'd lose 
> about half our filesystems etc.
> 
> Neither is "thousands of lines of code", or "it hasn't always 
> worked". Again, if it was, then we'd have to get rid of just about 
> all drivers out there.
> 
> So give some real reasons. "It's a maintenance nightmare because 
> it does xyz" might be a reason. But then we really need to see the 
> "xyz" part too.
> 
> Alan is definitely right that we're likely to see more of the 
> "non-PC" platforms as x86 tries to do embedded.

That is true, and the whole painful conversion from the build-time 
32-bit sub-arch code to the unified runtime quirk code (which is 
really 'non-PC platform driver' kind of thing) that i did a few 
months ago was partly about that.

I dont dispute that aspect - in fact we merged a rare subarch as 
well.

But Voyager has been the most painful subarchitecture by far - for 
an extended period of time the code didnt even build in its 
defconfig - and it is also the most useless one as well. So it was 
just a token really.

The only action i got from James was when _I_ implemented the 
proper, runtime subarch mechanism and when we actually _removed_ the 
voyager code. Before it James resisted such changes, see this thread 
almost a year ago, in the Nth "voyager breaks the build" discussion, 
where i suggested:

  " btw., any chance to turn it into a quirk space thing? "

  http://lkml.org/lkml/2008/4/21/441

I got no action from James for my technical requests. Right now i 
dont see any guarantee that this wont be repeated, once the code is 
upstream again after meeting some threshold minimally.

Voyager was a painful experience to all x86 maintainers and i'd 
expect pushers and supporters of rarely used code to do such work, 
not maintainers.

Are the quality thresholds i outlined in the previous thread(s) 
unreasonable? They were:

 " If the code is absolutely trouble-free out of tree, for an
   equivalent amount of time (3 kernel releases or so), and gathers
   users/testers/etc., then we might add it, after a thorough
   technical review. "

  http://lkml.org/lkml/2009/4/19/181

Given that there are only two known boxes left (both James's, the 
other one went missing in action somewhere in Brasil), the 'gather 
testers' bit is unreasonable i guess. 'Prove you can stay trouble 
free' is more realistic. Dunno.

See for example what happened in linux-next today: Voyager broke x86 
and didnt even build, and Stephen had to keep it out of today's 
linux-next build.

	Ingo

Re: Request for linux-next inclusion of the voyager tree

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> See for example what happened in linux-next today: Voyager broke 
> x86 and didnt even build, and Stephen had to keep it out of 
> today's linux-next build.

I also took a look at that tree:

  git://git.kernel.org/pub/scm/linux/kernel/git/jejb/voyager-2.6.git master

A couple of quick, mostly high-level observations:

1)

The Voyager bits got rebased _yesterday_. _All_ the commits:

 commit 0ff51d1467af91bca4210b0d09372b6e7ded7524
 Author:     James Bottomley <James.Bottomley@HansenPartnership.com>
 AuthorDate: Mon Feb 23 15:02:04 2009 -0600
 Commit:     James Bottomley <James.Bottomley@HansenPartnership.com>
 CommitDate: Mon Jun 8 09:49:08 2009 -0500

I told James before that he should _not_ rebase:

   http://lkml.org/lkml/2009/2/1/76

In no uncertain terms. He completely ignored me and he messes up 
trees that way again. Now i should pull such a damaged tree while 
all the other x86 contributors we pull from do a thorough job?

( And note that the above link points to _another_ similar incident,
  where James rebased a tree and broke the build. It is a pattern of 
  behavior. )

2)

The tree structure is unacceptable:

 a087b5f: [VOYAGER] x86: add prefill_possible_map to x86_quirks
 f5ef55a: [VOYAGER] x86/mca: make mca_nmi_hook external
 55c8430: [VOYAGER] x86: add {safe,hard}_smp_processor_id to smp_ops
 fd1ab45: Revert "MAINTAINERS - Remove x86/Voyager no longer in tree"
 0ff51d1: Revert "x86: remove the Voyager 32-bit subarch"

That is crap. It starts with a huge 'revert' - which of course 
breaks the tree and needs 16 commits to bring back into action.

It might be OK to _revive_ the source code that way privately - but 
the history completely incorrectly suggests a 'revert'. We revert 
buggy commits.

What we want here is a clean submission, and a tidy, well 
constructed, non-rebased, well-tested Git tree. Or plain email 
submissions initially, because frankly i shouldnt Git-pull such a 
messy tree.

3)

The comments. For example the revert:

 From 0ff51d1467af91bca4210b0d09372b6e7ded7524 Mon Sep 17 00:00:00 2001
 From: James Bottomley <James.Bottomley@HansenPartnership.com>
 Date: Mon, 23 Feb 2009 15:02:04 -0600
 Subject: [PATCH] Revert "x86: remove the Voyager 32-bit subarch"

 This reverts commit 965c7ecaf2e2b083d711a01ab33735a4bdeee1a4.
 ---

That is not how we do reverts! We always give a reason for a revert. 

4)

Small details like standard commit titles in the x86 tree. It 
shouldnt be:

 [VOYAGER] x86: eliminate subarchitecture file do_timer.h

It should be:

 x86: Voyager: Eliminate subarchitecture file do_timer.h

Note the different order and different capitalization.

5)

And as i requested it in an earier thread, quirky platforms should 
be supported via a _single_ quirks file.

That makes it easy to handle and makes it easy to ignore as well. 
It's basically a "weird hardware driver". We have examples of that, 
for example arch/x86/kernel/visws_quirks.c. Voyager will be larger 
than that but it's OK - it's not like it will undergo massive 
development.

Putting it back into arch/x86/mach-voyager/ again reintroduces the 
subarch directory structure we got rid of.

6)

This ordering of subsequent patches:

5c173bb: [VOYAGER] x86: eliminate subarchitecture file do_timer.h
18d3288: [VOYAGER] x86: eliminate subarchitecture file entry_arch.h
42c7289: [VOYAGER] x86: eliminate subarchitecture file setup_arch.h

is totally wrong - it removes something that should not have been 
put there in the first place.

If then Voyager should be added as a clean series of patches: first 
the generic patches that introduce infrastructure changes (to 
x86_quirks and smp_ops, etc.), then a single final patch that adds 
in the Voyager quirks platform driver.

And we've been through similar excercises multiple times before. 

All in one, this tree is quite poor and needs a lot of work.

	Ingo

Re: Request for linux-next inclusion of the voyager tree

[James Bottomley]

On Wed, 2009-06-10 at 03:00 +0200, Ingo Molnar wrote:
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> > See for example what happened in linux-next today: Voyager broke 
> > x86 and didnt even build, and Stephen had to keep it out of 
> > today's linux-next build.
> 
> I also took a look at that tree:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/jejb/voyager-2.6.git master
> 
> A couple of quick, mostly high-level observations:
> 
> 1)
> 
> The Voyager bits got rebased _yesterday_. _All_ the commits:
> 
>  commit 0ff51d1467af91bca4210b0d09372b6e7ded7524
>  Author:     James Bottomley <James.Bottomley@HansenPartnership.com>
>  AuthorDate: Mon Feb 23 15:02:04 2009 -0600
>  Commit:     James Bottomley <James.Bottomley@HansenPartnership.com>
>  CommitDate: Mon Jun 8 09:49:08 2009 -0500
> 
> I told James before that he should _not_ rebase:
> 
>    http://lkml.org/lkml/2009/2/1/76
> 
> In no uncertain terms. He completely ignored me and he messes up 
> trees that way again. Now i should pull such a damaged tree while 
> all the other x86 contributors we pull from do a thorough job?
> 
> ( And note that the above link points to _another_ similar incident,
>   where James rebased a tree and broke the build. It is a pattern of 
>   behavior. )

OK, so we disagree on this.  But think what I'm having to do ... I need
a patch set to apply on top of your somewhat volatile changes to x86 ...
I'm using git to manage that patch set and I have no downstream users of
the voyager tree, so rebasing is a good management tool.

If I go the merge point route, I get a tree with more non trivial merge
points than commits, so it becomes incredibly difficult for anyone to
follow what's going on.  I also can no longer use git-email to send my
patch series anywhere.

So I beg to differ and conclude the rebase maintenance of this patch
series is the logically correct thing for me to do at the current time.

> 2)
> 
> The tree structure is unacceptable:
> 
>  a087b5f: [VOYAGER] x86: add prefill_possible_map to x86_quirks
>  f5ef55a: [VOYAGER] x86/mca: make mca_nmi_hook external
>  55c8430: [VOYAGER] x86: add {safe,hard}_smp_processor_id to smp_ops
>  fd1ab45: Revert "MAINTAINERS - Remove x86/Voyager no loner in tree"
>  0ff51d1: Revert "x86: remove the Voyager 32-bit subarch"
> 
> That is crap. It starts with a huge 'revert' - which of course 
> breaks the tree and needs 16 commits to bring back into action.

The revert doesn't actually break anything ... you saw to that yourself
by marking CONFIG_VOYAGER BROKEN and by pulling all it's files out of
the central makefile about two weeks before you actually removed the
code.

> It might be OK to _revive_ the source code that way privately - but 
> the history completely incorrectly suggests a 'revert'. We revert 
> buggy commits.
> 
> What we want here is a clean submission, and a tidy, well 
> constructed, non-rebased, well-tested Git tree. Or plain email 
> submissions initially, because frankly i shouldnt Git-pull such a 
> messy tree.
> 
> 3)
> 
> The comments. For example the revert:
> 
>  From 0ff51d1467af91bca4210b0d09372b6e7ded7524 Mon Sep 17 00:00:00 2001
>  From: James Bottomley <James.Bottomley@HansenPartnership.com>
>  Date: Mon, 23 Feb 2009 15:02:04 -0600
>  Subject: [PATCH] Revert "x86: remove the Voyager 32-bit subarch"
> 
>  This reverts commit 965c7ecaf2e2b083d711a01ab33735a4bdeee1a4.
>  ---
> 
> That is not how we do reverts! We always give a reason for a revert. 

Well "put it back again" is the reason ... however, it did seem to be
obvious in the Subject line ... but I can add an additional reason if
you insist ... what apart from "revive voyager" do you want it to say?

> 4)
> 
> Small details like standard commit titles in the x86 tree. It 
> shouldnt be:
> 
>  [VOYAGER] x86: eliminate subarchitecture file do_timer.h
> 
> It should be:
> 
>  x86: Voyager: Eliminate subarchitecture file do_timer.h
> 
> Note the different order and different capitalization.

Well [TREE] is a standard.  I've been using it for my trees for ages.
If you were to take voyager through your tree, then it would follow your
standards ... but you're forcing me to send it through my own, so it
follows mine.

> 5)
> 
> And as i requested it in an earier thread, quirky platforms should 
> be supported via a _single_ quirks file.
> 
> That makes it easy to handle and makes it easy to ignore as well. 
> It's basically a "weird hardware driver". We have examples of that, 
> for example arch/x86/kernel/visws_quirks.c. Voyager will be larger 
> than that but it's OK - it's not like it will undergo massive 
> development.
> 
> Putting it back into arch/x86/mach-voyager/ again reintroduces the 
> subarch directory structure we got rid of.

It's just a convenient place to put the voyager stuff ... I can move it
somewhere, but code motion just makes the diffstats explode for no good
reason and makes it hard to judge the actual changes.

I'd argue having voyager in a separate directory which is never built
unless you enable the config option is further out of sight and out of
mind.

But this is a tiny detail ... the three voyager files can go anywhere,
I'm not wedded to their actual location.

> 6)
> 
> This ordering of subsequent patches:
> 
> 5c173bb: [VOYAGER] x86: eliminate subarchitecture file do_timer.h
> 18d3288: [VOYAGER] x86: eliminate subarchitecture file entry_arch.h
> 42c7289: [VOYAGER] x86: eliminate subarchitecture file setup_arch.h
> 
> is totally wrong - it removes something that should not have been 
> put there in the first place.

I think you'll find do_timer.h, entry_arch.h and setup_arch.h still
exist in the current tree.

What these patches are actually doing is completing your partial
subarchitecture elimination by folding the splits back into the mainline
code and removing the header files that enabled them.

Out of all the patches, I would have thought you'd like these the best,
since they do finally eliminate the rest of the subarchitecture stuff
which still exists as trace elements in arch/x86 today.

> If then Voyager should be added as a clean series of patches: first 
> the generic patches that introduce infrastructure changes (to 
> x86_quirks and smp_ops, etc.), then a single final patch that adds 
> in the Voyager quirks platform driver.
> 
> And we've been through similar excercises multiple times before. 
> 
> All in one, this tree is quite poor and needs a lot of work.

So basically, if the only problems you have are commit log naming and
patch sequencing, you find the actual code technically fine?

James

Re: Request for linux-next inclusion of the voyager tree

[Linus Torvalds]

On Wed, 10 Jun 2009, James Bottomley wrote:
> 
> If I go the merge point route, I get a tree with more non trivial merge
> points than commits, so it becomes incredibly difficult for anyone to
> follow what's going on.  I also can no longer use git-email to send my
> patch series anywhere.

Why do you need to merge at all?

Do you get constant conflicts? If so, _that_ is likely the problem.

The rule should be that you should _never_ need to merge from Ingo or me, 
and things should be smooth. And if there are too frequent conflicts for 
that to work, then the rule should be that things get cleaned up so that 
those conflicts don't happen!

Constant rebasing, or constant merging, is a symptom of something simply 
not working. It's probably why Ingo is fed up with Voyager in the first 
place. Can you guys just start telling me what the actual maintenance 
problem is? Where do you actually step on each others toes? What are the 
conflicts? Why is Voyager so special?

			Linus

Re: Request for linux-next inclusion of the voyager tree

[James Bottomley]

On Wed, 2009-06-10 at 08:20 -0700, Linus Torvalds wrote:
> 
> On Wed, 10 Jun 2009, James Bottomley wrote:
> > 
> > If I go the merge point route, I get a tree with more non trivial merge
> > points than commits, so it becomes incredibly difficult for anyone to
> > follow what's going on.  I also can no longer use git-email to send my
> > patch series anywhere.
> 
> Why do you need to merge at all?
> 
> Do you get constant conflicts? If so, _that_ is likely the problem.

Pretty much, yes.  The problem isn't in the voyager code, it's in the
residual subarchitecture clean up.  As the x86 tree evolves, that's what
keeps conflicting mainly because adjacent areas get altered.  Once
that's upstream, the voyager piece should be a smooth ride.

The other piece that gets conflicts is the fact that voyager needs two
additions to the quirks and smp ops pointer indirections to cope with
some of it's unique features ... these have all been over linux-scsi
several times, with feedback from the affected users.  However, while
they remain out of tree, there another source of conflict.

> The rule should be that you should _never_ need to merge from Ingo or me, 
> and things should be smooth. And if there are too frequent conflicts for 
> that to work, then the rule should be that things get cleaned up so that 
> those conflicts don't happen!
> 
> Constant rebasing, or constant merging, is a symptom of something simply 
> not working. It's probably why Ingo is fed up with Voyager in the first 
> place. Can you guys just start telling me what the actual maintenance 
> problem is? Where do you actually step on each others toes? What are the 
> conflicts? Why is Voyager so special?

Once the last piece of the subarchitecture is removed and the pieces
needed to support voyager are in, that should be it.

The two additional pieces are:

1. prefill_possible_map initialisation ... voyager boots with a non apic
config, so it needs to prefill the possible map from its VIC CPU
registers
2. indirection of hard_smp_processor_id() ... voyager doesn't have an
apic, so it has to read a special VIC register instead.

There's also an addition to the boot code to make voyager detection
dynamic instead of static based on subarchitecture.

James

Re: Request for linux-next inclusion of the voyager tree

[Linus Torvalds]

On Wed, 10 Jun 2009, James Bottomley wrote:
> 
> Pretty much, yes.  The problem isn't in the voyager code, it's in the
> residual subarchitecture clean up.  As the x86 tree evolves, that's what
> keeps conflicting mainly because adjacent areas get altered.  Once
> that's upstream, the voyager piece should be a smooth ride.

Quite frankly, in that case I think that in order to get Voyager merged, 
we should just get the subarchitecture code cleaned up _first_.

The thing is, I do agree with Ingo that Voyager is not _nearly_ important 
enough to be rammed through in some ugly manner. And if the plan is to get 
it all done cleanly in the end anyway, then there is certainly no hurry 
what-so-ever in getting Voyager merged _before_ it's possible to merge it 
cleanly.

			Linus

Re: Request for linux-next inclusion of the voyager tree

[Ingo Molnar]

* James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> On Wed, 2009-06-10 at 08:20 -0700, Linus Torvalds wrote:
> > 
> > On Wed, 10 Jun 2009, James Bottomley wrote:
> > > 
> > > If I go the merge point route, I get a tree with more non trivial merge
> > > points than commits, so it becomes incredibly difficult for anyone to
> > > follow what's going on.  I also can no longer use git-email to send my
> > > patch series anywhere.
> > 
> > Why do you need to merge at all?
> > 
> > Do you get constant conflicts? If so, _that_ is likely the problem.
> 
> Pretty much, yes.  The problem isn't in the voyager code, it's in 
> the residual subarchitecture clean up.  As the x86 tree evolves, 
> that's what keeps conflicting mainly because adjacent areas get 
> altered.  Once that's upstream, the voyager piece should be a 
> smooth ride.

The problem causing problems with Voyager are long-lived, and it 
goes well before the time we got rid of subarchitectures and added 
the x86-quirks mechanism to express "non PC" properties cleanly.

The problem was simply that the old Voyager code had an unreasonably 
large cross-section to the x86 code originally, under the old 32-bit 
subarch code concepts. (which, in hindsight, should never have been 
merged in that form.)

We had this mach-default/mach-voyager splitup and the asm headers 
were duplicated as well, creating a large, hard to maintain 
build-time kludge of subarchitecture code. Voyager broke again and 
again in the past ~2 years because its assumptions were hidden in 
build-time wrappers and were not visible to developers changing the 
generic code.

The other key problem was that for a long time you runtime tested 
Voyager only in late -rc's (i remember an -rc8 Voyager patchset from 
you - for issues that were introduced months before that point!) - 
which gave us little oppotunity to do things properly.

The thing is, you are the only Linux person known to us on this 
planet who has access to the hardware and runs recent Linux on it 
and thus _can_ test Voyager/Linux, so unless we reduce the cross 
section to the rest of the code it will frequently 'break' - because 
no-one can actually test its assumptions.

So unless someone else mails us that they boot recent kernels and 
care about Voyager, this is basically a one-person show for your 
sake only. So the requirements are that it 1) must not hurt any 
other code 2) the introduction must be done so cleanly that everyone 
else benefits too indirectly, by virtue of an improved 
infrastructure.

> Once the last piece of the subarchitecture is removed and the 
> pieces needed to support voyager are in, that should be it. [...]

Ok, let me make this abundantly clear:

We _already_ removed the 32-bit subarchitecture code. We converted 
summit, numaq and visws over to the x86-quirks mechanism.

So your "once the last piece of the subarchitecture is removed" 
statement makes no sense. We dont have subarchitecture code left and 
we are not adding it back.

	Ingo

Re: Request for linux-next inclusion of the voyager tree

[Ingo Molnar]

* James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> On Wed, 2009-06-10 at 03:00 +0200, Ingo Molnar wrote:
> > * Ingo Molnar <mingo@elte.hu> wrote:
> > 
> > > See for example what happened in linux-next today: Voyager broke 
> > > x86 and didnt even build, and Stephen had to keep it out of 
> > > today's linux-next build.
> > 
> > I also took a look at that tree:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/jejb/voyager-2.6.git master
> > 
> > A couple of quick, mostly high-level observations:
> > 
> > 1)
> > 
> > The Voyager bits got rebased _yesterday_. _All_ the commits:
> > 
> >  commit 0ff51d1467af91bca4210b0d09372b6e7ded7524
> >  Author:     James Bottomley <James.Bottomley@HansenPartnership.com>
> >  AuthorDate: Mon Feb 23 15:02:04 2009 -0600
> >  Commit:     James Bottomley <James.Bottomley@HansenPartnership.com>
> >  CommitDate: Mon Jun 8 09:49:08 2009 -0500
> > 
> > I told James before that he should _not_ rebase:
> > 
> >    http://lkml.org/lkml/2009/2/1/76
> > 
> > In no uncertain terms. He completely ignored me and he messes up 
> > trees that way again. Now i should pull such a damaged tree while 
> > all the other x86 contributors we pull from do a thorough job?
> > 
> > ( And note that the above link points to _another_ similar incident,
> >   where James rebased a tree and broke the build. It is a pattern of 
> >   behavior. )
> 
> OK, so we disagree on this.  But think what I'm having to do ... I 
> need a patch set to apply on top of your somewhat volatile changes 
> to x86 ... I'm using git to manage that patch set and I have no 
> downstream users of the voyager tree, so rebasing is a good 
> management tool.

Rebasing is not a "management tool" - it's a destructive "past 
history is so messy or harmful that it's worthless so throw it
away" tool. As explained be me in the link above.

Also, care to clarify what you meant under "somewhat volatile 
changes to x86"? We dont rebase the x86 tree, so there's nothing 
'volatile' about it.

> If I go the merge point route, I get a tree with more non trivial 
> merge points than commits, so it becomes incredibly difficult for 
> anyone to follow what's going on.  I also can no longer use 
> git-email to send my patch series anywhere.

The proper work flow is to get the generic changes into the x86 tree 
and then to send the voyager bits without affecting anything else. 
There wont be any conflict in a normal workflow.

> So I beg to differ and conclude the rebase maintenance of this 
> patch series is the logically correct thing for me to do at the 
> current time.
> 
> > 2)
> > 
> > The tree structure is unacceptable:
> > 
> >  a087b5f: [VOYAGER] x86: add prefill_possible_map to x86_quirks
> >  f5ef55a: [VOYAGER] x86/mca: make mca_nmi_hook external
> >  55c8430: [VOYAGER] x86: add {safe,hard}_smp_processor_id to smp_ops
> >  fd1ab45: Revert "MAINTAINERS - Remove x86/Voyager no loner in tree"
> >  0ff51d1: Revert "x86: remove the Voyager 32-bit subarch"
> > 
> > That is crap. It starts with a huge 'revert' - which of course 
> > breaks the tree and needs 16 commits to bring back into action.
> 
> The revert doesn't actually break anything ... you saw to that 
> yourself by marking CONFIG_VOYAGER BROKEN and by pulling all it's 
> files out of the central makefile about two weeks before you 
> actually removed the code.

But that is not what matters: the revert is _ugly_ and unacceptable.

> > It might be OK to _revive_ the source code that way privately - 
> > but the history completely incorrectly suggests a 'revert'. We 
> > revert buggy commits.
> > 
> > What we want here is a clean submission, and a tidy, well 
> > constructed, non-rebased, well-tested Git tree. Or plain email 
> > submissions initially, because frankly i shouldnt Git-pull such 
> > a messy tree.
> > 
> > 3)
> > 
> > The comments. For example the revert:
> > 
> >  From 0ff51d1467af91bca4210b0d09372b6e7ded7524 Mon Sep 17 00:00:00 2001
> >  From: James Bottomley <James.Bottomley@HansenPartnership.com>
> >  Date: Mon, 23 Feb 2009 15:02:04 -0600
> >  Subject: [PATCH] Revert "x86: remove the Voyager 32-bit subarch"
> > 
> >  This reverts commit 965c7ecaf2e2b083d711a01ab33735a4bdeee1a4.
> >  ---
> > 
> > That is not how we do reverts! We always give a reason for a 
> > revert.
> 
> Well "put it back again" is the reason ... however, it did seem to 
> be obvious in the Subject line ... but I can add an additional 
> reason if you insist ... what apart from "revive voyager" do you 
> want it to say?

No, we dont want these reverts at all - see tglx's mails. We want a 
clean series adding hw support to the x86 architecture.

> > 4)
> > 
> > Small details like standard commit titles in the x86 tree. It 
> > shouldnt be:
> > 
> >  [VOYAGER] x86: eliminate subarchitecture file do_timer.h
> > 
> > It should be:
> > 
> >  x86: Voyager: Eliminate subarchitecture file do_timer.h
> > 
> > Note the different order and different capitalization.
> 
> Well [TREE] is a standard.  I've been using it for my trees for 
> ages. If you were to take voyager through your tree, then it would 
> follow your standards ... but you're forcing me to send it through 
> my own, so it follows mine.

If as a sub-maintainer of a particular piece of code you are not 
willing to follow the commit standards of your upstream partner, 
then i simply will refuse to pull from you.

> > 5)
> > 
> > And as i requested it in an earier thread, quirky platforms should 
> > be supported via a _single_ quirks file.
> > 
> > That makes it easy to handle and makes it easy to ignore as well. 
> > It's basically a "weird hardware driver". We have examples of that, 
> > for example arch/x86/kernel/visws_quirks.c. Voyager will be larger 
> > than that but it's OK - it's not like it will undergo massive 
> > development.
> > 
> > Putting it back into arch/x86/mach-voyager/ again reintroduces the 
> > subarch directory structure we got rid of.
> 
> It's just a convenient place to put the voyager stuff ... I can 
> move it somewhere, but code motion just makes the diffstats 
> explode for no good reason and makes it hard to judge the actual 
> changes.
> 
> I'd argue having voyager in a separate directory which is never 
> built unless you enable the config option is further out of sight 
> and out of mind.
> 
> But this is a tiny detail ... the three voyager files can go 
> anywhere, I'm not wedded to their actual location.

Please put them into a single .c file so that we can demonstrate 
that even a non-trivial non-PC sub-architecture can be added via a 
single driver - like other, existing subarchitectures already do. 
We'll expect the same stringent quality requirements from any future 
embedded-space submissions as well.

> > 6)
> > 
> > This ordering of subsequent patches:
> > 
> > 5c173bb: [VOYAGER] x86: eliminate subarchitecture file do_timer.h
> > 18d3288: [VOYAGER] x86: eliminate subarchitecture file entry_arch.h
> > 42c7289: [VOYAGER] x86: eliminate subarchitecture file setup_arch.h
> > 
> > is totally wrong - it removes something that should not have 
> > been put there in the first place.
> 
> I think you'll find do_timer.h, entry_arch.h and setup_arch.h 
> still exist in the current tree.
> 
> What these patches are actually doing is completing your partial 
> subarchitecture elimination by folding the splits back into the 
> mainline code and removing the header files that enabled them.
> 
> Out of all the patches, I would have thought you'd like these the 
> best, since they do finally eliminate the rest of the 
> subarchitecture stuff which still exists as trace elements in 
> arch/x86 today.

Heh, you are right - the '[VOYAGER]' tag fooled me there. Mea culpa.

> > If then Voyager should be added as a clean series of patches: 
> > first the generic patches that introduce infrastructure changes 
> > (to x86_quirks and smp_ops, etc.), then a single final patch 
> > that adds in the Voyager quirks platform driver.
> > 
> > And we've been through similar excercises multiple times before.
> > 
> > All in one, this tree is quite poor and needs a lot of work.
> 
> So basically, if the only problems you have are commit log naming 
> and patch sequencing, you find the actual code technically fine?

I havent gone to that detail yet - i'd like to see the generic 
changes first, and i'd like to see the general structure and 
principle of the patches be correct.

But hey ... here it goes: I just did a quick 3-places sample into 
the voyager code and there's 3 easy-to-solve visual things i 
noticed:

one thing you should take care of is to please use the customary 
comment style:

  /*
   * Comment .....
   * ...... goes here:
   */

specified in Documentation/CodingStyle. Also, please get rid of 
VDEBUG() and VOYAGER_DEBUG or convert it to pr_debug().

Plus there's a number of places where you use __u8 types needlessly, 
such as in before_handle_vic_irq():

        __u8 cpu = smp_processor_id();

We only use __u8 where it truly matters when interacting with 
hardware. Here (and in other places) it should be 'int'.

Please re-read the whole code with such general cleanliness 
principles in mind, i'm sure there are other places to fix as well.

I can re-review it if you think it's all 100% squeaky clean and in 
accordancy with the typical Linux kernel and arch/x86 coding style 
and principles.

Thanks,

        Ingo

Re: Request for linux-next inclusion of the voyager tree

[James Bottomley]

On Wed, 2009-06-10 at 02:30 +0200, Ingo Molnar wrote:
> * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > On Wed, 10 Jun 2009, Ingo Molnar wrote:
> > 
> > > 
> > > * Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > > 
> > > > > This code has been NAK-ed by the x86 maintainers:
> > > > > 
> > > > >  - Due to the absurd irrelevance of Voyager/x86/Linux hardware
> > > > > 
> > > > >  - Due to the thousands of lines of of code it adds to arch/x86
> > > > >    to support a 486/P5 era piece of hardware
> > > > > 
> > > > >  - and due to its negative track record of:
> > > > > 
> > > > >     v2.6.27.0:   Voyager was broken - it did not even build. (!)
> > > > >     v2.6.28.0:   Voyager was broken - it did not even build. (!)
> > > > >     v2.6.29-rc5: Voyager was broken - it did not even build. (!)
> > > > 
> > > > So Ingo you are arguing "It didn't work in some releases so we 
> > > > want to make it continue not to work by trying to keep the fixes 
> > > > out" ?
> > > 
> > > No. This code is not in Linux right now, and that i see no reason to 
> > > put it back, for the (many) reasons outlined.
> > 
> > Ingo, "absurd irrelevance" is not a reason. If it was, we'd lose 
> > about half our filesystems etc.
> > 
> > Neither is "thousands of lines of code", or "it hasn't always 
> > worked". Again, if it was, then we'd have to get rid of just about 
> > all drivers out there.
> > 
> > So give some real reasons. "It's a maintenance nightmare because 
> > it does xyz" might be a reason. But then we really need to see the 
> > "xyz" part too.
> > 
> > Alan is definitely right that we're likely to see more of the 
> > "non-PC" platforms as x86 tries to do embedded.
> 
> That is true, and the whole painful conversion from the build-time 
> 32-bit sub-arch code to the unified runtime quirk code (which is 
> really 'non-PC platform driver' kind of thing) that i did a few 
> months ago was partly about that.
> 
> I dont dispute that aspect - in fact we merged a rare subarch as 
> well.
> 
> But Voyager has been the most painful subarchitecture by far - for 
> an extended period of time the code didnt even build in its 
> defconfig - and it is also the most useless one as well. So it was 
> just a token really.
> 
> The only action i got from James was when _I_ implemented the 
> proper, runtime subarch mechanism and when we actually _removed_ the 
> voyager code. Before it James resisted such changes, see this thread 
> almost a year ago, in the Nth "voyager breaks the build" discussion, 
> where i suggested:
> 
>   " btw., any chance to turn it into a quirk space thing? "
> 
>   http://lkml.org/lkml/2008/4/21/441
> 
> I got no action from James for my technical requests.

However, you did get a reply worrying about the technical aspects:

http://marc.info/?l=linux-kernel&m=120881937304045

And later on me worrying about the increase in pointer function
indirections.  When you forcibly did the conversion anyway the voyager
conversion was done within a couple of weeks.

>  Right now i 
> dont see any guarantee that this wont be repeated, once the code is 
> upstream again after meeting some threshold minimally.
> 
> Voyager was a painful experience to all x86 maintainers and i'd 
> expect pushers and supporters of rarely used code to do such work, 
> not maintainers.
> 
> Are the quality thresholds i outlined in the previous thread(s) 
> unreasonable? They were:
> 
>  " If the code is absolutely trouble-free out of tree, for an
>    equivalent amount of time (3 kernel releases or so), and gathers
>    users/testers/etc., then we might add it, after a thorough
>    technical review. "
> 
>   http://lkml.org/lkml/2009/4/19/181
> 
> Given that there are only two known boxes left (both James's, the 
> other one went missing in action somewhere in Brasil), the 'gather 
> testers' bit is unreasonable i guess. 'Prove you can stay trouble 
> free' is more realistic. Dunno.
> 
> See for example what happened in linux-next today: Voyager broke x86 
> and didnt even build, and Stephen had to keep it out of today's 
> linux-next build.

Integration testing in configurations I either can't build or don't have
the machine power to run over is one of the incredible values of
linux-next, and why stuff needs to be in there before being pulled into
mainline.  The bug was fixed within about an hour of being found, but I
need linux-next to help me find this and other problems.

James

Re: Request for linux-next inclusion of the voyager tree

[Thomas Gleixner]

On Tue, 9 Jun 2009, Linus Torvalds wrote:
> So give some real reasons. "It's a maintenance nightmare because it does 
> xyz" might be a reason. But then we really need to see the "xyz" part too.

I looked at the patch in detail.

  - 42c72898662a1aed8bf94c6edc8d98ebc00a23a3 breaks xen paravirt guest
    kernels

  - 55c8430e8ec69811f31808251835087abea3e2fe adds new smp_ops, which
    are only necessary for Voyager and can not be optimized out for
    !VOYAGER builds. Also the patch needs to be split in "introduce
    ops" and "use the new ops" parts. Aside of that an analysis
    whether the new smp_ops affect any hotpath operations is missing.

  - 18d3288b053fdf49c062187cb58a50dfef228c70 adds duplicated
    declarations of voyager functions to a generic x86 header, while
    all of them are already declared in some voyager specific header.

  - 6057dbac12b9e85c4814b8e324ccde6e09f00e66 does subtle changes to
    the APIC code, which are neither explained by the changelog nor
    reviewed and acked by the people working on that code.

  - 5c173bb94876e34da1f257e211324424e3b0fc82 modifies the timer
    interrupt. by introducing an extra pointer check even for !VOYAGER
    builds w/o giving a reason for that change. Also adding an
    explicit voyager_timer_interrupt() has nothing to do with the
    x86_quirks model we asked for.

  - the changelogs are partially useless:

    	e.g. f5ef55ae426d811cf6f3650760da3ea526644eef
    	"[VOYAGER] x86/mca: make mca_nmi_hook external
    
	Part of the apic rework brought the mca_nmi_hook to a place where it
    	can't be accessed by architecture specific code.  Publish it again,
    	this time as a settable function vector so that voyager can use it."

    The patch actually does not modify an existing thing, it
    introduces the hook. Also we asked that such quirks should be
    handled via the x86_quirks functionality and not by adding extra
    hooks into the code again.

Aside of that the patch starts with a full revert of the voyager
removal instead of adding the necessary modifications to the x86
generic code in the first place and on top of them the voyager
implementation.

This is exactly the workflow we ask other contributors for as well.

In fact some of the patches are valuable cleanups on their own but
they need to go through us as independent commits on top of which a
non intrusive selfcontained reintroduction of voyager can be done.

Looking at the diffstats of the non voyager related files:

 arch/x86/include/asm/do_timer.h     |   16 -------
 arch/x86/include/asm/entry_arch.h   |   63 -----------------------------
 arch/x86/include/asm/setup_arch.h   |    3 -
 b/MAINTAINERS                       |    6 ++
 b/arch/x86/Kbuild                   |    3 +
 b/arch/x86/Kconfig                  |   14 ++++++
 b/arch/x86/boot/Makefile            |    3 +
 b/arch/x86/boot/a20.c               |    7 +++
 b/arch/x86/boot/boot.h              |    5 +-
 b/arch/x86/boot/main.c              |    5 ++
 b/arch/x86/configs/i386_defconfig   |    3 +
 b/arch/x86/configs/x86_64_defconfig |    3 +
 b/arch/x86/include/asm/Kbuild       |    1 
 b/arch/x86/include/asm/apic.h       |    8 +++
 b/arch/x86/include/asm/bootparam.h  |    5 +-
 b/arch/x86/include/asm/hw_irq.h     |   11 +++++
 b/arch/x86/include/asm/mca.h        |    3 +
 b/arch/x86/include/asm/setup.h      |    7 +--
 b/arch/x86/include/asm/smp.h        |   22 +++++++++-
 b/arch/x86/kernel/apic/apic.c       |    8 +--
 b/arch/x86/kernel/apic/ipi.c        |    2 
 b/arch/x86/kernel/apic/probe_32.c   |    3 +
 b/arch/x86/kernel/apic/summit_32.c  |    4 -
 b/arch/x86/kernel/entry_32.S        |   76 +++++++++++++++++++++++++++++++++++-
 b/arch/x86/kernel/irqinit.c         |   15 +++++--
 b/arch/x86/kernel/mca_32.c          |   12 +++++
 b/arch/x86/kernel/probe_roms_32.c   |    1 
 b/arch/x86/kernel/setup.c           |   35 +++-------------
 b/arch/x86/kernel/smp.c             |    7 +++
 b/arch/x86/kernel/smpboot.c         |    2 
 b/arch/x86/kernel/time_32.c         |   11 +++--
 b/arch/x86/kernel/visws_quirks.c    |    7 ---
 b/arch/x86/lguest/Kconfig           |    1 
 b/arch/x86/xen/Kconfig              |    2 
 b/arch/x86/xen/smp.c                |    7 +++
 b/drivers/lguest/Kconfig            |    2 
 36 files changed, 236 insertions(+), 147 deletions(-)

The voyager related files:

 boot/voyager.c                |   41 +
 include/asm/vic.h             |   61 +
 include/asm/voyager.h         |  553 ++++++++++++++
 include/asm/voyager_bios.h    |   22 
 include/asm/voyager_boot.h    |   27 
 include/asm/voyager_vectors.h |   37 
 mach-voyager/Makefile         |    8 
 mach-voyager/setup.c          |  123 +++
 mach-voyager/voyager_basic.c  |  304 +++++++
 mach-voyager/voyager_cat.c    | 1197 +++++++++++++++++++++++++++++++
 mach-voyager/voyager_smp.c    | 1597 ++++++++++++++++++++++++++++++++++++++++++
 mach-voyager/voyager_thread.c |  131 +++
 12 files changed, 4101 insertions(+)

So while the real voyager stuff lives in 12 new files the patches
changes 36 generic x86 files in subtle areas like boot and SMP
bringup/detection w/o any review of the responsible maintainers.

> Alan is definitely right that we're likely to see more of the "non-PC" 
> platforms as x86 tries to do embedded.

I agree, but the way voyager is done is _not_ a good example for the
embedded x86 folks who will probably start to send in their scoop in
the foreseable future.

I'm not fundamentally against bringing Voyager back, but it needs to
go through a useful patch submission and review process and not by
forcing voyager wreckage into our code base.

I talked to Ingo and he too agrees that we can bring it back under
strict requirements of having a single-.c-file x86_quirks based driver
on top of a clean stack of cleanups and preparatory patches which make
the voyager add on a complete selfcontained plugin.

That's the way we want to deal with the upcoming embedded x86 horror
as well.

I'm going to look at any related patch to which I'm cc'ed on and I
will make sure that the end result is satisfying for all involved
parties.

Thanks,

	tglx

Re: Request for linux-next inclusion of the voyager tree

[Linus Torvalds]

On Wed, 10 Jun 2009, Thomas Gleixner wrote:
> 
> > Alan is definitely right that we're likely to see more of the "non-PC" 
> > platforms as x86 tries to do embedded.
> 
> I agree, but the way voyager is done is _not_ a good example for the
> embedded x86 folks who will probably start to send in their scoop in
> the foreseable future.
> 
> I'm not fundamentally against bringing Voyager back, but it needs to
> go through a useful patch submission and review process and not by
> forcing voyager wreckage into our code base.

Ok, thanks. This was exactly the kind of thing I wanted to hear. It does 
sound like the Voyager tree is doing things I myself wouldn't approve of 
as a maintainer, so I can't really say that I'm upset by the x86 
maintainers then not pulling it.

			Linus

Re: Request for linux-next inclusion of the voyager tree

[Ingo Molnar]

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

> On Wed, 10 Jun 2009, Thomas Gleixner wrote:
> > 
> > > Alan is definitely right that we're likely to see more of the "non-PC" 
> > > platforms as x86 tries to do embedded.
> > 
> > I agree, but the way voyager is done is _not_ a good example for the
> > embedded x86 folks who will probably start to send in their scoop in
> > the foreseable future.
> > 
> > I'm not fundamentally against bringing Voyager back, but it 
> > needs to go through a useful patch submission and review process 
> > and not by forcing voyager wreckage into our code base.
> 
> Ok, thanks. This was exactly the kind of thing I wanted to hear. 
> It does sound like the Voyager tree is doing things I myself 
> wouldn't approve of as a maintainer, so I can't really say that 
> I'm upset by the x86 maintainers then not pulling it.

I also take back the "it's obsolete" and "it didnt even build" 
portion of my NAK - that was overboard as Alan and you pointed it 
out.

I think we can work out something and a clear(er) platform driver 
interface abstraction with a thin cross section to generic x86 code 
will be helpful to a lot more than just Voyager.

In fact we have implemented that largely and it went upstream in 
2.6.30, via the massive changes around this bit:

  6bda2c8: x86: remove subarchitecture support

This is what _already_ happened to other (ex-)subarchitecture code: 
visws, numaq were frequent trouble spots too, and with the 
x86-quirks model they basically vanished from our regression lists.

So it's a successful model in practice, and if Voyager is done in a 
similar way we wont see many Voyager problems in the future either.

	Ingo

Re: Request for linux-next inclusion of the voyager tree

[James Bottomley]

On Wed, 2009-06-10 at 17:39 +0200, Ingo Molnar wrote:
> * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > On Wed, 10 Jun 2009, Thomas Gleixner wrote:
> > > 
> > > > Alan is definitely right that we're likely to see more of the "non-PC" 
> > > > platforms as x86 tries to do embedded.
> > > 
> > > I agree, but the way voyager is done is _not_ a good example for the
> > > embedded x86 folks who will probably start to send in their scoop in
> > > the foreseable future.
> > > 
> > > I'm not fundamentally against bringing Voyager back, but it 
> > > needs to go through a useful patch submission and review process 
> > > and not by forcing voyager wreckage into our code base.
> > 
> > Ok, thanks. This was exactly the kind of thing I wanted to hear. 
> > It does sound like the Voyager tree is doing things I myself 
> > wouldn't approve of as a maintainer, so I can't really say that 
> > I'm upset by the x86 maintainers then not pulling it.
> 
> I also take back the "it's obsolete" and "it didnt even build" 
> portion of my NAK - that was overboard as Alan and you pointed it 
> out.
> 
> I think we can work out something and a clear(er) platform driver 
> interface abstraction with a thin cross section to generic x86 code 
> will be helpful to a lot more than just Voyager.
> 
> In fact we have implemented that largely and it went upstream in 
> 2.6.30, via the massive changes around this bit:
> 
>   6bda2c8: x86: remove subarchitecture support
> 
> This is what _already_ happened to other (ex-)subarchitecture code: 
> visws, numaq were frequent trouble spots too, and with the 
> x86-quirks model they basically vanished from our regression lists.
> 
> So it's a successful model in practice, and if Voyager is done in a 
> similar way we wont see many Voyager problems in the future either.

OK, so this is an acceptable compromise for me too.

What I think now is needed (from me) are three patch sets:

1. The final subarchitecture cleanups
2. The quirk model/smp ops additions
3. The voyager put back.

James

Re: Request for linux-next inclusion of the voyager tree

[Ingo Molnar]

* James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> On Wed, 2009-06-10 at 17:39 +0200, Ingo Molnar wrote:
> > * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > 
> > > On Wed, 10 Jun 2009, Thomas Gleixner wrote:
> > > > 
> > > > > Alan is definitely right that we're likely to see more of the "non-PC" 
> > > > > platforms as x86 tries to do embedded.
> > > > 
> > > > I agree, but the way voyager is done is _not_ a good example for the
> > > > embedded x86 folks who will probably start to send in their scoop in
> > > > the foreseable future.
> > > > 
> > > > I'm not fundamentally against bringing Voyager back, but it 
> > > > needs to go through a useful patch submission and review process 
> > > > and not by forcing voyager wreckage into our code base.
> > > 
> > > Ok, thanks. This was exactly the kind of thing I wanted to hear. 
> > > It does sound like the Voyager tree is doing things I myself 
> > > wouldn't approve of as a maintainer, so I can't really say that 
> > > I'm upset by the x86 maintainers then not pulling it.
> > 
> > I also take back the "it's obsolete" and "it didnt even build" 
> > portion of my NAK - that was overboard as Alan and you pointed it 
> > out.
> > 
> > I think we can work out something and a clear(er) platform driver 
> > interface abstraction with a thin cross section to generic x86 code 
> > will be helpful to a lot more than just Voyager.
> > 
> > In fact we have implemented that largely and it went upstream in 
> > 2.6.30, via the massive changes around this bit:
> > 
> >   6bda2c8: x86: remove subarchitecture support
> > 
> > This is what _already_ happened to other (ex-)subarchitecture code: 
> > visws, numaq were frequent trouble spots too, and with the 
> > x86-quirks model they basically vanished from our regression lists.
> > 
> > So it's a successful model in practice, and if Voyager is done in a 
> > similar way we wont see many Voyager problems in the future either.
> 
> OK, so this is an acceptable compromise for me too.
> 
> What I think now is needed (from me) are three patch sets:
> 
> 1. The final subarchitecture cleanups
> 2. The quirk model/smp ops additions
> 3. The voyager put back.

Yes, that looks fine.

You can have them in a single series for convenience if you want to 
(it's probably easier for you to test that way) - but 3 separate 
series are fine too, no strong preference either way - as long as 
the internal structure and details follows the ordering and 
parameters we outlined in previous mails.

	Ingo

Re: Request for linux-next inclusion of the voyager tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

On Wed, 10 Jun 2009 18:53:31 +0200 Ingo Molnar <mingo@elte.hu> wrote:
>
> * James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> > 
> > OK, so this is an acceptable compromise for me too.
> > 
> > What I think now is needed (from me) are three patch sets:
> > 
> > 1. The final subarchitecture cleanups
> > 2. The quirk model/smp ops additions
> > 3. The voyager put back.
> 
> Yes, that looks fine.
> 
> You can have them in a single series for convenience if you want to 
> (it's probably easier for you to test that way) - but 3 separate 
> series are fine too, no strong preference either way - as long as 
> the internal structure and details follows the ordering and 
> parameters we outlined in previous mails.

OK, given this looks like a rewrite of the voyager tree, I will drop it
from linux-next for a while.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Request for linux-next inclusion of the voyager tree

[James Bottomley]

On Thu, 2009-06-11 at 11:35 +1000, Stephen Rothwell wrote:
> On Wed, 10 Jun 2009 18:53:31 +0200 Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> > > 
> > > OK, so this is an acceptable compromise for me too.
> > > 
> > > What I think now is needed (from me) are three patch sets:
> > > 
> > > 1. The final subarchitecture cleanups
> > > 2. The quirk model/smp ops additions
> > > 3. The voyager put back.
> > 
> > Yes, that looks fine.
> > 
> > You can have them in a single series for convenience if you want to 
> > (it's probably easier for you to test that way) - but 3 separate 
> > series are fine too, no strong preference either way - as long as 
> > the internal structure and details follows the ordering and 
> > parameters we outlined in previous mails.
> 
> OK, given this looks like a rewrite of the voyager tree, I will drop it
> from linux-next for a while.

Yes, I think that's the correct thing in the circumstances.

Thanks for running it through linux-next; it certainly turned up a
couple of problems I wouldn't have seen otherwise.

James