* [RFC PATCH] math.h: Account for 64-bit division on i386 @ 2025-04-19 11:51 cem 2025-04-20 15:40 ` Guenter Roeck 0 siblings, 1 reply; 6+ messages in thread From: cem @ 2025-04-19 11:51 UTC (permalink / raw) To: linux-kernel Cc: Hans.Holmberg, oe-kbuild-all, hch, lukas, angelogioacchino.delregno, Jonathan.Cameron From: Carlos Maiolino <cem@kernel.org> Building linux on i386 might fail if a 64bit type is passed to mult_fract(). To prevent the failure, use do_div() for the division calculation instead of hardcoding a / b. Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202504181233.F7D9Atra-lkp@intel.com/ --- I'm sending it as a RFC because I didn't to extensive testing on this patch, also I'm not sure if mult_frac() was intended to work on 32-bit only types. If that's the case, perhaps, a new mult_frac64() might be a better idea?! include/linux/math.h | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/include/linux/math.h b/include/linux/math.h index 0198c92cbe3e..05ea853b75b4 100644 --- a/include/linux/math.h +++ b/include/linux/math.h @@ -133,15 +133,16 @@ __STRUCT_FRACT(u32) #undef __STRUCT_FRACT /* Calculate "x * n / d" without unnecessary overflow or loss of precision. */ -#define mult_frac(x, n, d) \ -({ \ - typeof(x) x_ = (x); \ - typeof(n) n_ = (n); \ - typeof(d) d_ = (d); \ - \ - typeof(x_) q = x_ / d_; \ - typeof(x_) r = x_ % d_; \ - q * n_ + r * n_ / d_; \ +#define mult_frac(x, n, d) \ +({ \ + typeof(x) x_ = (x); \ + typeof(n) n_ = (n); \ + typeof(d) d_ = (d); \ + \ + typeof(x_) r = do_div(x_, d_); \ + r *= n_; \ + do_div(r, d_); \ + x_ * n_ + r; \ }) #define sector_div(a, b) do_div(a, b) -- 2.49.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] math.h: Account for 64-bit division on i386 2025-04-19 11:51 [RFC PATCH] math.h: Account for 64-bit division on i386 cem @ 2025-04-20 15:40 ` Guenter Roeck 2025-04-20 17:42 ` Guenter Roeck 2025-04-20 18:01 ` Carlos Maiolino 0 siblings, 2 replies; 6+ messages in thread From: Guenter Roeck @ 2025-04-20 15:40 UTC (permalink / raw) To: cem Cc: linux-kernel, Hans.Holmberg, oe-kbuild-all, hch, lukas, angelogioacchino.delregno, Jonathan.Cameron On Sat, Apr 19, 2025 at 01:51:46PM +0200, cem@kernel.org wrote: > From: Carlos Maiolino <cem@kernel.org> > > Building linux on i386 might fail if a 64bit type is passed to i386 actually builds. Its compiler is probably able to convert the offending mult_frac() without helpers since the divisor is a constant. I see the problem with openrisc and parisc, with gcc 13.3.0. > mult_fract(). To prevent the failure, use do_div() for the division > calculation instead of hardcoding a / b. > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202504181233.F7D9Atra-lkp@intel.com/ > --- > > I'm sending it as a RFC because I didn't to extensive testing on this > patch, also I'm not sure if mult_frac() was intended to work on 32-bit > only types. If that's the case, perhaps, a new mult_frac64() might be a > better idea?! > > include/linux/math.h | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/include/linux/math.h b/include/linux/math.h > index 0198c92cbe3e..05ea853b75b4 100644 > --- a/include/linux/math.h > +++ b/include/linux/math.h > @@ -133,15 +133,16 @@ __STRUCT_FRACT(u32) > #undef __STRUCT_FRACT > > /* Calculate "x * n / d" without unnecessary overflow or loss of precision. */ > -#define mult_frac(x, n, d) \ > -({ \ > - typeof(x) x_ = (x); \ > - typeof(n) n_ = (n); \ > - typeof(d) d_ = (d); \ > - \ > - typeof(x_) q = x_ / d_; \ > - typeof(x_) r = x_ % d_; \ > - q * n_ + r * n_ / d_; \ > +#define mult_frac(x, n, d) \ > +({ \ > + typeof(x) x_ = (x); \ > + typeof(n) n_ = (n); \ > + typeof(d) d_ = (d); \ > + \ > + typeof(x_) r = do_div(x_, d_); \ > + r *= n_; \ > + do_div(r, d_); \ > + x_ * n_ + r; \ > }) > Unfortunately that doesn't work. I get build errors on parisc. In file included from ./arch/parisc/include/generated/asm/div64.h:1, from ./include/linux/math.h:6, from ./include/linux/kernel.h:27, from ./arch/parisc/include/asm/bug.h:5, from ./include/linux/bug.h:5, from ./include/linux/mmdebug.h:5, from ./include/linux/mm.h:6, from mm/page_alloc.c:19: mm/page_alloc.c: In function 'boost_watermark': ./include/asm-generic/div64.h:183:35: error: comparison of distinct pointer types lacks a cast [-Werror] 183 | (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ | ^~ ./include/linux/math.h:142:24: note: in expansion of macro 'do_div' 142 | typeof(x_) r = do_div(x_, d_); \ | ^~~~~~ mm/page_alloc.c:2010:21: note: in expansion of macro 'mult_frac' 2010 | max_boost = mult_frac(zone->_watermark[WMARK_HIGH], That is just one example. It seems to affect all uses of mult_frac(). Guenter ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] math.h: Account for 64-bit division on i386 2025-04-20 15:40 ` Guenter Roeck @ 2025-04-20 17:42 ` Guenter Roeck 2025-04-20 18:05 ` Carlos Maiolino 2025-04-23 7:02 ` Geert Uytterhoeven 2025-04-20 18:01 ` Carlos Maiolino 1 sibling, 2 replies; 6+ messages in thread From: Guenter Roeck @ 2025-04-20 17:42 UTC (permalink / raw) To: cem Cc: linux-kernel, Hans.Holmberg, oe-kbuild-all, hch, lukas, angelogioacchino.delregno, Jonathan.Cameron On Sun, Apr 20, 2025 at 08:40:27AM -0700, Guenter Roeck wrote: > On Sat, Apr 19, 2025 at 01:51:46PM +0200, cem@kernel.org wrote: > > From: Carlos Maiolino <cem@kernel.org> > > > > Building linux on i386 might fail if a 64bit type is passed to > > i386 actually builds. Its compiler is probably able to convert > the offending mult_frac() without helpers since the divisor is > a constant. I see the problem with openrisc and parisc, with > gcc 13.3.0. > > > mult_fract(). To prevent the failure, use do_div() for the division > > calculation instead of hardcoding a / b. > > > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > > Reported-by: kernel test robot <lkp@intel.com> > > Closes: https://lore.kernel.org/oe-kbuild-all/202504181233.F7D9Atra-lkp@intel.com/ > > --- > > > > I'm sending it as a RFC because I didn't to extensive testing on this > > patch, also I'm not sure if mult_frac() was intended to work on 32-bit > > only types. If that's the case, perhaps, a new mult_frac64() might be a > > better idea?! > > > > include/linux/math.h | 19 ++++++++++--------- > > 1 file changed, 10 insertions(+), 9 deletions(-) > > > > diff --git a/include/linux/math.h b/include/linux/math.h > > index 0198c92cbe3e..05ea853b75b4 100644 > > --- a/include/linux/math.h > > +++ b/include/linux/math.h > > @@ -133,15 +133,16 @@ __STRUCT_FRACT(u32) > > #undef __STRUCT_FRACT > > > > /* Calculate "x * n / d" without unnecessary overflow or loss of precision. */ > > -#define mult_frac(x, n, d) \ > > -({ \ > > - typeof(x) x_ = (x); \ > > - typeof(n) n_ = (n); \ > > - typeof(d) d_ = (d); \ > > - \ > > - typeof(x_) q = x_ / d_; \ > > - typeof(x_) r = x_ % d_; \ > > - q * n_ + r * n_ / d_; \ > > +#define mult_frac(x, n, d) \ > > +({ \ > > + typeof(x) x_ = (x); \ > > + typeof(n) n_ = (n); \ > > + typeof(d) d_ = (d); \ > > + \ > > + typeof(x_) r = do_div(x_, d_); \ > > + r *= n_; \ > > + do_div(r, d_); \ > > + x_ * n_ + r; \ > > }) > > > > Unfortunately that doesn't work. I get build errors on parisc. > Turns out the first parameter of do_div needs to be u64, not s64, at least on parisc. Guenter > In file included from ./arch/parisc/include/generated/asm/div64.h:1, > from ./include/linux/math.h:6, > from ./include/linux/kernel.h:27, > from ./arch/parisc/include/asm/bug.h:5, > from ./include/linux/bug.h:5, > from ./include/linux/mmdebug.h:5, > from ./include/linux/mm.h:6, > from mm/page_alloc.c:19: > mm/page_alloc.c: In function 'boost_watermark': > ./include/asm-generic/div64.h:183:35: error: comparison of distinct pointer types lacks a cast [-Werror] > 183 | (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ > | ^~ > ./include/linux/math.h:142:24: note: in expansion of macro 'do_div' > 142 | typeof(x_) r = do_div(x_, d_); \ > | ^~~~~~ > mm/page_alloc.c:2010:21: note: in expansion of macro 'mult_frac' > 2010 | max_boost = mult_frac(zone->_watermark[WMARK_HIGH], > > That is just one example. It seems to affect all uses of > mult_frac(). > > Guenter ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] math.h: Account for 64-bit division on i386 2025-04-20 17:42 ` Guenter Roeck @ 2025-04-20 18:05 ` Carlos Maiolino 2025-04-23 7:02 ` Geert Uytterhoeven 1 sibling, 0 replies; 6+ messages in thread From: Carlos Maiolino @ 2025-04-20 18:05 UTC (permalink / raw) To: Guenter Roeck Cc: linux-kernel, Hans.Holmberg, oe-kbuild-all, hch, lukas, angelogioacchino.delregno, Jonathan.Cameron On Sun, Apr 20, 2025 at 10:42:18AM -0700, Guenter Roeck wrote: > On Sun, Apr 20, 2025 at 08:40:27AM -0700, Guenter Roeck wrote: > > On Sat, Apr 19, 2025 at 01:51:46PM +0200, cem@kernel.org wrote: > > > From: Carlos Maiolino <cem@kernel.org> > > > > > > Building linux on i386 might fail if a 64bit type is passed to > > > > i386 actually builds. Its compiler is probably able to convert > > the offending mult_frac() without helpers since the divisor is > > a constant. I see the problem with openrisc and parisc, with > > gcc 13.3.0. > > > > > mult_fract(). To prevent the failure, use do_div() for the division > > > calculation instead of hardcoding a / b. > > > > > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > > > Reported-by: kernel test robot <lkp@intel.com> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202504181233.F7D9Atra-lkp@intel.com/ > > > --- > > > > > > I'm sending it as a RFC because I didn't to extensive testing on this > > > patch, also I'm not sure if mult_frac() was intended to work on 32-bit > > > only types. If that's the case, perhaps, a new mult_frac64() might be a > > > better idea?! > > > > > > include/linux/math.h | 19 ++++++++++--------- > > > 1 file changed, 10 insertions(+), 9 deletions(-) > > > > > > diff --git a/include/linux/math.h b/include/linux/math.h > > > index 0198c92cbe3e..05ea853b75b4 100644 > > > --- a/include/linux/math.h > > > +++ b/include/linux/math.h > > > @@ -133,15 +133,16 @@ __STRUCT_FRACT(u32) > > > #undef __STRUCT_FRACT > > > > > > /* Calculate "x * n / d" without unnecessary overflow or loss of precision. */ > > > -#define mult_frac(x, n, d) \ > > > -({ \ > > > - typeof(x) x_ = (x); \ > > > - typeof(n) n_ = (n); \ > > > - typeof(d) d_ = (d); \ > > > - \ > > > - typeof(x_) q = x_ / d_; \ > > > - typeof(x_) r = x_ % d_; \ > > > - q * n_ + r * n_ / d_; \ > > > +#define mult_frac(x, n, d) \ > > > +({ \ > > > + typeof(x) x_ = (x); \ > > > + typeof(n) n_ = (n); \ > > > + typeof(d) d_ = (d); \ > > > + \ > > > + typeof(x_) r = do_div(x_, d_); \ > > > + r *= n_; \ > > > + do_div(r, d_); \ > > > + x_ * n_ + r; \ > > > }) > > > > > > > Unfortunately that doesn't work. I get build errors on parisc. > > > > Turns out the first parameter of do_div needs to be u64, not s64, > at least on parisc. Yup, that's true for x86 too, needs to be fixed once we decide which path to take to fix it. > > Guenter > > > In file included from ./arch/parisc/include/generated/asm/div64.h:1, > > from ./include/linux/math.h:6, > > from ./include/linux/kernel.h:27, > > from ./arch/parisc/include/asm/bug.h:5, > > from ./include/linux/bug.h:5, > > from ./include/linux/mmdebug.h:5, > > from ./include/linux/mm.h:6, > > from mm/page_alloc.c:19: > > mm/page_alloc.c: In function 'boost_watermark': > > ./include/asm-generic/div64.h:183:35: error: comparison of distinct pointer types lacks a cast [-Werror] > > 183 | (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ > > | ^~ > > ./include/linux/math.h:142:24: note: in expansion of macro 'do_div' > > 142 | typeof(x_) r = do_div(x_, d_); \ > > | ^~~~~~ > > mm/page_alloc.c:2010:21: note: in expansion of macro 'mult_frac' > > 2010 | max_boost = mult_frac(zone->_watermark[WMARK_HIGH], > > > > That is just one example. It seems to affect all uses of > > mult_frac(). > > > > Guenter ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] math.h: Account for 64-bit division on i386 2025-04-20 17:42 ` Guenter Roeck 2025-04-20 18:05 ` Carlos Maiolino @ 2025-04-23 7:02 ` Geert Uytterhoeven 1 sibling, 0 replies; 6+ messages in thread From: Geert Uytterhoeven @ 2025-04-23 7:02 UTC (permalink / raw) To: Guenter Roeck Cc: cem, linux-kernel, Hans.Holmberg, oe-kbuild-all, hch, lukas, angelogioacchino.delregno, Jonathan.Cameron On Sun, 20 Apr 2025 at 19:42, Guenter Roeck <linux@roeck-us.net> wrote: > On Sun, Apr 20, 2025 at 08:40:27AM -0700, Guenter Roeck wrote: > > On Sat, Apr 19, 2025 at 01:51:46PM +0200, cem@kernel.org wrote: > > > From: Carlos Maiolino <cem@kernel.org> > > > > > > Building linux on i386 might fail if a 64bit type is passed to > > > > i386 actually builds. Its compiler is probably able to convert > > the offending mult_frac() without helpers since the divisor is > > a constant. I see the problem with openrisc and parisc, with > > gcc 13.3.0. > > > > > mult_fract(). To prevent the failure, use do_div() for the division > > > calculation instead of hardcoding a / b. > > > > > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > > > Reported-by: kernel test robot <lkp@intel.com> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202504181233.F7D9Atra-lkp@intel.com/ > > > --- > > > > > > I'm sending it as a RFC because I didn't to extensive testing on this > > > patch, also I'm not sure if mult_frac() was intended to work on 32-bit > > > only types. If that's the case, perhaps, a new mult_frac64() might be a > > > better idea?! > > > > > > include/linux/math.h | 19 ++++++++++--------- > > > 1 file changed, 10 insertions(+), 9 deletions(-) > > > > > > diff --git a/include/linux/math.h b/include/linux/math.h > > > index 0198c92cbe3e..05ea853b75b4 100644 > > > --- a/include/linux/math.h > > > +++ b/include/linux/math.h > > > @@ -133,15 +133,16 @@ __STRUCT_FRACT(u32) > > > #undef __STRUCT_FRACT > > > > > > /* Calculate "x * n / d" without unnecessary overflow or loss of precision. */ > > > -#define mult_frac(x, n, d) \ > > > -({ \ > > > - typeof(x) x_ = (x); \ > > > - typeof(n) n_ = (n); \ > > > - typeof(d) d_ = (d); \ > > > - \ > > > - typeof(x_) q = x_ / d_; \ > > > - typeof(x_) r = x_ % d_; \ > > > - q * n_ + r * n_ / d_; \ > > > +#define mult_frac(x, n, d) \ > > > +({ \ > > > + typeof(x) x_ = (x); \ > > > + typeof(n) n_ = (n); \ > > > + typeof(d) d_ = (d); \ > > > + \ > > > + typeof(x_) r = do_div(x_, d_); \ > > > + r *= n_; \ > > > + do_div(r, d_); \ > > > + x_ * n_ + r; \ > > > }) > > > > > > > Unfortunately that doesn't work. I get build errors on parisc. > > > > Turns out the first parameter of do_div needs to be u64, not s64, > at least on parisc. That is correct: the first parameter must be u64, the second must be u32: https://elixir.bootlin.com/linux/v6.14.3/source/include/asm-generic/div64.h As you must never use open-coded 64-bit divisions in the kernel, simple helpers like mult_frac() can only be used for 32-bit values. For anything involving 64-bit divisions, you must use the helpers from <linux/math64.h>, e.g. mul_u64_u32_div(). https://elixir.bootlin.com/linux/v6.14.3/source/include/linux/math64.h#L257 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] math.h: Account for 64-bit division on i386 2025-04-20 15:40 ` Guenter Roeck 2025-04-20 17:42 ` Guenter Roeck @ 2025-04-20 18:01 ` Carlos Maiolino 1 sibling, 0 replies; 6+ messages in thread From: Carlos Maiolino @ 2025-04-20 18:01 UTC (permalink / raw) To: Guenter Roeck Cc: linux-kernel, Hans.Holmberg, oe-kbuild-all, hch, lukas, angelogioacchino.delregno, Jonathan.Cameron On Sun, Apr 20, 2025 at 08:40:25AM -0700, Guenter Roeck wrote: > On Sat, Apr 19, 2025 at 01:51:46PM +0200, cem@kernel.org wrote: > > From: Carlos Maiolino <cem@kernel.org> > > > > Building linux on i386 might fail if a 64bit type is passed to > > i386 actually builds. Its compiler is probably able to convert > the offending mult_frac() without helpers since the divisor is > a constant. I see the problem with openrisc and parisc, with > gcc 13.3.0. It does fail to build, that's how it got identified in the first place. I managed to reproduce here according to the reproducer sent by the test robot. I think it works if you don't enable build checks (in this case W=1) though. I didn't try other architectures other than x86 though, thanks for the heads up. > > > mult_fract(). To prevent the failure, use do_div() for the division > > calculation instead of hardcoding a / b. > > > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > > Reported-by: kernel test robot <lkp@intel.com> > > Closes: https://lore.kernel.org/oe-kbuild-all/202504181233.F7D9Atra-lkp@intel.com/ > > --- > > > > I'm sending it as a RFC because I didn't to extensive testing on this > > patch, also I'm not sure if mult_frac() was intended to work on 32-bit > > only types. If that's the case, perhaps, a new mult_frac64() might be a > > better idea?! > > > > include/linux/math.h | 19 ++++++++++--------- > > 1 file changed, 10 insertions(+), 9 deletions(-) > > > > diff --git a/include/linux/math.h b/include/linux/math.h > > index 0198c92cbe3e..05ea853b75b4 100644 > > --- a/include/linux/math.h > > +++ b/include/linux/math.h > > @@ -133,15 +133,16 @@ __STRUCT_FRACT(u32) > > #undef __STRUCT_FRACT > > > > /* Calculate "x * n / d" without unnecessary overflow or loss of precision. */ > > -#define mult_frac(x, n, d) \ > > -({ \ > > - typeof(x) x_ = (x); \ > > - typeof(n) n_ = (n); \ > > - typeof(d) d_ = (d); \ > > - \ > > - typeof(x_) q = x_ / d_; \ > > - typeof(x_) r = x_ % d_; \ > > - q * n_ + r * n_ / d_; \ > > +#define mult_frac(x, n, d) \ > > +({ \ > > + typeof(x) x_ = (x); \ > > + typeof(n) n_ = (n); \ > > + typeof(d) d_ = (d); \ > > + \ > > + typeof(x_) r = do_div(x_, d_); \ > > + r *= n_; \ > > + do_div(r, d_); \ > > + x_ * n_ + r; \ > > }) > > > > Unfortunately that doesn't work. I get build errors on parisc. > > In file included from ./arch/parisc/include/generated/asm/div64.h:1, > from ./include/linux/math.h:6, > from ./include/linux/kernel.h:27, > from ./arch/parisc/include/asm/bug.h:5, > from ./include/linux/bug.h:5, > from ./include/linux/mmdebug.h:5, > from ./include/linux/mm.h:6, > from mm/page_alloc.c:19: > mm/page_alloc.c: In function 'boost_watermark': > ./include/asm-generic/div64.h:183:35: error: comparison of distinct pointer types lacks a cast [-Werror] > 183 | (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ > | ^~ > ./include/linux/math.h:142:24: note: in expansion of macro 'do_div' > 142 | typeof(x_) r = do_div(x_, d_); \ > | ^~~~~~ > mm/page_alloc.c:2010:21: note: in expansion of macro 'mult_frac' > 2010 | max_boost = mult_frac(zone->_watermark[WMARK_HIGH], > > That is just one example. It seems to affect all uses of > mult_frac(). > > Guenter ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-04-23 7:02 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-04-19 11:51 [RFC PATCH] math.h: Account for 64-bit division on i386 cem 2025-04-20 15:40 ` Guenter Roeck 2025-04-20 17:42 ` Guenter Roeck 2025-04-20 18:05 ` Carlos Maiolino 2025-04-23 7:02 ` Geert Uytterhoeven 2025-04-20 18:01 ` Carlos Maiolino
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox