From: Ingo Molnar <mingo@elte•hu>
To: Linus Torvalds <torvalds@linux-foundation•org>
Cc: Alan Cox <alan@lxorguk•ukuu.org.uk>,
James Bottomley <James.Bottomley@HansenPartnership•com>,
Thomas Gleixner <tglx@linutronix•de>,
"H. Peter Anvin" <hpa@zytor•com>,
Andrew Morton <akpm@linux-foundation•org>,
Stephen Rothwell <sfr@canb•auug.org.au>,
linux-next@vger•kernel.org
Subject: Re: Request for linux-next inclusion of the voyager tree
Date: Wed, 10 Jun 2009 03:00:14 +0200 [thread overview]
Message-ID: <20090610010014.GA28345@elte.hu> (raw)
In-Reply-To: <20090610003055.GA26492@elte.hu>
* 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
next prev parent reply other threads:[~2009-06-10 1:00 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-06-08 16:10 Request for linux-next inclusion of the voyager tree James Bottomley
2009-06-08 23:28 ` Tony Breeds
2009-06-10 14:45 ` James Bottomley
2009-06-09 9:45 ` Stephen Rothwell
2009-06-09 13:49 ` James Bottomley
2009-06-09 20:21 ` Ingo Molnar
2009-06-09 20:33 ` James Bottomley
2009-06-09 21:18 ` Ingo Molnar
2009-06-09 23:41 ` Alan Cox
2009-06-09 23:56 ` Ingo Molnar
2009-06-10 0:04 ` Linus Torvalds
2009-06-10 0:30 ` Ingo Molnar
2009-06-10 1:00 ` Ingo Molnar [this message]
2009-06-10 14:38 ` James Bottomley
2009-06-10 15:20 ` Linus Torvalds
2009-06-10 15:28 ` James Bottomley
2009-06-10 15:33 ` Linus Torvalds
2009-06-10 16:19 ` Ingo Molnar
2009-06-10 16:42 ` Ingo Molnar
2009-06-10 14:23 ` James Bottomley
2009-06-10 15:13 ` Thomas Gleixner
2009-06-10 15:23 ` Linus Torvalds
2009-06-10 15:39 ` Ingo Molnar
2009-06-10 16:02 ` James Bottomley
2009-06-10 16:53 ` Ingo Molnar
2009-06-11 1:35 ` Stephen Rothwell
2009-06-11 1:39 ` James Bottomley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090610010014.GA28345@elte.hu \
--to=mingo@elte$(echo .)hu \
--cc=James.Bottomley@HansenPartnership$(echo .)com \
--cc=akpm@linux-foundation$(echo .)org \
--cc=alan@lxorguk$(echo .)ukuu.org.uk \
--cc=hpa@zytor$(echo .)com \
--cc=linux-next@vger$(echo .)kernel.org \
--cc=sfr@canb$(echo .)auug.org.au \
--cc=tglx@linutronix$(echo .)de \
--cc=torvalds@linux-foundation$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox