* Re: [PATCH 0/3] clean out Perl from build system. [not found] <200912080317.08254.rob@landley.net> @ 2009-12-08 10:23 ` Michal Marek 2009-12-08 15:08 ` Rob Landley [not found] ` <200912080319.30679.rob@landley.net> 1 sibling, 1 reply; 12+ messages in thread From: Michal Marek @ 2009-12-08 10:23 UTC (permalink / raw) To: Rob Landley; +Cc: linux-kernel, linux-kbuild On 8.12.2009 10:17, Rob Landley wrote: > Perl was introduced as a build dependency in 2.6.25. My cross-compile > environment does not use perl, so ever since then I've patched it back out. > > With these patches I've personally built kernels for x86, x86_64, arm, mips, > powerpc, sparc, sh4, and m68k. Hi Rob, you should CC linux-kbuild@ on patches like this (added now, the whole series is at http://lkml.org/lkml/2009/12/8/93). Michal ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] clean out Perl from build system. 2009-12-08 10:23 ` [PATCH 0/3] clean out Perl from build system Michal Marek @ 2009-12-08 15:08 ` Rob Landley 0 siblings, 0 replies; 12+ messages in thread From: Rob Landley @ 2009-12-08 15:08 UTC (permalink / raw) To: Michal Marek; +Cc: linux-kernel, linux-kbuild On Tuesday 08 December 2009 04:23:06 Michal Marek wrote: > On 8.12.2009 10:17, Rob Landley wrote: > > Perl was introduced as a build dependency in 2.6.25. My cross-compile > > environment does not use perl, so ever since then I've patched it back > > out. > > > > With these patches I've personally built kernels for x86, x86_64, arm, > > mips, powerpc, sparc, sh4, and m68k. > > Hi Rob, > > you should CC linux-kbuild@ on patches like this (added now, the whole > series is at http://lkml.org/lkml/2009/12/8/93). > > Michal Thanks. LWN said Sam Ravnborg was stepping down, and I haven't kept up with who's in charge now. Rob -- Latency is more important than throughput. It's that simple. - Linus Torvalds ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <200912080319.30679.rob@landley.net>]
* Re: [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh [not found] ` <200912080319.30679.rob@landley.net> @ 2009-12-09 15:45 ` Michal Marek 2009-12-09 23:40 ` Rob Landley 0 siblings, 1 reply; 12+ messages in thread From: Michal Marek @ 2009-12-09 15:45 UTC (permalink / raw) To: Rob Landley; +Cc: linux-kernel, H. Peter Anvin, linux-kbuild [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: with HZ=250: --- 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 29 +#define HZ_TO_MSEC_SHR32 30 #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) 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 #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 #define MSEC_TO_HZ_NUM U64_C(1) #define MSEC_TO_HZ_DEN U64_C(1) #define HZ_TO_USEC_MUL32 U64_C(0xfa000000) $ bash --version GNU bash, version 4.0.33(1)-release (x86_64-suse-linux-gnu) ... You're trying to avoid the build dependency on Perl. What about adding a timeconst.h_shipped with the precomputed values from timeconst.pl: #if HZ == 24 #define ... ... #endif #if HZ == 32 ... #endif ... #ifndef HZ_TO_MSEC_MUL32 # error "Unknown HZ value, please update kernel/timeconst.h using kernel/timeconst.pl" #endif plus some makefile automagic to run the script iff the HZ value isn't precomputed. Then you would only need Perl for exotic HZ configurations. > > Signed-off-by: Rob Landley <rob@landley.net> > -- > > kernel/Makefile | 4 > kernel/timeconst.pl | 378 ------------------------------------------ > kernel/timeconst.sh | 91 ++++++++++ > 3 files changed, 93 insertions(+), 380 deletions(-) > > diff -ruN linux-2.6.30/kernel/Makefile linux-2.6.30.new/kernel/Makefile > --- linux-2.6.30/kernel/Makefile 2009-06-09 22:05:27.000000000 -0500 > +++ linux-2.6.30.new/kernel/Makefile 2009-06-22 14:29:16.000000000 -0500 > @@ -123,7 +123,7 @@ > $(obj)/time.o: $(obj)/timeconst.h > > quiet_cmd_timeconst = TIMEC $@ > - cmd_timeconst = $(PERL) $< $(CONFIG_HZ) > $@ > + cmd_timeconst = $(CONFIG_SHELL) $< $(CONFIG_HZ) $@ > targets += timeconst.h > -$(obj)/timeconst.h: $(src)/timeconst.pl FORCE > +$(obj)/timeconst.h: $(src)/timeconst.sh FORCE > $(call if_changed,timeconst) > diff -ruN linux-2.6.30/kernel/timeconst.pl linux-2.6.30.new/kernel/timeconst.pl > --- linux-2.6.30/kernel/timeconst.pl 2009-06-09 22:05:27.000000000 -0500 > +++ linux-2.6.30.new/kernel/timeconst.pl 1969-12-31 18:00:00.000000000 -0600 > @@ -1,378 +0,0 @@ > -#!/usr/bin/perl > -# ----------------------------------------------------------------------- > -# > -# Copyright 2007-2008 rPath, Inc. - All Rights Reserved > -# > -# This file is part of the Linux kernel, and is made available under > -# the terms of the GNU General Public License version 2 or (at your > -# option) any later version; incorporated herein by reference. > -# > -# ----------------------------------------------------------------------- > -# > - > -# > -# Usage: timeconst.pl HZ > timeconst.h [snip] > diff -ruN linux-2.6.30/kernel/timeconst.sh linux-2.6.30.new/kernel/timeconst.sh > --- linux-2.6.30/kernel/timeconst.sh 1969-12-31 18:00:00.000000000 -0600 > +++ linux-2.6.30.new/kernel/timeconst.sh 2009-06-22 14:29:16.000000000 -0500 > @@ -0,0 +1,148 @@ > +#!/bin/sh > + > +if [ $# -ne 2 ] > +then > + echo "Usage: timeconst.sh HZ FILENAME" > + echo > + echo "Generate a header file with constants for coverting between" > + echo "decimal HZ timer ticks and milisecond or microsecond delays." > + echo > + exit 1 > +fi > + > +HZ=$1 > +shift > +FILENAME=$1 > + > +# Sanity test: even the shell in Red Hat 9 (circa 2003) supported 64 bit math. > + > +if [ $((1 << 32)) -lt 0 ] > +then > + echo "timeconst.sh needs a shell with 64 bit math, such as bash," > + echo "busybox ash, or dash running on a 64 bit host." > + exit 1 > +fi > + > +# If this script exits for any reason before this trap is removed, > +# delete the output file so a partial file won't confuse the build. > + > +trap "rm $FILENAME" EXIT > + > +# Output start of header file > + > +cat > $FILENAME << EOF || exit 1 > +/* Automatically generated by kernel/timeconst.sh */ > +/* Conversion constants for HZ == $HZ */ > + > +#ifndef __KERNEL_TIMECONST_H > +#define __KERNEL_TIMECONST_H > + > +#include <linux/param.h> > +#include <linux/types.h> > + > +#if HZ != $HZ > +#error "kernel/timeconst.h has the wrong HZ value!" > +#endif > + > +EOF > + > +# For both Milliseconds and Microseconds > + > +cat << EOF | > +MSEC 1000 > +USEC 1000000 > +EOF > +while read NAME PERIOD > +do > + # Find greatest common denominator (using Euclid's algorithm) > + > + A=$HZ > + B=$PERIOD > + > + while [ $B -ne 0 ] > + do > + C=$(( $A % $B )) > + A=$B > + B=$C > + done > + > + GCD=$A > + > + # Do this for each direction (HZ_TO_PERIOD and PERIOD_TO_HZ) > + > + for DIRECTION in 0 1 > + do > + if [ $DIRECTION -eq 0 ] > + then > + CONVERT="HZ_TO_${NAME}" > + FROM=$HZ > + TO=$PERIOD > + else > + CONVERT="${NAME}_TO_HZ" > + FROM=$PERIOD > + TO=$HZ > + fi > + > + # Calculate 32 significant bits of MUL32 data. > + > + SHIFT=0 > + while true > + do > + # This can't overflow 64 bit math. Pathological case > + # (TO=1, FROM=1000000) uses around 32+20=52 bits. > + > + MUL32=$(( ( ( $TO << $SHIFT ) + $FROM - 1 ) / $FROM )) > + > + # Keep increasing $SHIFT until we've got 32 bits. > + > + [ $MUL32 -gt $(( 1 << 31 )) ] && break > + SHIFT=$(( $SHIFT + 1 )) > + done > + MUL32=$( printf %x $MUL32 ) > + > + # ADJ32 is just (((FROM/GCD)-1)<<SHIFT)/(FROM/GCD) but this > + # can overflow 64 bit math (examples, HZ=24 or HZ=122). > + # Pathological case could use 32+20+20=72 bits. (And this is > + # the pathological case because a larger $HZ results in a > + # smaller $SHIFT, so even insane HZ>USEC cases should be ok.) > + > + # To get around this, we chop the bottom 32 bits off the > + # calculation and then reassemble it to avoid overflow: > + # 32+64=96, which is > 72. > + > + ADJ32=$(( $FROM / $GCD )) > + if [ $SHIFT -gt 32 ] > + then > + UPPER=$(( ( $ADJ32 - 1 ) << ( $SHIFT - 32 ) )) > + LOWER=$(( ( $UPPER % $ADJ32 ) << 32 )) > + ADJ32=$(( ( ( $UPPER / $ADJ32 ) << 32 ) + ( $LOWER / $ADJ32 ))) > + else > + ADJ32=$(( ( ( $ADJ32 - 1 ) << $SHIFT) / $ADJ32 )) > + fi > + ADJ32=$( printf %x $ADJ32 ) > + > + NUM=$(( $TO / $GCD )) > + DEN=$(( $FROM / $GCD )) > + > + # Output next chunk of header data to file > + > + ( > + echo "#define ${CONVERT}_MUL32 U64_C(0x$MUL32)" && > + echo "#define ${CONVERT}_ADJ32 U64_C(0x$ADJ32)" && > + echo "#define ${CONVERT}_SHR32 $SHIFT" && > + echo "#define ${CONVERT}_NUM U64_C($NUM)" && > + echo "#define ${CONVERT}_DEN U64_C($DEN)" > + ) >> $FILENAME || exit 1 > + done > +done > + > +( > + echo > + echo "#endif /* __KERNEL_TIMECHONST_H */" ^ Should be "__KERNEL_TIMECONST_H". > +) >> $FILENAME || exit 1 > + > +# Don't rm $FILENAME on exit anymore. > + > +trap "" EXIT > + > +exit 0 > Michal ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh 2009-12-09 15:45 ` [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh Michal Marek @ 2009-12-09 23:40 ` Rob Landley 2009-12-09 23:45 ` H. Peter Anvin 2009-12-10 13:52 ` Michal Marek 0 siblings, 2 replies; 12+ messages in thread From: Rob Landley @ 2009-12-09 23:40 UTC (permalink / raw) To: Michal Marek; +Cc: linux-kernel, H. Peter Anvin, linux-kbuild 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh 2009-12-09 23:40 ` Rob Landley @ 2009-12-09 23:45 ` H. Peter Anvin 2009-12-10 1:50 ` Rob Landley 2009-12-10 13:52 ` Michal Marek 1 sibling, 1 reply; 12+ messages in thread From: H. Peter Anvin @ 2009-12-09 23:45 UTC (permalink / raw) To: Rob Landley; +Cc: Michal Marek, linux-kernel, linux-kbuild On 12/09/2009 03:40 PM, Rob Landley wrote: > > 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.) > It's not "a random CPAN library" - it's a core module. -hpa ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh 2009-12-09 23:45 ` H. Peter Anvin @ 2009-12-10 1:50 ` Rob Landley 2009-12-10 1:53 ` H. Peter Anvin 0 siblings, 1 reply; 12+ messages in thread From: Rob Landley @ 2009-12-10 1:50 UTC (permalink / raw) To: H. Peter Anvin; +Cc: Michal Marek, linux-kernel, linux-kbuild On Wednesday 09 December 2009 17:45:21 H. Peter Anvin wrote: > On 12/09/2009 03:40 PM, Rob Landley wrote: > > 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.) > > It's not "a random CPAN library" - it's a core module. Then why cache large quantities of its output and test for its existence? Rob -- Latency is more important than throughput. It's that simple. - Linus Torvalds ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh 2009-12-10 1:50 ` Rob Landley @ 2009-12-10 1:53 ` H. Peter Anvin 0 siblings, 0 replies; 12+ messages in thread From: H. Peter Anvin @ 2009-12-10 1:53 UTC (permalink / raw) To: Rob Landley; +Cc: Michal Marek, linux-kernel, linux-kbuild On 12/09/2009 05:50 PM, Rob Landley wrote: > On Wednesday 09 December 2009 17:45:21 H. Peter Anvin wrote: >> On 12/09/2009 03:40 PM, Rob Landley wrote: >>> 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.) >> >> It's not "a random CPAN library" - it's a core module. > > Then why cache large quantities of its output and test for its existence? > To support archaic Perl versions and Perl binaries without libraries. -hpa ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh 2009-12-09 23:40 ` Rob Landley 2009-12-09 23:45 ` H. Peter Anvin @ 2009-12-10 13:52 ` Michal Marek 2009-12-10 23:16 ` Rob Landley 1 sibling, 1 reply; 12+ messages in thread From: Michal Marek @ 2009-12-10 13:52 UTC (permalink / raw) To: Rob Landley; +Cc: linux-kernel, H. Peter Anvin, linux-kbuild On 10.12.2009 00:40, Rob Landley wrote: > On Wednesday 09 December 2009 09:45:37 Michal Marek wrote: >> 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.) That's why I wrote >> plus some makefile automagic to run the script iff the HZ value isn't >> precomputed. Then you would only need Perl for exotic HZ configurations. E.g. make it ... #elif HZ == 1200 ... #else #include "timeconst_custom.h" #endif and the makefile would run timeconst.pl to generate timeconst_custom.h iff HZ is set to something arbitrary. I don't have a patch for that, but I don't see a fundamental problem with such approach. Michal ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh 2009-12-10 13:52 ` Michal Marek @ 2009-12-10 23:16 ` Rob Landley 2009-12-11 15:31 ` Michal Marek 0 siblings, 1 reply; 12+ messages in thread From: Rob Landley @ 2009-12-10 23:16 UTC (permalink / raw) To: Michal Marek; +Cc: linux-kernel, H. Peter Anvin, linux-kbuild On Thursday 10 December 2009 07:52:44 Michal Marek wrote: > On 10.12.2009 00:40, Rob Landley wrote: > > On Wednesday 09 December 2009 09:45:37 Michal Marek wrote: > >> 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/pat > >ches/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.) > > That's why I wrote > > >> plus some makefile automagic to run the script iff the HZ value isn't > >> precomputed. Then you would only need Perl for exotic HZ configurations. > > E.g. make it > ... > #elif HZ == 1200 > ... > #else > #include "timeconst_custom.h" > #endif > > and the makefile would run timeconst.pl to generate timeconst_custom.h > iff HZ is set to something arbitrary. I don't have a patch for that, but > I don't see a fundamental problem with such approach. I looked at that when I was attempting to update (rather than replace) my original patch. There's a reason I didn't go there. You're suggesting there be two code paths, one maintained by hand and the other just about never tested against bit-rot. You want to add logic to the makefile so it knows when to run the perl script and when not to, meaning the makefile needs its own list of the cached HZ values, which needs to be kept in sync with the header of cached values, and thus the same information has to live in two places. Meanwhile the shell script exists, works, and takes a fraction of a second to run. Using it every build provides a single consistent code path which works the same for everybody. I'm not seeing any advantage in cacheing values that are easy to calculate at compile time. It's just as easy to completely eliminate the perl dependency as to merely mitigate it. > Michal Rob -- Latency is more important than throughput. It's that simple. - Linus Torvalds ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh 2009-12-10 23:16 ` Rob Landley @ 2009-12-11 15:31 ` Michal Marek 2009-12-11 19:13 ` H. Peter Anvin 0 siblings, 1 reply; 12+ messages in thread From: Michal Marek @ 2009-12-11 15:31 UTC (permalink / raw) To: Rob Landley; +Cc: linux-kernel, H. Peter Anvin, linux-kbuild Rob Landley wrote: > On Thursday 10 December 2009 07:52:44 Michal Marek wrote: >> and the makefile would run timeconst.pl to generate timeconst_custom.h >> iff HZ is set to something arbitrary. I don't have a patch for that, but >> I don't see a fundamental problem with such approach. > > I looked at that when I was attempting to update (rather than replace) my > original patch. There's a reason I didn't go there. You're suggesting there > be two code paths, one maintained by hand and the other just about never > tested against bit-rot. OK, that's valid point, indeed. Peter, would you ack Rob's patch with the oneline fix added (http://lkml.org/lkml/2009/12/8/94 plus http://lkml.org/lkml/2009/12/9/435)? Michal ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh 2009-12-11 15:31 ` Michal Marek @ 2009-12-11 19:13 ` H. Peter Anvin 2009-12-12 0:49 ` Rob Landley 0 siblings, 1 reply; 12+ messages in thread From: H. Peter Anvin @ 2009-12-11 19:13 UTC (permalink / raw) To: Michal Marek; +Cc: Rob Landley, linux-kernel, linux-kbuild On 12/11/2009 07:31 AM, Michal Marek wrote: > > OK, that's valid point, indeed. Peter, would you ack Rob's patch with > the oneline fix added (http://lkml.org/lkml/2009/12/8/94 plus > http://lkml.org/lkml/2009/12/9/435)? > I strongly dislike his patch, as he open-codes specific multiprecision arithmetic. This makes it hard for other people to maintain, and makes it prone to errors -- as evidenced by the fact that it didn't even replicate the known-good results. I have made my position clear on this and other patches several times before: I consider it a fool's errand, and a result of a completely pointless crusade to make a particular science fair-type project a wee bit easier. We have already seen real damage caused by it, since people have used awk instead, and have gotten bitten by incompatibilities between awk implementations. As such, no, I will not ack this patch, and will consider myself released of any obligation to maintain the code if this goes in anyway. I would consider acking a C program which does proper multiprecision arithmetic, but I'm also not going to spend my time on it. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh 2009-12-11 19:13 ` H. Peter Anvin @ 2009-12-12 0:49 ` Rob Landley 0 siblings, 0 replies; 12+ messages in thread From: Rob Landley @ 2009-12-12 0:49 UTC (permalink / raw) To: H. Peter Anvin; +Cc: Michal Marek, linux-kernel, linux-kbuild On Friday 11 December 2009 13:13:39 H. Peter Anvin wrote: > On 12/11/2009 07:31 AM, Michal Marek wrote: > > OK, that's valid point, indeed. Peter, would you ack Rob's patch with > > the oneline fix added (http://lkml.org/lkml/2009/12/8/94 plus > > http://lkml.org/lkml/2009/12/9/435)? > > I strongly dislike his patch, as he open-codes specific multiprecision > arithmetic. This makes it hard for other people to maintain, and makes > it prone to errors It's always easier to understand code that you wrote, doubly so with perl. The perl code this is replacing has comments, function names like fmul() and fmuls() with no way to distinguish what they do except by reading their implementation, and half of it is devoting to cacheing the output of the other half for reasons which are never explained in the code. The few comments the perl code does have are sometimes actually misleading. For example, bignum_hex() has the comment "generate a hex value if the result fits in 64 bits; otherwise skip". What would skpping mean in this context? Would it silently emit a broken header, or would it break the build? (My first guess is that "undef" without an expr would be a build break, but maybe this is perl's way of saying "None"? I do know that whatever happened the result wouldn't be a usable kernel, so "skip" is disingenous.) That said, I read through and managed to understand the perl code. But it took me more than a day, and I was motivated by the desire to replace it. > -- as evidenced by the fact that it didn't even > replicate the known-good results. Actually I had noticed the difference back at the time I wrote the script, but the value plus shift were functionally equivalent, and at the time (a year ago) I vaguely recall working through the math (and looking at the kernel code using the output) convinced me that that integer overflow couldn't actually happen for real inputs. But I don't remember why (perhaps the value to be converted is range checked somewhere so ffffffff couldn't actually make it to this code, I don't remember) and thus my version would give more precision to the result (which seemed to be the whole point of the exercise). No point in trying to reconstruct my reasoning from a year ago when it's a one line change to make it non-controversial, and the "value always fits in 32 bits so you can safely multiply by an unsigned int due to LP64" is easier to explain/document anyway. (On the other end of things, I do remember that the admittedly insane HZ of ffffffff would have a GCD of 5 with 1000 and thus not be saved as that. That came up because I was worried an ffffffff value times ffffffff might _round_ up to overflow when adj32 was added. I did attempt to document some of the edge cases in the shell script comments...) > I have made my position clear on this and other patches several times > before: I consider it a fool's errand, I.E. your fundamental objection is to removing perl form the build. You are the one who added this perl to the build system. You're also the one who added perl to all the other projects you maintain (klibc and syslinux I noticed) at approximately the same time, after years of those not requiring perl. I don't know why you suddenly decided perl needed to be everywhere, but the Linux kernel is not the only project you simultaneously applied this philosophy to. I strongly suspect the other objections you've raised in the past two years are a proxy for the objection to removing perl. However, your central objection to removing perl hasn't convinced people like Alan Cox, who called Perl "a bad candidate for our toolchain dependencies" here: http://lkml.indiana.edu/hypermail/linux/kernel/0901.1/02108.html Thus you raise other issues, which seem to be proxies for that fundamental objection. I've tried to address the ones that look like they were actually intended to be addressed, but I note that your current objection is to the something like the seventh submission of this patch, and I believe this is the first time you've raised this particular issue. I believe I first submitted an ancestor of this patch on February 15, 2008. Just this year I've submitted previous versions of this patch (more or less in its current form) on January 2, 2009 (with a resubmit on January 3 in response to feedback), on June 22, and on September 18. And again now. The patch hasn't changed much, your objections to it have. I don't even remember you previously complaining about the output differing. (At a guess, you never actually tried my patch?) But it has been a while, I could easily be forgetting details by now. I'm posting these here because they're useful to other people, not because I expect you to ever stop objecting to them no matter what changes I make. You've made it clear you object to the _goal_ of the patch. The implementation seems to be a side issue. > and a result of a completely > pointless crusade to make a particular science fair-type project a wee > bit easier. Embedded developers' desire to restrict their build environment to something controllable seems to strike you as weird, yes. Would it help to explain that it's the same general theory behind Linux From Scratch making a temporary system, chrooting into it, and then building in there in Chapter 5? Leaking random crap from the host is bad, often in subtle ways you don't immediately catch. Some programs (perl, libtool, syscfg) are champions at leaking context, and to be avoided if at all possible. (I suppose you could always suggest setting up a different build environment for the kernel as for userspace, the way Red Hat used to do with kgcc. Doesn't strike me as a good idea, though.) > We have already seen real damage caused by it, since people > have used awk instead, and have gotten bitten by incompatibilities > between awk implementations. I'm not using AWK, and I'm sticking with POSIX shell constructs that were there in susv3, let alone susv4. Since I wasn't the one proposing any patches extensively using awk (I need the man page open to make awk do anything more complicated than '{print $2}'), and this patch isn't using awk at all, I guess you're once again objecting to the removal of perl in general, not to the patch in front of you. And in doing so, you're providing evidence that I'm not the only one motivated to submit perl removal patches, which might undermine your "particular science fair project" argument a bit. I am motivated to follow up on this. I've been maintaining perl removal patches for a couple years now because I personally need them, but I'm not the only one using these patches, and I've been thanked for tackling this problem by a dozen embedded developers. I wasn't the one who first noticed the perl dependency when it went in back in .25, David Anders notified me of the issue before I'd tested the -rc that introduced it. The most recent guy to say, and I quote, "Thanks for eliminating the perl dependency." to me in private email was Joe Perches (on Tuesday). Embedded developers tend to do stuff off-list. This is because every new kernel breaks random stuff on non-x86 targets. For example, I finally got powerpc's pmac_zilog fixed a few days ago (allowing me to use the serial console on qemu's mac99 target), after a thread stretching back two months (it broke after 2.6.28). The "works for me" fix is here: http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-December/078700.html The most recent arm breakage turned out to be simple (a new CONFIG_MMU symbol showed up between 2.6.31 and 2.6.32 and needed to be switched on), but diagnosing "it doesn't boot, no console messages" is always a pain. The most recent mips breakage was fixed by commit d71789b6fa37c21ce5eb588d279f57904a62e7e2 which I asked to be backported to 2.6.31 in the stable series and Greg KH acknowledged on November 5th. (I'd link to my submission to stable@kernel.org but there's no entry for that in vger lists and googling doesn't seem to come up with a public archive.) That's just in the past month or so. Trying to git bisect each of those problems found huge ranges of "it doesn't even compile" in the repository, because that's the _norm_ for non-x86 targets during development. Most embedded developers don't want to deal with this, so they stay a year or three behind the bleeding edge and only upgrade after crazy people like me have gotten it working. Since even the ones who _don't_ start their messages by apologizing about their English know they'll be flamed or ignored if they raise an issue about an 18 month old kernel here, they develop a fear of posting on linux-kernel at all because it's an "unfriendly" place. (I try to talk them out of it, but it's a cultural thing at this point. The new kernel embedded list hasn't really helped more than 10% of this problem.) I regularly tackle these issues anyway, am generally friendly to newbies on the lists I follow, and apparently have no shame, so people sometimes forward issues to me and hope I'll push them upstream so they don't have to. (And then go away again once they've managed to upgrade to whatever snapshot they'll be using for the next 3 years, leaving me to follow up.) *shrug* I'm used to it. You feed stray issues patches and they wind up adopting you... > As such, no, I will not ack this patch, and will consider myself > released of any obligation to maintain the code if this goes in anyway. I've been maintaining this patch series for 2 years already. I would be happy to maintain my changed version in future if it gets in. I expected that. > I would consider acking a C program which does proper multiprecision > arithmetic, but I'm also not going to spend my time on it. Hmmm, "would consider acking" (not "would ack", not even "could ack"), with the additional caveat that the multiprecision arithmetic must be an unspecified "proper", whatever that means. The "proxy objections" thesis stated above would label this an obvious delaying tactic, but if I promise to call your bluff, then the current patch gets dropped on the floor for another development cycle... Well played, sir. I must plead the 5th for now. > -hpa Rob -- Latency is more important than throughput. It's that simple. - Linus Torvalds ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2009-12-12 0:49 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <200912080317.08254.rob@landley.net>
2009-12-08 10:23 ` [PATCH 0/3] clean out Perl from build system Michal Marek
2009-12-08 15:08 ` Rob Landley
[not found] ` <200912080319.30679.rob@landley.net>
2009-12-09 15:45 ` [PATCH 1/3] Replace kernel/timeconst.pl with kernel/timeconst.sh Michal Marek
2009-12-09 23:40 ` Rob Landley
2009-12-09 23:45 ` H. Peter Anvin
2009-12-10 1:50 ` Rob Landley
2009-12-10 1:53 ` H. Peter Anvin
2009-12-10 13:52 ` Michal Marek
2009-12-10 23:16 ` Rob Landley
2009-12-11 15:31 ` Michal Marek
2009-12-11 19:13 ` H. Peter Anvin
2009-12-12 0:49 ` Rob Landley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).