Message ID | 1464367686-3475-1-git-send-email-ferruh.yigit@intel.com (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Thomas Monjalon |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 32B9156A9; Fri, 27 May 2016 18:48:15 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id C61C15690 for <dev@dpdk.org>; Fri, 27 May 2016 18:48:13 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga101.fm.intel.com with ESMTP; 27 May 2016 09:48:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,374,1459839600"; d="scan'208";a="963697710" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by orsmga001.jf.intel.com with ESMTP; 27 May 2016 09:48:12 -0700 Received: from sivswdev02.ir.intel.com (sivswdev02.ir.intel.com [10.237.217.46]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id u4RGm80X000478; Fri, 27 May 2016 17:48:08 +0100 Received: from sivswdev02.ir.intel.com (localhost [127.0.0.1]) by sivswdev02.ir.intel.com with ESMTP id u4RGm8po003512; Fri, 27 May 2016 17:48:08 +0100 Received: (from fyigit@localhost) by sivswdev02.ir.intel.com with id u4RGm8XZ003508; Fri, 27 May 2016 17:48:08 +0100 From: Ferruh Yigit <ferruh.yigit@intel.com> To: dev@dpdk.org Cc: Panu Matilainen <pmatilai@redhat.com>, Christian Ehrhardt <christian.ehrhardt@canonical.com>, Thomas Monjalon <thomas.monjalon@6wind.com>, Ferruh Yigit <ferruh.yigit@intel.com> Date: Fri, 27 May 2016 17:48:05 +0100 Message-Id: <1464367686-3475-1-git-send-email-ferruh.yigit@intel.com> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <574872B3.6040702@intel.com> References: <574872B3.6040702@intel.com> Subject: [dpdk-dev] [PATCH 1/2] mk: prevent overlinking in applications X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Ferruh Yigit
May 27, 2016, 4:48 p.m. UTC
Replace --no-as-needed linker flag with --as-needed flag, which will
only link libraries directly called by application. This requires inter
library dependencies resolved correctly.
Not linking all libraries cause a compile error for lpcap and possible
to have other similar compiler errors, so increasing the scope of
--start-group argument.
cmdline_test application causes compile error because of cyclic
dependency between librte_eal <-> librte_mempool. A workaround added to
cmdline_test for compile error.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
This patch is on top of patch:
http://dpdk.org/dev/patchwork/patch/12987/
---
app/cmdline_test/Makefile | 1 +
mk/exec-env/linuxapp/rte.vars.mk | 2 +-
mk/rte.app.mk | 3 +--
3 files changed, 3 insertions(+), 3 deletions(-)
Comments
Hi Ferruh, 2016-05-27 17:48, Ferruh Yigit: > Replace --no-as-needed linker flag with --as-needed flag, which will > only link libraries directly called by application. This requires inter > library dependencies resolved correctly. > > Not linking all libraries cause a compile error for lpcap and possible > to have other similar compiler errors, so increasing the scope of > --start-group argument. What is the error? > cmdline_test application causes compile error because of cyclic > dependency between librte_eal <-> librte_mempool. A workaround added to > cmdline_test for compile error. > > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> > --- a/app/cmdline_test/Makefile > +++ b/app/cmdline_test/Makefile > @@ -46,6 +46,7 @@ SRCS-y += commands.c > > CFLAGS += -O3 > CFLAGS += $(WERROR_FLAGS) A comment is required here to explain the workaround. > +LDFLAGS += -no-as-needed The option should be --no-as-needed. > --- a/mk/exec-env/linuxapp/rte.vars.mk > +++ b/mk/exec-env/linuxapp/rte.vars.mk > @@ -46,7 +46,7 @@ EXECENV_CFLAGS = -pthread > endif > > # Workaround lack of DT_NEEDED entry This comment is obsolete now. > -EXECENV_LDFLAGS = --no-as-needed > +EXECENV_LDFLAGS = --as-needed Why put this option for Linux only? Shouldn't we restrict this option to app.mk only? > --- a/mk/rte.app.mk > +++ b/mk/rte.app.mk > @@ -58,6 +58,7 @@ _LDLIBS-y += -L$(RTE_SDK_BIN)/lib > # > > _LDLIBS-y += --whole-archive > +_LDLIBS-y += --start-group --start-group must be used only to solve circular dependencies. Ideally we should not use it. I think it's needed only because of EAL logs using mempool (must be removed). Why extending it? I'm afraid we are masking some issues. > _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += -lrte_distributor > _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) += -lrte_reorder > @@ -111,8 +112,6 @@ _LDLIBS-y += -lcrypto > endif > endif # !CONFIG_RTE_BUILD_SHARED_LIBS > > -_LDLIBS-y += --start-group > - > _LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) += -lrte_kvargs > _LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) += -lrte_mbuf > _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag
On 6/9/2016 11:10 AM, Thomas Monjalon wrote: > Hi Ferruh, > > 2016-05-27 17:48, Ferruh Yigit: >> Replace --no-as-needed linker flag with --as-needed flag, which will >> only link libraries directly called by application. This requires inter >> library dependencies resolved correctly. >> >> Not linking all libraries cause a compile error for lpcap and possible >> to have other similar compiler errors, so increasing the scope of >> --start-group argument. > > What is the error? > This is with -as-needed flag, and static library compilation: .../dpdk/build/lib/librte_pmd_pcap.a(rte_eth_pcap.o): In function `eth_dev_stop': rte_eth_pcap.c:(.text+0xcc1): undefined reference to `pcap_dump_close' rte_eth_pcap.c:(.text+0xcd6): undefined reference to `pcap_close' rte_eth_pcap.c:(.text+0xd19): undefined reference to `pcap_close' rte_eth_pcap.c:(.text+0xd58): undefined reference to `pcap_close' /root/development/dpdk/build/lib/librte_pmd_pcap.a(rte_eth_pcap.o): In function `open_tx_pcap': rte_eth_pcap.c:(.text+0x190b): undefined reference to `pcap_open_dead' rte_eth_pcap.c:(.text+0x191b): undefined reference to `pcap_dump_open' ... pcap pmd has libpcap calls, but while linking final application (testpmd), linker is not able to find objects that has these. The command line for compilation is: gcc ... -o test ... -Wl,--whole-archive <dpdk libs> -Wl,-lpcap -Wl,--start-group <more dpdk libs> -Wl,-lrte_pmd_pcap <more dpdk libs> -Wl,--end-group -Wl,--no-whole-archive -lpcap is provided, but since before -lrte_pmd_pcap, references not resolved. Previously, with "-no-as-needed" flag, all shared libraries linked always, which was preventing compile error. >> cmdline_test application causes compile error because of cyclic >> dependency between librte_eal <-> librte_mempool. A workaround added to >> cmdline_test for compile error. >> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> > >> --- a/app/cmdline_test/Makefile >> +++ b/app/cmdline_test/Makefile >> @@ -46,6 +46,7 @@ SRCS-y += commands.c >> >> CFLAGS += -O3 >> CFLAGS += $(WERROR_FLAGS) > > A comment is required here to explain the workaround. > Sure, I will add a comment and send a new version of the patch. >> +LDFLAGS += -no-as-needed > > The option should be --no-as-needed. Right > >> --- a/mk/exec-env/linuxapp/rte.vars.mk >> +++ b/mk/exec-env/linuxapp/rte.vars.mk >> @@ -46,7 +46,7 @@ EXECENV_CFLAGS = -pthread >> endif >> >> # Workaround lack of DT_NEEDED entry > > This comment is obsolete now. Will remove. > >> -EXECENV_LDFLAGS = --no-as-needed >> +EXECENV_LDFLAGS = --as-needed > > Why put this option for Linux only? When this option not defined at all, different compilers behave different, some has --as-needed as default and cause compilation issues, I guess that is why for Linux --no-as-needed is forces. I assume BSD compilers by default use --no-as-needed, so we don't have any compilation problem. And it looks like there is no specific reason to not force BSD to --as-needed, I will test it. > Shouldn't we restrict this option to app.mk only? Having --no-needed flag only for app should be OK. Let me test first, and as a result should we move this to app.mk? > >> --- a/mk/rte.app.mk >> +++ b/mk/rte.app.mk >> @@ -58,6 +58,7 @@ _LDLIBS-y += -L$(RTE_SDK_BIN)/lib >> # >> >> _LDLIBS-y += --whole-archive >> +_LDLIBS-y += --start-group > > --start-group must be used only to solve circular dependencies. > Ideally we should not use it. I think it's needed only because > of EAL logs using mempool (must be removed). No, this is not the reason, please check above lpcap compile error, that is the reason of this update. > Why extending it? > I'm afraid we are masking some issues. But if we have a convention to add external libraries after dpdk libraries, that also should solve this issue, but that is more update than just updating --start-group. > >> _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += -lrte_distributor >> _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) += -lrte_reorder >> @@ -111,8 +112,6 @@ _LDLIBS-y += -lcrypto >> endif >> endif # !CONFIG_RTE_BUILD_SHARED_LIBS >> >> -_LDLIBS-y += --start-group >> - >> _LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) += -lrte_kvargs >> _LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) += -lrte_mbuf >> _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag >
This is a respin of the ideas of Christian and Ferruh to limit the static application size or dynamic links to shared libraries. It also brings some clean-up in rte.app.mk. Ferruh Yigit (2): mk: prevent overlinking in applications mk: reduce scope of whole-archive static linking Thomas Monjalon (4): mk: sort drivers in static application link list mk: fix driver dependencies order for static application mk: remove library grouping during application linking mk: sort libraries in level order when linking mk/exec-env/linuxapp/rte.vars.mk | 3 - mk/rte.app.mk | 134 ++++++++++++++++----------------------- 2 files changed, 55 insertions(+), 82 deletions(-)
This patch set updates application linking, main motivation is to reduce overlinking in application, also removes library grouping by re-ordering libraries and for static compilation reduces whole library linking to PMD libs. There is a workaround because of librte_eal <-> librte_mempool cyclic dependency in rte.app.mk, which can be removed when issue solved. It also brings some clean-up in rte.app.mk. Ferruh Yigit (3): mk: sort libraries when linking, move pmd libs to higher level mk: sort libraries when linking, move external libs to lower level mk: prevent overlinking in applications Thomas Monjalon (3): mk: sort drivers in static application link list mk: fix driver dependencies order for static application mk: remove library grouping during application linking mk/exec-env/linuxapp/rte.vars.mk | 3 - mk/rte.app.mk | 152 +++++++++++++++++---------------------- 2 files changed, 66 insertions(+), 89 deletions(-)
2016-06-10 15:19, Thomas Monjalon: > This is a respin of the ideas of Christian and Ferruh to limit > the static application size or dynamic links to shared libraries. > > It also brings some clean-up in rte.app.mk. > > Ferruh Yigit (2): > mk: prevent overlinking in applications > mk: reduce scope of whole-archive static linking > > Thomas Monjalon (4): > mk: sort drivers in static application link list > mk: fix driver dependencies order for static application > mk: remove library grouping during application linking > mk: sort libraries in level order when linking Applied
diff --git a/app/cmdline_test/Makefile b/app/cmdline_test/Makefile index c6169f5..5b7a2a2 100644 --- a/app/cmdline_test/Makefile +++ b/app/cmdline_test/Makefile @@ -46,6 +46,7 @@ SRCS-y += commands.c CFLAGS += -O3 CFLAGS += $(WERROR_FLAGS) +LDFLAGS += -no-as-needed # this application needs libraries first DEPDIRS-y += lib drivers diff --git a/mk/exec-env/linuxapp/rte.vars.mk b/mk/exec-env/linuxapp/rte.vars.mk index d51bd17..10d37d5 100644 --- a/mk/exec-env/linuxapp/rte.vars.mk +++ b/mk/exec-env/linuxapp/rte.vars.mk @@ -46,7 +46,7 @@ EXECENV_CFLAGS = -pthread endif # Workaround lack of DT_NEEDED entry -EXECENV_LDFLAGS = --no-as-needed +EXECENV_LDFLAGS = --as-needed EXECENV_LDLIBS = EXECENV_ASFLAGS = diff --git a/mk/rte.app.mk b/mk/rte.app.mk index b84b56d..e12226c 100644 --- a/mk/rte.app.mk +++ b/mk/rte.app.mk @@ -58,6 +58,7 @@ _LDLIBS-y += -L$(RTE_SDK_BIN)/lib # _LDLIBS-y += --whole-archive +_LDLIBS-y += --start-group _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += -lrte_distributor _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) += -lrte_reorder @@ -111,8 +112,6 @@ _LDLIBS-y += -lcrypto endif endif # !CONFIG_RTE_BUILD_SHARED_LIBS -_LDLIBS-y += --start-group - _LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) += -lrte_kvargs _LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) += -lrte_mbuf _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag