Page MenuHomeFreeBSD

Fix make universe with WITH_OFED=yes.
ClosedPublic

Authored by kib on Dec 1 2017, 10:47 PM.

Details

Summary

This patch does the following:

  • Corrects include paths.
  • Remove duplicated symbols from the linker version script. Bfd linker is permissive but lld issues an error.
  • Correctly selects the unordered_map definition namespace.
  • Defines barriers for mips/arm using strongest CPU barriers.

RISC-V was not handled but the logistics required to make it included into make universe on the machine is not worth the efforts.

Test Plan

The

make -s -j 32 SRCCONF=src.conf MAKE_JUST_WORLDS=yes tinderbox

command returns success with src.conf containing the line

WITH_OFED=yes

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kib created this revision.Dec 1 2017, 10:47 PM
kib edited the summary of this revision. (Show Details)Dec 1 2017, 10:48 PM
hselasky added inline comments.Dec 2 2017, 10:34 PM
contrib/ofed/infiniband-diags/build/Makefile.inc
11 ↗(On Diff #36077)

Style: Space before "-I".

Can you explain why you need this CFLAGS? All the relevant includes should be installed in /usr/obj/xxx in the correct location and that should be used instead of the source tree!

contrib/ofed/libibnetdisc/g_hash_table.cpp
34 ↗(On Diff #36077)

Please test this change with both clang and gcc. Last time I tried this on my box, clang didn't define __cplusplus ...

kib added a reviewer: bdrewery.Dec 3 2017, 8:55 AM
kib added inline comments.Dec 3 2017, 8:58 AM
contrib/ofed/infiniband-diags/build/Makefile.inc
11 ↗(On Diff #36077)

I do not know for sure why exactly I needed the change, from the PoV of internals of the build system, but without it compiler cannot find the includes.

But what I suspect happens in your case, where you did not see the build errors, is that the headers were installed into the host and your build was polluted.

Anyway, I do not know about build enough to answer this and I added Brian to might be provide some feedback.

contrib/ofed/libibnetdisc/g_hash_table.cpp
34 ↗(On Diff #36077)

If you read the revision annotation, you would note that I build whole tinderbox with the change. Which includes the architectures which are (cross-)build with gcc.

Clang does not define __cplusplus IFF it builds in non-C++ mode.

hselasky added inline comments.Dec 4 2017, 12:15 PM
contrib/ofed/libibnetdisc/g_hash_table.cpp
34 ↗(On Diff #36077)

I see, so adding the -std=c++11 for clang makes __cplusplus defined.

hselasky added inline comments.Dec 4 2017, 12:17 PM
contrib/ofed/infiniband-diags/build/Makefile.inc
11 ↗(On Diff #36077)

I'll double check this.

hselasky added inline comments.Dec 4 2017, 12:28 PM
contrib/ofed/infiniband-diags/build/Makefile.inc
11 ↗(On Diff #36077)

Did you clean /usr/obj and /usr/include/infiniband before doing this build?

kib added inline comments.Dec 4 2017, 12:37 PM
contrib/ofed/infiniband-diags/build/Makefile.inc
11 ↗(On Diff #36077)

/usr/include/infiniband is empty on my machine, and /usr/obj was not used due to custom MAKEOBJDIRPEFIX.

contrib/ofed/libibnetdisc/g_hash_table.cpp
34 ↗(On Diff #36077)

No, absolutely NOT. __cplusplus must be defined for any compilation of c++ sources. -std=c++XX changes its value, but not presence.

Can you check inside your MAKEOBJDIRPEFIX if there is "include/infiniband" and what its contents might be?

kib added a comment.Dec 4 2017, 1:40 PM

Can you check inside your MAKEOBJDIRPEFIX if there is "include/infiniband" and what its contents might be?

[root@r-freeb43 ~]# find !$ -type d -name infiniband
find obj/root/deviant2/sparc64.sparc64/ -type d -name infiniband
obj/root/deviant2/sparc64.sparc64/tmp/usr/include/infiniband
obj/root/deviant2/sparc64.sparc64/tmp/legacy/usr/include/infiniband
[root@r-freeb43 ~]# ls obj/root/deviant2/sparc64.sparc64/tmp/usr/include/infiniband
arch.h          driver.h        kern-abi.h      sa-kern-abi.h   umad_sa.h
byteorder.h     endian.h        mad.h           sa.h            umad_sm.h
byteswap.h      ib.h            mad_osd.h       types.h         umad_str.h
cm.h            iba             marshall.h      udma_barrier.h  umad_types.h
cm_abi.h        ibnetdisc.h     opcode.h        umad.h          vendor
complib         ibnetdisc_osd.h opensm          umad_cm.h       verbs.h
[root@r-freeb43 ~]# ls obj/root/deviant2/sparc64.sparc64/tmp/legacy/usr/include/infiniband
complib iba     opensm  vendor

Hi,

Yes, it looks like you're right. My test-builds picked up /usr/include/xxx instead of /usr/obj/xxx . In the Makefiles for OFED I've added these CFLAGS, but INCLUDEDIR is still pointing to /usr/include instead of /usr/obj/xxx :-(

contrib/ofed/infiniband-diags/build/Makefile.inc:CFLAGS+= -I${INCLUDEDIR}/infiniband
contrib/ofed/libibnetdisc/Makefile:CFLAGS+= -I${INCLUDEDIR}/infiniband
contrib/ofed/opensm/complib/Makefile:CFLAGS+= -I${INCLUDEDIR}/infiniband
contrib/ofed/opensm/libopensm/Makefile:CFLAGS+= -I${INCLUDEDIR}/infiniband
contrib/ofed/opensm/libvendor/Makefile:CFLAGS+= -I${INCLUDEDIR}/infiniband
contrib/ofed/opensm/opensm/Makefile:CFLAGS+= -I${INCLUDEDIR}/infiniband

If you see how to solve this let me know. I'm looking into it.

I found it. This change should be the correct one:

-CFLAGS+= -I${INCLUDEDIR}/infiniband
+CFLAGS+= -I${SYSROOT:U${DESTDIR}}/${INCLUDEDIR}/infiniband

This revision was not accepted when it landed; it landed in state Needs Review.Dec 11 2017, 11:58 AM
Closed by commit rS326764: ofed: Remove duplicated symbols from the version file. (authored by kib, committed by ). · Explain Why
This revision was automatically updated to reflect the committed changes.