lkml.org 
[lkml]   [2008]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [git pull] core fixes
    Date
    On Monday 18 August 2008 15:22, Nick Piggin wrote:
    > On Friday 15 August 2008 22:58, Ingo Molnar wrote:
    > > * Nick Piggin <nickpiggin@yahoo.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.



    \
     
     \ /
      Last update: 2008-08-18 08:19    [W:6.651 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site