| Message ID | 1419521597-31978-8-git-send-email-rkerur@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Thu, Dec 25, 2014 at 10:33:17AM -0500, Ravi Kerur wrote: > Move common functions in eal_memory.c to librte_eal/common > directory. > Use RTE_EXEC_ENV_BSDAPP to differentiate minor differences in > common functions. > Fix checkpatch warnings and errors. > > Signed-off-by: Ravi Kerur <rkerur@gmail.com> > --- > lib/librte_eal/bsdapp/eal/eal_memory.c | 36 ++------------------------ > lib/librte_eal/common/eal_common_memory.c | 43 +++++++++++++++++++++++++++++-- > lib/librte_eal/common/eal_private.h | 28 ++++++++++++++++++++ > lib/librte_eal/linuxapp/eal/eal_memory.c | 36 ++------------------------ > 4 files changed, 73 insertions(+), 70 deletions(-) > > diff --git a/lib/librte_eal/bsdapp/eal/eal_memory.c b/lib/librte_eal/bsdapp/eal/eal_memory.c > index 65ee87d..b192705 100644 > --- a/lib/librte_eal/bsdapp/eal/eal_memory.c > +++ b/lib/librte_eal/bsdapp/eal/eal_memory.c > @@ -59,7 +59,7 @@ rte_mem_virt2phy(const void *virtaddr) > return RTE_BAD_PHYS_ADDR; > } > > -static int > +int > rte_eal_contigmem_init(void) > { > struct rte_mem_config *mcfg; > @@ -130,7 +130,7 @@ rte_eal_contigmem_init(void) > return 0; > } > > -static int > +int > rte_eal_contigmem_attach(void) > { > const struct hugepage_info *hpi; > @@ -190,35 +190,3 @@ error: > return -1; > } > > - > -static int > -rte_eal_memdevice_init(void) > -{ > - struct rte_config *config; > - > - if (rte_eal_process_type() == RTE_PROC_SECONDARY) > - return 0; > - > - config = rte_eal_get_configuration(); > - config->mem_config->nchannel = internal_config.force_nchannel; > - config->mem_config->nrank = internal_config.force_nrank; > - > - return 0; > -} > - > -/* init memory subsystem */ > -int > -rte_eal_memory_init(void) > -{ > - RTE_LOG(INFO, EAL, "Setting up physically contiguous memory...\n"); > - const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? > - rte_eal_contigmem_init() : > - rte_eal_contigmem_attach(); > - if (retval < 0) > - return -1; > - > - if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() < 0) > - return -1; > - > - return 0; > -} > diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c > index 77830f8..da7aa98 100644 > --- a/lib/librte_eal/common/eal_common_memory.c > +++ b/lib/librte_eal/common/eal_common_memory.c > @@ -46,6 +46,7 @@ > #include <rte_log.h> > > #include "eal_private.h" > +#include "eal_internal_cfg.h" > > /* > * Return a pointer to a read-only table of struct rte_physmem_desc > @@ -70,7 +71,7 @@ rte_eal_get_physmem_size(void) > /* get pointer to global configuration */ > mcfg = rte_eal_get_configuration()->mem_config; > > - for (i=0; i<RTE_MAX_MEMSEG; i++) { > + for (i = 0; i < RTE_MAX_MEMSEG; i++) { > if (mcfg->memseg[i].addr == NULL) > break; > > @@ -90,7 +91,7 @@ rte_dump_physmem_layout(FILE *f) > /* get pointer to global configuration */ > mcfg = rte_eal_get_configuration()->mem_config; > > - for (i=0; i<RTE_MAX_MEMSEG; i++) { > + for (i = 0; i < RTE_MAX_MEMSEG; i++) { > if (mcfg->memseg[i].addr == NULL) > break; > > @@ -119,3 +120,41 @@ unsigned rte_memory_get_nrank(void) > { > return rte_eal_get_configuration()->mem_config->nrank; > } > + > +static int > +rte_eal_memdevice_init(void) > +{ > + struct rte_config *config; > + > + if (rte_eal_process_type() == RTE_PROC_SECONDARY) > + return 0; > + > + config = rte_eal_get_configuration(); > + config->mem_config->nchannel = internal_config.force_nchannel; > + config->mem_config->nrank = internal_config.force_nrank; > + > + return 0; > +} > + > +/* init memory subsystem */ > +int > +rte_eal_memory_init(void) > +{ > + RTE_LOG(INFO, EAL, "Setting up physically contiguous memory...\n"); > + const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? > +#ifdef RTE_EXEC_ENV_BSDAPP > + rte_eal_contigmem_init() : > + rte_eal_contigmem_attach(); > +#else /* RTE_EXEC_ENV_BSDAPP */ > + rte_eal_hugepage_init() : > + rte_eal_hugepage_attach(); > +#endif /* RTE_EXEC_ENV_BSDAPP */ Given that the functions you are calling here are only ever build for the platform they are called from, it seems to me that you can give them a shared name, and just build the appropriate one. I.e. you don't need to add ifdeffery here. Neil > + > + if (retval < 0) > + return -1; > + > + if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() < 0) > + return -1; > + > + return 0; > +} > diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h > index 19af23d..16338a2 100644 > --- a/lib/librte_eal/common/eal_private.h > +++ b/lib/librte_eal/common/eal_private.h > @@ -286,6 +286,20 @@ int get_ncpus(void); > */ > int set_tsc_freq_from_sysctl(void); > > +/** > + * This function prepares physical memory mapping > + * > + * This function is private to the EAL. > + */ > +int rte_eal_contigmem_init(void); > + > +/** > + * This function creates memory mapping in secondary > + * > + * This function is private to the EAL. > + */ > +int rte_eal_contigmem_attach(void); > + > #else /* RTE_EXEC_ENV_BSDAPP */ > /** > * This function check if cpu is present > @@ -301,6 +315,20 @@ int cpu_detected(unsigned lcore_id); > */ > int set_tsc_freq_from_clock(void); > > +/** > + * This function prepares physical memory mapping > + * > + * This function is private to the EAL. > + */ > +int rte_eal_hugepage_init(void); > + > +/** > + * This function creates memory mapping in secondary > + * > + * This function is private to the EAL. > + */ > +int rte_eal_hugepage_attach(void); > + > #endif /* RTE_EXEC_ENV_BSDAPP */ > > #endif /* _EAL_PRIVATE_H_ */ > diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c > index bae2507..f4d91df 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_memory.c > +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c > @@ -1031,7 +1031,7 @@ calc_num_pages_per_socket(uint64_t * memory, > * 6. unmap the first mapping > * 7. fill memsegs in configuration with contiguous zones > */ > -static int > +int > rte_eal_hugepage_init(void) > { > struct rte_mem_config *mcfg; > @@ -1369,7 +1369,7 @@ getFileSize(int fd) > * configuration and finds the hugepages which form that segment, mapping them > * in order to form a contiguous block in the virtual memory space > */ > -static int > +int > rte_eal_hugepage_attach(void) > { > const struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config; > @@ -1530,35 +1530,3 @@ error: > return -1; > } > > -static int > -rte_eal_memdevice_init(void) > -{ > - struct rte_config *config; > - > - if (rte_eal_process_type() == RTE_PROC_SECONDARY) > - return 0; > - > - config = rte_eal_get_configuration(); > - config->mem_config->nchannel = internal_config.force_nchannel; > - config->mem_config->nrank = internal_config.force_nrank; > - > - return 0; > -} > - > - > -/* init memory subsystem */ > -int > -rte_eal_memory_init(void) > -{ > - RTE_LOG(INFO, EAL, "Setting up memory...\n"); > - const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? > - rte_eal_hugepage_init() : > - rte_eal_hugepage_attach(); > - if (retval < 0) > - return -1; > - > - if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() < 0) > - return -1; > - > - return 0; > -} > -- > 1.9.1 > >
Inline <rk> On Thu, Dec 25, 2014 at 9:46 AM, Neil Horman <nhorman@tuxdriver.com> wrote: > On Thu, Dec 25, 2014 at 10:33:17AM -0500, Ravi Kerur wrote: > > Move common functions in eal_memory.c to librte_eal/common > > directory. > > Use RTE_EXEC_ENV_BSDAPP to differentiate minor differences in > > common functions. > > Fix checkpatch warnings and errors. > > > > Signed-off-by: Ravi Kerur <rkerur@gmail.com> > > --- > > lib/librte_eal/bsdapp/eal/eal_memory.c | 36 > ++------------------------ > > lib/librte_eal/common/eal_common_memory.c | 43 > +++++++++++++++++++++++++++++-- > > lib/librte_eal/common/eal_private.h | 28 ++++++++++++++++++++ > > lib/librte_eal/linuxapp/eal/eal_memory.c | 36 > ++------------------------ > > 4 files changed, 73 insertions(+), 70 deletions(-) > > > > diff --git a/lib/librte_eal/bsdapp/eal/eal_memory.c > b/lib/librte_eal/bsdapp/eal/eal_memory.c > > index 65ee87d..b192705 100644 > > --- a/lib/librte_eal/bsdapp/eal/eal_memory.c > > +++ b/lib/librte_eal/bsdapp/eal/eal_memory.c > > @@ -59,7 +59,7 @@ rte_mem_virt2phy(const void *virtaddr) > > return RTE_BAD_PHYS_ADDR; > > } > > > > -static int > > +int > > rte_eal_contigmem_init(void) > > { > > struct rte_mem_config *mcfg; > > @@ -130,7 +130,7 @@ rte_eal_contigmem_init(void) > > return 0; > > } > > > > -static int > > +int > > rte_eal_contigmem_attach(void) > > { > > const struct hugepage_info *hpi; > > @@ -190,35 +190,3 @@ error: > > return -1; > > } > > > > - > > -static int > > -rte_eal_memdevice_init(void) > > -{ > > - struct rte_config *config; > > - > > - if (rte_eal_process_type() == RTE_PROC_SECONDARY) > > - return 0; > > - > > - config = rte_eal_get_configuration(); > > - config->mem_config->nchannel = internal_config.force_nchannel; > > - config->mem_config->nrank = internal_config.force_nrank; > > - > > - return 0; > > -} > > - > > -/* init memory subsystem */ > > -int > > -rte_eal_memory_init(void) > > -{ > > - RTE_LOG(INFO, EAL, "Setting up physically contiguous memory...\n"); > > - const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? > > - rte_eal_contigmem_init() : > > - rte_eal_contigmem_attach(); > > - if (retval < 0) > > - return -1; > > - > > - if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() < 0) > > - return -1; > > - > > - return 0; > > -} > > diff --git a/lib/librte_eal/common/eal_common_memory.c > b/lib/librte_eal/common/eal_common_memory.c > > index 77830f8..da7aa98 100644 > > --- a/lib/librte_eal/common/eal_common_memory.c > > +++ b/lib/librte_eal/common/eal_common_memory.c > > @@ -46,6 +46,7 @@ > > #include <rte_log.h> > > > > #include "eal_private.h" > > +#include "eal_internal_cfg.h" > > > > /* > > * Return a pointer to a read-only table of struct rte_physmem_desc > > @@ -70,7 +71,7 @@ rte_eal_get_physmem_size(void) > > /* get pointer to global configuration */ > > mcfg = rte_eal_get_configuration()->mem_config; > > > > - for (i=0; i<RTE_MAX_MEMSEG; i++) { > > + for (i = 0; i < RTE_MAX_MEMSEG; i++) { > > if (mcfg->memseg[i].addr == NULL) > > break; > > > > @@ -90,7 +91,7 @@ rte_dump_physmem_layout(FILE *f) > > /* get pointer to global configuration */ > > mcfg = rte_eal_get_configuration()->mem_config; > > > > - for (i=0; i<RTE_MAX_MEMSEG; i++) { > > + for (i = 0; i < RTE_MAX_MEMSEG; i++) { > > if (mcfg->memseg[i].addr == NULL) > > break; > > > > @@ -119,3 +120,41 @@ unsigned rte_memory_get_nrank(void) > > { > > return rte_eal_get_configuration()->mem_config->nrank; > > } > > + > > +static int > > +rte_eal_memdevice_init(void) > > +{ > > + struct rte_config *config; > > + > > + if (rte_eal_process_type() == RTE_PROC_SECONDARY) > > + return 0; > > + > > + config = rte_eal_get_configuration(); > > + config->mem_config->nchannel = internal_config.force_nchannel; > > + config->mem_config->nrank = internal_config.force_nrank; > > + > > + return 0; > > +} > > + > > +/* init memory subsystem */ > > +int > > +rte_eal_memory_init(void) > > +{ > > + RTE_LOG(INFO, EAL, "Setting up physically contiguous memory...\n"); > > + const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? > > +#ifdef RTE_EXEC_ENV_BSDAPP > > + rte_eal_contigmem_init() : > > + rte_eal_contigmem_attach(); > > +#else /* RTE_EXEC_ENV_BSDAPP */ > > + rte_eal_hugepage_init() : > > + rte_eal_hugepage_attach(); > > +#endif /* RTE_EXEC_ENV_BSDAPP */ > Given that the functions you are calling here are only ever build for the > platform they are called from, it seems to me that you can give them a > shared > name, and just build the appropriate one. I.e. you don't need to add > ifdeffery > here. > > <rk> Agreed and will be done. Only reason I left it as is because I thought it might create confusion since in the code/document in BSD it's referred as contigmem rather than hugepage. > Neil > > > + > > + if (retval < 0) > > + return -1; > > + > > + if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() < 0) > > + return -1; > > + > > + return 0; > > +} > > diff --git a/lib/librte_eal/common/eal_private.h > b/lib/librte_eal/common/eal_private.h > > index 19af23d..16338a2 100644 > > --- a/lib/librte_eal/common/eal_private.h > > +++ b/lib/librte_eal/common/eal_private.h > > @@ -286,6 +286,20 @@ int get_ncpus(void); > > */ > > int set_tsc_freq_from_sysctl(void); > > > > +/** > > + * This function prepares physical memory mapping > > + * > > + * This function is private to the EAL. > > + */ > > +int rte_eal_contigmem_init(void); > > + > > +/** > > + * This function creates memory mapping in secondary > > + * > > + * This function is private to the EAL. > > + */ > > +int rte_eal_contigmem_attach(void); > > + > > #else /* RTE_EXEC_ENV_BSDAPP */ > > /** > > * This function check if cpu is present > > @@ -301,6 +315,20 @@ int cpu_detected(unsigned lcore_id); > > */ > > int set_tsc_freq_from_clock(void); > > > > +/** > > + * This function prepares physical memory mapping > > + * > > + * This function is private to the EAL. > > + */ > > +int rte_eal_hugepage_init(void); > > + > > +/** > > + * This function creates memory mapping in secondary > > + * > > + * This function is private to the EAL. > > + */ > > +int rte_eal_hugepage_attach(void); > > + > > #endif /* RTE_EXEC_ENV_BSDAPP */ > > > > #endif /* _EAL_PRIVATE_H_ */ > > diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c > b/lib/librte_eal/linuxapp/eal/eal_memory.c > > index bae2507..f4d91df 100644 > > --- a/lib/librte_eal/linuxapp/eal/eal_memory.c > > +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c > > @@ -1031,7 +1031,7 @@ calc_num_pages_per_socket(uint64_t * memory, > > * 6. unmap the first mapping > > * 7. fill memsegs in configuration with contiguous zones > > */ > > -static int > > +int > > rte_eal_hugepage_init(void) > > { > > struct rte_mem_config *mcfg; > > @@ -1369,7 +1369,7 @@ getFileSize(int fd) > > * configuration and finds the hugepages which form that segment, > mapping them > > * in order to form a contiguous block in the virtual memory space > > */ > > -static int > > +int > > rte_eal_hugepage_attach(void) > > { > > const struct rte_mem_config *mcfg = > rte_eal_get_configuration()->mem_config; > > @@ -1530,35 +1530,3 @@ error: > > return -1; > > } > > > > -static int > > -rte_eal_memdevice_init(void) > > -{ > > - struct rte_config *config; > > - > > - if (rte_eal_process_type() == RTE_PROC_SECONDARY) > > - return 0; > > - > > - config = rte_eal_get_configuration(); > > - config->mem_config->nchannel = internal_config.force_nchannel; > > - config->mem_config->nrank = internal_config.force_nrank; > > - > > - return 0; > > -} > > - > > - > > -/* init memory subsystem */ > > -int > > -rte_eal_memory_init(void) > > -{ > > - RTE_LOG(INFO, EAL, "Setting up memory...\n"); > > - const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? > > - rte_eal_hugepage_init() : > > - rte_eal_hugepage_attach(); > > - if (retval < 0) > > - return -1; > > - > > - if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() < 0) > > - return -1; > > - > > - return 0; > > -} > > -- > > 1.9.1 > > > > >
On Thu, Dec 25, 2014 at 11:22:53AM -0800, Ravi Kerur wrote: > Inline <rk> > > On Thu, Dec 25, 2014 at 9:46 AM, Neil Horman <nhorman@tuxdriver.com> wrote: > > > On Thu, Dec 25, 2014 at 10:33:17AM -0500, Ravi Kerur wrote: > > > Move common functions in eal_memory.c to librte_eal/common > > > directory. > > > Use RTE_EXEC_ENV_BSDAPP to differentiate minor differences in > > > common functions. > > > Fix checkpatch warnings and errors. > > > > > > Signed-off-by: Ravi Kerur <rkerur@gmail.com> > > > --- > > > lib/librte_eal/bsdapp/eal/eal_memory.c | 36 > > ++------------------------ > > > lib/librte_eal/common/eal_common_memory.c | 43 > > +++++++++++++++++++++++++++++-- > > > lib/librte_eal/common/eal_private.h | 28 ++++++++++++++++++++ > > > lib/librte_eal/linuxapp/eal/eal_memory.c | 36 > > ++------------------------ > > > 4 files changed, 73 insertions(+), 70 deletions(-) > > > > > > diff --git a/lib/librte_eal/bsdapp/eal/eal_memory.c > > b/lib/librte_eal/bsdapp/eal/eal_memory.c > > > index 65ee87d..b192705 100644 > > > --- a/lib/librte_eal/bsdapp/eal/eal_memory.c > > > +++ b/lib/librte_eal/bsdapp/eal/eal_memory.c > > > @@ -59,7 +59,7 @@ rte_mem_virt2phy(const void *virtaddr) > > > return RTE_BAD_PHYS_ADDR; > > > } > > > > > > -static int > > > +int > > > rte_eal_contigmem_init(void) > > > { > > > struct rte_mem_config *mcfg; > > > @@ -130,7 +130,7 @@ rte_eal_contigmem_init(void) > > > return 0; > > > } > > > > > > -static int > > > +int > > > rte_eal_contigmem_attach(void) > > > { > > > const struct hugepage_info *hpi; > > > @@ -190,35 +190,3 @@ error: > > > return -1; > > > } > > > > > > - > > > -static int > > > -rte_eal_memdevice_init(void) > > > -{ > > > - struct rte_config *config; > > > - > > > - if (rte_eal_process_type() == RTE_PROC_SECONDARY) > > > - return 0; > > > - > > > - config = rte_eal_get_configuration(); > > > - config->mem_config->nchannel = internal_config.force_nchannel; > > > - config->mem_config->nrank = internal_config.force_nrank; > > > - > > > - return 0; > > > -} > > > - > > > -/* init memory subsystem */ > > > -int > > > -rte_eal_memory_init(void) > > > -{ > > > - RTE_LOG(INFO, EAL, "Setting up physically contiguous memory...\n"); > > > - const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? > > > - rte_eal_contigmem_init() : > > > - rte_eal_contigmem_attach(); > > > - if (retval < 0) > > > - return -1; > > > - > > > - if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() < 0) > > > - return -1; > > > - > > > - return 0; > > > -} > > > diff --git a/lib/librte_eal/common/eal_common_memory.c > > b/lib/librte_eal/common/eal_common_memory.c > > > index 77830f8..da7aa98 100644 > > > --- a/lib/librte_eal/common/eal_common_memory.c > > > +++ b/lib/librte_eal/common/eal_common_memory.c > > > @@ -46,6 +46,7 @@ > > > #include <rte_log.h> > > > > > > #include "eal_private.h" > > > +#include "eal_internal_cfg.h" > > > > > > /* > > > * Return a pointer to a read-only table of struct rte_physmem_desc > > > @@ -70,7 +71,7 @@ rte_eal_get_physmem_size(void) > > > /* get pointer to global configuration */ > > > mcfg = rte_eal_get_configuration()->mem_config; > > > > > > - for (i=0; i<RTE_MAX_MEMSEG; i++) { > > > + for (i = 0; i < RTE_MAX_MEMSEG; i++) { > > > if (mcfg->memseg[i].addr == NULL) > > > break; > > > > > > @@ -90,7 +91,7 @@ rte_dump_physmem_layout(FILE *f) > > > /* get pointer to global configuration */ > > > mcfg = rte_eal_get_configuration()->mem_config; > > > > > > - for (i=0; i<RTE_MAX_MEMSEG; i++) { > > > + for (i = 0; i < RTE_MAX_MEMSEG; i++) { > > > if (mcfg->memseg[i].addr == NULL) > > > break; > > > > > > @@ -119,3 +120,41 @@ unsigned rte_memory_get_nrank(void) > > > { > > > return rte_eal_get_configuration()->mem_config->nrank; > > > } > > > + > > > +static int > > > +rte_eal_memdevice_init(void) > > > +{ > > > + struct rte_config *config; > > > + > > > + if (rte_eal_process_type() == RTE_PROC_SECONDARY) > > > + return 0; > > > + > > > + config = rte_eal_get_configuration(); > > > + config->mem_config->nchannel = internal_config.force_nchannel; > > > + config->mem_config->nrank = internal_config.force_nrank; > > > + > > > + return 0; > > > +} > > > + > > > +/* init memory subsystem */ > > > +int > > > +rte_eal_memory_init(void) > > > +{ > > > + RTE_LOG(INFO, EAL, "Setting up physically contiguous memory...\n"); > > > + const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? > > > +#ifdef RTE_EXEC_ENV_BSDAPP > > > + rte_eal_contigmem_init() : > > > + rte_eal_contigmem_attach(); > > > +#else /* RTE_EXEC_ENV_BSDAPP */ > > > + rte_eal_hugepage_init() : > > > + rte_eal_hugepage_attach(); > > > +#endif /* RTE_EXEC_ENV_BSDAPP */ > > Given that the functions you are calling here are only ever build for the > > platform they are called from, it seems to me that you can give them a > > shared > > name, and just build the appropriate one. I.e. you don't need to add > > ifdeffery > > here. > > > > > <rk> Agreed and will be done. Only reason I left it as is because I thought > it might create confusion since in the code/document in BSD it's referred > as contigmem rather than hugepage. > I agree, it might be a bit confusing, but I think documenting it would be superior to creating separate function names with additional ifdeffery. cscope illustrates pretty well multiple function definitions. Neil
On Fri, Dec 26, 2014 at 6:44 AM, Neil Horman <nhorman@tuxdriver.com> wrote: > On Thu, Dec 25, 2014 at 11:22:53AM -0800, Ravi Kerur wrote: > > Inline <rk> > > > > On Thu, Dec 25, 2014 at 9:46 AM, Neil Horman <nhorman@tuxdriver.com> > wrote: > > > > > On Thu, Dec 25, 2014 at 10:33:17AM -0500, Ravi Kerur wrote: > > > > Move common functions in eal_memory.c to librte_eal/common > > > > directory. > > > > Use RTE_EXEC_ENV_BSDAPP to differentiate minor differences in > > > > common functions. > > > > Fix checkpatch warnings and errors. > > > > > > > > Signed-off-by: Ravi Kerur <rkerur@gmail.com> > > > > --- > > > > lib/librte_eal/bsdapp/eal/eal_memory.c | 36 > > > ++------------------------ > > > > lib/librte_eal/common/eal_common_memory.c | 43 > > > +++++++++++++++++++++++++++++-- > > > > lib/librte_eal/common/eal_private.h | 28 ++++++++++++++++++++ > > > > lib/librte_eal/linuxapp/eal/eal_memory.c | 36 > > > ++------------------------ > > > > 4 files changed, 73 insertions(+), 70 deletions(-) > > > > > > > > diff --git a/lib/librte_eal/bsdapp/eal/eal_memory.c > > > b/lib/librte_eal/bsdapp/eal/eal_memory.c > > > > index 65ee87d..b192705 100644 > > > > --- a/lib/librte_eal/bsdapp/eal/eal_memory.c > > > > +++ b/lib/librte_eal/bsdapp/eal/eal_memory.c > > > > @@ -59,7 +59,7 @@ rte_mem_virt2phy(const void *virtaddr) > > > > return RTE_BAD_PHYS_ADDR; > > > > } > > > > > > > > -static int > > > > +int > > > > rte_eal_contigmem_init(void) > > > > { > > > > struct rte_mem_config *mcfg; > > > > @@ -130,7 +130,7 @@ rte_eal_contigmem_init(void) > > > > return 0; > > > > } > > > > > > > > -static int > > > > +int > > > > rte_eal_contigmem_attach(void) > > > > { > > > > const struct hugepage_info *hpi; > > > > @@ -190,35 +190,3 @@ error: > > > > return -1; > > > > } > > > > > > > > - > > > > -static int > > > > -rte_eal_memdevice_init(void) > > > > -{ > > > > - struct rte_config *config; > > > > - > > > > - if (rte_eal_process_type() == RTE_PROC_SECONDARY) > > > > - return 0; > > > > - > > > > - config = rte_eal_get_configuration(); > > > > - config->mem_config->nchannel = internal_config.force_nchannel; > > > > - config->mem_config->nrank = internal_config.force_nrank; > > > > - > > > > - return 0; > > > > -} > > > > - > > > > -/* init memory subsystem */ > > > > -int > > > > -rte_eal_memory_init(void) > > > > -{ > > > > - RTE_LOG(INFO, EAL, "Setting up physically contiguous > memory...\n"); > > > > - const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? > > > > - rte_eal_contigmem_init() : > > > > - rte_eal_contigmem_attach(); > > > > - if (retval < 0) > > > > - return -1; > > > > - > > > > - if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() > < 0) > > > > - return -1; > > > > - > > > > - return 0; > > > > -} > > > > diff --git a/lib/librte_eal/common/eal_common_memory.c > > > b/lib/librte_eal/common/eal_common_memory.c > > > > index 77830f8..da7aa98 100644 > > > > --- a/lib/librte_eal/common/eal_common_memory.c > > > > +++ b/lib/librte_eal/common/eal_common_memory.c > > > > @@ -46,6 +46,7 @@ > > > > #include <rte_log.h> > > > > > > > > #include "eal_private.h" > > > > +#include "eal_internal_cfg.h" > > > > > > > > /* > > > > * Return a pointer to a read-only table of struct rte_physmem_desc > > > > @@ -70,7 +71,7 @@ rte_eal_get_physmem_size(void) > > > > /* get pointer to global configuration */ > > > > mcfg = rte_eal_get_configuration()->mem_config; > > > > > > > > - for (i=0; i<RTE_MAX_MEMSEG; i++) { > > > > + for (i = 0; i < RTE_MAX_MEMSEG; i++) { > > > > if (mcfg->memseg[i].addr == NULL) > > > > break; > > > > > > > > @@ -90,7 +91,7 @@ rte_dump_physmem_layout(FILE *f) > > > > /* get pointer to global configuration */ > > > > mcfg = rte_eal_get_configuration()->mem_config; > > > > > > > > - for (i=0; i<RTE_MAX_MEMSEG; i++) { > > > > + for (i = 0; i < RTE_MAX_MEMSEG; i++) { > > > > if (mcfg->memseg[i].addr == NULL) > > > > break; > > > > > > > > @@ -119,3 +120,41 @@ unsigned rte_memory_get_nrank(void) > > > > { > > > > return rte_eal_get_configuration()->mem_config->nrank; > > > > } > > > > + > > > > +static int > > > > +rte_eal_memdevice_init(void) > > > > +{ > > > > + struct rte_config *config; > > > > + > > > > + if (rte_eal_process_type() == RTE_PROC_SECONDARY) > > > > + return 0; > > > > + > > > > + config = rte_eal_get_configuration(); > > > > + config->mem_config->nchannel = internal_config.force_nchannel; > > > > + config->mem_config->nrank = internal_config.force_nrank; > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +/* init memory subsystem */ > > > > +int > > > > +rte_eal_memory_init(void) > > > > +{ > > > > + RTE_LOG(INFO, EAL, "Setting up physically contiguous > memory...\n"); > > > > + const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? > > > > +#ifdef RTE_EXEC_ENV_BSDAPP > > > > + rte_eal_contigmem_init() : > > > > + rte_eal_contigmem_attach(); > > > > +#else /* RTE_EXEC_ENV_BSDAPP */ > > > > + rte_eal_hugepage_init() : > > > > + rte_eal_hugepage_attach(); > > > > +#endif /* RTE_EXEC_ENV_BSDAPP */ > > > Given that the functions you are calling here are only ever build for > the > > > platform they are called from, it seems to me that you can give them a > > > shared > > > name, and just build the appropriate one. I.e. you don't need to add > > > ifdeffery > > > here. > > > > > > > > <rk> Agreed and will be done. Only reason I left it as is because I > thought > > it might create confusion since in the code/document in BSD it's referred > > as contigmem rather than hugepage. > > > I agree, it might be a bit confusing, but I think documenting it would be > superior to creating separate function names with additional ifdeffery. > cscope > illustrates pretty well multiple function definitions. > > Neil > > <rk> WIll take care of using common function names.
diff --git a/lib/librte_eal/bsdapp/eal/eal_memory.c b/lib/librte_eal/bsdapp/eal/eal_memory.c index 65ee87d..b192705 100644 --- a/lib/librte_eal/bsdapp/eal/eal_memory.c +++ b/lib/librte_eal/bsdapp/eal/eal_memory.c @@ -59,7 +59,7 @@ rte_mem_virt2phy(const void *virtaddr) return RTE_BAD_PHYS_ADDR; } -static int +int rte_eal_contigmem_init(void) { struct rte_mem_config *mcfg; @@ -130,7 +130,7 @@ rte_eal_contigmem_init(void) return 0; } -static int +int rte_eal_contigmem_attach(void) { const struct hugepage_info *hpi; @@ -190,35 +190,3 @@ error: return -1; } - -static int -rte_eal_memdevice_init(void) -{ - struct rte_config *config; - - if (rte_eal_process_type() == RTE_PROC_SECONDARY) - return 0; - - config = rte_eal_get_configuration(); - config->mem_config->nchannel = internal_config.force_nchannel; - config->mem_config->nrank = internal_config.force_nrank; - - return 0; -} - -/* init memory subsystem */ -int -rte_eal_memory_init(void) -{ - RTE_LOG(INFO, EAL, "Setting up physically contiguous memory...\n"); - const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? - rte_eal_contigmem_init() : - rte_eal_contigmem_attach(); - if (retval < 0) - return -1; - - if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() < 0) - return -1; - - return 0; -} diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c index 77830f8..da7aa98 100644 --- a/lib/librte_eal/common/eal_common_memory.c +++ b/lib/librte_eal/common/eal_common_memory.c @@ -46,6 +46,7 @@ #include <rte_log.h> #include "eal_private.h" +#include "eal_internal_cfg.h" /* * Return a pointer to a read-only table of struct rte_physmem_desc @@ -70,7 +71,7 @@ rte_eal_get_physmem_size(void) /* get pointer to global configuration */ mcfg = rte_eal_get_configuration()->mem_config; - for (i=0; i<RTE_MAX_MEMSEG; i++) { + for (i = 0; i < RTE_MAX_MEMSEG; i++) { if (mcfg->memseg[i].addr == NULL) break; @@ -90,7 +91,7 @@ rte_dump_physmem_layout(FILE *f) /* get pointer to global configuration */ mcfg = rte_eal_get_configuration()->mem_config; - for (i=0; i<RTE_MAX_MEMSEG; i++) { + for (i = 0; i < RTE_MAX_MEMSEG; i++) { if (mcfg->memseg[i].addr == NULL) break; @@ -119,3 +120,41 @@ unsigned rte_memory_get_nrank(void) { return rte_eal_get_configuration()->mem_config->nrank; } + +static int +rte_eal_memdevice_init(void) +{ + struct rte_config *config; + + if (rte_eal_process_type() == RTE_PROC_SECONDARY) + return 0; + + config = rte_eal_get_configuration(); + config->mem_config->nchannel = internal_config.force_nchannel; + config->mem_config->nrank = internal_config.force_nrank; + + return 0; +} + +/* init memory subsystem */ +int +rte_eal_memory_init(void) +{ + RTE_LOG(INFO, EAL, "Setting up physically contiguous memory...\n"); + const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? +#ifdef RTE_EXEC_ENV_BSDAPP + rte_eal_contigmem_init() : + rte_eal_contigmem_attach(); +#else /* RTE_EXEC_ENV_BSDAPP */ + rte_eal_hugepage_init() : + rte_eal_hugepage_attach(); +#endif /* RTE_EXEC_ENV_BSDAPP */ + + if (retval < 0) + return -1; + + if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() < 0) + return -1; + + return 0; +} diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h index 19af23d..16338a2 100644 --- a/lib/librte_eal/common/eal_private.h +++ b/lib/librte_eal/common/eal_private.h @@ -286,6 +286,20 @@ int get_ncpus(void); */ int set_tsc_freq_from_sysctl(void); +/** + * This function prepares physical memory mapping + * + * This function is private to the EAL. + */ +int rte_eal_contigmem_init(void); + +/** + * This function creates memory mapping in secondary + * + * This function is private to the EAL. + */ +int rte_eal_contigmem_attach(void); + #else /* RTE_EXEC_ENV_BSDAPP */ /** * This function check if cpu is present @@ -301,6 +315,20 @@ int cpu_detected(unsigned lcore_id); */ int set_tsc_freq_from_clock(void); +/** + * This function prepares physical memory mapping + * + * This function is private to the EAL. + */ +int rte_eal_hugepage_init(void); + +/** + * This function creates memory mapping in secondary + * + * This function is private to the EAL. + */ +int rte_eal_hugepage_attach(void); + #endif /* RTE_EXEC_ENV_BSDAPP */ #endif /* _EAL_PRIVATE_H_ */ diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c index bae2507..f4d91df 100644 --- a/lib/librte_eal/linuxapp/eal/eal_memory.c +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c @@ -1031,7 +1031,7 @@ calc_num_pages_per_socket(uint64_t * memory, * 6. unmap the first mapping * 7. fill memsegs in configuration with contiguous zones */ -static int +int rte_eal_hugepage_init(void) { struct rte_mem_config *mcfg; @@ -1369,7 +1369,7 @@ getFileSize(int fd) * configuration and finds the hugepages which form that segment, mapping them * in order to form a contiguous block in the virtual memory space */ -static int +int rte_eal_hugepage_attach(void) { const struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config; @@ -1530,35 +1530,3 @@ error: return -1; } -static int -rte_eal_memdevice_init(void) -{ - struct rte_config *config; - - if (rte_eal_process_type() == RTE_PROC_SECONDARY) - return 0; - - config = rte_eal_get_configuration(); - config->mem_config->nchannel = internal_config.force_nchannel; - config->mem_config->nrank = internal_config.force_nrank; - - return 0; -} - - -/* init memory subsystem */ -int -rte_eal_memory_init(void) -{ - RTE_LOG(INFO, EAL, "Setting up memory...\n"); - const int retval = rte_eal_process_type() == RTE_PROC_PRIMARY ? - rte_eal_hugepage_init() : - rte_eal_hugepage_attach(); - if (retval < 0) - return -1; - - if (internal_config.no_shconf == 0 && rte_eal_memdevice_init() < 0) - return -1; - - return 0; -}
Move common functions in eal_memory.c to librte_eal/common directory. Use RTE_EXEC_ENV_BSDAPP to differentiate minor differences in common functions. Fix checkpatch warnings and errors. Signed-off-by: Ravi Kerur <rkerur@gmail.com> --- lib/librte_eal/bsdapp/eal/eal_memory.c | 36 ++------------------------ lib/librte_eal/common/eal_common_memory.c | 43 +++++++++++++++++++++++++++++-- lib/librte_eal/common/eal_private.h | 28 ++++++++++++++++++++ lib/librte_eal/linuxapp/eal/eal_memory.c | 36 ++------------------------ 4 files changed, 73 insertions(+), 70 deletions(-)