Re: [git pull] core fixes

Nick Piggin
2008-08-14T04:45:27Z

On Tuesday 12 August 2008 18:05, Nick Piggin wrote:
> On Tuesday 12 August 2008 16:13, Nick Piggin wrote:
> > On Tuesday 12 August 2008 08:20, Ingo Molnar wrote:
> > > Nick Piggin (1):
> > > generic-ipi: fix stack and rcu interaction bug in
> > > smp_call_function_mask()
> >
> > I'm still not 100% sure that I have this patch right... I might have seen
> > a lockup trace implicating the smp call function path... which may have
> > been due to some other problem or a different bug in the new call
> > function code, but if some more people can take a look at it before
> > merging?
>
> OK indeed it did have a couple of bugs. Firstly, I wasn't freeing the
> data properly in the alloc && wait case. Secondly, I wasn't resetting
> CSD_FLAG_WAIT in the for each cpu loop (so only the first CPU would
> wait).
>
> After those fixes, the patch boots and runs with the kmalloc commented
> out (so it always executes the slowpath).

Hmm, thanks for merging this Ingo, but it didn't get upstream very
nicely (first was merged the one totally broken patch I didn't even
really test let alone propose for inclusion!). Then some window of
other patches, then the incremental patch to fix it.

I know it probably is easiest for your "vacuum the mailing list" mode
of workflow. And it's not that I don't appreciate it, but what I care
about most is the end result and that is suboptimal in this case.

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[*].

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.

[*] BTW. it would be nice to have some formal regression: and/or link:
tags in git patches which I think would help that kind of analysis and
also help distros to pick up fixes...

Do you agree? Or if not, can you explain your views?

Thanks,
Nick