Message ID | 20200424121051.5056-1-geert@linux-m68k.org |
---|---|
State | New, archived |
Headers | show |
Series |
|
Related | show |
On 4/24/20 5:10 AM, Geert Uytterhoeven wrote: > On 32-bit architectures (e.g. m68k): > > ERROR: modpost: "__udivdi3" [net/openvswitch/openvswitch.ko] undefined! > ERROR: modpost: "__divdi3" [net/openvswitch/openvswitch.ko] undefined! > > Fixes: e57358873bb5d6ca ("net: openvswitch: use u64 for meter bucket") > Reported-by: noreply@ellerman.id.au > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- > net/openvswitch/meter.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c > index 915f31123f235c03..3498a5ab092ab2b8 100644 > --- a/net/openvswitch/meter.c > +++ b/net/openvswitch/meter.c > @@ -393,7 +393,7 @@ static struct dp_meter *dp_meter_create(struct nlattr **a) > * Start with a full bucket. > */ > band->bucket = (band->burst_size + band->rate) * 1000ULL; > - band_max_delta_t = band->bucket / band->rate; > + band_max_delta_t = do_div(band->bucket, band->rate); > if (band_max_delta_t > meter->max_delta_t) > meter->max_delta_t = band_max_delta_t; > band++; > This is fascinating... Have you tested this patch ? Please double check what do_div() return value is supposed to be ! Thanks.
Hi Eric, On Fri, Apr 24, 2020 at 5:05 PM Eric Dumazet <eric.dumazet@gmail.com> wrote: > On 4/24/20 5:10 AM, Geert Uytterhoeven wrote: > > On 32-bit architectures (e.g. m68k): > > > > ERROR: modpost: "__udivdi3" [net/openvswitch/openvswitch.ko] undefined! > > ERROR: modpost: "__divdi3" [net/openvswitch/openvswitch.ko] undefined! > > > > Fixes: e57358873bb5d6ca ("net: openvswitch: use u64 for meter bucket") > > Reported-by: noreply@ellerman.id.au > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > > --- > > net/openvswitch/meter.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c > > index 915f31123f235c03..3498a5ab092ab2b8 100644 > > --- a/net/openvswitch/meter.c > > +++ b/net/openvswitch/meter.c > > @@ -393,7 +393,7 @@ static struct dp_meter *dp_meter_create(struct nlattr **a) > > * Start with a full bucket. > > */ > > band->bucket = (band->burst_size + band->rate) * 1000ULL; > > - band_max_delta_t = band->bucket / band->rate; > > + band_max_delta_t = do_div(band->bucket, band->rate); > > if (band_max_delta_t > meter->max_delta_t) > > meter->max_delta_t = band_max_delta_t; > > band++; > > > > This is fascinating... Have you tested this patch ? Sorry, I should have said this is compile-tested only. > Please double check what do_div() return value is supposed to be ! I do not have any openvswitch setups, let alone on the poor m68k box. Gr{oetje,eeting}s, Geert
Geert Uytterhoeven <geert@linux-m68k.org> writes: > Hi Eric, > > On Fri, Apr 24, 2020 at 5:05 PM Eric Dumazet <eric.dumazet@gmail.com> wrote: >> On 4/24/20 5:10 AM, Geert Uytterhoeven wrote: >> > On 32-bit architectures (e.g. m68k): >> > >> > ERROR: modpost: "__udivdi3" [net/openvswitch/openvswitch.ko] undefined! >> > ERROR: modpost: "__divdi3" [net/openvswitch/openvswitch.ko] undefined! >> > >> > Fixes: e57358873bb5d6ca ("net: openvswitch: use u64 for meter bucket") >> > Reported-by: noreply@ellerman.id.au >> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> >> > --- >> > net/openvswitch/meter.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c >> > index 915f31123f235c03..3498a5ab092ab2b8 100644 >> > --- a/net/openvswitch/meter.c >> > +++ b/net/openvswitch/meter.c >> > @@ -393,7 +393,7 @@ static struct dp_meter *dp_meter_create(struct nlattr **a) >> > * Start with a full bucket. >> > */ >> > band->bucket = (band->burst_size + band->rate) * 1000ULL; >> > - band_max_delta_t = band->bucket / band->rate; >> > + band_max_delta_t = do_div(band->bucket, band->rate); >> > if (band_max_delta_t > meter->max_delta_t) >> > meter->max_delta_t = band_max_delta_t; >> > band++; >> > >> >> This is fascinating... Have you tested this patch ? > > Sorry, I should have said this is compile-tested only. > >> Please double check what do_div() return value is supposed to be ! > > I do not have any openvswitch setups, let alone on the poor m68k box. I think what Eric is referring to is this, from the documentation of do_div: * do_div - returns 2 values: calculate remainder and update new dividend * @n: uint64_t dividend (will be updated) * @base: uint32_t divisor * * Summary: * ``uint32_t remainder = n % base;`` * ``n = n / base;`` * * Return: (uint32_t)remainder Specifically that last part :) -Toke
Hi Toke, On Fri, Apr 24, 2020 at 6:45 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > Geert Uytterhoeven <geert@linux-m68k.org> writes: > > On Fri, Apr 24, 2020 at 5:05 PM Eric Dumazet <eric.dumazet@gmail.com> wrote: > >> On 4/24/20 5:10 AM, Geert Uytterhoeven wrote: > >> > On 32-bit architectures (e.g. m68k): > >> > > >> > ERROR: modpost: "__udivdi3" [net/openvswitch/openvswitch.ko] undefined! > >> > ERROR: modpost: "__divdi3" [net/openvswitch/openvswitch.ko] undefined! > >> > > >> > Fixes: e57358873bb5d6ca ("net: openvswitch: use u64 for meter bucket") > >> > Reported-by: noreply@ellerman.id.au > >> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > >> > --- > >> > net/openvswitch/meter.c | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c > >> > index 915f31123f235c03..3498a5ab092ab2b8 100644 > >> > --- a/net/openvswitch/meter.c > >> > +++ b/net/openvswitch/meter.c > >> > @@ -393,7 +393,7 @@ static struct dp_meter *dp_meter_create(struct nlattr **a) > >> > * Start with a full bucket. > >> > */ > >> > band->bucket = (band->burst_size + band->rate) * 1000ULL; > >> > - band_max_delta_t = band->bucket / band->rate; > >> > + band_max_delta_t = do_div(band->bucket, band->rate); > >> > if (band_max_delta_t > meter->max_delta_t) > >> > meter->max_delta_t = band_max_delta_t; > >> > band++; > >> > > >> > >> This is fascinating... Have you tested this patch ? > > > > Sorry, I should have said this is compile-tested only. > > > >> Please double check what do_div() return value is supposed to be ! > > > > I do not have any openvswitch setups, let alone on the poor m68k box. > > I think what Eric is referring to is this, from the documentation of > do_div: > > * do_div - returns 2 values: calculate remainder and update new dividend > * @n: uint64_t dividend (will be updated) > * @base: uint32_t divisor > * > * Summary: > * ``uint32_t remainder = n % base;`` > * ``n = n / base;`` > * > * Return: (uint32_t)remainder Oops, that was a serious brain fart. Sorry for that! That should have been div_u64() instead of do_div(). Feel free to update my patch while applying, else I'll send a v2 later. /me hides in a brown paper bag for the whole weekend... Gr{oetje,eeting}s, Geert
On Sat, Apr 25, 2020 at 1:54 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Toke, > > On Fri, Apr 24, 2020 at 6:45 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > > Geert Uytterhoeven <geert@linux-m68k.org> writes: > > > On Fri, Apr 24, 2020 at 5:05 PM Eric Dumazet <eric.dumazet@gmail.com> wrote: > > >> On 4/24/20 5:10 AM, Geert Uytterhoeven wrote: > > >> > On 32-bit architectures (e.g. m68k): > > >> > > > >> > ERROR: modpost: "__udivdi3" [net/openvswitch/openvswitch.ko] undefined! > > >> > ERROR: modpost: "__divdi3" [net/openvswitch/openvswitch.ko] undefined! > > >> > > > >> > Fixes: e57358873bb5d6ca ("net: openvswitch: use u64 for meter bucket") > > >> > Reported-by: noreply@ellerman.id.au > > >> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > > >> > --- > > >> > net/openvswitch/meter.c | 2 +- > > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > > >> > diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c > > >> > index 915f31123f235c03..3498a5ab092ab2b8 100644 > > >> > --- a/net/openvswitch/meter.c > > >> > +++ b/net/openvswitch/meter.c > > >> > @@ -393,7 +393,7 @@ static struct dp_meter *dp_meter_create(struct nlattr **a) > > >> > * Start with a full bucket. > > >> > */ > > >> > band->bucket = (band->burst_size + band->rate) * 1000ULL; > > >> > - band_max_delta_t = band->bucket / band->rate; > > >> > + band_max_delta_t = do_div(band->bucket, band->rate); > > >> > if (band_max_delta_t > meter->max_delta_t) > > >> > meter->max_delta_t = band_max_delta_t; > > >> > band++; > > >> > > > >> > > >> This is fascinating... Have you tested this patch ? > > > > > > Sorry, I should have said this is compile-tested only. > > > > > >> Please double check what do_div() return value is supposed to be ! > > > > > > I do not have any openvswitch setups, let alone on the poor m68k box. > > > > I think what Eric is referring to is this, from the documentation of > > do_div: > > > > * do_div - returns 2 values: calculate remainder and update new dividend > > * @n: uint64_t dividend (will be updated) > > * @base: uint32_t divisor > > * > > * Summary: > > * ``uint32_t remainder = n % base;`` > > * ``n = n / base;`` > > * > > * Return: (uint32_t)remainder > > Oops, that was a serious brain fart. Sorry for that! > > That should have been div_u64() instead of do_div(). > Feel free to update my patch while applying, else I'll send a v2 later. > > /me hides in a brown paper bag for the whole weekend... I sent the patch with updated, change the reported tag, test it, and add more commit msg, and author should be you. Thanks. > 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
diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c index 915f31123f235c03..3498a5ab092ab2b8 100644 --- a/net/openvswitch/meter.c +++ b/net/openvswitch/meter.c @@ -393,7 +393,7 @@ static struct dp_meter *dp_meter_create(struct nlattr **a) * Start with a full bucket. */ band->bucket = (band->burst_size + band->rate) * 1000ULL; - band_max_delta_t = band->bucket / band->rate; + band_max_delta_t = do_div(band->bucket, band->rate); if (band_max_delta_t > meter->max_delta_t) meter->max_delta_t = band_max_delta_t; band++;
On 32-bit architectures (e.g. m68k): ERROR: modpost: "__udivdi3" [net/openvswitch/openvswitch.ko] undefined! ERROR: modpost: "__divdi3" [net/openvswitch/openvswitch.ko] undefined! Fixes: e57358873bb5d6ca ("net: openvswitch: use u64 for meter bucket") Reported-by: noreply@ellerman.id.au Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> --- net/openvswitch/meter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)