Messages in this thread | | | Date | Mon, 14 May 2007 12:25:02 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: [PATCH] timekeeping fix mismerge |
| |
On Mon, 14 May 2007, Thomas Gleixner wrote: > > The time keeping code move to kernel/time/timekeeping.c broke the > clocksource resume logic patch. Fix it up and move the > clocksource_resume() call to the appropriate place.
Yeah, looks obvious enough. It had just enough context in the *wrong* place, that the patch would continue to apply if you used the default (insane) GNU patch semantics, because GNU patch defaults to --fuzz=2, and is thus ok if it can find even just a single line around the patch that is valid.
So it looks like Andrew's patch-application scripts happily added the "clocksource_resume()" to an insane place, because the one-line context was
> > + clocksource_resume(); > + > write_seqlock_irqsave(&xtime_lock, flags);
ie an empty line and a single write_seqlock_irqsave(). And those two lines could be found elsewhere in the wrong file.
There's a real reason I consider GNU patch defaults to be totally insane.
I personally use much stricter rules, but what happens is that Andrew's scripts will apply the patch to the wrong place, and then the diff gets *re-generated*, so by the time I see it, I see a nice diff that applies with no fuzz at all.
I don't think re-generating the diff is wrong, and in fact I think you have to do it, but I think Andrew should use "--fuzz=0" or at least "--fuzz=1" instead of the default 2. Yeah, it obviously causes more patch application failures, and it can be very irritating if *most* of those patches would have applied cleanly and correctly with --fuzz=2, but --fuzz=2 really is very dangerous. It literally just needed two lines to match in the wrong place (and as mentioned, those two lines can be trivial, like in the example - totally empty, even!)
Linus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |