Re: Linux 3.1-rc9

[Linus Torvalds]

On Thu, Oct 13, 2011 at 4:25 PM, Simon Kirby <sim@xxxxxxxxxx> wrote:
>
> Looks good. No hangs or crashes for two days on any of them running
> 3.1-rc9 plus this patch. Not sure if you want to deuglify it, but it
> seems to work...
>
> Tested-by: Simon Kirby <sim@xxxxxxxxxx>

Peter, what's the status of this one?

Quite frankly, I personally consider it to be broken - why are we
introducing this new lock for this very special thing? A spinlock to
protect a *single* word of counter seems broken.

It seems more likely that the real bug is that kernel/sched_stats.h
currently takes cputimer->lock without disabling interrupts. Everybody
else uses irq-safe locking, why would sched_stats.h not need that?

However, I don't see why that spinlock is needed at all. Why aren't
those fields just atomics (or at least just "sum_exec_runtime")? And
why does "cputime_add()" exist at all? It seems to always be just a
plain add, and nothing else would seem to ever make sense *anyway*?

In other words, none of that code makes any sense to me at all. And
the patch in question that fixes a hang for Simon seems to make it
even worse. Can somebody explain to me why it looks that crappy?

Please?

That stupid definition of cputime_add() has apparently existed as-is
since it was introduced in 2005. Why do we have code like this:

    times->utime = cputime_add(times->utime, t->utime);

instead of just

    times->utime += t->utime;

which seems not just shorter, but more readable too? The reason is not
some type safety in the cputime_add() thing, it's just a macro.

Added Martin and Ingo to the discussion - Martin because he added that
cputime_add in the first place, and Ingo because he gets the most hits
on kernel/sched_stats.h. Guys - you can see the history on lkml.

                                 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/