Message-ID: <Pine.LNX.4.58.0805061635450.28962@gandalf.stny.rr.com>
Date:	Tue, 6 May 2008 16:59:40 -0400 (EDT)
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Daniel Walker <dwalker@...sta.com>
cc:	Thomas Gleixner <tglx@...utronix.de>,
	Sven-Thorsten Dietrich <sven@...bigcorporation.com>,
	Remy Bohmer <linux@...mer.net>,
	LKML <linux-kernel@...r.kernel.org>,
	RT <linux-rt-users@...r.kernel.org>,
	Jon Masters <jonathan@...masters.org>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: Preempt-RT patch for 2.6.25




On Tue, 6 May 2008, Daniel Walker wrote:
> >
> > But you admit that you didn't do that yourself. This is also the time that
> > we were very busy at working on ftrace and hoping it would make it into
> > 2.6.26 (which it did not). Remember our goal is to get as much as possible
> > into mainline Linux. And getting something from -rt (ftrace) into Linux
> > was the higher priority for me than to go review your series after you
> > tell me that you dropped archs and made changes to the code for the sake
> > of bisectability.
>
> Who am I to dictate priorities to you? However, that's no excuse for
> ignoring something important.

It is only important for merging to a new tree, nothing more here. A
bisectable tree is important to make sure all new enhancements can be
tracked to see which feature broke the kernel. The reason we want it
bisectable in Linux, is when a bug appears we can do a binary search
(git-bisect) to find the culprit.

Doing this with the -rt patch doesn't solve that key issue. The reason why
is that new enhancements to the -rt patch happen in small patches. In
fact, by folding in a bunch of patches, we have just lost that ability!
The -rt patch has a bunch of features that it adds to the kernel. Having
the -rt patch queue bisectable doesn't find the bug that was introduced by
a new change to a patch deep in the queue (caused by folding). Ideally,
and I've done this to an extent, is to have each change as a separate
patch. This can help us find which change caused a bug.

Having the -rt patch queue as a bisectable queue (all 350+ patches) ONLY
HELPS IN FORWARD PORTING TO ANOTHER VERSION OF LINUX! That said, it is
usually the maintainers that do this. I have certain points in the queue
that I make sure can compile and test a new feature, to make sure
something didn't break in the port. I don't need each patch of that
feature bisectable, because the feature in the whole may be broken by
something that happened in upstream.


>
> > I thought my response was good enough to give you a clue that you needed
> > to show that your work didn't make any changes. Making the tree bisectable
> > is a "clean up" not a code design. And with all clean ups, they should not
> > cause any changes to code behaviour. You've been doing kernel development
> > long enough to know this. I'm not about to hold your hand and step you
> > through the proper process of getting things accepted.
>
> Setting a requirement of binary compatibility is un-reasonable. I told
> you it wasn't making functional changes, that combined with a concerned
> review of the code should have been enough.

I told you what to do, and so did Thomas. It's not rocket science, and it
is something I would do to make sure as well. Hell, Ingo and Thomas did
this for the x86 merge which was 10 times more complex than making the -rt
queue bisectable.

This is what you should have done.

1) if you needed to make changes to the code (your renaming), do that at
the end with extra patches.

2) save that final result.

3) go through and modify all the patches to your hearts content, to get a
bisectable queue out the door.

4) compare the end result with the result from step 2.

What's so fscking difficult about the above?????

If Ingo and Thomas told Linus to just accept the x86 merge without doing
this exact thing, and then told Linus that it is his job as maintainer to
review the changes to make sure they are correct, there would have not
been any x86 merge today. In fact, if they had the nerve to do that, they
would have lost all respect in the Linux community.

Guess what? You're doing exactly what I said Ingo and Thomas DID NOT DO!


>
> > If you don't make an effort to work with the maintainers instead of just
> > shoving out a bunch of code and expect us to do the dirty work to make
> > sure its ok, then you might as well give up. Take a lesson from Gregory
> > Haskins. He came out with a lot of changes that were controversial at the
> > time. Instead of shoving his code down our throats, he took our critisms
> > seriously and worked hard to work with us. Now most of his code, even the
> > controversial parts, has been incorporated.
>
> You have a responsibility as a maintainer .. You have to at least look
> at code, and decided exactly how to handle it. You have to decided how
> you want to merge the code, and when to merge the code.

The patch queue is 350+ patches. It is not up to me to go through and sort
out exactly what you did. That would take me a week to look at each
individual patch and know what was in your head. Sorry, that is your
responsibilty to help out here.

>
> Bisection is not an optional feature. It's default acceptable, and it
> should have been first on your merge list.

As I said, the paradigm for the -rt patch is completely different. Having
a bisectable patch queue to go into Linux is to find out where a bug was
introduced. Linux evolves in a simple straight time order of events. This
is not true with the -rt patch. To do this, I would keep all patches
unchanged in the tree, and only add new patches at the end, and never fold
anything. We do this to a point, and after a few releases, we start to
fold patches in that we feel are stable.

Again, having a bisectable -rt tree is to help with forward porting only.
And it doesn't even need to be fully bisectable. Just needs to have
certain points to verify.  Usually a bug is introduced by something that
changed in upstream that gets broken not by one patch in the -rt tree, but
by the new concept that the -rt tree introduces. For example, something
might break because we have interrupts as threads. This isn't found by a
bisectable tree, its found by turning off different configuration options
of the -rt patch.


>
> Instead you didn't want it. You default rejected it. From my perspective
> not wanting to merge something like that is itself unreasonable.

I did not default reject it. I told you what we wanted, and you made
absolutely no effort in complying. So in return, I made absolutely no
effort in looking at what you did.

>
> How is discussing this shoving my code down your throat?


Because you refuse to listen to what we say, and you keep pandering that
your changes are so damn important that we need to forget all our own
principles to listen to the word of Daniel.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists