Re: [dpdk-dev] [PATCH v3 13/14] eal: unify internal config initialization

[David Marchand <david.marchand@redhat.com>]

On Thu, Jul 4, 2019 at 9:50 AM David Marchand <david.marchand@redhat.com>
wrote:

>
>
> On Thu, Jun 27, 2019 at 1:39 PM Anatoly Burakov <anatoly.burakov@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.burakov@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 :-)


-- 
David Marchand