lkml.org 
[lkml]   [2009]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh
Date
On Wednesday 09 December 2009 09:45:37 Michal Marek wrote:
> [CC hpa who wrote the timeconst.pl script]
>
> On 8.12.2009 10:19, Rob Landley wrote:
> > From: Rob Landley <rob@landley.net>
> >
> > Replace kernel/timeconst.pl with kernel/timeconst.sh. The new shell
> > script is much simpler, about 1/4 the size, and runs on Red Hat 9 from
> > 2003.
>
> I tried the shell script with the precomputed values in timeconst.pl and
> it gave me different results than the perl version for 250 and 1000:

You're right.

They're functionally equivalent (due to the relationship between MUL32 and
SHR32), which is why this code has worked for me and other people for a year
now. The difference is the possibility of an integer overflow if you try to
convert a time period of "0xffffffff".

Trivial fix:

--- a/sources/patches/linux-2.6.25-rc1-noperl.patch Tue Dec 08 20:22:53 2009
-0600
+++ b/sources/patches/linux-2.6.25-rc1-noperl.patch Wed Dec 09 17:28:26 2009
-0600
@@ -507,7 +507,7 @@
+
+ # Keep increasing $SHIFT until we've got 32 bits.
+
-+ [ $MUL32 -gt $(( 1 << 31 )) ] && break
++ [ $MUL32 -ge $(( 1 << 31 )) ] && break
+ SHIFT=$(( $SHIFT + 1 ))
+ done
+ MUL32=$( printf %x $MUL32 )
As long as MUL32 fits in 32 bits than you can multiply it by another 32 bit
number without overflow, and that's probably all the kernel's enforcing.

> #define HZ_TO_MSEC_NUM U64_C(4)
> #define HZ_TO_MSEC_DEN U64_C(1)
> -#define MSEC_TO_HZ_MUL32 U64_C(0x80000000)
> -#define MSEC_TO_HZ_ADJ32 U64_C(0x180000000)
> -#define MSEC_TO_HZ_SHR32 33
> +#define MSEC_TO_HZ_MUL32 U64_C(0x100000000)
> +#define MSEC_TO_HZ_ADJ32 U64_C(0x300000000)
> +#define MSEC_TO_HZ_SHR32 34
> #define MSEC_TO_HZ_NUM U64_C(1)
> #define MSEC_TO_HZ_DEN U64_C(4)
> #define HZ_TO_USEC_MUL32 U64_C(0xfa000000)

The same.

> and with HZ=1000:
> --- perl
> +++ bash
> @@ -8,14 +8,14 @@
> #error "kernel/timeconst.h has the wrong HZ value!"
> #endif
>
> -#define HZ_TO_MSEC_MUL32 U64_C(0x80000000)
> +#define HZ_TO_MSEC_MUL32 U64_C(0x100000000)
> #define HZ_TO_MSEC_ADJ32 U64_C(0x0)
> -#define HZ_TO_MSEC_SHR32 31
> +#define HZ_TO_MSEC_SHR32 32

And again.

> #define HZ_TO_MSEC_NUM U64_C(1)
> #define HZ_TO_MSEC_DEN U64_C(1)
> -#define MSEC_TO_HZ_MUL32 U64_C(0x80000000)
> +#define MSEC_TO_HZ_MUL32 U64_C(0x100000000)
> #define MSEC_TO_HZ_ADJ32 U64_C(0x0)
> -#define MSEC_TO_HZ_SHR32 31
> +#define MSEC_TO_HZ_SHR32 32

Same thing.

> You're trying to avoid the build dependency on Perl. What about adding a
> timeconst.h_shipped with the precomputed values from timeconst.pl:

Been there, done that. My first patch (way back for 2.6.25) took that
approach:

http://landley.net/hg/hgwebdir.cgi/firmware/file/a791ca629d9c/sources/patches/linux-2.6.25-
rc1-noperl.patch

But it turns out various non-x86 targets (such as ARM OMAP) allow HZ to be
specified by an entry field in the config file, into which the user can type a
range of numbers. See this post from last year for details:

http://lists.impactlinux.com/pipermail/firmware-impactlinux.com/2008-
December/000022.html
This is why reducing the perl version to just the precomputed constants
wouldn't work either. (They're there so that you only need to install a
random cpan library when surprised by a build break on non-x86 machines.)

> > + echo
> > + echo "#endif /* __KERNEL_TIMECHONST_H */"
>
> ^
>
> Should be "__KERNEL_TIMECONST_H".

Yeah, Joe Perches pointed that out to me off-list. It's just a comment so I
didn't resubmit the patch for that, but I've fixed my local version already.

Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds


\
 
 \ /
  Last update: 2009-12-10 00:43    [W:0.074 / U:0.276 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site