[dpdk-dev] [PATCH] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic"

[dpdk-dev] [PATCH] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic"

[Kefu Chai]

RTE_MAX_MEMSEG_LISTS = 128 is not enough for some use cases, so add an
option so user can override it.
RTE_MBUF_REFCNT_ATOMIC = 0 is enough for applications like Seastar,
where it's safe to assume that the mbuf refcnt is only updated by a single
core only.

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

diff --git a/config/meson.build b/config/meson.build
index 3b5966ec2f..72dd461198 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('mbuf_refcnt_atomic')
+  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..c0e8958797 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -38,6 +38,11 @@ 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('mbuf_refcnt_atomic', 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

Re: [dpdk-dev] [PATCH] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic"

[Bruce Richardson]

On Thu, Sep 02, 2021 at 10:48:05PM +0800, Kefu Chai wrote:
> RTE_MAX_MEMSEG_LISTS = 128 is not enough for some use cases, so add an
> option so user can override it.

How big a value do you need? Can you share anything about the use-case
where you are hitting this limit?

> RTE_MBUF_REFCNT_ATOMIC = 0 is enough for applications like Seastar,
> where it's safe to assume that the mbuf refcnt is only updated by a single
> core only.
> 
> Signed-off-by: Kefu Chai <tchaikov@gmail.com>
> ---
>  config/meson.build  | 5 ++++-
>  config/rte_config.h | 2 --
>  meson_options.txt   | 5 +++++
>  3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/config/meson.build b/config/meson.build
> index 3b5966ec2f..72dd461198 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('mbuf_refcnt_atomic')
> +  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..c0e8958797 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -38,6 +38,11 @@ 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('mbuf_refcnt_atomic', type: 'boolean', value: true, description:
> +       'atomically access the mbuf refcnt')
> +

No blank line after the option. I'd suggest renaming it to
"atomic_mbuf_ref_counts" to make the entry name more readable - at the cost
of it being longer, unfortunately.

/Bruce

[dpdk-dev] [PATCH v2] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic"

[Kefu Chai]

RTE_MAX_MEMSEG_LISTS = 128 is not enough for high-memory machines, 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.

---
v2:
   revise the commit message.

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

diff --git a/config/meson.build b/config/meson.build
index 3b5966ec2f..72dd461198 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('mbuf_refcnt_atomic')
+  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..c0e8958797 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -38,6 +38,11 @@ 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('mbuf_refcnt_atomic', 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

[dpdk-dev] [PATCH v3] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts

[Kefu Chai]

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.

---
v3:

* s/mbuf_refcnt_atomic/atomic_mbuf_ref_counts/, and update the 
  title of the commit message accordingly
* explain the expected number of MEMSEG_LISTS in commit message.
* remove the empty line after the newly added option

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

[dpdk-dev] [PATCH v4] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts

[Kefu Chai]

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

Re: [dpdk-dev] [PATCH v4] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts

[kefu chai]

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
>


-- 
Regards
Kefu Chai

Re: [dpdk-dev] [PATCH v4] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts

[Bruce Richardson]

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>

Re: [dpdk-dev] [PATCH v4] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts

[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 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>



-- 
Regards
Kefu Chai

Re: [dpdk-dev] [PATCH v4] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts

[Thomas Monjalon]

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.

[dpdk-dev] [PATCH v5 0/2] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic"

[Kefu Chai]

---
v5:
* split the changes of two options into two separated commits
  one for atomic_mbuf_ref_counts, the other for
  max_memseg_lists

v4:

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

v3:

* s/mbuf_refcnt_atomic/atomic_mbuf_ref_counts/, and update the 
  title of the commit message accordingly
* explain the expected number of MEMSEG_LISTS in commit message.
* remove the empty line after the newly added option

v2:
* revise the commit message.

Kefu Chai (2):
  build: add meson options of atomic_mbuf_ref_counts
  build: add meson options of max_memseg_lists

 config/meson.build  | 5 ++++-
 config/rte_config.h | 2 --
 meson_options.txt   | 4 ++++
 3 files changed, 8 insertions(+), 3 deletions(-)

-- 
2.33.0

[dpdk-dev] [PATCH v5 1/2] build: add meson options of atomic_mbuf_ref_counts

[Kefu Chai]

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.

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

diff --git a/config/meson.build b/config/meson.build
index 4cdf589e20..c90c7a0bfe 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -299,7 +299,9 @@ 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
-
+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..208d916a1f 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -50,7 +50,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 9beedabe4c..222ad6d9d9 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -38,6 +38,8 @@ option('max_lcores', type: 'string', value: 'default', description:
        'Set maximum number of cores/threads supported by EAL; "default" is different per-arch, "detect" detects the number of cores on the build machine.')
 option('max_numa_nodes', type: 'string', value: 'default', description:
        'Set the highest NUMA node supported by EAL; "default" is different per-arch, "detect" detects the highest NUMA node on the build machine.')
+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

[dpdk-dev] [PATCH v5 2/2] build: add meson options of max_memseg_lists

[Kefu Chai]

RTE_MAX_MEMSEG_LISTS = 128 is not enough for many-core machines, in our
case, we need to increase it to 8192. so add an option so user can
override it.

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

diff --git a/config/meson.build b/config/meson.build
index c90c7a0bfe..8d03dc6471 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -299,6 +299,7 @@ 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
diff --git a/config/rte_config.h b/config/rte_config.h
index 208d916a1f..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
diff --git a/meson_options.txt b/meson_options.txt
index 222ad6d9d9..9127f8556f 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -38,6 +38,8 @@ option('max_lcores', type: 'string', value: 'default', description:
        'Set maximum number of cores/threads supported by EAL; "default" is different per-arch, "detect" detects the number of cores on the build machine.')
 option('max_numa_nodes', type: 'string', value: 'default', description:
        'Set the highest NUMA node supported by EAL; "default" is different per-arch, "detect" detects the highest NUMA node on the build machine.')
+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:
-- 
2.33.0

Re: [dpdk-dev] [PATCH v5 1/2] build: add meson options of atomic_mbuf_ref_counts

[Bruce Richardson]

On Thu, Oct 14, 2021 at 04:54:18AM +0800, Kefu Chai wrote:
> 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.
> 
> Signed-off-by: Kefu Chai <tchaikov@gmail.com>
> ---

For this, I think it's a setting that needs to be a global one for DPDK, so
I'm ok with adding it as a meson option.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH v5 2/2] build: add meson options of max_memseg_lists

[Bruce Richardson]

On Thu, Oct 14, 2021 at 04:54:19AM +0800, Kefu Chai wrote:
> RTE_MAX_MEMSEG_LISTS = 128 is not enough for many-core machines, in our
> case, we need to increase it to 8192. so add an option so user can
> override it.
> 
> Signed-off-by: Kefu Chai <tchaikov@gmail.com>

This seems a very low-level option to be exposing to the user. Some
thoughts/questions:

- can you give some more detail on why you need such a massive number, 64
  times the default?
- what would be the impact of increasing the default to 8192? I assume this
  is only used in a few places in EAL, so would the memory footprint
  increase be large?
- rather than a single specified value, would an alternative be to make
  this be a computed value at config time, scaled by number of lcores (or
  number of numa nodes)?

Regards,
/Bruce

Re: [dpdk-dev] [PATCH v5 2/2] build: add meson options of max_memseg_lists

[Thomas Monjalon]

14/10/2021 10:25, Bruce Richardson:
> On Thu, Oct 14, 2021 at 04:54:19AM +0800, Kefu Chai wrote:
> > RTE_MAX_MEMSEG_LISTS = 128 is not enough for many-core machines, in our
> > case, we need to increase it to 8192. so add an option so user can
> > override it.
> > 
> > Signed-off-by: Kefu Chai <tchaikov@gmail.com>
> 
> This seems a very low-level option to be exposing to the user. Some
> thoughts/questions:
> 
> - can you give some more detail on why you need such a massive number, 64
>   times the default?
> - what would be the impact of increasing the default to 8192? I assume this
>   is only used in a few places in EAL, so would the memory footprint
>   increase be large?
> - rather than a single specified value, would an alternative be to make
>   this be a computed value at config time, scaled by number of lcores (or
>   number of numa nodes)?

+1 for these suggestions

Re: [dpdk-dev] [PATCH v5 1/2] build: add meson options of atomic_mbuf_ref_counts

[Thomas Monjalon]

14/10/2021 10:20, Bruce Richardson:
> On Thu, Oct 14, 2021 at 04:54:18AM +0800, Kefu Chai wrote:
> > 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.
> > 
> > Signed-off-by: Kefu Chai <tchaikov@gmail.com>
> > ---
> 
> For this, I think it's a setting that needs to be a global one for DPDK, so
> I'm ok with adding it as a meson option.
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Changed the option name to "mbuf_refcnt_atomic" to match the flag.
Applied, thanks.