On 04-Jul-19 11:54 AM, David Marchand wrote:


On Thu, Jul 4, 2019 at 12:50 PM Burakov, Anatoly <anatoly.bura...@intel.com <mailto:anatoly.bura...@intel.com>> wrote:

    On 04-Jul-19 8:56 AM, David Marchand wrote:
     > On Thu, Jul 4, 2019 at 9:50 AM David Marchand
    <david.march...@redhat.com <mailto:david.march...@redhat.com>>
     > wrote:
     >
     >>
     >>
     >> On Thu, Jun 27, 2019 at 1:39 PM Anatoly Burakov
    <anatoly.bura...@intel.com <mailto:anatoly.bura...@intel.com>>
     >> wrote:
     >>
     >>> Currently, each EAL will update internal/shared config in their
     >>> own way at init, resulting in needless duplication of code and
     >>> OS-dependent behavior. Move the functions to a common file and
     >>> add missing FreeBSD steps.
     >>>
     >>> Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com
    <mailto:anatoly.bura...@intel.com>>
     >>> ---
     >>>   lib/librte_eal/common/eal_common_mcfg.c | 18 ++++++++++++++++++
     >>>   lib/librte_eal/common/eal_memcfg.h      |  8 ++++++++
     >>>   lib/librte_eal/freebsd/eal/eal.c        |  2 ++
     >>>   lib/librte_eal/linux/eal/eal.c          | 22
    ++--------------------
     >>>   4 files changed, 30 insertions(+), 20 deletions(-)
     >>>
     >>> diff --git a/lib/librte_eal/common/eal_common_mcfg.c
     >>> b/lib/librte_eal/common/eal_common_mcfg.c
     >>> index dc6665d6a..fe8d2b726 100644
     >>> --- a/lib/librte_eal/common/eal_common_mcfg.c
     >>> +++ b/lib/librte_eal/common/eal_common_mcfg.c
     >>> @@ -31,6 +31,24 @@ eal_mcfg_wait_complete(void)
     >>>                  rte_pause();
     >>>   }
     >>>
     >>> +void
     >>> +eal_mcfg_update_internal(void)
     >>> +{
     >>> +       struct rte_mem_config *mcfg =
     >>> rte_eal_get_configuration()->mem_config;
     >>> +
     >>> +       internal_config.legacy_mem = mcfg->legacy_mem;
     >>> +       internal_config.single_file_segments =
    mcfg->single_file_segments;
     >>> +}
     >>> +
     >>> +void
     >>> +eal_mcfg_update_from_internal(void)
     >>> +{
     >>> +       struct rte_mem_config *mcfg =
     >>> rte_eal_get_configuration()->mem_config;
     >>> +
     >>> +       mcfg->legacy_mem = internal_config.legacy_mem;
     >>> +       mcfg->single_file_segments =
    internal_config.single_file_segments;
     >>> +}
     >>> +
     >>>   void
     >>>   rte_mcfg_mem_read_lock(void)
     >>>   {
     >>> diff --git a/lib/librte_eal/common/eal_memcfg.h
     >>> b/lib/librte_eal/common/eal_memcfg.h
     >>> index a2434417e..d02ac1621 100644
     >>> --- a/lib/librte_eal/common/eal_memcfg.h
     >>> +++ b/lib/librte_eal/common/eal_memcfg.h
     >>> @@ -68,6 +68,14 @@ struct rte_mem_config {
     >>>          uint8_t dma_maskbits;
     >>>   };
     >>>
     >>> +/* update internal config from shared mem config */
     >>> +void
     >>> +eal_mcfg_update_internal(void);
     >>> +
     >>> +/* update shared mem config from internal config */
     >>> +void
     >>> +eal_mcfg_update_from_internal(void);
     >>> +
     >>>   /* wait until primary process initialization is complete */
     >>>   void
     >>>   eal_mcfg_wait_complete(void);
     >>> diff --git a/lib/librte_eal/freebsd/eal/eal.c
     >>> b/lib/librte_eal/freebsd/eal/eal.c
     >>> index 13e230fc8..6bfe203fd 100644
     >>> --- a/lib/librte_eal/freebsd/eal/eal.c
     >>> +++ b/lib/librte_eal/freebsd/eal/eal.c
     >>> @@ -379,6 +379,7 @@ rte_config_init(void)
     >>>          case RTE_PROC_PRIMARY:
     >>>                  if (rte_eal_config_create() < 0)
     >>>                          return -1;
     >>> +               eal_mcfg_update_internal();
     >>>                  break;
     >>>          case RTE_PROC_SECONDARY:
     >>>                  if (rte_eal_config_attach() < 0)
     >>> @@ -386,6 +387,7 @@ rte_config_init(void)
     >>>                  eal_mcfg_wait_complete();
     >>>                  if (rte_eal_config_reattach() < 0)
     >>>                          return -1;
     >>> +               eal_mcfg_update_from_internal();
     >>>                  break;
     >>>          case RTE_PROC_AUTO:
     >>>          case RTE_PROC_INVALID:
     >>>
     >>
     >>
     >> Hum, you swapped eal_mcfg_update_internal and
     >> eal_mcfg_update_from_internal.
     >> The names are a bit ambiguous, and I wonder if we really need those
     >> separate helpers.
     >>
     >
     > Replying to myself... at least those helpers try to do something
     > explicitly: synchronising the local copy of the configuration
    with the
     > shared mem config.
     > Keeping them separate documents this step.
     >
     > Anyway, your choice, but you must fix freebsd :-)
     >

    Sorry, i don't follow - what needs to be fixed in FreeBSD?


eal_mcfg_update_from_internal() must be called in primary process.
eal_mcfg_update_internal() must be called in secondary processes.


Oh, right, good catch, thanks!


--
David Marchand


--
Thanks,
Anatoly

Reply via email to