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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • Success! was Re: tvtime audio vs pcHDTV-3000 card and pvHDTV-1.6 software
    ... On Wednesday 16 March 2005 20:15, Gene Heskett wrote: ... > that a diff actually outputs only the src code differences, ... nor in that simple little 10 line patch that ... >Unforch, the 2.6.11 plain tree has not, in this case been built yet ...
    (Linux-Kernel)
  • Re: how to compile and install a new driver
    ... Warren Block wrote: ... but you'd better include enough info so that they could make FreeBSD-stype diffs: diff has the unfortunate default of making an output that is compatible with ed. ... This supplies extremely little information to use, in case the file you're trying to patch with that diff has changed, and is also damned hard for mere humans to understand. ... then you can compile. ...
    (freebsd-questions)
  • [PATCH][CFT] mm swapping improvements
    ... Nikita's patches and one of my own, and backs out the RSS limit patch ... diff -puN include/linux/mmzone.h~rollup include/linux/mmzone.h ... -int FASTCALL); ...
    (Linux-Kernel)
  • Re: Reading and applying patches
    ... It's worth mentioning that patch is pretty forgiving. ... text up to the actual patch data (the output of a diff command, ... often the "old" tree will be inside ... Commands like "cvs diff" will generate diff output, ...
    (Fedora)
  • Re: TCP listenall
    ... of the patch is at the end of this email. ... retrieving revision 1.40 ... diff -u -r1.370 tcp_input.c ...
    (freebsd-net)