[v4] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts

  RTE_MAX_MEMSEG_LISTS = 128 is not enough for high-memory machines, in our
case, we need to increase it to 8192. so add an option so user can
override it. RTE_MBUF_REFCNT_ATOMIC = 0 is not necessary for applications
like Seastar, where it's safe to assume that the mbuf refcnt is only
updated by a single core only.

---

v4:

fix the coding style issue by reduce the line length to under 75.
this change should silence the warning like:

WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#81: 
RTE_MAX_MEMSEG_LISTS = 128 is not enough for high-memory machines, in our case,

total: 0 errors, 1 warnings, 35 lines checked

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
---
 config/meson.build  | 5 ++++-
 config/rte_config.h | 2 --
 meson_options.txt   | 4 ++++
 3 files changed, 8 insertions(+), 3 deletions(-)
  

hello Bruce,

do you have any further concerns? is there anything i can do to move
this forward?

cheers,

On Thu, Sep 9, 2021 at 12:51 AM Kefu Chai <tchaikov@gmail.com> wrote:
>
> RTE_MAX_MEMSEG_LISTS = 128 is not enough for high-memory machines, in our
> case, we need to increase it to 8192. so add an option so user can
> override it. RTE_MBUF_REFCNT_ATOMIC = 0 is not necessary for applications
> like Seastar, where it's safe to assume that the mbuf refcnt is only
> updated by a single core only.
>
> ---
>
> v4:
>
> fix the coding style issue by reduce the line length to under 75.
> this change should silence the warning like:
>
> WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> #81:
> RTE_MAX_MEMSEG_LISTS = 128 is not enough for high-memory machines, in our case,
>
> total: 0 errors, 1 warnings, 35 lines checked
>
> Signed-off-by: Kefu Chai <tchaikov@gmail.com>
> ---
>  config/meson.build  | 5 ++++-
>  config/rte_config.h | 2 --
>  meson_options.txt   | 4 ++++
>  3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/config/meson.build b/config/meson.build
> index 3b5966ec2f..d95dccdbcc 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -301,7 +301,10 @@ if dpdk_conf.get('RTE_ARCH_64')
>  else # for 32-bit we need smaller reserved memory areas
>      dpdk_conf.set('RTE_MAX_MEM_MB', 2048)
>  endif
> -
> +dpdk_conf.set('RTE_MAX_MEMSEG_LISTS', get_option('max_memseg_lists'))
> +if get_option('atomic_mbuf_ref_counts')
> +  dpdk_conf.set('RTE_MBUF_REFCNT_ATOMIC', true)
> +endif
>
>  compile_time_cpuflags = []
>  subdir(arch_subdir)
> diff --git a/config/rte_config.h b/config/rte_config.h
> index 590903c07d..0a659f5e1a 100644
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> @@ -29,7 +29,6 @@
>
>  /* EAL defines */
>  #define RTE_MAX_HEAPS 32
> -#define RTE_MAX_MEMSEG_LISTS 128
>  #define RTE_MAX_MEMSEG_PER_LIST 8192
>  #define RTE_MAX_MEM_MB_PER_LIST 32768
>  #define RTE_MAX_MEMSEG_PER_TYPE 32768
> @@ -50,7 +49,6 @@
>
>  /* mbuf defines */
>  #define RTE_MBUF_DEFAULT_MEMPOOL_OPS "ring_mp_mc"
> -#define RTE_MBUF_REFCNT_ATOMIC 1
>  #define RTE_PKTMBUF_HEADROOM 128
>
>  /* ether defines */
> diff --git a/meson_options.txt b/meson_options.txt
> index 0e92734c49..6aeae211cd 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -38,6 +38,10 @@ option('max_lcores', type: 'integer', value: 128, description:
>         'maximum number of cores/threads supported by EAL')
>  option('max_numa_nodes', type: 'integer', value: 32, description:
>         'maximum number of NUMA nodes supported by EAL')
> +option('max_memseg_lists', type: 'integer', value: 128, description:
> +       'maximum number of dynamic arrays holding memsegs')
> +option('atomic_mbuf_ref_counts', type: 'boolean', value: true, description:
> +       'atomically access the mbuf refcnt')
>  option('platform', type: 'string', value: 'native', description:
>         'Platform to build, either "native", "generic" or a SoC. Please refer to the Linux build guide for more information.')
>  option('enable_trace_fp', type: 'boolean', value: false, description:
> --
> 2.33.0
>
  

On Mon, Sep 20, 2021 at 03:51:06PM +0800, kefu chai wrote:
> hello Bruce,
> 
> do you have any further concerns? is there anything i can do to move
> this forward?
> 
> cheers,
>

+Anatoly, for his input for the memory segments change.

I still would prefer not to have these as config options, but perhaps one
or both needs to be. The atomic refcount seems more reasonable to add of
the two. For the max memseg lists, what is the impact if we were to
increase this value globally?

/Bruce
 
> On Thu, Sep 9, 2021 at 12:51 AM Kefu Chai <tchaikov@gmail.com> wrote:
> >
> > RTE_MAX_MEMSEG_LISTS = 128 is not enough for high-memory machines, in our
> > case, we need to increase it to 8192. so add an option so user can
> > override it. RTE_MBUF_REFCNT_ATOMIC = 0 is not necessary for applications
> > like Seastar, where it's safe to assume that the mbuf refcnt is only
> > updated by a single core only.
> >
> > ---
> >
> > v4:
> >
> > fix the coding style issue by reduce the line length to under 75.
> > this change should silence the warning like:
> >
> > WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> > #81:
> > RTE_MAX_MEMSEG_LISTS = 128 is not enough for high-memory machines, in our case,
> >
> > total: 0 errors, 1 warnings, 35 lines checked
> >
> > Signed-off-by: Kefu Chai <tchaikov@gmail.com>
> > ---
> >  config/meson.build  | 5 ++++-
> >  config/rte_config.h | 2 --
> >  meson_options.txt   | 4 ++++
> >  3 files changed, 8 insertions(+), 3 deletions(-)
> >

<snip>
  

On Mon, Sep 20, 2021 at 4:08 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Mon, Sep 20, 2021 at 03:51:06PM +0800, kefu chai wrote:
> > hello Bruce,
> >
> > do you have any further concerns? is there anything i can do to move
> > this forward?
> >
> > cheers,
> >
>
> +Anatoly, for his input for the memory segments change.
>
> I still would prefer not to have these as config options, but perhaps one
> or both needs to be. The atomic refcount seems more reasonable to add of
> the two. For the max memseg lists, what is the impact if we were to
> increase this value globally?

hi Bruce, thank you for your insights.

yeah, as i explained in the previous email.the atomic refcount is more
critical for my work on integration of DPDK+SPDK+Seastar. since
Seastar enforces share-nothing in its design, there is no need to use
atomic refcount under almost all circumstances. regarding to the max
memseg list, what i am trying is to port the change of
https://github.com/scylladb/seastar/commit/716c7c04db693c266f52de6b0cced0252d70b3bf
to the DPDK used by the latest release of SPDK.

i am copying Avi also for his input. he is the author of the change above.

cheers,

>
> /Bruce
>
> > On Thu, Sep 9, 2021 at 12:51 AM Kefu Chai <tchaikov@gmail.com> wrote:
> > >
> > > RTE_MAX_MEMSEG_LISTS = 128 is not enough for high-memory machines, in our
> > > case, we need to increase it to 8192. so add an option so user can
> > > override it. RTE_MBUF_REFCNT_ATOMIC = 0 is not necessary for applications
> > > like Seastar, where it's safe to assume that the mbuf refcnt is only
> > > updated by a single core only.
> > >
> > > ---
> > >
> > > v4:
> > >
> > > fix the coding style issue by reduce the line length to under 75.
> > > this change should silence the warning like:
> > >
> > > WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> > > #81:
> > > RTE_MAX_MEMSEG_LISTS = 128 is not enough for high-memory machines, in our case,
> > >
> > > total: 0 errors, 1 warnings, 35 lines checked
> > >
> > > Signed-off-by: Kefu Chai <tchaikov@gmail.com>
> > > ---
> > >  config/meson.build  | 5 ++++-
> > >  config/rte_config.h | 2 --
> > >  meson_options.txt   | 4 ++++
> > >  3 files changed, 8 insertions(+), 3 deletions(-)
> > >
>
> <snip>
  

27/09/2021 17:03, kefu chai:
> On Mon, Sep 20, 2021 at 4:08 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Mon, Sep 20, 2021 at 03:51:06PM +0800, kefu chai wrote:
> > > hello Bruce,
> > >
> > > do you have any further concerns? is there anything i can do to move
> > > this forward?
> > >
> > > cheers,
> > >
> >
> > +Anatoly, for his input for the memory segments change.
> >
> > I still would prefer not to have these as config options, but perhaps one
> > or both needs to be. The atomic refcount seems more reasonable to add of
> > the two. For the max memseg lists, what is the impact if we were to
> > increase this value globally?
> 
> hi Bruce, thank you for your insights.
> 
> yeah, as i explained in the previous email.the atomic refcount is more
> critical for my work on integration of DPDK+SPDK+Seastar. since
> Seastar enforces share-nothing in its design, there is no need to use
> atomic refcount under almost all circumstances. regarding to the max
> memseg list, what i am trying is to port the change of
> https://github.com/scylladb/seastar/commit/716c7c04db693c266f52de6b0cced0252d70b3bf
> to the DPDK used by the latest release of SPDK.

I think it would help acceptance to send these changes as 2 separate patches.