linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
@ 2019-09-30 14:13 Borislav Petkov
  2019-09-30 15:45 ` Michal Kubecek
  0 siblings, 1 reply; 9+ messages in thread
From: Borislav Petkov @ 2019-09-30 14:13 UTC (permalink / raw)
  To: linux-rdma; +Cc: Saeed Mahameed, Leon Romanovsky, netdev, lkml

Hi!

JFYI,

I'm seeing this on i386 allyesconfig builds of current Linus master:

ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
  2019-09-30 14:13 ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined! Borislav Petkov
@ 2019-09-30 15:45 ` Michal Kubecek
  2019-09-30 16:29   ` Borislav Petkov
  0 siblings, 1 reply; 9+ messages in thread
From: Michal Kubecek @ 2019-09-30 15:45 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: linux-rdma, Saeed Mahameed, Leon Romanovsky, netdev, lkml

On Mon, Sep 30, 2019 at 04:13:17PM +0200, Borislav Petkov wrote:
> I'm seeing this on i386 allyesconfig builds of current Linus master:
> 
> ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
> make[1]: *** [__modpost] Error 1
> make: *** [modules] Error 2

This is usually result of dividing (or modulo) by a 64-bit integer. Can
you identify where (file and line number) is the __umoddi3() call
generated?

Michal

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
  2019-09-30 15:45 ` Michal Kubecek
@ 2019-09-30 16:29   ` Borislav Petkov
  2019-09-30 16:52     ` Stephen Hemminger
  2019-09-30 16:55     ` Stephen Hemminger
  0 siblings, 2 replies; 9+ messages in thread
From: Borislav Petkov @ 2019-09-30 16:29 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: linux-rdma, Saeed Mahameed, Leon Romanovsky, netdev, lkml

On Mon, Sep 30, 2019 at 05:45:35PM +0200, Michal Kubecek wrote:
> On Mon, Sep 30, 2019 at 04:13:17PM +0200, Borislav Petkov wrote:
> > I'm seeing this on i386 allyesconfig builds of current Linus master:
> > 
> > ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
> > make[1]: *** [__modpost] Error 1
> > make: *** [modules] Error 2
> 
> This is usually result of dividing (or modulo) by a 64-bit integer. Can
> you identify where (file and line number) is the __umoddi3() call
> generated?

Did another 32-bit allyesconfig build. It said:

ld: drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.o: in function `mlx5dr_icm_alloc_chunk':
dr_icm_pool.c:(.text+0x733): undefined reference to `__umoddi3'
make: *** [vmlinux] Error 1

The .s file then points to the exact location:

# drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c:140:   align_diff = icm_mr->icm_start_addr % align_base;
        pushl   %ebx    # align_base
        pushl   %ecx    # align_base
        call    __umoddi3       #
        popl    %edx    #
        popl    %ecx    #

HTH.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
  2019-09-30 16:29   ` Borislav Petkov
@ 2019-09-30 16:52     ` Stephen Hemminger
  2019-09-30 16:55     ` Stephen Hemminger
  1 sibling, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2019-09-30 16:52 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Michal Kubecek, linux-rdma, Saeed Mahameed, Leon Romanovsky,
	netdev, lkml

On Mon, 30 Sep 2019 18:29:10 +0200
Borislav Petkov <bp@alien8.de> wrote:

> On Mon, Sep 30, 2019 at 05:45:35PM +0200, Michal Kubecek wrote:
> > On Mon, Sep 30, 2019 at 04:13:17PM +0200, Borislav Petkov wrote:  
> > > I'm seeing this on i386 allyesconfig builds of current Linus master:
> > > 
> > > ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
> > > make[1]: *** [__modpost] Error 1
> > > make: *** [modules] Error 2  
> > 
> > This is usually result of dividing (or modulo) by a 64-bit integer. Can
> > you identify where (file and line number) is the __umoddi3() call
> > generated?  
> 
> Did another 32-bit allyesconfig build. It said:
> 
> ld: drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.o: in function `mlx5dr_icm_alloc_chunk':
> dr_icm_pool.c:(.text+0x733): undefined reference to `__umoddi3'
> make: *** [vmlinux] Error 1
> 
> The .s file then points to the exact location:
> 
> # drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c:140:   align_diff = icm_mr->icm_start_addr % align_base;
>         pushl   %ebx    # align_base
>         pushl   %ecx    # align_base
>         call    __umoddi3       #
>         popl    %edx    #
>         popl    %ecx    #

Since align_base is a power of 2 masking should work as well.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
  2019-09-30 16:29   ` Borislav Petkov
  2019-09-30 16:52     ` Stephen Hemminger
@ 2019-09-30 16:55     ` Stephen Hemminger
  2019-09-30 18:40       ` Borislav Petkov
  1 sibling, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2019-09-30 16:55 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Michal Kubecek, linux-rdma, Saeed Mahameed, Leon Romanovsky,
	netdev, lkml

On Mon, 30 Sep 2019 18:29:10 +0200
Borislav Petkov <bp@alien8.de> wrote:

> On Mon, Sep 30, 2019 at 05:45:35PM +0200, Michal Kubecek wrote:
> > On Mon, Sep 30, 2019 at 04:13:17PM +0200, Borislav Petkov wrote:  
> > > I'm seeing this on i386 allyesconfig builds of current Linus master:
> > > 
> > > ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
> > > make[1]: *** [__modpost] Error 1
> > > make: *** [modules] Error 2  
> > 
> > This is usually result of dividing (or modulo) by a 64-bit integer. Can
> > you identify where (file and line number) is the __umoddi3() call
> > generated?  
> 
> Did another 32-bit allyesconfig build. It said:
> 
> ld: drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.o: in function `mlx5dr_icm_alloc_chunk':
> dr_icm_pool.c:(.text+0x733): undefined reference to `__umoddi3'
> make: *** [vmlinux] Error 1
> 
> The .s file then points to the exact location:
> 
> # drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c:140:   align_diff = icm_mr->icm_start_addr % align_base;
>         pushl   %ebx    # align_base
>         pushl   %ecx    # align_base
>         call    __umoddi3       #
>         popl    %edx    #
>         popl    %ecx    #
> 
> HTH.
> 

Could also us div_u64_rem here?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
  2019-09-30 16:55     ` Stephen Hemminger
@ 2019-09-30 18:40       ` Borislav Petkov
  2019-10-01 15:14         ` Michal Kubecek
  0 siblings, 1 reply; 9+ messages in thread
From: Borislav Petkov @ 2019-09-30 18:40 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Michal Kubecek, linux-rdma, Saeed Mahameed, Leon Romanovsky,
	netdev, lkml

On Mon, Sep 30, 2019 at 09:55:16AM -0700, Stephen Hemminger wrote:
> Could also us div_u64_rem here?

Yah, the below seems to work and the resulting asm looks sensible to me
but someone should definitely double-check me as I don't know this code
at all.

Thx.

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
index 913f1e5aaaf2..b4302658e5f8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
@@ -137,7 +137,7 @@ dr_icm_pool_mr_create(struct mlx5dr_icm_pool *pool,
 
 	icm_mr->icm_start_addr = icm_mr->dm.addr;
 
-	align_diff = icm_mr->icm_start_addr % align_base;
+	div_u64_rem(icm_mr->icm_start_addr, align_base, &align_diff);
 	if (align_diff)
 		icm_mr->used_length = align_base - align_diff;
 

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
  2019-09-30 18:40       ` Borislav Petkov
@ 2019-10-01 15:14         ` Michal Kubecek
  2019-10-15 20:29           ` Saeed Mahameed
  0 siblings, 1 reply; 9+ messages in thread
From: Michal Kubecek @ 2019-10-01 15:14 UTC (permalink / raw)
  To: netdev
  Cc: Borislav Petkov, Alex Vesker, Stephen Hemminger, linux-rdma,
	Saeed Mahameed, Leon Romanovsky, lkml

On Mon, Sep 30, 2019 at 08:40:31PM +0200, Borislav Petkov wrote:
> On Mon, Sep 30, 2019 at 09:55:16AM -0700, Stephen Hemminger wrote:
> > Could also us div_u64_rem here?
> 
> Yah, the below seems to work and the resulting asm looks sensible to me
> but someone should definitely double-check me as I don't know this code
> at all.
> 
> Thx.
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
> index 913f1e5aaaf2..b4302658e5f8 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
> @@ -137,7 +137,7 @@ dr_icm_pool_mr_create(struct mlx5dr_icm_pool *pool,
>  
>  	icm_mr->icm_start_addr = icm_mr->dm.addr;
>  
> -	align_diff = icm_mr->icm_start_addr % align_base;
> +	div_u64_rem(icm_mr->icm_start_addr, align_base, &align_diff);
>  	if (align_diff)
>  		icm_mr->used_length = align_base - align_diff;
>  
> 

While this fixes 32-bit builds, it breaks 64-bit ones as align_diff is
64-bit and div_u64_rem expects pointer to u32. :-(

I checked that align_base is always a power of two so that we could get
away with

	align_diff = icm_mr->icm_start_addr & (align_base - 1)

I'm not sure, however, if it's safe to assume align_base will always
have to be a power of two or if we should add a check for safety.

(Cc-ing also author of commit 29cf8febd185 ("net/mlx5: DR, ICM pool
memory allocator").)

Michal

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
  2019-10-01 15:14         ` Michal Kubecek
@ 2019-10-15 20:29           ` Saeed Mahameed
  2019-10-15 20:45             ` Saeed Mahameed
  0 siblings, 1 reply; 9+ messages in thread
From: Saeed Mahameed @ 2019-10-15 20:29 UTC (permalink / raw)
  To: mkubecek, netdev; +Cc: stephen, leon, linux-kernel, linux-rdma, bp, Alex Vesker

On Tue, 2019-10-01 at 17:14 +0200, Michal Kubecek wrote:
> On Mon, Sep 30, 2019 at 08:40:31PM +0200, Borislav Petkov wrote:
> > On Mon, Sep 30, 2019 at 09:55:16AM -0700, Stephen Hemminger wrote:
> > > Could also us div_u64_rem here?
> > 
> > Yah, the below seems to work and the resulting asm looks sensible
> > to me
> > but someone should definitely double-check me as I don't know this
> > code
> > at all.
> > 
> > Thx.
> > 
> > diff --git
> > a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
> > index 913f1e5aaaf2..b4302658e5f8 100644
> > ---
> > a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
> > +++
> > b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
> > @@ -137,7 +137,7 @@ dr_icm_pool_mr_create(struct mlx5dr_icm_pool
> > *pool,
> >  
> >  	icm_mr->icm_start_addr = icm_mr->dm.addr;
> >  
> > -	align_diff = icm_mr->icm_start_addr % align_base;
> > +	div_u64_rem(icm_mr->icm_start_addr, align_base, &align_diff);
> >  	if (align_diff)
> >  		icm_mr->used_length = align_base - align_diff;
> >  
> > 
> 
> While this fixes 32-bit builds, it breaks 64-bit ones as align_diff
> is
> 64-bit and div_u64_rem expects pointer to u32. :-(
> 
> I checked that align_base is always a power of two so that we could
> get
> away with
> 
> 	align_diff = icm_mr->icm_start_addr & (align_base - 1)
> 
> I'm not sure, however, if it's safe to assume align_base will always
> have to be a power of two or if we should add a check for safety.
> 
> (Cc-ing also author of commit 29cf8febd185 ("net/mlx5: DR, ICM pool
> memory allocator").)
> 

Thanks everyone for your input on this, Alex will take care of this and
we will submit a patch ..


> Michal

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
  2019-10-15 20:29           ` Saeed Mahameed
@ 2019-10-15 20:45             ` Saeed Mahameed
  0 siblings, 0 replies; 9+ messages in thread
From: Saeed Mahameed @ 2019-10-15 20:45 UTC (permalink / raw)
  To: mkubecek, netdev; +Cc: stephen, Alex Vesker, linux-kernel, linux-rdma, leon, bp

On Tue, 2019-10-15 at 20:29 +0000, Saeed Mahameed wrote:
> On Tue, 2019-10-01 at 17:14 +0200, Michal Kubecek wrote:
> > On Mon, Sep 30, 2019 at 08:40:31PM +0200, Borislav Petkov wrote:
> > > On Mon, Sep 30, 2019 at 09:55:16AM -0700, Stephen Hemminger
> > > wrote:
> > > > Could also us div_u64_rem here?
> > > 
> > > Yah, the below seems to work and the resulting asm looks sensible
> > > to me
> > > but someone should definitely double-check me as I don't know
> > > this
> > > code
> > > at all.
> > > 
> > > Thx.
> > > 
> > > diff --git
> > > a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
> > > b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
> > > index 913f1e5aaaf2..b4302658e5f8 100644
> > > ---
> > > a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
> > > +++
> > > b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
> > > @@ -137,7 +137,7 @@ dr_icm_pool_mr_create(struct mlx5dr_icm_pool
> > > *pool,
> > >  
> > >  	icm_mr->icm_start_addr = icm_mr->dm.addr;
> > >  
> > > -	align_diff = icm_mr->icm_start_addr % align_base;
> > > +	div_u64_rem(icm_mr->icm_start_addr, align_base, &align_diff);
> > >  	if (align_diff)
> > >  		icm_mr->used_length = align_base - align_diff;
> > >  
> > > 
> > 
> > While this fixes 32-bit builds, it breaks 64-bit ones as align_diff
> > is
> > 64-bit and div_u64_rem expects pointer to u32. :-(
> > 
> > I checked that align_base is always a power of two so that we could
> > get
> > away with
> > 
> > 	align_diff = icm_mr->icm_start_addr & (align_base - 1)
> > 
> > I'm not sure, however, if it's safe to assume align_base will
> > always
> > have to be a power of two or if we should add a check for safety.
> > 
> > (Cc-ing also author of commit 29cf8febd185 ("net/mlx5: DR, ICM pool
> > memory allocator").)
> > 
> 
> Thanks everyone for your input on this, Alex will take care of this
> and
> we will submit a patch ..
> 

Please disregard, i see this was fixed in 
"[net] mlx5: avoid 64-bit division in dr_icm_pool_mr_create()"

Just came back from a long vacation, got a lot of catching up to do
:).. 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-10-15 20:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-30 14:13 ERROR: "__umoddi3" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined! Borislav Petkov
2019-09-30 15:45 ` Michal Kubecek
2019-09-30 16:29   ` Borislav Petkov
2019-09-30 16:52     ` Stephen Hemminger
2019-09-30 16:55     ` Stephen Hemminger
2019-09-30 18:40       ` Borislav Petkov
2019-10-01 15:14         ` Michal Kubecek
2019-10-15 20:29           ` Saeed Mahameed
2019-10-15 20:45             ` Saeed Mahameed

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).