Re: [PATCH] Avoid overflows in kernel/time.c (version 4)
> diff --git a/kernel/Makefile b/kernel/Makefile > index dfa9695..749825a 100644 > --- a/kernel/Makefile > +++ b/kernel/Makefile > @@ -80,3 +80,11 @@ quiet_cmd_ikconfiggz = IKCFG $@ > targets += config_data.h > $(obj)/config_data.h: $(obj)/config_data.gz FORCE > $(call if_changed,ikconfiggz) > + > +$(obj)/time.o: $(obj)/timeconst.h > + > +quiet_cmd_timeconst = TIMEC $@ > + cmd_timeconst = $(PERL) $< $(CONFIG_HZ) > $@ > +targets += timeconst.h > +$(obj)/timeconst.h: $(src)/timeconst.pl $(wildcard include/config/hz.h) FORCE > + $(call if_changed,timeconst) The prerequisite $(wildcard include/config/hz.h) is not needed. You will run the perl script if: - timeconst.h is missing - if commandline changes (new CONFIG_HZ value) Otherwise it looks OK to me. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Avoid overflows in kernel/time.c (version 4)
When the conversion factor between jiffies and milli- or microseconds is not a single multiply or divide, as for the case of HZ == 300, we currently do a multiply followed by a divide. The intervening result, however, is subject to overflows, especially since the fraction is not simplified (for HZ == 300, we multiply by 300 and divide by 1000). This is exposed to the user when passing a large timeout to poll(), for example. This patch replaces the multiply-divide with a reciprocal multiplication on 32-bit platforms. When the input is an unsigned long, there is no portable way to do this on 64-bit platforms there is no portable way to do this since it requires a 128-bit intermediate result (which gcc does support on 64-bit platforms but may generate libgcc calls, e.g. on 64-bit s390), but since the output is a 32-bit integer in the cases affected, just simplify the multiply-divide (*3/10 instead of *300/1000). The reciprocal multiply used can have off-by-one errors in the upper half of the valid output range. This could be avoided at the expense of having to deal with a potential 65-bit intermediate result. Since the intent is to avoid overflow problems and most of the other time conversions are only semiexact, the off-by-one errors were considered an acceptable tradeoff. At Ralf Baechle's suggestion, this version uses a Perl script to compute the necessary constants. We already have dependencies on Perl for kernel compiles. This does, however, require the Perl module Math::BigInt, which is included in the standard Perl distribution starting with version 5.8.0. In order to support older versions of Perl, include a table of canned constants in the script itself, and structure the script so that Math::BigInt isn't required if pulling values from said table. Running the script requires that the HZ value is available from the Makefile. Thus, this patch also adds the Kconfig variable CONFIG_HZ to the architectures which didn't already have it (alpha, cris, frv, h8300, m32r, m68k, m68knommu, sh64, sparc, v850, and xtensa.) It does *not* resolve a weird case in the sh architecture, where HZ != CONFIG_HZ if CONFIG_SH_WDT is set. Paul Mundt has been contacted about that, and has agreed to fix it up. Signed-off-by: H. Peter Anvin <[EMAIL PROTECTED]> --- Delta from previous version: indentation fixed in timeconst.pl arch/alpha/Kconfig|5 + arch/cris/Kconfig |4 + arch/frv/Kconfig |4 + arch/h8300/Kconfig|4 + arch/m32r/Kconfig |4 + arch/m68k/Kconfig |4 + arch/m68knommu/Kconfig|5 + arch/sh64/Kconfig |6 + arch/sparc/Kconfig|4 + arch/v850/Kconfig |7 + arch/xtensa/Kconfig |4 + include/asm-alpha/param.h | 10 +- include/asm-cris/param.h |2 +- include/asm-frv/param.h |2 +- include/asm-h8300/param.h |2 +- include/asm-m32r/param.h |2 +- include/asm-m68k/param.h |2 +- include/asm-m68knommu/param.h |8 +- include/asm-sh64/param.h |6 +- include/asm-sparc/param.h |2 +- include/asm-v850/anna.h |6 - include/asm-v850/as85ep1.h|6 - include/asm-v850/fpga85e2c.h |6 - include/asm-v850/param.h |3 +- include/asm-v850/rte_cb.h |6 - include/asm-v850/sim.h|5 - include/asm-v850/sim85e2.h|6 - include/asm-xtensa/param.h|2 +- kernel/Makefile |8 + kernel/time.c | 29 +++- kernel/timeconst.pl | 340 + 31 files changed, 431 insertions(+), 73 deletions(-) create mode 100644 kernel/timeconst.pl diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig index 4c002ba..442e4e7 100644 --- a/arch/alpha/Kconfig +++ b/arch/alpha/Kconfig @@ -616,6 +616,11 @@ config VERBOSE_MCHECK_ON Take the default (1) unless you want more control or more info. +config HZ + int + default 1200 if ALPHA_RAWHIDE + default 1024 + source "drivers/pci/Kconfig" source "drivers/eisa/Kconfig" diff --git a/arch/cris/Kconfig b/arch/cris/Kconfig index 222da15..fcc6a9e 100644 --- a/arch/cris/Kconfig +++ b/arch/cris/Kconfig @@ -55,6 +55,10 @@ config CRIS bool default y +config HZ + int + default 100 + source "init/Kconfig" menu "General setup" diff --git a/arch/frv/Kconfig b/arch/frv/Kconfig index 43153e7..57bdf2d 100644 --- a/arch/frv/Kconfig +++ b/arch/frv/Kconfig @@ -57,6 +57,10 @@ config ARCH_HAS_ILOG2_U64 bool default y +config HZ + int + default 1000 + mainmenu "Fujitsu FR-V Kernel Configuration" source "init/Kconfig" diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig index ff6a871..8a40a6f 100644 --- a/arch/h8300/Kconfig +++ b/arch/h8300/Kconfig @@ -79,6 +79,10 @@ config PCI bool default n +config HZ + int + default
[PATCH] Avoid overflows in kernel/time.c (version 4)
When the conversion factor between jiffies and milli- or microseconds is not a single multiply or divide, as for the case of HZ == 300, we currently do a multiply followed by a divide. The intervening result, however, is subject to overflows, especially since the fraction is not simplified (for HZ == 300, we multiply by 300 and divide by 1000). This is exposed to the user when passing a large timeout to poll(), for example. This patch replaces the multiply-divide with a reciprocal multiplication on 32-bit platforms. When the input is an unsigned long, there is no portable way to do this on 64-bit platforms there is no portable way to do this since it requires a 128-bit intermediate result (which gcc does support on 64-bit platforms but may generate libgcc calls, e.g. on 64-bit s390), but since the output is a 32-bit integer in the cases affected, just simplify the multiply-divide (*3/10 instead of *300/1000). The reciprocal multiply used can have off-by-one errors in the upper half of the valid output range. This could be avoided at the expense of having to deal with a potential 65-bit intermediate result. Since the intent is to avoid overflow problems and most of the other time conversions are only semiexact, the off-by-one errors were considered an acceptable tradeoff. At Ralf Baechle's suggestion, this version uses a Perl script to compute the necessary constants. We already have dependencies on Perl for kernel compiles. This does, however, require the Perl module Math::BigInt, which is included in the standard Perl distribution starting with version 5.8.0. In order to support older versions of Perl, include a table of canned constants in the script itself, and structure the script so that Math::BigInt isn't required if pulling values from said table. Running the script requires that the HZ value is available from the Makefile. Thus, this patch also adds the Kconfig variable CONFIG_HZ to the architectures which didn't already have it (alpha, cris, frv, h8300, m32r, m68k, m68knommu, sh64, sparc, v850, and xtensa.) It does *not* resolve a weird case in the sh architecture, where HZ != CONFIG_HZ if CONFIG_SH_WDT is set. Paul Mundt has been contacted about that, and has agreed to fix it up. Signed-off-by: H. Peter Anvin [EMAIL PROTECTED] --- Delta from previous version: indentation fixed in timeconst.pl arch/alpha/Kconfig|5 + arch/cris/Kconfig |4 + arch/frv/Kconfig |4 + arch/h8300/Kconfig|4 + arch/m32r/Kconfig |4 + arch/m68k/Kconfig |4 + arch/m68knommu/Kconfig|5 + arch/sh64/Kconfig |6 + arch/sparc/Kconfig|4 + arch/v850/Kconfig |7 + arch/xtensa/Kconfig |4 + include/asm-alpha/param.h | 10 +- include/asm-cris/param.h |2 +- include/asm-frv/param.h |2 +- include/asm-h8300/param.h |2 +- include/asm-m32r/param.h |2 +- include/asm-m68k/param.h |2 +- include/asm-m68knommu/param.h |8 +- include/asm-sh64/param.h |6 +- include/asm-sparc/param.h |2 +- include/asm-v850/anna.h |6 - include/asm-v850/as85ep1.h|6 - include/asm-v850/fpga85e2c.h |6 - include/asm-v850/param.h |3 +- include/asm-v850/rte_cb.h |6 - include/asm-v850/sim.h|5 - include/asm-v850/sim85e2.h|6 - include/asm-xtensa/param.h|2 +- kernel/Makefile |8 + kernel/time.c | 29 +++- kernel/timeconst.pl | 340 + 31 files changed, 431 insertions(+), 73 deletions(-) create mode 100644 kernel/timeconst.pl diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig index 4c002ba..442e4e7 100644 --- a/arch/alpha/Kconfig +++ b/arch/alpha/Kconfig @@ -616,6 +616,11 @@ config VERBOSE_MCHECK_ON Take the default (1) unless you want more control or more info. +config HZ + int + default 1200 if ALPHA_RAWHIDE + default 1024 + source drivers/pci/Kconfig source drivers/eisa/Kconfig diff --git a/arch/cris/Kconfig b/arch/cris/Kconfig index 222da15..fcc6a9e 100644 --- a/arch/cris/Kconfig +++ b/arch/cris/Kconfig @@ -55,6 +55,10 @@ config CRIS bool default y +config HZ + int + default 100 + source init/Kconfig menu General setup diff --git a/arch/frv/Kconfig b/arch/frv/Kconfig index 43153e7..57bdf2d 100644 --- a/arch/frv/Kconfig +++ b/arch/frv/Kconfig @@ -57,6 +57,10 @@ config ARCH_HAS_ILOG2_U64 bool default y +config HZ + int + default 1000 + mainmenu Fujitsu FR-V Kernel Configuration source init/Kconfig diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig index ff6a871..8a40a6f 100644 --- a/arch/h8300/Kconfig +++ b/arch/h8300/Kconfig @@ -79,6 +79,10 @@ config PCI bool default n +config HZ + int + default 100 + source
Re: [PATCH] Avoid overflows in kernel/time.c (version 4)
diff --git a/kernel/Makefile b/kernel/Makefile index dfa9695..749825a 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -80,3 +80,11 @@ quiet_cmd_ikconfiggz = IKCFG $@ targets += config_data.h $(obj)/config_data.h: $(obj)/config_data.gz FORCE $(call if_changed,ikconfiggz) + +$(obj)/time.o: $(obj)/timeconst.h + +quiet_cmd_timeconst = TIMEC $@ + cmd_timeconst = $(PERL) $ $(CONFIG_HZ) $@ +targets += timeconst.h +$(obj)/timeconst.h: $(src)/timeconst.pl $(wildcard include/config/hz.h) FORCE + $(call if_changed,timeconst) The prerequisite $(wildcard include/config/hz.h) is not needed. You will run the perl script if: - timeconst.h is missing - if commandline changes (new CONFIG_HZ value) Otherwise it looks OK to me. Sam -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
