* Re: [dpdk-dev] [PATCH] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic"
2021-09-02 14:48 [dpdk-dev] [PATCH] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic" Kefu Chai
@ 2021-09-02 15:02 ` Bruce Richardson
2021-09-02 15:05 ` [dpdk-dev] [PATCH v2] " Kefu Chai
` (4 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Bruce Richardson @ 2021-09-02 15:02 UTC (permalink / raw)
To: Kefu Chai; +Cc: dev
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
^ permalink raw reply [flat|nested] 16+ messages in thread* [dpdk-dev] [PATCH v2] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic"
2021-09-02 14:48 [dpdk-dev] [PATCH] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic" Kefu Chai
2021-09-02 15:02 ` Bruce Richardson
@ 2021-09-02 15:05 ` Kefu Chai
2021-09-02 15:11 ` [dpdk-dev] [PATCH v3] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts Kefu Chai
` (3 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Kefu Chai @ 2021-09-02 15:05 UTC (permalink / raw)
To: dev; +Cc: Kefu Chai, Bruce Richardson
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
^ permalink raw reply [flat|nested] 16+ messages in thread* [dpdk-dev] [PATCH v3] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts
2021-09-02 14:48 [dpdk-dev] [PATCH] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic" Kefu Chai
2021-09-02 15:02 ` Bruce Richardson
2021-09-02 15:05 ` [dpdk-dev] [PATCH v2] " Kefu Chai
@ 2021-09-02 15:11 ` Kefu Chai
2021-09-08 16:51 ` [dpdk-dev] [PATCH v4] " Kefu Chai
2021-10-13 20:54 ` [dpdk-dev] [PATCH v5 0/2] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic" Kefu Chai
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Kefu Chai @ 2021-09-02 15:11 UTC (permalink / raw)
To: dev; +Cc: Kefu Chai, Bruce Richardson
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
^ permalink raw reply [flat|nested] 16+ messages in thread* [dpdk-dev] [PATCH v4] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts
2021-09-02 15:11 ` [dpdk-dev] [PATCH v3] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts Kefu Chai
@ 2021-09-08 16:51 ` Kefu Chai
2021-09-20 7:51 ` kefu chai
0 siblings, 1 reply; 16+ messages in thread
From: Kefu Chai @ 2021-09-08 16:51 UTC (permalink / raw)
To: dev; +Cc: Kefu Chai, Bruce Richardson
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH v4] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts
2021-09-08 16:51 ` [dpdk-dev] [PATCH v4] " Kefu Chai
@ 2021-09-20 7:51 ` kefu chai
2021-09-20 8:08 ` Bruce Richardson
0 siblings, 1 reply; 16+ messages in thread
From: kefu chai @ 2021-09-20 7:51 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH v4] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts
2021-09-20 7:51 ` kefu chai
@ 2021-09-20 8:08 ` Bruce Richardson
2021-09-27 15:03 ` kefu chai
0 siblings, 1 reply; 16+ messages in thread
From: Bruce Richardson @ 2021-09-20 8:08 UTC (permalink / raw)
To: kefu chai; +Cc: dev, anatoly.burakov
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v4] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts
2021-09-20 8:08 ` Bruce Richardson
@ 2021-09-27 15:03 ` kefu chai
2021-10-13 15:38 ` Thomas Monjalon
0 siblings, 1 reply; 16+ messages in thread
From: kefu chai @ 2021-09-27 15:03 UTC (permalink / raw)
To: Bruce Richardson, Avi Kivity; +Cc: dev, anatoly.burakov
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v4] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts
2021-09-27 15:03 ` kefu chai
@ 2021-10-13 15:38 ` Thomas Monjalon
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2021-10-13 15:38 UTC (permalink / raw)
To: kefu chai; +Cc: Bruce Richardson, Avi Kivity, dev, anatoly.burakov
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH v5 0/2] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic"
2021-09-02 14:48 [dpdk-dev] [PATCH] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic" Kefu Chai
` (2 preceding siblings ...)
2021-09-02 15:11 ` [dpdk-dev] [PATCH v3] build: add meson options of max_memseg_lists and atomic_mbuf_ref_counts Kefu Chai
@ 2021-10-13 20:54 ` Kefu Chai
2021-10-13 20:54 ` [dpdk-dev] [PATCH v5 1/2] build: add meson options of atomic_mbuf_ref_counts Kefu Chai
2021-10-13 20:54 ` [dpdk-dev] [PATCH v5 2/2] build: add meson options of max_memseg_lists Kefu Chai
5 siblings, 0 replies; 16+ messages in thread
From: Kefu Chai @ 2021-10-13 20:54 UTC (permalink / raw)
To: dev
Cc: Kefu Chai, Bruce Richardson, Thomas Monjalon, Avi Kivity,
anatoly.burakov
---
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
^ permalink raw reply [flat|nested] 16+ messages in thread* [dpdk-dev] [PATCH v5 1/2] build: add meson options of atomic_mbuf_ref_counts
2021-09-02 14:48 [dpdk-dev] [PATCH] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic" Kefu Chai
` (3 preceding siblings ...)
2021-10-13 20:54 ` [dpdk-dev] [PATCH v5 0/2] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic" Kefu Chai
@ 2021-10-13 20:54 ` Kefu Chai
2021-10-14 8:20 ` Bruce Richardson
2021-10-13 20:54 ` [dpdk-dev] [PATCH v5 2/2] build: add meson options of max_memseg_lists Kefu Chai
5 siblings, 1 reply; 16+ messages in thread
From: Kefu Chai @ 2021-10-13 20:54 UTC (permalink / raw)
To: dev
Cc: Kefu Chai, Bruce Richardson, Thomas Monjalon, Avi Kivity,
anatoly.burakov
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH v5 1/2] build: add meson options of atomic_mbuf_ref_counts
2021-10-13 20:54 ` [dpdk-dev] [PATCH v5 1/2] build: add meson options of atomic_mbuf_ref_counts Kefu Chai
@ 2021-10-14 8:20 ` Bruce Richardson
2021-10-25 15:55 ` Thomas Monjalon
0 siblings, 1 reply; 16+ messages in thread
From: Bruce Richardson @ 2021-10-14 8:20 UTC (permalink / raw)
To: Kefu Chai; +Cc: dev, Thomas Monjalon, Avi Kivity, anatoly.burakov
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v5 1/2] build: add meson options of atomic_mbuf_ref_counts
2021-10-14 8:20 ` Bruce Richardson
@ 2021-10-25 15:55 ` Thomas Monjalon
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2021-10-25 15:55 UTC (permalink / raw)
To: Kefu Chai; +Cc: dev, Avi Kivity, anatoly.burakov, Bruce Richardson
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH v5 2/2] build: add meson options of max_memseg_lists
2021-09-02 14:48 [dpdk-dev] [PATCH] build: add meson option of "max_memseg_lists" and "mbuf_refcnt_atomic" Kefu Chai
` (4 preceding siblings ...)
2021-10-13 20:54 ` [dpdk-dev] [PATCH v5 1/2] build: add meson options of atomic_mbuf_ref_counts Kefu Chai
@ 2021-10-13 20:54 ` Kefu Chai
2021-10-14 8:25 ` Bruce Richardson
5 siblings, 1 reply; 16+ messages in thread
From: Kefu Chai @ 2021-10-13 20:54 UTC (permalink / raw)
To: dev
Cc: Kefu Chai, Bruce Richardson, Thomas Monjalon, Avi Kivity,
anatoly.burakov
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH v5 2/2] build: add meson options of max_memseg_lists
2021-10-13 20:54 ` [dpdk-dev] [PATCH v5 2/2] build: add meson options of max_memseg_lists Kefu Chai
@ 2021-10-14 8:25 ` Bruce Richardson
2021-10-14 8:29 ` Thomas Monjalon
0 siblings, 1 reply; 16+ messages in thread
From: Bruce Richardson @ 2021-10-14 8:25 UTC (permalink / raw)
To: Kefu Chai; +Cc: dev, Thomas Monjalon, Avi Kivity, anatoly.burakov
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v5 2/2] build: add meson options of max_memseg_lists
2021-10-14 8:25 ` Bruce Richardson
@ 2021-10-14 8:29 ` Thomas Monjalon
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2021-10-14 8:29 UTC (permalink / raw)
To: Kefu Chai, Bruce Richardson; +Cc: dev, Avi Kivity, anatoly.burakov
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
^ permalink raw reply [flat|nested] 16+ messages in thread