All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Christian Ehrhardt <christian.ehrhardt@canonical.com>,
	Panu Matilainen <pmatilai@redhat.com>
Cc: Thomas Monjalon <thomas.monjalon@6wind.com>, dev <dev@dpdk.org>
Subject: Re: [PATCH] mk: fix underlinking issues of most librte libraries
Date: Thu, 26 May 2016 14:13:38 +0100	[thread overview]
Message-ID: <5746F682.9070003@intel.com> (raw)
In-Reply-To: <CAATJJ0LXpKn1bq9Akjx-LtLrQPKKL_m=DgQ19=sC9sHFXQHTDQ@mail.gmail.com>

On 5/24/2016 11:11 AM, Christian Ehrhardt wrote:
> Hi Panu,
> I already agreed to Thomas on IRC but won't have time until next week.
> Thanks for making a patch that does that already - I'll give it a look and
> some test on my end next week then.
> 
> 
> 
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
> 
> On Tue, May 24, 2016 at 11:56 AM, Panu Matilainen <pmatilai@redhat.com>
> wrote:
> 
>> On 05/20/2016 08:08 PM, Thomas Monjalon wrote:
>>
>>> 2016-05-20 18:50, Christian Ehrhardt:
>>>
>>>> The individual libraries have various cross dependencies.
>>>> This is already refelcted in the DEPDIR dependency, but not yet in
>>>> proper DT_NEEDED flags in the .so's.
>>>> This adds the -l flags so that is properly stored in the .so's ELF
>>>> headers.
>>>>
>>>
>>> Why not filling LDLIBS by parsing DEPDIRS-y in rte.lib.mk?
>>>
>>>
>> A fair question :) The thought has passed my mind too, but I thought it'd
>> be too messy with differing library vs directory names and other
>> exceptions. But in reality manually maintained separate LDLIBS is only
>> going to get out of sync sooner or later, and turns out its not that bad
>> anyway:
>>
>> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
>> index b420280..88b4e98 100644
>> --- a/mk/rte.lib.mk
>> +++ b/mk/rte.lib.mk
>> @@ -77,6 +77,12 @@ else
>>  _CPU_LDFLAGS := $(CPU_LDFLAGS)
>>  endif
>>
>> +# Translate DEPDIRS-y into LDLIBS
>> +IGNORE_DEPS = -lrte_eal/% -lrte_net -lrte_compat
>> +_LDDIRS = $(subst librte_ether,libethdev,$(DEPDIRS-y))
>> +_LDDEPS = $(subst lib/lib,-l,$(_LDDIRS))
>> +LDLIBS += $(filter-out $(IGNORE_DEPS), $(_LDDEPS))
>> +
>>  O_TO_A = $(AR) crDs $(LIB) $(OBJS-y)
>>  O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
>>  O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  AR $(@)")
>>
>> I'm also sure somebody more familiar with gmake could improve it, but it
>> seems to work quite fine here. I'll wait a bit to see if there are other
>> suggestions before sending it as a proper patch.
>>
>>         - Panu -
>>

I did test the patch with minor diff in rte.vars.mk [1], otherwise all
libraries kept needed in final library.

Patch itself looks good, overlinking problem in final binary seems fixed
[2] [3].

But this cause a compilation error for cmdline_test [4]. This is because
of cyclic dependency between librte_eal <-> librte_mempool.

Other applications don't have this compilation error because they use
librte_mempool, and since it is linked against application,  this error
not observed. cmdline_test itself doesn't have any direct call to
librte_mempool.

It is possible to add workaround to cmdline_test [5], (introduce
overlinking back), but should we attack on cyclic dependency instead?


Thanks,
ferruh

---

[1]
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 =


[2]
# ldd build/app/testpmd
        linux-vdso.so.1 (0x00007ffcff92e000)
        librte_mbuf.so.2.1 => .../build/lib/librte_mbuf.so.2.1
        libethdev.so.4.1 => .../build/lib/libethdev.so.4.1
        librte_mempool.so.2.1 => .../build/lib/librte_mempool.so.2.1
        librte_ring.so.1.1 => .../build/lib/librte_ring.so.1.1
        librte_eal.so.2.1 => .../build/lib/librte_eal.so.2.1
        librte_cmdline.so.2.1 => .../build/lib/librte_cmdline.so.2.1
        librte_pmd_bond.so.1.1 => .../build/lib/librte_pmd_bond.so.1.1
        libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f6879ff1000)
        libc.so.6 => /lib64/libc.so.6 (0x00007f6879c30000)
        /lib64/ld-linux-x86-64.so.2 (0x0000564998dbd000)
        libdl.so.2 => /lib64/libdl.so.2 (0x00007f6879a2c000)
        libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f6879814000)
        librt.so.1 => /lib64/librt.so.1 (0x00007f687960c000)
        librte_kvargs.so.1.1 => .../build/lib/librte_kvargs.so.1.1

[3]
# ldd -u -r build/app/testpmd
#


[4]
== Build app/cmdline_test
  CC cmdline_test.o
  CC commands.o
  LD cmdline_test
.../build/lib/librte_eal.so: undefined reference to `rte_mempool_lookup'
.../build/lib/librte_eal.so: undefined reference to `rte_mempool_create'
collect2: error: ld returned 1 exit status
.../mk/rte.app.mk:243: recipe for target 'cmdline_test' failed
make[3]: *** [cmdline_test] Error 1


[5]
diff --git a/app/cmdline_test/Makefile b/app/cmdline_test/Makefile
index c6169f5..69ebfb2 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

  reply	other threads:[~2016-05-26 13:14 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-19 15:38 Underlinked libs and overlinked applications - an issue? Christian Ehrhardt
2016-05-19 18:17 ` Thomas Monjalon
2016-05-20 11:01   ` Panu Matilainen
2016-05-20 12:41     ` Christian Ehrhardt
2016-05-20 16:43       ` Christian Ehrhardt
2016-05-20 16:50         ` [PATCH] mk: fix underlinking issues of most librte libraries Christian Ehrhardt
2016-05-20 17:08           ` Thomas Monjalon
2016-05-24  9:56             ` Panu Matilainen
2016-05-24 10:11               ` Christian Ehrhardt
2016-05-26 13:13                 ` Ferruh Yigit [this message]
2016-05-27  5:59                   ` Panu Matilainen
2016-05-27 16:15                     ` Ferruh Yigit
2016-05-27 16:48                       ` [PATCH 1/2] mk: prevent overlinking in applications Ferruh Yigit
2016-05-27 16:48                         ` [PATCH 2/2] mk: reduce scope of whole-archive to pmd libraries Ferruh Yigit
2016-06-10  9:03                           ` Thomas Monjalon
2016-06-10  9:57                             ` Ferruh Yigit
2016-06-10 10:18                               ` Thomas Monjalon
2016-06-10 11:06                                 ` Ferruh Yigit
2016-06-10 12:21                                   ` Thomas Monjalon
2016-06-10 13:35                                     ` Ferruh Yigit
2016-06-09 10:10                         ` [PATCH 1/2] mk: prevent overlinking in applications Thomas Monjalon
2016-06-09 13:05                           ` Ferruh Yigit
2016-06-10 13:19                         ` [PATCH v2 0/6] reduce " Thomas Monjalon
2016-06-10 13:19                           ` [PATCH v2 1/6] mk: sort drivers in static application link list Thomas Monjalon
2016-06-10 13:19                           ` [PATCH v2 2/6] mk: fix driver dependencies order for static application Thomas Monjalon
2016-06-10 14:49                             ` Ferruh Yigit
2016-06-10 14:54                               ` Thomas Monjalon
2016-06-10 13:19                           ` [PATCH v2 3/6] mk: remove library grouping during application linking Thomas Monjalon
2016-06-10 14:54                             ` Ferruh Yigit
2016-06-10 15:14                               ` Thomas Monjalon
2016-06-10 13:19                           ` [PATCH v2 4/6] mk: prevent overlinking in applications Thomas Monjalon
2016-06-10 14:56                             ` Ferruh Yigit
2016-06-10 15:15                               ` Thomas Monjalon
2016-06-10 13:19                           ` [PATCH v2 5/6] mk: sort libraries in level order when linking Thomas Monjalon
2016-06-10 13:19                           ` [PATCH v2 6/6] mk: reduce scope of whole-archive static linking Thomas Monjalon
2016-06-10 15:03                             ` Ferruh Yigit
2016-06-10 15:18                               ` Thomas Monjalon
2016-06-10 18:32                           ` [PATCH v3 0/6] reduce overlinking in applications Ferruh Yigit
2016-06-10 18:32                             ` [PATCH v3 1/6] mk: sort drivers in static application link list Ferruh Yigit
2016-06-10 19:38                               ` Wiles, Keith
2016-06-10 18:32                             ` [PATCH v3 2/6] mk: fix driver dependencies order for static application Ferruh Yigit
2016-06-10 18:32                             ` [PATCH v3 3/6] mk: sort libraries when linking, move pmd libs to higher level Ferruh Yigit
2016-06-13  9:29                               ` Thomas Monjalon
2016-06-13 10:04                                 ` Ferruh Yigit
2016-06-13 10:21                                   ` Thomas Monjalon
2016-06-13 10:29                                     ` Ferruh Yigit
2016-06-10 18:32                             ` [PATCH v3 4/6] mk: sort libraries when linking, move external libs to lower level Ferruh Yigit
2016-06-10 18:32                             ` [PATCH v3 5/6] mk: remove library grouping during application linking Ferruh Yigit
2016-06-10 18:32                             ` [PATCH v3 6/6] mk: prevent overlinking in applications Ferruh Yigit
2016-06-11  6:34                               ` Thomas Monjalon
2016-06-13  9:05                                 ` Ferruh Yigit
2016-06-13  9:24                                   ` Christian Ehrhardt
2016-06-13  9:48                                     ` [PATCH] mk: fix missing DEPDIRS to avoid libarary underlinking Christian Ehrhardt
2016-06-13 10:08                                       ` Thomas Monjalon
2016-06-13 10:20                                         ` Christian Ehrhardt
2016-06-13 11:06                                           ` [PATCH v2 0/2] mk: fix more library underlinking Christian Ehrhardt
2016-06-13 11:06                                             ` [PATCH v2 1/2] mk: fix missing vhost dependency to pthread to avoid libarary underlinking Christian Ehrhardt
2016-06-13 11:06                                             ` [PATCH v2 2/2] mk: fix missing DEPDIRS " Christian Ehrhardt
2016-06-13 14:16                                             ` [PATCH v2 0/2] mk: fix more library underlinking Thomas Monjalon
2016-06-13 14:14                           ` [PATCH v2 0/6] reduce overlinking in applications Thomas Monjalon
2016-06-07  8:23               ` [PATCH] mk: fix underlinking issues of most librte libraries Thomas Monjalon
2016-06-07  9:25                 ` Panu Matilainen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5746F682.9070003@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=christian.ehrhardt@canonical.com \
    --cc=dev@dpdk.org \
    --cc=pmatilai@redhat.com \
    --cc=thomas.monjalon@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.