| Message ID | 1464367686-3475-2-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 579315A32; Fri, 27 May 2016 18:48:24 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 752385690 for <dev@dpdk.org>; Fri, 27 May 2016 18:48:22 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 27 May 2016 09:48:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,374,1459839600"; d="scan'208";a="986030034" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by orsmga002.jf.intel.com with ESMTP; 27 May 2016 09:48:20 -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 u4RGmHJJ000517; Fri, 27 May 2016 17:48:17 +0100 Received: from sivswdev02.ir.intel.com (localhost [127.0.0.1]) by sivswdev02.ir.intel.com with ESMTP id u4RGmHuF003521; Fri, 27 May 2016 17:48:17 +0100 Received: (from fyigit@localhost) by sivswdev02.ir.intel.com with id u4RGmH3f003517; Fri, 27 May 2016 17:48:17 +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:06 +0100 Message-Id: <1464367686-3475-2-git-send-email-ferruh.yigit@intel.com> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <1464367686-3475-1-git-send-email-ferruh.yigit@intel.com> References: <574872B3.6040702@intel.com> <1464367686-3475-1-git-send-email-ferruh.yigit@intel.com> Subject: [dpdk-dev] [PATCH 2/2] mk: reduce scope of whole-archive to pmd libraries 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
--whole-archive argument only required for pmd libraries, and currently
it covers more libraries. Reducing scope of the argument to pmd
libraries slightly reduce final application size.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
mk/rte.app.mk | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Comments
2016-05-27 17:48, Ferruh Yigit: > --whole-archive argument only required for pmd libraries, and currently > it covers more libraries. Reducing scope of the argument to pmd > libraries slightly reduce final application size. In my understanding, --whole-archive is required for static libraries used by plugins: http://dpdk.org/commit/20afd76a If we want to restrict it, I would say it must be around EAL, ring, mbuf, mempool, ethdev, cryptodev, etc.
On 6/10/2016 10:03 AM, Thomas Monjalon wrote: > 2016-05-27 17:48, Ferruh Yigit: >> --whole-archive argument only required for pmd libraries, and currently >> it covers more libraries. Reducing scope of the argument to pmd >> libraries slightly reduce final application size. > > In my understanding, --whole-archive is required for static libraries used > by plugins: > http://dpdk.org/commit/20afd76a Right, required for static libraries. But more than used by plugins, required for plugin to work. > > If we want to restrict it, I would say it must be around EAL, ring, > mbuf, mempool, ethdev, cryptodev, etc. > We should restrict to plugins. What happens is, since there is no direct call to PMDs from application, PMD code is not included into final application. To force PMDs to be included into final binary, --whole-archive is required. But --whole-archive is not required for other libraries, and just cause unnecessary increase in binary size. Sample logs, with same setup, [1] compiled without --whole-archive, [2] is current code: [1]: # ./testpmd -- -i EAL: Detected 32 lcore(s) EAL: Probing VFIO support... EAL: No probed ethernet devices Interactive-mode selected USER1: create a new mbuf pool <mbuf_pool_socket_0>: n=395456, size=2176, socket=0 Done testpmd> show port info all testpmd> quit [2] # ./testpmd -- -i EAL: Detected 32 lcore(s) EAL: Probing VFIO support... EAL: PCI device 0000:04:00.0 on NUMA socket 0 EAL: probe driver: 8086:1521 rte_igb_pmd EAL: PCI device 0000:08:00.0 on NUMA socket 0 EAL: probe driver: 8086:10fb rte_ixgbe_pmd EAL: PCI device 0000:08:00.1 on NUMA socket 0 EAL: probe driver: 8086:10fb rte_ixgbe_pmd EAL: PCI device 0000:0a:00.0 on NUMA socket 0 EAL: probe driver: 8086:1584 rte_i40e_pmd EAL: PCI device 0000:81:00.0 on NUMA socket 1 EAL: probe driver: 8086:10fb rte_ixgbe_pmd EAL: PCI device 0000:81:00.1 on NUMA socket 1 EAL: probe driver: 8086:10fb rte_ixgbe_pmd EAL: PCI device 0000:86:00.0 on NUMA socket 1 EAL: probe driver: 8086:15a4 rte_pmd_fm10k EAL: PCI device 0000:87:00.0 on NUMA socket 1 EAL: probe driver: 8086:15a4 rte_pmd_fm10k EAL: PCI device 0000:88:00.0 on NUMA socket 1 EAL: probe driver: 8086:1572 rte_i40e_pmd EAL: PCI device 0000:88:00.1 on NUMA socket 1 EAL: probe driver: 8086:1572 rte_i40e_pmd EAL: PCI device 0000:88:00.2 on NUMA socket 1 EAL: probe driver: 8086:1572 rte_i40e_pmd EAL: PCI device 0000:88:00.3 on NUMA socket 1 EAL: probe driver: 8086:1572 rte_i40e_pmd Interactive-mode selected USER1: create a new mbuf pool <mbuf_pool_socket_0>: n=395456, size=2176, socket=0 Configuring Port 0 (socket 0) Port 0: 90:E2:BA:0E:49:B9 Configuring Port 1 (socket 0) Port 1: 00:1B:21:76:FA:21 Checking link statuses... Port 0 Link Up - speed 10000 Mbps - full-duplex Port 1 Link Up - speed 10000 Mbps - full-duplex Done testpmd>
2016-06-10 10:57, Ferruh Yigit: > On 6/10/2016 10:03 AM, Thomas Monjalon wrote: > > 2016-05-27 17:48, Ferruh Yigit: > >> --whole-archive argument only required for pmd libraries, and currently > >> it covers more libraries. Reducing scope of the argument to pmd > >> libraries slightly reduce final application size. > > > > In my understanding, --whole-archive is required for static libraries used > > by plugins: > > http://dpdk.org/commit/20afd76a > Right, required for static libraries. But more than used by plugins, > required for plugin to work. > > > If we want to restrict it, I would say it must be around EAL, ring, > > mbuf, mempool, ethdev, cryptodev, etc. > > > > We should restrict to plugins. What happens is, since there is no direct > call to PMDs from application, PMD code is not included into final > application. To force PMDs to be included into final binary, > --whole-archive is required. > > But --whole-archive is not required for other libraries, and just cause > unnecessary increase in binary size. Yes it is necessary for other libraries, otherwise some symbols needed by plugins won't be available. Example: In function `rte_pmd_af_packet_devuninit': rte_eth_af_packet.c:(.text+0x8bf): undefined reference to `rte_eth_dev_allocated' rte_eth_af_packet.c:(.text+0x930): undefined reference to `rte_eth_dev_release_port'
On 6/10/2016 11:18 AM, Thomas Monjalon wrote: > 2016-06-10 10:57, Ferruh Yigit: >> On 6/10/2016 10:03 AM, Thomas Monjalon wrote: >>> 2016-05-27 17:48, Ferruh Yigit: >>>> --whole-archive argument only required for pmd libraries, and currently >>>> it covers more libraries. Reducing scope of the argument to pmd >>>> libraries slightly reduce final application size. >>> >>> In my understanding, --whole-archive is required for static libraries used >>> by plugins: >>> http://dpdk.org/commit/20afd76a >> Right, required for static libraries. But more than used by plugins, >> required for plugin to work. >> >>> If we want to restrict it, I would say it must be around EAL, ring, >>> mbuf, mempool, ethdev, cryptodev, etc. >>> >> >> We should restrict to plugins. What happens is, since there is no direct >> call to PMDs from application, PMD code is not included into final >> application. To force PMDs to be included into final binary, >> --whole-archive is required. >> >> But --whole-archive is not required for other libraries, and just cause >> unnecessary increase in binary size. > > Yes it is necessary for other libraries, otherwise some symbols needed by > plugins won't be available. > Example: > In function `rte_pmd_af_packet_devuninit': > rte_eth_af_packet.c:(.text+0x8bf): undefined reference to `rte_eth_dev_allocated' > rte_eth_af_packet.c:(.text+0x930): undefined reference to `rte_eth_dev_release_port' > If there is a direct call to that API, linker should include required object file into final binary, --whole-archive shouldn't be required for this. What is special for PMD's is there are not directly called, but initialized using constructors. How are you getting above compilation error? Thanks, ferruh
2016-06-10 12:06, Ferruh Yigit: > On 6/10/2016 11:18 AM, Thomas Monjalon wrote: > > 2016-06-10 10:57, Ferruh Yigit: > >> On 6/10/2016 10:03 AM, Thomas Monjalon wrote: > >>> 2016-05-27 17:48, Ferruh Yigit: > >>>> --whole-archive argument only required for pmd libraries, and currently > >>>> it covers more libraries. Reducing scope of the argument to pmd > >>>> libraries slightly reduce final application size. > >>> > >>> In my understanding, --whole-archive is required for static libraries used > >>> by plugins: > >>> http://dpdk.org/commit/20afd76a > >> Right, required for static libraries. But more than used by plugins, > >> required for plugin to work. > >> > >>> If we want to restrict it, I would say it must be around EAL, ring, > >>> mbuf, mempool, ethdev, cryptodev, etc. > >>> > >> > >> We should restrict to plugins. What happens is, since there is no direct > >> call to PMDs from application, PMD code is not included into final > >> application. To force PMDs to be included into final binary, > >> --whole-archive is required. > >> > >> But --whole-archive is not required for other libraries, and just cause > >> unnecessary increase in binary size. > > > > Yes it is necessary for other libraries, otherwise some symbols needed by > > plugins won't be available. > > Example: > > In function `rte_pmd_af_packet_devuninit': > > rte_eth_af_packet.c:(.text+0x8bf): undefined reference to `rte_eth_dev_allocated' > > rte_eth_af_packet.c:(.text+0x930): undefined reference to `rte_eth_dev_release_port' > > > If there is a direct call to that API, linker should include required > object file into final binary, --whole-archive shouldn't be required for > this. > > What is special for PMD's is there are not directly called, but > initialized using constructors. > > How are you getting above compilation error? The error was generated with your patch on top of others I'm going to send. I suggest to continue the discussion/review based on the v2 coming soon.
On 6/10/2016 1:21 PM, Thomas Monjalon wrote: > 2016-06-10 12:06, Ferruh Yigit: >> On 6/10/2016 11:18 AM, Thomas Monjalon wrote: >>> 2016-06-10 10:57, Ferruh Yigit: >>>> On 6/10/2016 10:03 AM, Thomas Monjalon wrote: >>>>> 2016-05-27 17:48, Ferruh Yigit: >>>>>> --whole-archive argument only required for pmd libraries, and currently >>>>>> it covers more libraries. Reducing scope of the argument to pmd >>>>>> libraries slightly reduce final application size. >>>>> >>>>> In my understanding, --whole-archive is required for static libraries used >>>>> by plugins: >>>>> http://dpdk.org/commit/20afd76a >>>> Right, required for static libraries. But more than used by plugins, >>>> required for plugin to work. >>>> >>>>> If we want to restrict it, I would say it must be around EAL, ring, >>>>> mbuf, mempool, ethdev, cryptodev, etc. >>>>> >>>> >>>> We should restrict to plugins. What happens is, since there is no direct >>>> call to PMDs from application, PMD code is not included into final >>>> application. To force PMDs to be included into final binary, >>>> --whole-archive is required. >>>> >>>> But --whole-archive is not required for other libraries, and just cause >>>> unnecessary increase in binary size. >>> >>> Yes it is necessary for other libraries, otherwise some symbols needed by >>> plugins won't be available. >>> Example: >>> In function `rte_pmd_af_packet_devuninit': >>> rte_eth_af_packet.c:(.text+0x8bf): undefined reference to `rte_eth_dev_allocated' >>> rte_eth_af_packet.c:(.text+0x930): undefined reference to `rte_eth_dev_release_port' >>> >> If there is a direct call to that API, linker should include required >> object file into final binary, --whole-archive shouldn't be required for >> this. >> >> What is special for PMD's is there are not directly called, but >> initialized using constructors. >> >> How are you getting above compilation error? > > The error was generated with your patch on top of others I'm going to send. > I suggest to continue the discussion/review based on the v2 coming soon. > I have quickly checked v2, you are getting this compiler error because --start-group/--end-group removed, so pmd not able to resolve the references. And you are working around this by adding all object of library unconditionally (--whole-archive). I will comment more of v2 patch. Thanks, ferruh
diff --git a/mk/rte.app.mk b/mk/rte.app.mk index e12226c..8bfe240 100644 --- a/mk/rte.app.mk +++ b/mk/rte.app.mk @@ -57,7 +57,6 @@ _LDLIBS-y += -L$(RTE_SDK_BIN)/lib # Order is important: from higher level to lower level # -_LDLIBS-y += --whole-archive _LDLIBS-y += --start-group _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += -lrte_distributor @@ -129,6 +128,8 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lrte_pmd_xenvirt ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n) # plugins (link only if static libraries) +_LDLIBS-y += --whole-archive + _LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += -lrte_pmd_vmxnet3_uio _LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += -lrte_pmd_virtio _LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD) += -lrte_pmd_bnx2x @@ -174,11 +175,12 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += -lrte_pmd_vhost endif # $(CONFIG_RTE_LIBRTE_VHOST) +_LDLIBS-y += --no-whole-archive + endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB) _LDLIBS-y += $(EXECENV_LDLIBS) _LDLIBS-y += --end-group -_LDLIBS-y += --no-whole-archive LDLIBS += $(_LDLIBS-y) $(CPU_LDLIBS) $(EXTRA_LDLIBS)