From: Ravi Kerur <rkerur@gmail.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v7 1/6] Move common functions in eal_thread.c
Date: Fri, 24 Apr 2015 12:21:23 -0700 [thread overview]
Message-ID: <CAFb4SLBPzaux+=y+iBSBM=bovC=zSY8HXh4-rGmSHO9J-efA8w@mail.gmail.com> (raw)
In-Reply-To: <20150424185327.GE32445@hmsreliant.think-freely.org>
On Fri, Apr 24, 2015 at 11:53 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Fri, Apr 24, 2015 at 09:45:24AM -0700, Ravi Kerur wrote:
> > On Fri, Apr 24, 2015 at 8:22 AM, Neil Horman <nhorman@tuxdriver.com>
> wrote:
> >
> > > On Fri, Apr 24, 2015 at 08:14:04AM -0700, Ravi Kerur wrote:
> > > > On Fri, Apr 24, 2015 at 6:51 AM, Neil Horman <nhorman@tuxdriver.com>
> > > wrote:
> > > >
> > > > > On Thu, Apr 23, 2015 at 02:35:31PM -0700, Ravi Kerur wrote:
> > > > > > Changes in v7
> > > > > > Remove _setname_ pthread calls.
> > > > > > Use rte_gettid() API in RTE_LOG to print thread_id.
> > > > > >
> > > > > > Changes in v6
> > > > > > Remove RTE_EXEC_ENV_BSDAPP from eal_common_thread.c file.
> > > > > > Add pthread_setname_np/pthread_set_name_np for Linux/FreeBSD
> > > > > > respectively. Plan to use _getname_ in RTE_LOG when available.
> > > > > > Use existing rte_get_systid() in RTE_LOG to print thread_id.
> > > > > >
> > > > > > Changes in v5
> > > > > > Rebase to latest code.
> > > > > >
> > > > > > Changes in v4
> > > > > > None
> > > > > >
> > > > > > Changes in v3
> > > > > > Changed subject to be more explicit on file name inclusion.
> > > > > >
> > > > > > Changes in v2
> > > > > > None
> > > > > >
> > > > > > Changes in v1
> > > > > > eal_thread.c has minor differences between Linux and BSD, move
> > > > > > entire file into common directory.
> > > > > > Use RTE_EXEC_ENV_BSDAPP to differentiate on minor differences.
> > > > > > Rename eal_thread.c to eal_common_thread.c
> > > > > > Makefile changes to reflect file move and name change.
> > > > > > Fix checkpatch warnings.
> > > > > >
> > > > > > Signed-off-by: Ravi Kerur <rkerur@gmail.com>
> > > > > > ---
> > > > > > lib/librte_eal/bsdapp/eal/Makefile | 2 +-
> > > > > > lib/librte_eal/bsdapp/eal/eal_thread.c | 152
> > > > > ------------------------------
> > > > > > lib/librte_eal/common/eal_common_thread.c | 147
> > > > > ++++++++++++++++++++++++++++-
> > > > > > lib/librte_eal/linuxapp/eal/eal_thread.c | 152
> > > > > +-----------------------------
> > > > > > 4 files changed, 148 insertions(+), 305 deletions(-)
> > > > > >
> > > > > > diff --git a/lib/librte_eal/bsdapp/eal/Makefile
> > > > > b/lib/librte_eal/bsdapp/eal/Makefile
> > > > > > index 2357cfa..55971b9 100644
> > > > > > --- a/lib/librte_eal/bsdapp/eal/Makefile
> > > > > > +++ b/lib/librte_eal/bsdapp/eal/Makefile
> > > > > > @@ -87,7 +87,7 @@ CFLAGS_eal_common_log.o := -D_GNU_SOURCE
> > > > > > # workaround for a gcc bug with noreturn attribute
> > > > > > # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
> > > > > > ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
> > > > > > -CFLAGS_eal_thread.o += -Wno-return-type
> > > > > > +CFLAGS_eal_common_thread.o += -Wno-return-type
> > > > > > CFLAGS_eal_hpet.o += -Wno-return-type
> > > > > > endif
> > > > > >
> > > > > > diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c
> > > > > b/lib/librte_eal/bsdapp/eal/eal_thread.c
> > > > > > index 9a03437..5714b8f 100644
> > > > > > --- a/lib/librte_eal/bsdapp/eal/eal_thread.c
> > > > > > +++ b/lib/librte_eal/bsdapp/eal/eal_thread.c
> > > > > > @@ -35,163 +35,11 @@
> > > > > > #include <stdio.h>
> > > > > > #include <stdlib.h>
> > > > > > #include <stdint.h>
> > > > > > -#include <unistd.h>
> > > > > > -#include <sched.h>
> > > > > > -#include <pthread_np.h>
> > > > > > -#include <sys/queue.h>
> > > > > > #include <sys/thr.h>
> > > > > >
> > > > > > -#include <rte_debug.h>
> > > > > > -#include <rte_atomic.h>
> > > > > > -#include <rte_launch.h>
> > > > > > -#include <rte_log.h>
> > > > > > -#include <rte_memory.h>
> > > > > > -#include <rte_memzone.h>
> > > > > > -#include <rte_per_lcore.h>
> > > > > > -#include <rte_eal.h>
> > > > > > -#include <rte_per_lcore.h>
> > > > > > -#include <rte_lcore.h>
> > > > > > -
> > > > > > #include "eal_private.h"
> > > > > > #include "eal_thread.h"
> > > > > >
> > > > > > -RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = LCORE_ID_ANY;
> > > > > NAK, these are exported symbols, you can't remove them without
> going
> > > > > through the
> > > > > deprecation process.
> > > > >
> > > > >
> > > > They are not removed/deleted, they are moved from eal_thread.c to
> > > > eal_common_thread.c file since it is common to both Linux and BSD.
> > > >
> > > Then perhaps you forgot to export the symbol? Its showing up as
> removed
> > > on the
> > > ABI checker utility.
> > >
> > > Neil
> > >
> >
> > Can you please show me in the current code where it is being exported? I
> > have only moved definitions to _common_ files, not sure why it should be
> > exported now. I searched in the current code for RTE_DEFINE_PER_LCORE
> >
> > #home/rkerur/dpdk-tmp/dpdk# grep -ir RTE_DEFINE_PER_LCORE *
> > app/test/test_per_lcore.c:static RTE_DEFINE_PER_LCORE(unsigned, test) =
> > 0x12345678;
> > lib/librte_eal/linuxapp/eal/eal_thread.c:RTE_DEFINE_PER_LCORE(unsigned,
> > _lcore_id) = LCORE_ID_ANY;
> > lib/librte_eal/linuxapp/eal/eal_thread.c:RTE_DEFINE_PER_LCORE(unsigned,
> > _socket_id) = (unsigned)SOCKET_ID_ANY;
> >
> lib/librte_eal/linuxapp/eal/eal_thread.c:RTE_DEFINE_PER_LCORE(rte_cpuset_t,
> > _cpuset);
> > lib/librte_eal/bsdapp/eal/eal_thread.c:RTE_DEFINE_PER_LCORE(unsigned,
> > _lcore_id) = LCORE_ID_ANY;
> > lib/librte_eal/bsdapp/eal/eal_thread.c:RTE_DEFINE_PER_LCORE(unsigned,
> > _socket_id) = (unsigned)SOCKET_ID_ANY;
> > lib/librte_eal/bsdapp/eal/eal_thread.c:RTE_DEFINE_PER_LCORE(rte_cpuset_t,
> > _cpuset);
> > lib/librte_eal/common/include/rte_per_lcore.h:#define
> > RTE_DEFINE_PER_LCORE(type, name) \
> > lib/librte_eal/common/include/rte_eal.h: static
> > RTE_DEFINE_PER_LCORE(int, _thread_id) = -1;
> > lib/librte_eal/common/eal_common_errno.c:RTE_DEFINE_PER_LCORE(int,
> > _rte_errno);
> > lib/librte_eal/common/eal_common_errno.c: static
> > RTE_DEFINE_PER_LCORE(char[RETVAL_SZ], retval);
> >
> >
> > > > Thanks
> > > > Ravi
> > > >
> > > > Regards
> > > > > Neil
> > > > >
> > > > >
> > >
> Its exported in the version map file:
> per_lcore__lcore_id;
>
>
Thanks Neil, I checked and both linux and bsd rte_eal_version.map have it.
I compared .map file between "changed code" and the original, they are same
for both linux and bsd. In fact you had ACK'd v4 version of this patch
series and no major changes after that. Please let me know if I missed
something.
dpdk-common-eal-f-4 has all the common code movement changes
dpdk-tmp is original code
#/home/rkerur/dpdk-common-eal-f-4/dpdk# diff
lib/librte_eal/linuxapp/eal/rte_eal_version.map
/home/rkerur/dpdk-tmp/dpdk/lib/librte_eal/linuxapp/eal/rte_eal_version.map
<No difference>
#/home/rkerur/dpdk-common-eal-f-4/dpdk# diff
lib/librte_eal/bsdapp/eal/rte_eal_version.map
/home/rkerur/dpdk-tmp/dpdk/lib/librte_eal/bsdapp/eal/rte_eal_version.map
<No difference>
Thanks,
Ravi
Neil
>
>
next prev parent reply other threads:[~2015-04-24 19:21 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-23 21:35 [dpdk-dev] [PATCH v7 0/6] Move EAL common functions Ravi Kerur
2015-04-23 21:35 ` [dpdk-dev] [PATCH v7 1/6] Move common functions in eal_thread.c Ravi Kerur
2015-04-23 21:35 ` [dpdk-dev] [PATCH v7 2/6] Move common functions in eal.c Ravi Kerur
2015-04-23 21:35 ` [dpdk-dev] [PATCH v7 3/6] Move common functions in eal_lcore.c Ravi Kerur
2015-04-23 21:35 ` [dpdk-dev] [PATCH v7 4/6] Move common functions in eal_timer.c Ravi Kerur
2015-04-23 21:35 ` [dpdk-dev] [PATCH v7 5/6] Move common functions in eal_memory.c Ravi Kerur
2015-04-23 21:35 ` [dpdk-dev] [PATCH v7 6/6] Move common functions in eal_pci.c Ravi Kerur
2015-04-24 13:51 ` [dpdk-dev] [PATCH v7 1/6] Move common functions in eal_thread.c Neil Horman
2015-04-24 15:14 ` Ravi Kerur
2015-04-24 15:22 ` Neil Horman
2015-04-24 16:45 ` Ravi Kerur
2015-04-24 18:53 ` Neil Horman
2015-04-24 19:21 ` Ravi Kerur [this message]
2015-04-24 19:51 ` Neil Horman
2015-04-24 21:24 ` Ravi Kerur
2015-04-25 1:45 ` Ravi Kerur
2015-04-25 12:32 ` Neil Horman
2015-04-25 13:02 ` Neil Horman
2015-04-26 0:09 ` Ravi Kerur
2015-04-27 13:44 ` Neil Horman
2015-04-27 22:39 ` Ravi Kerur
2015-04-28 19:35 ` Neil Horman
2015-04-28 23:52 ` Ravi Kerur
2015-04-29 10:04 ` Neil Horman
2015-04-29 17:47 ` Ravi Kerur
2015-04-30 16:00 ` Neil Horman
2015-05-01 0:15 ` Ravi Kerur
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='CAFb4SLBPzaux+=y+iBSBM=bovC=zSY8HXh4-rGmSHO9J-efA8w@mail.gmail.com' \
--to=rkerur@gmail.com \
--cc=dev@dpdk.org \
--cc=nhorman@tuxdriver.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).