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

[Bruce Richardson <bruce.richardson@intel.com>]

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.

/Bruce