Message-Id: <200808181617.14671.nickpiggin@yahoo.com.au>
Date:	Mon, 18 Aug 2008 16:17:14 +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 Monday 18 August 2008 15:22, Nick Piggin wrote:
> 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.

OK, Ingo pointed out that his first pull request did not contain both
the initial broken fix and the corrected delta. I thought this is what
happened but I was mistaken about that.

I still didn't actually have much confidence in the patch and didn't
want it to be merged. But I maybe didn't make that clear.

So if this is a one off unfortunate event then yeah it is nothing to
get too worked up about.

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