Re: [git pull] cpus4096 fixes

[Nick Piggin]

On Monday 28 July 2008 16:34, Rusty Russell wrote:
> On Monday 28 July 2008 13:06:36 Andrew Morton wrote:
> > On Mon, 28 Jul 2008 10:42:12 +1000 Rusty Russell <rusty@xxxxxxxxxxxxxxx>
>
> wrote:
> > > The 4k CPU patches have been sliding in without review up until now.
> >
> > wot?
>
> This surprises you?  I stumbled across the cpumask_of_cpu() bug because I
> happened to want it for stop_machine and read the damned code.  But it lead
> me to the surrounding code, which is pretty questionable.  An arch-specific
> map, rather than depending on NR_CPUS?  Adding set_cpus_allowed_ptr()
> instead of changing set_cpus_allowed()?  Macros which declare things and
> may or may not do an allocation/free?  Finally a patch so horrifically ugly
> that it can't be ignored any more gets all the way to Linus.
>
> Overall, it seems like an attempt to sneak in gradual workarounds for
> cpumasks on the stack, rather than a coherent plan.  I understand the
> temptation to avoid an "are we prepared to pay this price for large
> NR_CPUS?" discussion, but we need it anyway.
>
> And that's what I call "review".

Well, I'm not talking about this case specifically, but lots of
people I see these days tend to "review" code without doing a lot
of high level critical thinking (is the overall design any good,
could it be better or completely different, are the tradeoffs
worthwhile etc).

Don't get me wrong, the lower level spot-the-bug and coding style
etc reviews are indespensible too...

Unfortunately the development process isn't exactly conducive to
that higher level kind of review... there are pros and cons to
this, it's not all bad...

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