From: Ingo Molnar <mingo@elte.hu>
To: Paul Mackerras <paulus@samba.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>,
dwalker@fifo99.com, mingo@redhat.com, hpa@zytor.com,
linux-kernel@vger.kernel.org, johnstul@us.ibm.com,
tglx@linutronix.de, linux-tip-commits@vger.kernel.org
Subject: Re: [tip:timers/core] timekeeping: Increase granularity of read_persistent_clock()
Date: Tue, 25 Aug 2009 15:50:17 +0200 [thread overview]
Message-ID: <20090825135017.GA31340@elte.hu> (raw)
In-Reply-To: <19091.52205.222449.441072@cargo.ozlabs.ibm.com>
* Paul Mackerras <paulus@samba.org> wrote:
> Ingo Molnar writes:
>
> > * Paul Mackerras <paulus@samba.org> wrote:
> >
> > > Ingo Molnar writes:
> > >
> > > > If you suggest that each and every subsystem maintainer who
> > > > touches code that can be built on non-x86 architectures has to
> > > > cross-build to 20+ architectures to be able to push out a tree,
> > > > all the time, and has to rebase if this ever gets omitted, you
> > > > are really defying reality and are hurting Linux.
> > >
> > > Nice straw man, but I never said or even suggested anything like
> > > that. :)
> >
> > ... but what you say in essence amounts to that. Every change to a
> > file that is built on an architecture 'affects' that architecture so
> > your arguments can be repeated for that one. It's a slippery slope
> > built on a wrong premise.
>
> Not at all. I'm saying that committing code, some of which has
> never ever been compiled on any architecture, is just sloppy.
But this is not what happened. We committed it because it was a good
patch and this _enabled_ the wider testing of it, even in areas that
developers dont normally use.
In other words while i do a lot of cross-testing and applied your
fix immediately i'm not willing to whitewash the "PowerPC is rarely
used" fact out of Git trees via ugly rebasing - especially if the
distance between the original commit and the fix is just 4 hops.
Nor am i going to require people/developers to test on a dozen of
architectures or more before we can commit something - it's not
reasonable.
( Nor is there any real upside to rebasing for this reason alone,
and there's a lot of downsides to it, many of which i already
mentioned in my prior replies. )
> It's basic good engineering practice to have at least compiled the
> code you're committing - at least once on at least one
> architecture. [...]
And i very much do that before i push such changes to Linus.
It's a push show-stopper but not a commit show-stopper.
You chose to see the intermediate steps by checking the development
stage, why are you surprised that less used and less tested areas of
the code breaks?
Everything that happens rarely, such as big pushes to Linus, can be
(and does get) tested widely like that. For for the most critical
part of the workflow, the commit latency, we dont want extra
buerocracy that buys us nothing.
Let the platforms do their own testing and _provide developers_ who
will make it damn sure new bits work on their architecture out of
box. Or if they dont (or cannot) and just sit there expecting the
upstream kernel to do development for them, let them deal with the
consequences.
> [...] It's like making changes inside #ifdef CONFIG_FOO but never
> testing with CONFIG_FOO turned on. You'd complain, and rightly,
> if someone did that.
You again seem to ignore the very valid case i pointed out: if i
change generic code (or an include, an inline function or a define)
which somehow affects an architecture, by your logic i'd be
compelled to test it on that architecture - because it affects it.
That's not reasonable overhead.
> > I can only repeat what i said because i did not see you really
> > counter the core of my argument (you snipped out much of my mail
> > that details this and only replied to the last paragraph):
>
> Well, though much of what you said is true, in the end it seems to
> boil down to saying that non-x86 architectures don't have any
> right to expect any degree of care or attention from you, because
> only x86 is important. And of course you have a perfect right to
> pay attention to whatever you find interesting and ignore anything
> else, including difficulties that you cause along the way for
> other kernel developers.
>
> There doesn't seem to be any useful response to that.
You paraphrased my opinion into something i did not say.
Exactly which bit of "i cross-build test every architecture that
builds fine upstream before i push out to linux-next" do you
understand as me not caring?
But it does not get as strong testing, it's not a commit
show-stopper (unless coupled with other badness) and sometimes bugs
slip through and get fixed after (quickly).
> > Say that patch also touches arch/xtensa/. Can you cross-build to
> > xtensa?
>
> Sure, I just ask Michael Ellerman to point kisskb at a tree with
> my proposed patch in it.
FYI, would be a challenge, xtensa's defconfig didnt build upstream
for quite a long time:
/home/mingo/tip/arch/xtensa/kernel/pci.c: In function '__pci_mmap_set_pgprot':
/home/mingo/tip/arch/xtensa/kernel/pci.c:362: error: '_PAGE_NO_CACHE' undeclared (first use in this function)
/home/mingo/tip/arch/xtensa/kernel/pci.c:362: error: (Each undeclared identifier is reported only once
/home/mingo/tip/arch/xtensa/kernel/pci.c:362: error: for each function it appears in.)
> > > Patches which touch multiple architecture's arch-specific code
> > > should also get sent to the maintainers of the affected
> > > architectures and the linux-arch mailing list. I don't recall
> > > seeing this patch on linux-arch, though I may have missed it
> > > (and anyway that would be Martin's responsibility not yours,
> > > but it does contribute to the sense of being blindsided).
> > >
> > > More generally - if you don't have the resources to do regular
> > > build testing for powerpc or other architectures, then publish
> > > a testing branch and we'll get kisskb
> > > (http://kisskb.ellerman.id.au/) to build a selection of
> > > configs and architectures automatically.
> >
> > Slowing down patches via buerocracy like that is not acceptable
> > at all. Make developers care more about your architecture via
> > natural means, not via buerocratic measures.
>
> The "should" in what I said is just good manners, not bureaucracy.
>
> The stuff about kisskb was an offer to help, not bureaucracy.
> No-one's forcing you to do anything.
Well, me pointing out that such measures slow down the critical path
of patch submission is a valid concern - and "you are not forced to
do that" pretty much defeats you having suggested it, doesnt it? (i
hope i'm not misprepresenting your words)
Ingo
next prev parent reply other threads:[~2009-08-25 13:50 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-08-14 13:47 [patch 00/15] clocksource / timekeeping rework V4 (resend V3 + bug fix) Martin Schwidefsky
2009-08-14 13:47 ` [patch 01/15] introduce timekeeping_leap_insert Martin Schwidefsky
2009-08-15 9:01 ` [tip:timers/core] timekeeping: Introduce timekeeping_leap_insert tip-bot for John Stultz
2009-08-14 13:47 ` [patch 02/15] remove clocksource inline functions Martin Schwidefsky
2009-08-15 9:01 ` [tip:timers/core] timekeeping: Remove " tip-bot for Martin Schwidefsky
2009-08-14 13:47 ` [patch 03/15] reset of cycle_last for tsc clocksource Martin Schwidefsky
2009-08-15 9:01 ` [tip:timers/core] timekeeping: Move reset of cycle_last for tsc clocksource to tsc tip-bot for Martin Schwidefsky
2009-08-14 13:47 ` [patch 04/15] cleanup clocksource selection Martin Schwidefsky
2009-08-15 1:42 ` john stultz
2009-08-15 1:43 ` john stultz
2009-08-17 7:34 ` Martin Schwidefsky
2009-08-15 9:02 ` [tip:timers/core] clocksource: Cleanup " tip-bot for Martin Schwidefsky
2009-08-14 13:47 ` [patch 05/15] clocksource watchdog highres enablement Martin Schwidefsky
2009-08-15 9:02 ` [tip:timers/core] clocksource: Delay " tip-bot for Martin Schwidefsky
2009-08-14 13:47 ` [patch 06/15] clocksource watchdog resume logic Martin Schwidefsky
2009-08-15 9:02 ` [tip:timers/core] clocksource: Simplify " tip-bot for Martin Schwidefsky
2009-08-14 13:47 ` [patch 07/15] clocksource watchdog refactoring Martin Schwidefsky
2009-08-15 9:02 ` [tip:timers/core] clocksource: Refactor clocksource watchdog tip-bot for Martin Schwidefsky
2009-08-14 13:47 ` [patch 08/15] clocksource watchdog work Martin Schwidefsky
2009-08-15 9:03 ` [tip:timers/core] clocksource: Move watchdog downgrade to a work queue thread tip-bot for Martin Schwidefsky
2009-08-14 13:47 ` [patch 09/15] introduce struct timekeeper Martin Schwidefsky
2009-08-15 9:03 ` [tip:timers/core] timekeeping: Introduce " tip-bot for Martin Schwidefsky
2009-08-14 13:47 ` [patch 10/15] add xtime_shift and ntp_error_shift to " Martin Schwidefsky
2009-08-15 9:03 ` [tip:timers/core] timekeeping: Add " tip-bot for Martin Schwidefsky
2009-08-15 9:04 ` [patch 10/15] add " Thomas Gleixner
2009-08-14 13:47 ` [patch 11/15] move NTP adjusted clock multiplier " Martin Schwidefsky
2009-08-15 9:03 ` [tip:timers/core] timekeeping: Move " tip-bot for Martin Schwidefsky
2009-08-14 13:47 ` [patch 12/15] timekeeper read clock helper functions Martin Schwidefsky
2009-08-15 9:03 ` [tip:timers/core] timekeeping: Add timekeeper read_clock " tip-bot for Martin Schwidefsky
2009-08-14 13:47 ` [patch 13/15] update clocksource with stop_machine Martin Schwidefsky
2009-08-15 9:04 ` [tip:timers/core] timekeeping: Update " tip-bot for Martin Schwidefsky
2009-08-14 13:47 ` [patch 14/15] read_persistent_clock should return a timespec Martin Schwidefsky
2009-08-15 9:04 ` [tip:timers/core] timekeeping: Increase granularity of read_persistent_clock() tip-bot for Martin Schwidefsky
2009-08-22 10:32 ` Ingo Molnar
2009-08-22 15:15 ` Martin Schwidefsky
2009-08-22 15:33 ` Ingo Molnar
2009-08-22 20:23 ` Martin Schwidefsky
2009-08-23 8:53 ` Ingo Molnar
2009-08-23 9:03 ` [tip:timers/core] timekeeping: Increase granularity of read_persistent_clock(), build fix tip-bot for Martin Schwidefsky
2009-08-23 3:33 ` [tip:timers/core] timekeeping: Increase granularity of read_persistent_clock() Paul Mackerras
2009-08-23 8:47 ` Ingo Molnar
2009-08-24 3:20 ` Paul Mackerras
2009-08-24 8:23 ` Ingo Molnar
2009-08-25 3:49 ` Paul Mackerras
2009-08-25 8:26 ` Ingo Molnar
2009-08-25 9:57 ` Paul Mackerras
2009-08-25 10:17 ` Ingo Molnar
2009-08-25 11:33 ` Paul Mackerras
2009-08-25 13:50 ` Ingo Molnar [this message]
2009-08-25 21:33 ` Theodore Tso
2009-08-25 22:03 ` Ingo Molnar
2009-08-26 0:26 ` Paul Mackerras
2009-08-26 0:22 ` Paul Mackerras
2009-08-25 23:48 ` Paul Mackerras
2009-08-26 9:44 ` Benjamin Herrenschmidt
2009-08-14 13:47 ` [patch 15/15] introduce read_boot_clock Martin Schwidefsky
2009-08-15 9:04 ` [tip:timers/core] timekeeping: Introduce read_boot_clock tip-bot for Martin Schwidefsky
2009-08-14 14:08 ` [patch 00/15] clocksource / timekeeping rework V4 (resend V3 + bug fix) Thomas Gleixner
2009-08-14 14:22 ` Martin Schwidefsky
2009-08-14 22:56 ` john stultz
2009-08-15 1:46 ` john stultz
2009-08-15 9:01 ` Thomas Gleixner
2009-08-15 9:52 ` Ingo Molnar
2009-08-15 10:08 ` Thomas Gleixner
2009-08-17 7:40 ` Martin Schwidefsky
2009-08-17 8:45 ` Thomas Gleixner
2009-08-17 9:28 ` [circular locking bug] " Ingo Molnar
2009-08-17 17:53 ` Martin Schwidefsky
2009-08-18 15:09 ` Martin Schwidefsky
2009-08-19 10:06 ` [tip:timers/core] clocksource: Avoid clocksource watchdog circular locking dependency tip-bot for Martin Schwidefsky
2009-08-19 20:25 ` [circular locking bug] Re: [patch 00/15] clocksource / timekeeping rework V4 (resend V3 + bug fix) Ingo Molnar
2009-08-20 9:28 ` Martin Schwidefsky
2009-08-20 9:58 ` Ingo Molnar
2009-08-20 10:35 ` Martin Schwidefsky
2009-08-20 16:14 ` Thomas Gleixner
2009-08-20 16:53 ` Martin Schwidefsky
2009-08-20 19:08 ` Thomas Gleixner
2009-08-19 9:46 ` [tip:timers/core] clocksource: Protect the watchdog rating changes with clocksource_mutex tip-bot for Thomas Gleixner
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=20090825135017.GA31340@elte.hu \
--to=mingo@elte.hu \
--cc=dwalker@fifo99.com \
--cc=hpa@zytor.com \
--cc=johnstul@us.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tip-commits@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=paulus@samba.org \
--cc=schwidefsky@de.ibm.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.