diff mbox series

[v3,13/14] eal: unify internal config initialization

Message ID b49510c5323cceaea7405292637f8bd3d24e8ad7.1561635481.git.anatoly.burakov@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Make shared memory config non-public |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply issues

Commit Message

Anatoly Burakov June 27, 2019, 11:39 a.m. UTC 
  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(-)

Comments

David Marchand July 4, 2019, 7:50 a.m. UTC | #1 
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.

rte_eal_config_create and rte_eal_config_attach are already awfully close
in linux and freebsd implementation.
Can't we have them as common code and put those helpers bits direct in them
?




> diff --git a/lib/librte_eal/linux/eal/eal.c
> b/lib/librte_eal/linux/eal/eal.c
> index 4fd18b15f..fa205fd29 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -472,24 +472,6 @@ eal_proc_type_detect(void)
>         return ptype;
>  }
>
> -/* copies data from internal config to shared config */
> -static void
> -eal_update_mem_config(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;
> -}
> -
> -/* copies data from shared config to internal config */
> -static void
> -eal_update_internal_config(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;
> -}
> -
>  /* Sets up rte_config structure with the pointer to shared memory
> config.*/
>  static int
>  rte_config_init(void)
> @@ -500,7 +482,7 @@ rte_config_init(void)
>         case RTE_PROC_PRIMARY:
>                 if (rte_eal_config_create() < 0)
>                         return -1;
> -               eal_update_mem_config();
> +               eal_mcfg_update_from_internal();
>                 break;
>         case RTE_PROC_SECONDARY:
>                 if (rte_eal_config_attach() < 0)
> @@ -508,7 +490,7 @@ rte_config_init(void)
>                 eal_mcfg_wait_complete();
>                 if (rte_eal_config_reattach() < 0)
>                         return -1;
> -               eal_update_internal_config();
> +               eal_mcfg_update_internal();
>                 break;
>         case RTE_PROC_AUTO:
>         case RTE_PROC_INVALID:
> --
> 2.17.1
>
David Marchand July 4, 2019, 7:56 a.m. UTC | #2 
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 :-)
Anatoly Burakov July 4, 2019, 10:50 a.m. UTC | #3 
On 04-Jul-19 8:56 AM, David Marchand wrote:
> 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 :-)
> 

Sorry, i don't follow - what needs to be fixed in FreeBSD?
David Marchand July 4, 2019, 10:54 a.m. UTC | #4 
On Thu, Jul 4, 2019 at 12:50 PM Burakov, Anatoly <anatoly.burakov@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.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 :-)
> >
>
> 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.
Anatoly Burakov July 4, 2019, 11:26 a.m. UTC | #5 
On 04-Jul-19 11:54 AM, David Marchand wrote:
> 
> 
> On Thu, Jul 4, 2019 at 12:50 PM Burakov, Anatoly 
> <anatoly.burakov@intel.com <mailto:anatoly.burakov@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.marchand@redhat.com <mailto:david.marchand@redhat.com>>
>      > wrote:
>      >
>      >>
>      >>
>      >> On Thu, Jun 27, 2019 at 1:39 PM Anatoly Burakov
>     <anatoly.burakov@intel.com <mailto: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
>     <mailto: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 :-)
>      >
> 
>     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
diff mbox series

Patch

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:
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c
index 4fd18b15f..fa205fd29 100644
--- a/lib/librte_eal/linux/eal/eal.c
+++ b/lib/librte_eal/linux/eal/eal.c
@@ -472,24 +472,6 @@  eal_proc_type_detect(void)
 	return ptype;
 }
 
-/* copies data from internal config to shared config */
-static void
-eal_update_mem_config(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;
-}
-
-/* copies data from shared config to internal config */
-static void
-eal_update_internal_config(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;
-}
-
 /* Sets up rte_config structure with the pointer to shared memory config.*/
 static int
 rte_config_init(void)
@@ -500,7 +482,7 @@  rte_config_init(void)
 	case RTE_PROC_PRIMARY:
 		if (rte_eal_config_create() < 0)
 			return -1;
-		eal_update_mem_config();
+		eal_mcfg_update_from_internal();
 		break;
 	case RTE_PROC_SECONDARY:
 		if (rte_eal_config_attach() < 0)
@@ -508,7 +490,7 @@  rte_config_init(void)
 		eal_mcfg_wait_complete();
 		if (rte_eal_config_reattach() < 0)
 			return -1;
-		eal_update_internal_config();
+		eal_mcfg_update_internal();
 		break;
 	case RTE_PROC_AUTO:
 	case RTE_PROC_INVALID: