Index Home About Blog
Newsgroups: fa.linux.kernel
From: Alexander Viro <viro@math.psu.edu>
Subject: Of Bundling, Dao and Cowardice
Original-Message-ID: <Pine.GSO.4.21.0202161146160.29124-100000@weyl.math.psu.edu>
Date: Sat, 16 Feb 2002 18:31:21 GMT
Message-ID: <fa.mgb1s2v.kkcqbk@ifi.uio.no>

On Sat, 16 Feb 2002, Eric S. Raymond wrote:

> Would you ask someone designing a new VM to make it crash and hang exactly
> the same way the old one did?
>
> Do you demand that a rewrite of a disk driver have the same data-corruption
> bugs as the original before it can go into the tree, and tell the developer
> to add fixes later?
>
> Pragmatically, the point of rewriting a system is to *fix bugs*.

	That, folks, is a fine example of very common problem.
It's very tempting to try and force your ideas of How The Things
Should Be Done(tm) into the tree by bundling them with genuine
bug fixes and (more or less gracefully) refusing to split the
patch.  Anybody who had done serious work on the tree had felt
that temptation at one point or another.  No arguments - it's very
attractive.  Indeed, you don't have to defend every point of your
design and implementation - you can always point to Bad Bugs(tm)
that are fixed by the entire thing and obfuscate the objections away.
The trouble being, _SUCH_ _BUNDLING_ _IS_ _NOT_ _A_ _VALID_ _STRATEGY_.

	It had been tried.  Many times.  Sometimes it even got the
thing into the tree.  _All_ such cases had lead to trouble.

	There is another way.  It takes more efforts, it requires
readiness to defend every damn part of your design and it means
that you will have to deal with the sad fact that parts of that
design need to be redone.  It will take more time.  It will look
less sexy.  It may very well mean that somebody else will get
alternative design into the tree on the top of your efforts.
There are only two positive things about it - it is honest and
it leads to better tree.

	How does it work?  Simple - look at the path from original
tree to tree with your modifications.  And no, "one big jump" doesn't
count.  Think of the way your ideas can be split into parts.  Think
of the way obstructions to the changes can be split into parts.
Find small and simple changes that can go in right now, would improve
the tree and would make the rest of path easier.  And merge them.
If you find that it can't be done - think what needs to be changed in
the tree (or in your modifications) to make the split possible.
If you see that something is ugly and stands in the way - help the thing
to achieve the form it wants.  That may change all your ideas about the
right design for your modifications.  That's OK - it's a Good Thing(tm).
And merge the small changes.  Step by step.

	_That_ works.  Less glory; more time; more work.  And better
tree afterwards.  The code wants to get cleaner.  In many directions.
The trick is to pick the right ones and let the thing grow into natural
form.  Do that many times in the right order and you will get it where
you want it to be.  Quite possibly - in a better place than you thought
originally.

	Before you start saying that it's impossible - THAT HAD BEEN DONE.
Between 2.3.40 and 2.5.2 (about two years) we got pretty much complete
redesign of VFS,  along with the stuff that was plain impossible with
the old design.  Get the old tree.  Get the current tree.  Compare.  And
realize that more than half of the way was covered during 2.4.  Yes, the
total size of patches had been about 2 times larger than the size of
combined patch.  Definitely took a lot of extra efforts.  You know what?
It was worth the trouble.  Result is better than what I've expected.  And
a lot of things in the first iteration of design had turned out to be
useless - stuff that was there to work around the problems that got _fixed_
by cleanups at some point during these two years.  And I fully expect to
see the same thing happening with the stuff I'm planning and doing now.

	While we are at it, look what Rik is doing right now.  He has
a huge patch pretty much rewriting (what a coincidence) VM.  He had tried
to shove its previous incarnations into the tree whole-sale.  Saying that
it didn't work is putting it _very_ mildly - resulting fight is in l-k
archives and boy, was it nasty...  Look what's going on now - same splitup
and gradual merge strategy.  Sure, extra work.  Guess what?  The thing had
already become better from that work.

	As for your motivations...  If you are doing that for bragging
rights ("I've thought up a new design and now it's in the tree, exactly
as I envisioned it") - find something else to brag about.  Your strategy
is basically a sign of cowardice - you are fighting tooth and nail to avoid
gradual changes and the only way I can interpret it is that you are afraid
to let parts of your design stand on their own.  Not exactly a bragging
material, that...


From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: x86 status was Re: -mm merge plans for 2.6.23
Date: Wed, 11 Jul 2007 22:10:51 UTC
Message-ID: <fa.ZwpQcEyS1YN2VvmLI9CmgFO1GLM@ifi.uio.no>

On Wed, 11 Jul 2007, Andrea Arcangeli wrote:
>
> I'm going to change topic big time because your sentence above
> perfectly applies to the O(1) scheduler too.

I disagree to a large degree.

We almost never have problems with code you can "think about".

Sure, bugs happen, but code that everybody runs the same generally doesn't
break. So a CPU scheduler doesn't worry me all that much. CPU schedulers
are "easy".

What worries me is interfaces to hardware that we know looks different for
different people. That means that any testing that one person has done
doesn't necessarily translate to anything at *all* on another persons
machine.

The timer problems we had when merging the stuff in 2.6.21 just scarred
me. I'd _really_ hate to have to go through that again. And no, the
"gradual" thing where the patch that actually *enables* something isn't
very gradual at all, so that's the absolutely worst kind of thing, because
then people can "git bisect" to the point where it got enabled and tell us
that's where things broke, but that doesn't actually say anything at all
about the patch that actually implements the new behaviour.

So the "enable" kind of patch is actually the worst of the lot, when it
comes to hardware.

When it comes to pure software algorithms, and things like schedulers,
you'll still obviously have timing issues and tuning, but generally things
*work*, which makes it a lot easier to debug and describe.

		Linus


From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: x86 status was Re: -mm merge plans for 2.6.23
Date: Wed, 11 Jul 2007 22:36:54 UTC
Message-ID: <fa.OLzvDrhhTsDSrQsHIFTWZIXTF4E@ifi.uio.no>

On Wed, 11 Jul 2007, Valdis.Kletnieks@vt.edu wrote:
>
> <Takes a closer look at the patches>  D'Oh! :)  Yeah, the -rc4 version I'm
> looking at is like a dozen 1-3K patches setting up and cleaning up, and then
> one monster 65K patch doing the clockevents conversion, then another 6 or 8
> small ones.
>
> Yeah, that one big patch really doesn't look separable to me.

I think it should be.

That big patch really does do a *lot* more than just the "clockevents
conversion". It does all the hpet clock setup changes etc that are about
the hardware, and have *nothing* to do with actually changing the
interfaces.

For example, look at the hpet.c part of that patch. Totally independent
cleanups of everything else.

Or look at the changes to __setup_APIC_LVTT(). Same thing.

All the actual hardware interface changes are *totally* independent of the
software interface changes, and a lot of them are just cleanups.

But those hardware interface changes are easily the things that can break,
where some cleanup results in register writes being done in a different
order or something, and so if there's a bug there (and it's not visible on
most setups), now you cannot tell where the bug is.

Another example: setup_APIC_timer() used to wait for a timer interrupt
trigger to happen on the i8259 timer (or HPET). That code just got
removed (or maybe it got moved so subtly that I just don't see it).

What has that got to do with switching from the old timer interface to the
new one?

NOTHING.

So those kinds of changes that change hardware access functions should
have been done separately. Maybe there's a machine where that early
synchronization was necessary for some subtle timing reason. If so,
removing it sounds like a bug, no? Wouldn't it have been nice to see that
removal as a separate patch that was independent of the interface switch-
over?

I'd be a *lot* happier with switching over interfaces if I thought that
the low-level hardware drivers didn't change at the same time. But they
*do* change, afaik.

		Linus


From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: x86 status was Re: -mm merge plans for 2.6.23
Date: Wed, 11 Jul 2007 21:44:06 UTC
Message-ID: <fa.HfSMm580j/rdyGkQeZBKTn8I+us@ifi.uio.no>

On Wed, 11 Jul 2007, Ingo Molnar wrote:
>
> What you just did here is a slap in the face to a lot of contributors
> who worked hard on this code :(

Ingo, I'm sorry to say so, but your answer just convinced me that you're
wrong, and we MUST NOT take that code.

That was *exactly* the same thing you talked about when I refused to take
the original timer changes into 2.6.20. You were talking about how lots of
people had worked really hard, and how it was really tested.

And it damn well was NOT really tested, and 2.6.21 ended up being a
horribly painful experience (one of the more painful kernel releases in
recent times), and we ended up having to fix a *lot* of stuff.

And you admitted you were wrong at the time.

Now you do the *exact* same thing.

Here's a big clue: it doesn't matter one _whit_ how much face-slapping you
get, or how much effort some programmers have put into the code. It's
untested. And no, we are *not* going to do another "rip everything out,
and replace it with new code" again.

Over my dead body.

We're going to do this thing gradually, or not at all.

And if somebody feels slighted by the face-slap, and thinks he has already
done enough, and isn't interested in doing it gradually, then good
riddance. The "not at all" seems like a good idea, and maybe we can
re-visit this in a year or two.

I'm not going to have another 2.6.21 on my hands.

		Linus


From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: x86 status was Re: -mm merge plans for 2.6.23
Date: Wed, 11 Jul 2007 22:22:36 UTC
Message-ID: <fa.YJubKNoiB/DI4j6mozZ0/RGvjH0@ifi.uio.no>

On Thu, 12 Jul 2007, Thomas Gleixner wrote:
>
> Can you please shed some light on me, how exactly you switch an
> architecture gradually to clock events.

For example, we can make sure that the code in question that actually
touches the hardware stays exactly the same, and then just move the
interfaces around - and basically guarantee that _zero_ hardware-specific
issues pop up when you switch over, for example.

That way there is a gradual change-over.

The other approach (which would be nice _too_) is to actually try to
convert one clock source at a time. Why is that not an option?

		Linus



From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: x86 status was Re: -mm merge plans for 2.6.23
Date: Wed, 11 Jul 2007 23:09:06 UTC
Message-ID: <fa.ghgeq823fgpnZwHmHb/0rpIUpcE@ifi.uio.no>

On Thu, 12 Jul 2007, Thomas Gleixner wrote:
>
> The HPET change, which is the larger part of the conversion set simply
> because we now share the code with i386, might be split out by disabling
> HPET in the first step, doing the PIT / APIC conversion and then the
> HPET one in a separate step.

But that misses the point. It means that the commit that actually
*changes* the code never actually gets tested on its own

Why not just fix up the HPET code so that it can be shared *first*.
Without the other conversion? Really - What's so wrong with the hpet.c
changes in the *absense* of conversion to clockevents? Those changes seem
to be totally independent - just abstracting ou tthe
"hpet_get_virt_address()" stuff etc.

None of that has anything to do with clockevents, as far as I can see.

In other words, you now change a i386-only file, and maybe it breaks
subtly on i386 as a result. Wouldn't it be nicer to see that breakage as a
separate event?

Then, the x86-64 clockevents code will switch over entirely, but now it
switches over to something we can say has gotten testing, and we know the
switch-over won't break any 32-bit code, because the switch-over literally
didn't change anything at all for that case.

See? THAT is what I mean by "gradual". Bugs happen, but if we can make
_independent_ bugs show up in _independent_ commits, that will make it
much easier to figure out what happened.

The same is true of a lot of the APIC timer code. Sure, that patch has the
actual conversion in it, and you don't have the cross-architecture issues,
but more than 50% of the patch seems to be just cleanup that is
independent of the actual switch-over, no?

Again, if it was done as a "one patch for cleanup, and another patch that
actually switches the higher-level interfaces around", then the two mostly
independent issues (of "hardware access/initialization" vs "higher-level
changes in how it got called") get done as two independent commits.

And no, I really probably wouldn't ask for this, but 2.6.21 showed
*exactly* this problem. Trivial debugging helps like "git bisect" didn't
help at all, because all the problems started when the new code was
"activated", not when it was actually brought in.

		Linus


From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: x86 status was Re: -mm merge plans for 2.6.23
Date: Thu, 12 Jul 2007 00:05:08 UTC
Message-ID: <fa.pSvrC2aI8fxNmjIT66WJnsx8kfg@ifi.uio.no>

On Thu, 12 Jul 2007, Ingo Molnar wrote:
>
> We also integrated _all_ feedback we got, and we had the capacity and
> capability to fix whatever other feedback comes back - it just never
> came ... until today.

One thing I'll happily talk about is that while 2.6.21 was painful, you
and Thomas in particular were both very responsible about the thing, so
no, I'm not at all complaining or worried about it in that sense!

I just really _really_ wish we could have two fairly stable releases in a
row. I think 2.6.22 has the potential to be a pretty good setup, and I'd
really like to avoid having another 2.6.21 immediately afterwards.

So I'm not worried about integration and getting fixes when things break
per se, but I *am* worried that this is an area where we've traditionally
had lots of unexpected problems.

And hey, maybe this time there will be none. I just still smart from the
last time, so I'd prefer it to go more smoothly this time around.

		Linus

Index Home About Blog