Discussion:
[dpdk-dev] [PATCH] net/mlx5: fix linkage error for glue lib
(too old to reply)
Shahaf Shuler
2018-07-24 08:54:45 UTC
Permalink
From: Yaroslav Brustinov <***@cisco.com>

Compiling with gcc 4.7.2 introduced the linkage error

"bin/ld: Warning: alignment 8 of symbol `mlx5_glue' in
src/dpdk/drivers/net/mlx5/mlx5_glue.c.21.o is smaller than 16 in
src/dpdk/drivers/net/mlx5/mlx5_rxq.c.21.o"

Fix it be forcing the alignment of the glue lib.

Fixes: 0e83b8e536c1 ("net/mlx5: move rdma-core calls to separate file")
Cc: ***@dpdk.org
Cc: ***@6wind.com
Cc: ***@6wind.com

Signed-off-by: Yaroslav Brustinov <***@cisco.com>
Signed-off-by: Shahaf Shuler <***@mellanox.com>
---
drivers/net/mlx5/mlx5_glue.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_glue.c b/drivers/net/mlx5/mlx5_glue.c
index c7965e51fe..c56c69bb13 100644
--- a/drivers/net/mlx5/mlx5_glue.c
+++ b/drivers/net/mlx5/mlx5_glue.c
@@ -343,7 +343,9 @@ mlx5_glue_dv_create_qp(struct ibv_context *context,
#endif
}

-const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue){
+const struct mlx5_glue *mlx5_glue __attribute__((__aligned__(64))) =
+ &(const struct mlx5_glue)
+{
.version = MLX5_GLUE_VERSION,
.fork_init = mlx5_glue_fork_init,
.alloc_pd = mlx5_glue_alloc_pd,
--
2.12.0
Adrien Mazarguil
2018-07-24 12:00:48 UTC
Permalink
Post by Shahaf Shuler
Compiling with gcc 4.7.2 introduced the linkage error
"bin/ld: Warning: alignment 8 of symbol `mlx5_glue' in
src/dpdk/drivers/net/mlx5/mlx5_glue.c.21.o is smaller than 16 in
src/dpdk/drivers/net/mlx5/mlx5_rxq.c.21.o"
Fix it be forcing the alignment of the glue lib.
Fixes: 0e83b8e536c1 ("net/mlx5: move rdma-core calls to separate file")
Odd, how could this happen assuming both files are compiled during the same
run using identical flags? Looks like a compiler issue.

Anyway, may I suggest an alignment constraint on the structure type in
mlx5_glue.h instead, so that all users inherit it. E.g. using C11 syntax:

#include <stdalign.h>
#include <stddef.h>

[...]
alignas(max_align_t)
struct mlx5_glue {
[...]
};
Post by Shahaf Shuler
---
drivers/net/mlx5/mlx5_glue.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/mlx5_glue.c b/drivers/net/mlx5/mlx5_glue.c
index c7965e51fe..c56c69bb13 100644
--- a/drivers/net/mlx5/mlx5_glue.c
+++ b/drivers/net/mlx5/mlx5_glue.c
@@ -343,7 +343,9 @@ mlx5_glue_dv_create_qp(struct ibv_context *context,
#endif
}
-const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue){
+const struct mlx5_glue *mlx5_glue __attribute__((__aligned__(64))) =
+ &(const struct mlx5_glue)
+{
.version = MLX5_GLUE_VERSION,
.fork_init = mlx5_glue_fork_init,
.alloc_pd = mlx5_glue_alloc_pd,
--
2.12.0
--
Adrien Mazarguil
6WIND
Adrien Mazarguil
2018-07-24 12:05:51 UTC
Permalink
Post by Adrien Mazarguil
Post by Shahaf Shuler
Compiling with gcc 4.7.2 introduced the linkage error
"bin/ld: Warning: alignment 8 of symbol `mlx5_glue' in
src/dpdk/drivers/net/mlx5/mlx5_glue.c.21.o is smaller than 16 in
src/dpdk/drivers/net/mlx5/mlx5_rxq.c.21.o"
Fix it be forcing the alignment of the glue lib.
Fixes: 0e83b8e536c1 ("net/mlx5: move rdma-core calls to separate file")
Odd, how could this happen assuming both files are compiled during the same
run using identical flags? Looks like a compiler issue.
Anyway, may I suggest an alignment constraint on the structure type in
#include <stdalign.h>
#include <stddef.h>
[...]
alignas(max_align_t)
struct mlx5_glue {
[...]
};
My bad, this is not a correct use for alignas(), it doesn't work on types.
How about this instead:

alignas(max_align_t)
const struct mlx5_glue *mlx5_glue;
Post by Adrien Mazarguil
Post by Shahaf Shuler
---
drivers/net/mlx5/mlx5_glue.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/mlx5_glue.c b/drivers/net/mlx5/mlx5_glue.c
index c7965e51fe..c56c69bb13 100644
--- a/drivers/net/mlx5/mlx5_glue.c
+++ b/drivers/net/mlx5/mlx5_glue.c
@@ -343,7 +343,9 @@ mlx5_glue_dv_create_qp(struct ibv_context *context,
#endif
}
-const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue){
+const struct mlx5_glue *mlx5_glue __attribute__((__aligned__(64))) =
+ &(const struct mlx5_glue)
+{
.version = MLX5_GLUE_VERSION,
.fork_init = mlx5_glue_fork_init,
.alloc_pd = mlx5_glue_alloc_pd,
--
2.12.0
--
Adrien Mazarguil
6WIND
--
Adrien Mazarguil
6WIND
Shahaf Shuler
2018-07-24 13:51:24 UTC
Permalink
Yaroslav,
Subject: Re: [PATCH] net/mlx5: fix linkage error for glue lib
Post by Adrien Mazarguil
Odd, how could this happen assuming both files are compiled during the
same run using identical flags? Looks like a compiler issue.
Anyway, may I suggest an alignment constraint on the structure type in
#include <stdalign.h>
#include <stddef.h>
[...]
alignas(max_align_t)
struct mlx5_glue {
[...]
};
My bad, this is not a correct use for alignas(), it doesn't work on types.
alignas(max_align_t)
const struct mlx5_glue *mlx5_glue;
Can you confirm the above suggestion fixes your issue?
Adrien Mazarguil
2018-07-24 16:03:46 UTC
Permalink
Hi,
Are you OK with C11?
* C11 code would include stdalign.h and use alignof(max_align_t) however
* we'll stick with C99 for the time being.
:)
Hah, this code was originally intended to become a generic EAL function
hence the C99 but got rejected. Mellanox PMDs otherwise rely on C11 features
already.
As far as I understand, the issue is alignment of the pointer itself, not the struct.
I'm not familiar with this command: "alignof(max_align_t)".
struct mlx5_glue {
...
} __attribute__((__aligned__(64)));
struct __attribute__((__aligned__(64))) mlx5_glue {
...
};
...still keeps the linker unhappy.
Right, this was my first (wrong) suggestion that doesn't work on types. How
about the second one instead? Here's how the diff on mlx5_glue.h should look
like:

+#include <stdalign.h>
#include <stddef.h>
#include <stdint.h>

[...]

+alignas(max_align_t)
const struct mlx5_glue *mlx5_glue;

Another comment regarding this patch, commit log should probably mention it
addresses a GCC bug that cannot be reproduced with latter versions. Keep in
mind DPDK recommends to use at least GCC version 4.9.
-----Original Message-----
Sent: Tuesday, July 24, 2018 16:51
Subject: RE: [PATCH] net/mlx5: fix linkage error for glue lib
Yaroslav,
Subject: Re: [PATCH] net/mlx5: fix linkage error for glue lib
Post by Adrien Mazarguil
Odd, how could this happen assuming both files are compiled during
the same run using identical flags? Looks like a compiler issue.
Anyway, may I suggest an alignment constraint on the structure type
#include <stdalign.h>
#include <stddef.h>
[...]
alignas(max_align_t)
struct mlx5_glue {
[...]
};
My bad, this is not a correct use for alignas(), it doesn't work on types.
alignas(max_align_t)
const struct mlx5_glue *mlx5_glue;
Can you confirm the above suggestion fixes your issue?
--
Adrien Mazarguil
6WIND
Adrien Mazarguil
2018-07-25 09:24:47 UTC
Permalink
+Hanoch
I've added -std=c11 flag to our compilation to check.
/usr/bin/ld: Warning: alignment 8 of symbol `mlx5_glue' in src/dpdk/drivers/net/mlx5/mlx5_glue.c.11.o is smaller than 32 in src/dpdk/drivers/net/mlx5/mlx5_rxq.c.11.o
/usr/bin/ld: Warning: alignment 16 of symbol `mlx5_glue' in src/dpdk/drivers/net/mlx5/mlx5_glue.c.11.o is smaller than 32 in src/dpdk/drivers/net/mlx5/mlx5_rxq.c.11.o
Using alignas(64) does not produce linker warning.
OK, let's forget max_align_t. Even better, how about
alignas(RTE_CACHE_LINE_SIZE) just in case the same GCC version complains
about the lack of a 128 byte alignment on architectures like IBM POWER8.

(remember to #include <rte_config.h> for RTE_CACHE_LINE_SIZE)
-----Original Message-----
Sent: Tuesday, July 24, 2018 19:04
Subject: Re: [PATCH] net/mlx5: fix linkage error for glue lib
Hi,
Are you OK with C11?
* C11 code would include stdalign.h and use alignof(max_align_t) however
* we'll stick with C99 for the time being.
:)
Hah, this code was originally intended to become a generic EAL function hence the C99 but got rejected. Mellanox PMDs otherwise rely on C11 features already.
As far as I understand, the issue is alignment of the pointer itself, not the struct.
I'm not familiar with this command: "alignof(max_align_t)".
struct mlx5_glue {
...
} __attribute__((__aligned__(64)));
struct __attribute__((__aligned__(64))) mlx5_glue {
...
};
...still keeps the linker unhappy.
Right, this was my first (wrong) suggestion that doesn't work on types. How about the second one instead? Here's how the diff on mlx5_glue.h should look
+#include <stdalign.h>
#include <stddef.h>
#include <stdint.h>
[...]
+alignas(max_align_t)
const struct mlx5_glue *mlx5_glue;
Another comment regarding this patch, commit log should probably mention it addresses a GCC bug that cannot be reproduced with latter versions. Keep in mind DPDK recommends to use at least GCC version 4.9.
-----Original Message-----
Sent: Tuesday, July 24, 2018 16:51
Subject: RE: [PATCH] net/mlx5: fix linkage error for glue lib
Yaroslav,
Subject: Re: [PATCH] net/mlx5: fix linkage error for glue lib
Post by Adrien Mazarguil
Odd, how could this happen assuming both files are compiled during
the same run using identical flags? Looks like a compiler issue.
Anyway, may I suggest an alignment constraint on the structure
#include <stdalign.h>
#include <stddef.h>
[...]
alignas(max_align_t)
struct mlx5_glue {
[...]
};
My bad, this is not a correct use for alignas(), it doesn't work on types.
alignas(max_align_t)
const struct mlx5_glue *mlx5_glue;
Can you confirm the above suggestion fixes your issue?
--
Adrien Mazarguil
6WIND
--
Adrien Mazarguil
6WIND
Yaroslav Brustinov (ybrustin)
2018-07-25 10:02:10 UTC
Permalink
Hi,

Sounds good to me.

Thanks,
Yaroslav.


-----Original Message-----
From: Adrien Mazarguil [mailto:***@6wind.com]
Sent: Wednesday, July 25, 2018 12:25
To: Yaroslav Brustinov (ybrustin) <***@cisco.com>
Cc: Shahaf Shuler <***@mellanox.com>; Yongseok Koh <***@mellanox.com>; ***@dpdk.org; ***@dpdk.org; Nélio Laranjeiro <***@6wind.com>; Hanoch Haim (hhaim) <***@cisco.com>
Subject: Re: [PATCH] net/mlx5: fix linkage error for glue lib
+Hanoch
I've added -std=c11 flag to our compilation to check.
/usr/bin/ld: Warning: alignment 8 of symbol `mlx5_glue' in
src/dpdk/drivers/net/mlx5/mlx5_glue.c.11.o is smaller than 32 in
src/dpdk/drivers/net/mlx5/mlx5_rxq.c.11.o
/usr/bin/ld: Warning: alignment 16 of symbol `mlx5_glue' in
src/dpdk/drivers/net/mlx5/mlx5_glue.c.11.o is smaller than 32 in
src/dpdk/drivers/net/mlx5/mlx5_rxq.c.11.o
Using alignas(64) does not produce linker warning.
OK, let's forget max_align_t. Even better, how about
alignas(RTE_CACHE_LINE_SIZE) just in case the same GCC version complains about the lack of a 128 byte alignment on architectures like IBM POWER8.

(remember to #include <rte_config.h> for RTE_CACHE_LINE_SIZE)
-----Original Message-----
Sent: Tuesday, July 24, 2018 19:04
Subject: Re: [PATCH] net/mlx5: fix linkage error for glue lib
Hi,
Are you OK with C11?
* C11 code would include stdalign.h and use alignof(max_align_t) however
* we'll stick with C99 for the time being.
:)
Hah, this code was originally intended to become a generic EAL function hence the C99 but got rejected. Mellanox PMDs otherwise rely on C11 features already.
As far as I understand, the issue is alignment of the pointer itself, not the struct.
I'm not familiar with this command: "alignof(max_align_t)".
struct mlx5_glue {
...
} __attribute__((__aligned__(64)));
struct __attribute__((__aligned__(64))) mlx5_glue {
...
};
...still keeps the linker unhappy.
Right, this was my first (wrong) suggestion that doesn't work on
types. How about the second one instead? Here's how the diff on
mlx5_glue.h should look
+#include <stdalign.h>
#include <stddef.h>
#include <stdint.h>
[...]
+alignas(max_align_t)
const struct mlx5_glue *mlx5_glue;
Another comment regarding this patch, commit log should probably mention it addresses a GCC bug that cannot be reproduced with latter versions. Keep in mind DPDK recommends to use at least GCC version 4.9.
-----Original Message-----
Sent: Tuesday, July 24, 2018 16:51
Subject: RE: [PATCH] net/mlx5: fix linkage error for glue lib
Yaroslav,
Subject: Re: [PATCH] net/mlx5: fix linkage error for glue lib
Post by Adrien Mazarguil
Odd, how could this happen assuming both files are compiled
during the same run using identical flags? Looks like a compiler issue.
Anyway, may I suggest an alignment constraint on the structure
#include <stdalign.h>
#include <stddef.h>
[...]
alignas(max_align_t)
struct mlx5_glue {
[...]
};
My bad, this is not a correct use for alignas(), it doesn't work on types.
alignas(max_align_t)
const struct mlx5_glue *mlx5_glue;
Can you confirm the above suggestion fixes your issue?
--
Adrien Mazarguil
Shahaf Shuler
2018-07-25 13:18:54 UTC
Permalink
From: Yaroslav Brustinov <***@cisco.com>

addressing a gcc 4.7.2 bug that cannot be reproduced with latter
versions:

"bin/ld: Warning: alignment 8 of symbol `mlx5_glue' in
src/dpdk/drivers/net/mlx5/mlx5_glue.c.21.o is smaller than 16 in
src/dpdk/drivers/net/mlx5/mlx5_rxq.c.21.o"

Fix it be forcing the alignment of the glue lib.

Fixes: 0e83b8e536c1 ("net/mlx5: move rdma-core calls to separate file")
Cc: ***@dpdk.org
Cc: ***@6wind.com
Cc: ***@6wind.com

Signed-off-by: Yaroslav Brustinov <***@cisco.com>
Signed-off-by: Shahaf Shuler <***@mellanox.com>
---
On v2:
- Forced alignment using alignas to the size of the cacheline.

---
drivers/net/mlx5/mlx5_glue.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_glue.c b/drivers/net/mlx5/mlx5_glue.c
index c7965e51fe..705232f614 100644
--- a/drivers/net/mlx5/mlx5_glue.c
+++ b/drivers/net/mlx5/mlx5_glue.c
@@ -6,6 +6,7 @@
#include <errno.h>
#include <stddef.h>
#include <stdint.h>
+#include <stdalign.h>

/*
* Not needed by this file; included to work around the lack of off_t
@@ -23,6 +24,8 @@
#pragma GCC diagnostic error "-Wpedantic"
#endif

+#include <rte_config.h>
+
#include "mlx5_autoconf.h"
#include "mlx5_glue.h"

@@ -343,7 +346,8 @@ mlx5_glue_dv_create_qp(struct ibv_context *context,
#endif
}

-const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue){
+alignas(RTE_CACHE_LINE_SIZE)
+const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue) {
.version = MLX5_GLUE_VERSION,
.fork_init = mlx5_glue_fork_init,
.alloc_pd = mlx5_glue_alloc_pd,
--
2.12.0
Adrien Mazarguil
2018-07-25 13:22:52 UTC
Permalink
Post by Shahaf Shuler
addressing a gcc 4.7.2 bug that cannot be reproduced with latter
"bin/ld: Warning: alignment 8 of symbol `mlx5_glue' in
src/dpdk/drivers/net/mlx5/mlx5_glue.c.21.o is smaller than 16 in
src/dpdk/drivers/net/mlx5/mlx5_rxq.c.21.o"
Fix it be forcing the alignment of the glue lib.
Fixes: 0e83b8e536c1 ("net/mlx5: move rdma-core calls to separate file")
Except for a couple of minor nits below,
Post by Shahaf Shuler
---
- Forced alignment using alignas to the size of the cacheline.
---
drivers/net/mlx5/mlx5_glue.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/mlx5_glue.c b/drivers/net/mlx5/mlx5_glue.c
index c7965e51fe..705232f614 100644
--- a/drivers/net/mlx5/mlx5_glue.c
+++ b/drivers/net/mlx5/mlx5_glue.c
@@ -6,6 +6,7 @@
#include <errno.h>
#include <stddef.h>
#include <stdint.h>
+#include <stdalign.h>
You should keep alphabetical order.
Post by Shahaf Shuler
/*
* Not needed by this file; included to work around the lack of off_t
@@ -23,6 +24,8 @@
#pragma GCC diagnostic error "-Wpedantic"
#endif
+#include <rte_config.h>
+
#include "mlx5_autoconf.h"
#include "mlx5_glue.h"
@@ -343,7 +346,8 @@ mlx5_glue_dv_create_qp(struct ibv_context *context,
#endif
}
-const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue){
+alignas(RTE_CACHE_LINE_SIZE)
+const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue) {
Extra space added before opening brace :)
Post by Shahaf Shuler
.version = MLX5_GLUE_VERSION,
.fork_init = mlx5_glue_fork_init,
.alloc_pd = mlx5_glue_alloc_pd,
--
2.12.0
--
Adrien Mazarguil
6WIND
Shahaf Shuler
2018-07-26 05:44:51 UTC
Permalink
Subject: Re: [PATCH v2] net/mlx5: fix linkage error for glue lib
Post by Shahaf Shuler
addressing a gcc 4.7.2 bug that cannot be reproduced with latter
"bin/ld: Warning: alignment 8 of symbol `mlx5_glue' in
src/dpdk/drivers/net/mlx5/mlx5_glue.c.21.o is smaller than 16 in
src/dpdk/drivers/net/mlx5/mlx5_rxq.c.21.o"
Fix it be forcing the alignment of the glue lib.
Fixes: 0e83b8e536c1 ("net/mlx5: move rdma-core calls to separate file")
Except for a couple of minor nits below,
Applied to next-net-mlx with the needed fixes. Thanks.

Loading...