[lkml]   [2011]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: Linux 3.1-rc9
On Thu, Oct 13, 2011 at 4:25 PM, Simon Kirby <> 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 <>

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?


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.


 \ /
  Last update: 2011-10-17 03:43    [W:0.198 / U:11.300 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site