svn commit: r253077 - head/sys/kern

[Bruce Evans]

On Tue, 9 Jul 2013, Andriy Gapon wrote:

> Log:
>  should_yield: protect from td_swvoltick being uninitialized or too stale
>
>  The distance between ticks and td_swvoltick should be calculated as
>  an unsigned number.  Previously we could end up comparing a negative
>  number with hogticks in which case should_yield() would give incorrect
>  answer.
>
>  We should probably ensure that td_swvoltick is properly initialized.
>
>  Sponsored by:	HybridCluster
>  MFC after:	5 days
>
> Modified:
>  head/sys/kern/kern_synch.c
>
> Modified: head/sys/kern/kern_synch.c
> ==============================================================================
> --- head/sys/kern/kern_synch.c	Tue Jul  9 08:59:39 2013	(r253076)
> +++ head/sys/kern/kern_synch.c	Tue Jul  9 09:01:44 2013	(r253077)
> @@ -581,7 +581,7 @@ int
> should_yield(void)
> {
>
> -	return (ticks - curthread->td_swvoltick >= hogticks);
> +	return ((unsigned int)(ticks - curthread->td_swvoltick) >= hogticks);
> }

Hrmph.  Perhaps it should be calculated as an unsigned number, but
this calculates it as a signed number, with undefined behaviour if
overflow occurs, and then bogusly casts the signed number to unsigned.

It also has a style bug in the cast -- "unsigned int" is verbose, and
even "unsigned" is too long, so it is spelled "u_int" in the kernel.

Next, after the cast the operand types are mismatched, and compilers
could reasonably warn about the possible sign extension bugs from this.
Compilers do warn about the mismatch for "i < size_t(foo)" in loops.
This warning is so annoying and it is not normally produced when both
the operands are variables.  Many "fixes" for this warning make sign
extension bugs worse by casting changing the type of the variable
instead of casting the size_t.

Bruce