Message-Id: <200808181522.50563.nickpiggin@yahoo.com.au>
Date:	Mon, 18 Aug 2008 15:22:50 +1000
From:	Nick Piggin <nickpiggin@...oo.com.au>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [git pull] core fixes

On Friday 15 August 2008 22:58, Ingo Molnar wrote:
> * Nick Piggin <nickpiggin@...oo.com.au> wrote:

> > The argument that we lose the "thought process" of coming up with a
> > correct patch I don't really buy into either. We want to document the
> > thought process of well thought out, well reviewed and tested patches
> > -- if they get merged and subsequently found to be broken, it is
> > really nice to be able to look back at how and why they went wrong[*].
>
> generally i agree and replace patches - but in this case i went for the
> delta because -rc3 was imminent and the previous patch was already
> well-tested with practical workloads, even though broken in the
> slowpath.

But the patch author in this case hadn't proposed it as a fix or
added his SOB.

> Also, in terms of judging risks, it was easier to look at the 
> delta between the two commits and say "that obviously cannot make it
> worse than the current code". But it's all a special-case really.

Which delta do you mean? My first patch did make the current code worse.
The delta between that and the corrected one fairly obviously didn't
make it worse because my first patch broke it so badly -- not something
I think needs to be reflected in upstream changeset.


> > But merging bits and pieces of such raw patches IMO just adds too much
> > noise to the tree, and breaks bisection too easily. In the case of my
> > patch, the kernel will still build and mostly run, but that is
> > actually even a worse way to break the biesection if you are hunting
> > for some obscure and hard to reproduce bug.
>
> Note that the two commits were kept together tightly so the chance of
> bisection going in the middle of it _and_ hitting the obscure slowpath
> is reasonably small.

Note I hit the obscure slowpath numerous times while testing other
patches. It wasn't particularly difficult.


> What is wrong is to keep them apart (say merge a 
> full upstream release between them) and break bisection on a wide basis.

Slipperly slope, right? It is slippery in multiple ways. How "obscure"
and rare does the problem have to be? How large a window is tolerable
between broken patch and correction? How many people are allowed to
commit this type of broken patch? (because if everybody does then the
kernel is always broken regardless if how soon they are corrected).

It is far better just to avoid the whole issue and do the right thing
and not merge commits like this, than to have a broken tree and justify
it because the commits are close together.


> Btw., we could add "dont bisect into this other commit" markers to later
> commits. Bisection is a relatively slow operation anyway, so checking a
> graph of bisection markers would be within its reach without causing any
> practical slowdown of bisection. (I think.)

Right, things like that could easily make sense for inevitable cases
where a patch makes it upstream that is later found out to be badly
broken. But we don't have to make it worse by merging known to be broken
patches even if we later soon merge the fix.

You didn't at all point to any *upsides* of your merging in this way, so
I assume there are none?

--
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