Re: [dpdk-dev] [PATCH v6 2/6] Move common functions in eal.c

[Ravi Kerur <rkerur@gmail.com>]

On Wed, Apr 22, 2015 at 2:21 AM, Bruce Richardson <
bruce.richardson@intel.com> wrote:

> On Tue, Apr 21, 2015 at 12:13:22PM -0700, Ravi Kerur wrote:
> > On Tue, Apr 21, 2015 at 7:36 AM, Bruce Richardson <
> > bruce.richardson@intel.com> wrote:
> >
> > > On Sat, Apr 18, 2015 at 12:43:07PM -0700, Ravi Kerur wrote:
> > > > Changes in v6
> > > > Split eal_common_system.c and eal_common_runtime.c into
> > > > eal_common_sysfs.c
> > > > eal_common_mem_cfg.c
> > > > eal_common_proc_type.c
> > > > eal_comm_app_usage.c
> > > > based on functionality.
> > > >
> > > > Changes in v5
> > > > Rebase to latest code.
> > > >
> > > > Changes in v4
> > > > Remove eal_externs.h file, instead use  _get_ and _set_ APIS
> > > > to access those variables.
> > > > Split eal_common.c into eal_common_system.c and
> > > > and eal_common_runtime.c
> > > > rte_eal prefix functions are moved to _runtime_ and
> > > > eal prefix functions are moved to _system_ files respectively.
> > > >
> > > > Changes in v3
> > > > Changed subject to be more explicit on file name inclusion.
> > > >
> > > > Changes in v2
> > > > In function rte_eal_config_create remove #ifdef _BSDAPP_
> > > > and initialize mem_cfg_addr unconditionally.
> > > >
> > > > Changes in v1
> > > > Move common functions in eal.c to librte_eal/common/eal_common.c.
> > > >
> > > > Following functions are moved to eal_common.c file.
> > > >
> > > > struct rte_config *rte_eal_get_configuration(void);
> > > > int eal_parse_sysfs_value(const char *filename, unsigned long *val);
> > > > static void rte_eal_config_create(void);
> > > > enum rte_proc_type_t eal_proc_type_detect(void);
> > > > void rte_eal_config_init(void);
> > > > rte_usage_hook_t rte_set_application_usage_hook(rte_usage_hook_t
> > > > usage_func);
> > > > inline size_t eal_get_hugepage_mem_size(void);
> > > > void eal_check_mem_on_local_socket(void);
> > > > int sync_func(__attribute__((unused)) void *arg);
> > > > inline void rte_eal_mcfg_complete(void);
> > > > int rte_eal_has_hugepages(void);
> > > > enum rte_lcore_role_t rte_eal_lcore_role(unsigned lcore_id);
> > > > enum rte_proc_type_t rte_eal_process_type(void);
> > > >
> > > > Makefile changes to reflect new files added.
> > > > Fix checkpatch warnings and errors.
> > > >
> > > > Signed-off-by: Ravi Kerur <rkerur@gmail.com>
> > > > ---
> > > >  lib/librte_eal/bsdapp/eal/Makefile           |   4 +
> > > >  lib/librte_eal/bsdapp/eal/eal.c              | 271
> > > +++---------------------
> > > >  lib/librte_eal/common/eal_common_app_usage.c |  63 ++++++
> > > >  lib/librte_eal/common/eal_common_mem_cfg.c   | 217
> ++++++++++++++++++++
> > > >  lib/librte_eal/common/eal_common_proc_type.c |  58 ++++++
> > > >  lib/librte_eal/common/eal_common_sysfs.c     | 163 +++++++++++++++
> > > >  lib/librte_eal/common/eal_hugepages.h        |   1 +
> > > >  lib/librte_eal/common/eal_private.h          |  78 +++++++
> > > >  lib/librte_eal/common/include/rte_eal.h      |   4 +
> > > >  lib/librte_eal/linuxapp/eal/Makefile         |   4 +
> > > >  lib/librte_eal/linuxapp/eal/eal.c            | 296
> > > ++++-----------------------
> > > >  11 files changed, 668 insertions(+), 491 deletions(-)
> > > >  create mode 100644 lib/librte_eal/common/eal_common_app_usage.c
> > > >  create mode 100644 lib/librte_eal/common/eal_common_mem_cfg.c
> > > >  create mode 100644 lib/librte_eal/common/eal_common_proc_type.c
> > > >  create mode 100644 lib/librte_eal/common/eal_common_sysfs.c
> > > >
> > > On applying this patch in the series I'm getting compiler errors. With
> > > clang on
> > > linux (and BSD), I get errors about undefined inline functions e.g.:
> > >
> > > In file included from /home/bruce/
> > > dpdk.org/lib/librte_eal/linuxapp/eal/eal.c:84:
> > > /home/bruce/dpdk.org/lib/librte_eal/common/eal_hugepages.h:66:15:
> error:
> > > inline function 'eal_get_hugepage_mem_size' is not defined
> > > [-Werror,-Wundefined-inline]
> > > inline size_t eal_get_hugepage_mem_size(void);
> > >               ^
> > >       /home/bruce/dpdk.org/lib/librte_eal/linuxapp/eal/eal.c:549:29:
> > > note: used here
> > >                               internal_config.memory =
> > > eal_get_hugepage_mem_size();
> > >
> > > ICC is also giving a (different error):
> > >
> > >   CC eal_common_sysfs.o
> > >   /home/bruce/dpdk.org/lib/librte_eal/common/eal_common_sysfs.c(159):
> > > error #188: enumerated type mixed with another type
> > >                   return -1;
> > >
> > >
> > Thanks Bruce for reporting these errors. My plan was to wait for final
> "OK"
> > from Thomas for v6 patches esp. on changes for split in eal.c and
> eal_pci.c
> > files. Currently my testing includes
> >
> > 1. Compiling with gcc 4.8.2 on Ubuntu and freeBSD
> > 2. "make test" and testpmd on Linux
> > 3. Basic testpmd on freeBSD
> >
> > I will definitely fix these before sending out next version.
> >
> > Regarding installation of "ICC" compiler, I had tried following Intel
> > reference
> >
> >
> https://software.intel.com/en-us/articles/intel-compilers-linux-installation-help
> >
> > At least my understanding was it required License from Intel. Please let
> me
> > know for any alternate way of getting ICC.
> >
> > Thanks,
> > Ravi
> >
> >
> I'm just flagging the ICC error as an FYI, given that you are fixing other
> compiler errors anyway. Since ICC is an Intel product and we
> don't expect all contributors to have it, we in Intel will take
> responsibility
> for submitting patches to ensure ICC compiles the DPDK code ok.
>

Thanks Bruce, I will make sure I fix reported ICC errors in v7.

>
> /Bruce
>
>