* [TRIVIAL patch] 2.6.2-rc2 Remove compile warnings from timer.o
@ 2004-02-11 0:39 Darren Williams
2004-02-11 0:55 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Darren Williams @ 2004-02-11 0:39 UTC (permalink / raw)
To: Linux Kern; +Cc: akpm
To allow the ia64 simulator to run correctly on slower machines
the value of HZ has been defined as 32, this has introduced a compiler
warning (not that much of issue):
kernel/timer.c: In function `second_overflow':
kernel/timer.c:589: warning: right shift count is negative
kernel/timer.c:592: warning: right shift count is negative
I asked if the value of HZ could be increased for the simulator in
include/asm-ia64/param.h,
which was rejected for reasons below.
Is the patch below acceptable.
Darren
On Tue, 10 Feb 2004, David Mosberger wrote:
>
> Darren> -# define HZ 32
> Darren> +# define HZ 96
>
> Doesn't strike me as the correct fix: looking at timex.h, clearly HZ
> values as low as 12Hz are supposed to be supported. Perhaps a better
> fix would be to change timer.c to verify that
> SHIFT_SCALE<=SHIFT_USEC+SHIFT_HZ before attempting to do the shifting
> (via a CPP #if, of course).
>
> Also, I'm a bit reluctant to increase the HZ value for the simulator,
> because too large a value will cause the simulated kernel to fail to
> make forward progress on slow host machines.
I thouht that this might be the case, though the fisrt attempt modified
the smallest amount of source.
> --david
Hope the attached is suitable.
Darren
--------------------------------------------------
Darren Williams <dsw AT gelato.unsw.edu.au>
Gelato@UNSW <www.gelato.unsw.edu.au>
--------------------------------------------------
--- linux-2.6.2-rc2.orig/kernel/timer.c 2004-02-11 10:45:02.000000000 +1100
+++ linux-2.6.2-rc2/kernel/timer.c 2004-02-11 10:01:26.000000000 +1100
@@ -584,12 +584,22 @@ static void second_overflow(void)
STA_PPSWANDER | STA_PPSERROR);
}
ltemp = time_freq + pps_freq;
+
+#if (SHIFT_SCALE <= (SHIFT_USEC + SHIFT_HZ))
if (ltemp < 0)
time_adj -= -ltemp >>
(SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE);
else
time_adj += ltemp >>
(SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE);
+
+#else /* (SHIFT_SCALE > (SHIFT_USEC + SHIFT_HZ)) */
+ if (ltemp < 0)
+ time_adj -= -ltemp >> SHIFT_SCALE;
+ else
+ time_adj += ltemp >> SHIFT_SCALE;
+
+#endif /* (SHIFT_SCALE > (SHIFT_USEC + SHIFT_HZ)) */
#if HZ == 100
/* Compensate for (HZ==100) != (1 << SHIFT_HZ).
--------------------------------------------------
Darren Williams <dsw AT gelato.unsw.edu.au>
Gelato@UNSW <www.gelato.unsw.edu.au>
--------------------------------------------------
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [TRIVIAL patch] 2.6.2-rc2 Remove compile warnings from timer.o
2004-02-11 0:39 [TRIVIAL patch] 2.6.2-rc2 Remove compile warnings from timer.o Darren Williams
@ 2004-02-11 0:55 ` Andrew Morton
2004-02-11 1:15 ` Darren Williams
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2004-02-11 0:55 UTC (permalink / raw)
To: Darren Williams; +Cc: linux-kernel, akpm
Darren Williams <dsw@gelato.unsw.edu.au> wrote:
>
>
> To allow the ia64 simulator to run correctly on slower machines
> the value of HZ has been defined as 32, this has introduced a compiler
> warning (not that much of issue):
> kernel/timer.c: In function `second_overflow':
> kernel/timer.c:589: warning: right shift count is negative
> kernel/timer.c:592: warning: right shift count is negative
>
> I asked if the value of HZ could be increased for the simulator in
> include/asm-ia64/param.h,
> which was rejected for reasons below.
Well we really should be able to support values lower than 32 anyway - just
from a quality-of-code point of view.
I do recall once spying a piece of code which would cause a div-by-zero if
HZ was set to less than 50. These things happen.
> ltemp = time_freq + pps_freq;
> +
> +#if (SHIFT_SCALE <= (SHIFT_USEC + SHIFT_HZ))
> if (ltemp < 0)
> time_adj -= -ltemp >>
> (SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE);
> else
> time_adj += ltemp >>
> (SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE);
> +
> +#else /* (SHIFT_SCALE > (SHIFT_USEC + SHIFT_HZ)) */
> + if (ltemp < 0)
> + time_adj -= -ltemp >> SHIFT_SCALE;
> + else
> + time_adj += ltemp >> SHIFT_SCALE;
> +
> +#endif /* (SHIFT_SCALE > (SHIFT_USEC + SHIFT_HZ)) */
You should lose the ifdefs. Just do:
if (SHIFT_SCALE <= (SHIFT_USEC + SHIFT_HZ)) {
if (ltemp < 0)
time_adj -= -ltemp >>
(SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE);
else
time_adj += ltemp >>
(SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE);
}
...
because
a) It is not revolting and
b) The compiler checks the unused code for you, then throws it away.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [TRIVIAL patch] 2.6.2-rc2 Remove compile warnings from timer.o
2004-02-11 0:55 ` Andrew Morton
@ 2004-02-11 1:15 ` Darren Williams
0 siblings, 0 replies; 3+ messages in thread
From: Darren Williams @ 2004-02-11 1:15 UTC (permalink / raw)
To: Linux Kern
On Tue, 10 Feb 2004, Andrew Morton wrote:
> You should lose the ifdefs. Just do:
>
> if (SHIFT_SCALE <= (SHIFT_USEC + SHIFT_HZ)) {
> if (ltemp < 0)
> time_adj -= -ltemp >>
> (SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE);
> else
> time_adj += ltemp >>
> (SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE);
> }
> ...
>
> because
>
> a) It is not revolting and
>
> b) The compiler checks the unused code for you, then throws it away.
>
However this does not remove the compile warnings.
Darren
--------------------------------------------------
Darren Williams <dsw AT gelato.unsw.edu.au>
Gelato@UNSW <www.gelato.unsw.edu.au>
--------------------------------------------------
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2004-02-11 1:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-02-11 0:39 [TRIVIAL patch] 2.6.2-rc2 Remove compile warnings from timer.o Darren Williams
2004-02-11 0:55 ` Andrew Morton
2004-02-11 1:15 ` Darren Williams
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox