Re: [patch] CFS scheduler, -v8

[Willy Tarreau]

Hi Ingo,

On Sun, May 06, 2007 at 10:29:11AM +0200, Ingo Molnar wrote:
> 
> * Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> 
> > So the _only_ valid way to handle timers is to
> >  - either not allow wrapping at all (in which case "unsigned" is better, 
> >    since it is bigger)
> >  - or use wrapping explicitly, and use unsigned arithmetic (which is 
> >    well-defined in C) and do something like "(long)(a-b) > 0".
> 
> hm, there is a corner-case in CFS where a fix like this is necessary.
> 
> CFS uses 64-bit values for almost everything, and the majority of values 
> are of 'relative' nature with no danger of overflow. (They are signed 
> because they are relative values that center around zero and can be 
> negative or positive.)

(...)

> -		if (key < entry->fair_key) {
> +		if ((s64)(entry->fair_key - key) > 0) {

Just a hint: while your code here is correct, it is a good practise to
check against < 0 instead, so that if for any reason you sometimes forget
to cast to signed, the compiler will emit a warning stating that the
condition is always false. This would simply become :

-		if (key < entry->fair_key) {
+		if ((s64)(key - entry->fair_key) < 0) {

Just my .02 euros :-)
Willy

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