* [dpdk-dev] [PATCH 1/4] vhost: remove unused macro
2018-02-14 4:01 [dpdk-dev] [PATCH 0/4] some fixes target for 18.05 Jianfeng Tan
@ 2018-02-14 4:01 ` Jianfeng Tan
2018-02-19 20:49 ` Maxime Coquelin
2018-02-14 4:01 ` [dpdk-dev] [PATCH 2/4] vhost: avoid function call in data path Jianfeng Tan
` (3 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Jianfeng Tan @ 2018-02-14 4:01 UTC (permalink / raw)
To: dev; +Cc: Jianfeng Tan, tomaszx.kulasek, maxime.coquelin, yliu
Cc: tomaszx.kulasek@intel.com
Cc: maxime.coquelin@redhat.com
Cc: yliu@fridaylinux.org
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
lib/librte_vhost/vhost.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index d947bc9..ecd5b7b 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -296,7 +296,6 @@ vhost_log_used_vring(struct virtio_net *dev, struct vhost_virtqueue *vq,
#ifdef RTE_LIBRTE_VHOST_DEBUG
#define VHOST_MAX_PRINT_BUFF 6072
-#define LOG_LEVEL RTE_LOG_DEBUG
#define LOG_DEBUG(log_type, fmt, args...) RTE_LOG(DEBUG, log_type, fmt, ##args)
#define PRINT_PACKET(device, addr, size, header) do { \
char *pkt_addr = (char *)(addr); \
@@ -316,7 +315,6 @@ vhost_log_used_vring(struct virtio_net *dev, struct vhost_virtqueue *vq,
LOG_DEBUG(VHOST_DATA, "%s", packet); \
} while (0)
#else
-#define LOG_LEVEL RTE_LOG_INFO
#define LOG_DEBUG(log_type, fmt, args...) do {} while (0)
#define PRINT_PACKET(device, addr, size, header) do {} while (0)
#endif
--
2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH 1/4] vhost: remove unused macro
2018-02-14 4:01 ` [dpdk-dev] [PATCH 1/4] vhost: remove unused macro Jianfeng Tan
@ 2018-02-19 20:49 ` Maxime Coquelin
2018-02-21 7:32 ` Maxime Coquelin
0 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2018-02-19 20:49 UTC (permalink / raw)
To: Jianfeng Tan, dev; +Cc: tomaszx.kulasek, yliu
On 02/14/2018 05:01 AM, Jianfeng Tan wrote:
> Cc: tomaszx.kulasek@intel.com
> Cc: maxime.coquelin@redhat.com
> Cc: yliu@fridaylinux.org
>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
> lib/librte_vhost/vhost.h | 2 --
> 1 file changed, 2 deletions(-)
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH 1/4] vhost: remove unused macro
2018-02-19 20:49 ` Maxime Coquelin
@ 2018-02-21 7:32 ` Maxime Coquelin
0 siblings, 0 replies; 23+ messages in thread
From: Maxime Coquelin @ 2018-02-21 7:32 UTC (permalink / raw)
To: Jianfeng Tan, dev; +Cc: tomaszx.kulasek, yliu
On 02/19/2018 09:49 PM, Maxime Coquelin wrote:
>
>
> On 02/14/2018 05:01 AM, Jianfeng Tan wrote:
>> Cc: tomaszx.kulasek@intel.com
>> Cc: maxime.coquelin@redhat.com
>> Cc: yliu@fridaylinux.org
>>
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>> ---
>> lib/librte_vhost/vhost.h | 2 --
>> 1 file changed, 2 deletions(-)
>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>Applied to
Applied to dpdk-next-virtio/master.
Thanks,
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread
* [dpdk-dev] [PATCH 2/4] vhost: avoid function call in data path
2018-02-14 4:01 [dpdk-dev] [PATCH 0/4] some fixes target for 18.05 Jianfeng Tan
2018-02-14 4:01 ` [dpdk-dev] [PATCH 1/4] vhost: remove unused macro Jianfeng Tan
@ 2018-02-14 4:01 ` Jianfeng Tan
2018-02-19 20:48 ` Maxime Coquelin
2018-02-14 4:01 ` [dpdk-dev] [PATCH 3/4] app/testpmd: add option to avoid lock all memory Jianfeng Tan
` (2 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Jianfeng Tan @ 2018-02-14 4:01 UTC (permalink / raw)
To: dev; +Cc: Jianfeng Tan, maxime.coquelin, yliu
Previously, get_device() is a function call. It's OK for slow path
configuration, but takes some cycles for data path.
To avoid that, we turn this function to inline type.
Cc: maxime.coquelin@redhat.com
Cc: yliu@fridaylinux.org
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
lib/librte_vhost/vhost.c | 13 -------------
lib/librte_vhost/vhost.h | 13 ++++++++++++-
2 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index a407067..f6f12a0 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -68,19 +68,6 @@ __vhost_iova_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq,
return 0;
}
-struct virtio_net *
-get_device(int vid)
-{
- struct virtio_net *dev = vhost_devices[vid];
-
- if (unlikely(!dev)) {
- RTE_LOG(ERR, VHOST_CONFIG,
- "(%d) device not found.\n", vid);
- }
-
- return dev;
-}
-
void
cleanup_vq(struct vhost_virtqueue *vq, int destroy)
{
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index ecd5b7b..3fce13b 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -343,7 +343,18 @@ gpa_to_hpa(struct virtio_net *dev, uint64_t gpa, uint64_t size)
return 0;
}
-struct virtio_net *get_device(int vid);
+static __rte_always_inline struct virtio_net *
+get_device(int vid)
+{
+ struct virtio_net *dev = vhost_devices[vid];
+
+ if (unlikely(!dev)) {
+ RTE_LOG(ERR, VHOST_CONFIG,
+ "(%d) device not found.\n", vid);
+ }
+
+ return dev;
+}
int vhost_new_device(void);
void cleanup_device(struct virtio_net *dev, int destroy);
--
2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH 2/4] vhost: avoid function call in data path
2018-02-14 4:01 ` [dpdk-dev] [PATCH 2/4] vhost: avoid function call in data path Jianfeng Tan
@ 2018-02-19 20:48 ` Maxime Coquelin
2018-02-21 7:32 ` Maxime Coquelin
0 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2018-02-19 20:48 UTC (permalink / raw)
To: Jianfeng Tan, dev; +Cc: yliu
On 02/14/2018 05:01 AM, Jianfeng Tan wrote:
> Previously, get_device() is a function call. It's OK for slow path
> configuration, but takes some cycles for data path.
>
> To avoid that, we turn this function to inline type.
>
> Cc: maxime.coquelin@redhat.com
> Cc: yliu@fridaylinux.org
>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
> lib/librte_vhost/vhost.c | 13 -------------
> lib/librte_vhost/vhost.h | 13 ++++++++++++-
> 2 files changed, 12 insertions(+), 14 deletions(-)
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH 2/4] vhost: avoid function call in data path
2018-02-19 20:48 ` Maxime Coquelin
@ 2018-02-21 7:32 ` Maxime Coquelin
0 siblings, 0 replies; 23+ messages in thread
From: Maxime Coquelin @ 2018-02-21 7:32 UTC (permalink / raw)
To: Jianfeng Tan, dev; +Cc: yliu
On 02/19/2018 09:48 PM, Maxime Coquelin wrote:
>
>
> On 02/14/2018 05:01 AM, Jianfeng Tan wrote:
>> Previously, get_device() is a function call. It's OK for slow path
>> configuration, but takes some cycles for data path.
>>
>> To avoid that, we turn this function to inline type.
>>
>> Cc: maxime.coquelin@redhat.com
>> Cc: yliu@fridaylinux.org
>>
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>> ---
>> lib/librte_vhost/vhost.c | 13 -------------
>> lib/librte_vhost/vhost.h | 13 ++++++++++++-
>> 2 files changed, 12 insertions(+), 14 deletions(-)
>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Applied to dpdk-next-virtio/master.
Thanks,
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread
* [dpdk-dev] [PATCH 3/4] app/testpmd: add option to avoid lock all memory
2018-02-14 4:01 [dpdk-dev] [PATCH 0/4] some fixes target for 18.05 Jianfeng Tan
2018-02-14 4:01 ` [dpdk-dev] [PATCH 1/4] vhost: remove unused macro Jianfeng Tan
2018-02-14 4:01 ` [dpdk-dev] [PATCH 2/4] vhost: avoid function call in data path Jianfeng Tan
@ 2018-02-14 4:01 ` Jianfeng Tan
2018-02-14 6:40 ` Wu, Jingjing
2018-02-24 3:26 ` [dpdk-dev] [PATCH v2] " Jianfeng Tan
2018-02-14 4:01 ` [dpdk-dev] [PATCH 4/4] vhost: avoid populate guest memory Jianfeng Tan
2018-02-21 7:35 ` [dpdk-dev] [PATCH 0/4] some fixes target for 18.05 Maxime Coquelin
4 siblings, 2 replies; 23+ messages in thread
From: Jianfeng Tan @ 2018-02-14 4:01 UTC (permalink / raw)
To: dev; +Cc: Jianfeng Tan, wenzhuo.lu, jingjing.wu
In some cases, we don't want to lock all memory.
Add an option --no-mlockall to avoid lock all memory.
Cc: wenzhuo.lu@intel.com
Cc: jingjing.wu@intel.com
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
app/test-pmd/parameters.c | 5 ++++-
app/test-pmd/testpmd.c | 32 ++++++++++++++++++--------------
app/test-pmd/testpmd.h | 1 +
3 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 97d22b8..5060a6c 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -186,6 +186,7 @@ usage(char* progname)
printf(" --flow-isolate-all: "
"requests flow API isolated mode on all ports at initialization time.\n");
printf(" --tx-offloads=0xXXXXXXXX: hexadecimal bitmask of TX queue offloads\n");
+ printf(" --no-mlockall: do not lock all memory\n");
}
#ifdef RTE_LIBRTE_CMDLINE
@@ -621,6 +622,7 @@ launch_args_parse(int argc, char** argv)
{ "print-event", 1, 0, 0 },
{ "mask-event", 1, 0, 0 },
{ "tx-offloads", 1, 0, 0 },
+ { "no-mlockall", 0, 0, 0 },
{ 0, 0, 0, 0 },
};
@@ -1102,7 +1104,8 @@ launch_args_parse(int argc, char** argv)
rte_exit(EXIT_FAILURE,
"invalid mask-event argument\n");
}
-
+ if (!strcmp(lgopts[opt_idx].name, "no-mlockall"))
+ no_mlockall = 1;
break;
case 'h':
usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 4c0e258..59bdc85 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -294,6 +294,10 @@ uint32_t event_print_mask = (UINT32_C(1) << RTE_ETH_EVENT_UNKNOWN) |
(UINT32_C(1) << RTE_ETH_EVENT_INTR_RESET) |
(UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
(UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV);
+/*
+ * Decide if all memory are locked for performance.
+ */
+int no_mlockall = 0;
/*
* NIC bypass mode configuration options.
@@ -2489,7 +2493,20 @@ main(int argc, char** argv)
rte_panic("Cannot register log type");
rte_log_set_level(testpmd_logtype, RTE_LOG_DEBUG);
- if (mlockall(MCL_CURRENT | MCL_FUTURE)) {
+ /* Bitrate/latency stats disabled by default */
+#ifdef RTE_LIBRTE_BITRATE
+ bitrate_enabled = 0;
+#endif
+#ifdef RTE_LIBRTE_LATENCY_STATS
+ latencystats_enabled = 0;
+#endif
+
+ argc -= diag;
+ argv += diag;
+ if (argc > 1)
+ launch_args_parse(argc, argv);
+
+ if (!no_mlockall && mlockall(MCL_CURRENT | MCL_FUTURE)) {
TESTPMD_LOG(NOTICE, "mlockall() failed with error \"%s\"\n",
strerror(errno));
}
@@ -2511,19 +2528,6 @@ main(int argc, char** argv)
rte_panic("Empty set of forwarding logical cores - check the "
"core mask supplied in the command parameters\n");
- /* Bitrate/latency stats disabled by default */
-#ifdef RTE_LIBRTE_BITRATE
- bitrate_enabled = 0;
-#endif
-#ifdef RTE_LIBRTE_LATENCY_STATS
- latencystats_enabled = 0;
-#endif
-
- argc -= diag;
- argv += diag;
- if (argc > 1)
- launch_args_parse(argc, argv);
-
if (tx_first && interactive)
rte_exit(EXIT_FAILURE, "--tx-first cannot be used on "
"interactive mode.\n");
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 153abea..028b873 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -319,6 +319,7 @@ extern volatile int test_done; /* stop packet forwarding when set to 1. */
extern uint8_t lsc_interrupt; /**< disabled by "--no-lsc-interrupt" parameter */
extern uint8_t rmv_interrupt; /**< disabled by "--no-rmv-interrupt" parameter */
extern uint32_t event_print_mask;
+extern int no_mlockall; /**< set by "--no-mlockall" parameter */
/**< set by "--print-event xxxx" and "--mask-event xxxx parameters */
#ifdef RTE_LIBRTE_IXGBE_BYPASS
--
2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH 3/4] app/testpmd: add option to avoid lock all memory
2018-02-14 4:01 ` [dpdk-dev] [PATCH 3/4] app/testpmd: add option to avoid lock all memory Jianfeng Tan
@ 2018-02-14 6:40 ` Wu, Jingjing
2018-02-24 3:26 ` [dpdk-dev] [PATCH v2] " Jianfeng Tan
1 sibling, 0 replies; 23+ messages in thread
From: Wu, Jingjing @ 2018-02-14 6:40 UTC (permalink / raw)
To: Tan, Jianfeng, dev; +Cc: Lu, Wenzhuo
New options is added, the doc also need to update.
> -----Original Message-----
> From: Tan, Jianfeng
> Sent: Wednesday, February 14, 2018 12:02 PM
> To: dev@dpdk.org
> Cc: Tan, Jianfeng <jianfeng.tan@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>
> Subject: [PATCH 3/4] app/testpmd: add option to avoid lock all memory
>
> In some cases, we don't want to lock all memory.
>
> Add an option --no-mlockall to avoid lock all memory.
>
> Cc: wenzhuo.lu@intel.com
> Cc: jingjing.wu@intel.com
>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
> app/test-pmd/parameters.c | 5 ++++-
> app/test-pmd/testpmd.c | 32 ++++++++++++++++++--------------
> app/test-pmd/testpmd.h | 1 +
> 3 files changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 97d22b8..5060a6c 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -186,6 +186,7 @@ usage(char* progname)
> printf(" --flow-isolate-all: "
> "requests flow API isolated mode on all ports at initialization time.\n");
> printf(" --tx-offloads=0xXXXXXXXX: hexadecimal bitmask of TX queue offloads\n");
> + printf(" --no-mlockall: do not lock all memory\n");
> }
>
> #ifdef RTE_LIBRTE_CMDLINE
> @@ -621,6 +622,7 @@ launch_args_parse(int argc, char** argv)
> { "print-event", 1, 0, 0 },
> { "mask-event", 1, 0, 0 },
> { "tx-offloads", 1, 0, 0 },
> + { "no-mlockall", 0, 0, 0 },
> { 0, 0, 0, 0 },
> };
>
> @@ -1102,7 +1104,8 @@ launch_args_parse(int argc, char** argv)
> rte_exit(EXIT_FAILURE,
> "invalid mask-event argument\n");
> }
> -
> + if (!strcmp(lgopts[opt_idx].name, "no-mlockall"))
> + no_mlockall = 1;
> break;
> case 'h':
> usage(argv[0]);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 4c0e258..59bdc85 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -294,6 +294,10 @@ uint32_t event_print_mask = (UINT32_C(1) <<
> RTE_ETH_EVENT_UNKNOWN) |
> (UINT32_C(1) << RTE_ETH_EVENT_INTR_RESET) |
> (UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
> (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV);
> +/*
> + * Decide if all memory are locked for performance.
> + */
> +int no_mlockall = 0;
>
> /*
> * NIC bypass mode configuration options.
> @@ -2489,7 +2493,20 @@ main(int argc, char** argv)
> rte_panic("Cannot register log type");
> rte_log_set_level(testpmd_logtype, RTE_LOG_DEBUG);
>
> - if (mlockall(MCL_CURRENT | MCL_FUTURE)) {
> + /* Bitrate/latency stats disabled by default */
> +#ifdef RTE_LIBRTE_BITRATE
> + bitrate_enabled = 0;
> +#endif
> +#ifdef RTE_LIBRTE_LATENCY_STATS
> + latencystats_enabled = 0;
> +#endif
> +
> + argc -= diag;
> + argv += diag;
> + if (argc > 1)
> + launch_args_parse(argc, argv);
> +
> + if (!no_mlockall && mlockall(MCL_CURRENT | MCL_FUTURE)) {
> TESTPMD_LOG(NOTICE, "mlockall() failed with error \"%s\"\n",
> strerror(errno));
> }
> @@ -2511,19 +2528,6 @@ main(int argc, char** argv)
> rte_panic("Empty set of forwarding logical cores - check the "
> "core mask supplied in the command parameters\n");
>
> - /* Bitrate/latency stats disabled by default */
> -#ifdef RTE_LIBRTE_BITRATE
> - bitrate_enabled = 0;
> -#endif
> -#ifdef RTE_LIBRTE_LATENCY_STATS
> - latencystats_enabled = 0;
> -#endif
> -
> - argc -= diag;
> - argv += diag;
> - if (argc > 1)
> - launch_args_parse(argc, argv);
> -
> if (tx_first && interactive)
> rte_exit(EXIT_FAILURE, "--tx-first cannot be used on "
> "interactive mode.\n");
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 153abea..028b873 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -319,6 +319,7 @@ extern volatile int test_done; /* stop packet forwarding when set
> to 1. */
> extern uint8_t lsc_interrupt; /**< disabled by "--no-lsc-interrupt" parameter */
> extern uint8_t rmv_interrupt; /**< disabled by "--no-rmv-interrupt" parameter */
> extern uint32_t event_print_mask;
> +extern int no_mlockall; /**< set by "--no-mlockall" parameter */
> /**< set by "--print-event xxxx" and "--mask-event xxxx parameters */
>
> #ifdef RTE_LIBRTE_IXGBE_BYPASS
> --
> 2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread
* [dpdk-dev] [PATCH v2] app/testpmd: add option to avoid lock all memory
2018-02-14 4:01 ` [dpdk-dev] [PATCH 3/4] app/testpmd: add option to avoid lock all memory Jianfeng Tan
2018-02-14 6:40 ` Wu, Jingjing
@ 2018-02-24 3:26 ` Jianfeng Tan
2018-04-22 23:05 ` Thomas Monjalon
1 sibling, 1 reply; 23+ messages in thread
From: Jianfeng Tan @ 2018-02-24 3:26 UTC (permalink / raw)
To: dev; +Cc: Jianfeng Tan, wenzhuo.lu, jingjing.wu
In some cases, we don't want to lock all memory.
Add an option --no-mlockall to avoid lock all memory.
Cc: wenzhuo.lu@intel.com
Cc: jingjing.wu@intel.com
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
app/test-pmd/parameters.c | 5 ++++-
app/test-pmd/testpmd.c | 32 ++++++++++++++++++--------------
app/test-pmd/testpmd.h | 1 +
doc/guides/testpmd_app_ug/run_app.rst | 4 ++++
4 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 97d22b8..5060a6c 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -186,6 +186,7 @@ usage(char* progname)
printf(" --flow-isolate-all: "
"requests flow API isolated mode on all ports at initialization time.\n");
printf(" --tx-offloads=0xXXXXXXXX: hexadecimal bitmask of TX queue offloads\n");
+ printf(" --no-mlockall: do not lock all memory\n");
}
#ifdef RTE_LIBRTE_CMDLINE
@@ -621,6 +622,7 @@ launch_args_parse(int argc, char** argv)
{ "print-event", 1, 0, 0 },
{ "mask-event", 1, 0, 0 },
{ "tx-offloads", 1, 0, 0 },
+ { "no-mlockall", 0, 0, 0 },
{ 0, 0, 0, 0 },
};
@@ -1102,7 +1104,8 @@ launch_args_parse(int argc, char** argv)
rte_exit(EXIT_FAILURE,
"invalid mask-event argument\n");
}
-
+ if (!strcmp(lgopts[opt_idx].name, "no-mlockall"))
+ no_mlockall = 1;
break;
case 'h':
usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 4c0e258..59bdc85 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -294,6 +294,10 @@ uint32_t event_print_mask = (UINT32_C(1) << RTE_ETH_EVENT_UNKNOWN) |
(UINT32_C(1) << RTE_ETH_EVENT_INTR_RESET) |
(UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
(UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV);
+/*
+ * Decide if all memory are locked for performance.
+ */
+int no_mlockall = 0;
/*
* NIC bypass mode configuration options.
@@ -2489,7 +2493,20 @@ main(int argc, char** argv)
rte_panic("Cannot register log type");
rte_log_set_level(testpmd_logtype, RTE_LOG_DEBUG);
- if (mlockall(MCL_CURRENT | MCL_FUTURE)) {
+ /* Bitrate/latency stats disabled by default */
+#ifdef RTE_LIBRTE_BITRATE
+ bitrate_enabled = 0;
+#endif
+#ifdef RTE_LIBRTE_LATENCY_STATS
+ latencystats_enabled = 0;
+#endif
+
+ argc -= diag;
+ argv += diag;
+ if (argc > 1)
+ launch_args_parse(argc, argv);
+
+ if (!no_mlockall && mlockall(MCL_CURRENT | MCL_FUTURE)) {
TESTPMD_LOG(NOTICE, "mlockall() failed with error \"%s\"\n",
strerror(errno));
}
@@ -2511,19 +2528,6 @@ main(int argc, char** argv)
rte_panic("Empty set of forwarding logical cores - check the "
"core mask supplied in the command parameters\n");
- /* Bitrate/latency stats disabled by default */
-#ifdef RTE_LIBRTE_BITRATE
- bitrate_enabled = 0;
-#endif
-#ifdef RTE_LIBRTE_LATENCY_STATS
- latencystats_enabled = 0;
-#endif
-
- argc -= diag;
- argv += diag;
- if (argc > 1)
- launch_args_parse(argc, argv);
-
if (tx_first && interactive)
rte_exit(EXIT_FAILURE, "--tx-first cannot be used on "
"interactive mode.\n");
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 153abea..028b873 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -319,6 +319,7 @@ extern volatile int test_done; /* stop packet forwarding when set to 1. */
extern uint8_t lsc_interrupt; /**< disabled by "--no-lsc-interrupt" parameter */
extern uint8_t rmv_interrupt; /**< disabled by "--no-rmv-interrupt" parameter */
extern uint32_t event_print_mask;
+extern int no_mlockall; /**< set by "--no-mlockall" parameter */
/**< set by "--print-event xxxx" and "--mask-event xxxx parameters */
#ifdef RTE_LIBRTE_IXGBE_BYPASS
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 1fd5395..bdc6262 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -479,3 +479,7 @@ The commandline options are:
Set the hexadecimal bitmask of TX queue offloads.
The default value is 0.
+
+* ``--no-mlockall``
+
+ Disable locking all memory.
--
2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v2] app/testpmd: add option to avoid lock all memory
2018-02-24 3:26 ` [dpdk-dev] [PATCH v2] " Jianfeng Tan
@ 2018-04-22 23:05 ` Thomas Monjalon
2018-05-03 11:32 ` Burakov, Anatoly
0 siblings, 1 reply; 23+ messages in thread
From: Thomas Monjalon @ 2018-04-22 23:05 UTC (permalink / raw)
To: wenzhuo.lu, jingjing.wu; +Cc: dev, Jianfeng Tan
This patch has been forgotten.
Review please?
24/02/2018 04:26, Jianfeng Tan:
> In some cases, we don't want to lock all memory.
>
> Add an option --no-mlockall to avoid lock all memory.
>
> Cc: wenzhuo.lu@intel.com
> Cc: jingjing.wu@intel.com
>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v2] app/testpmd: add option to avoid lock all memory
2018-04-22 23:05 ` Thomas Monjalon
@ 2018-05-03 11:32 ` Burakov, Anatoly
0 siblings, 0 replies; 23+ messages in thread
From: Burakov, Anatoly @ 2018-05-03 11:32 UTC (permalink / raw)
To: Thomas Monjalon, wenzhuo.lu, jingjing.wu; +Cc: dev, Jianfeng Tan
On 23-Apr-18 12:05 AM, Thomas Monjalon wrote:
> This patch has been forgotten.
> Review please?
>
> 24/02/2018 04:26, Jianfeng Tan:
>> In some cases, we don't want to lock all memory.
>>
>> Add an option --no-mlockall to avoid lock all memory.
>>
>> Cc: wenzhuo.lu@intel.com
>> Cc: jingjing.wu@intel.com
>>
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
I will take this over and submit a new version.
--
Thanks,
Anatoly
^ permalink raw reply [flat|nested] 23+ messages in thread
* [dpdk-dev] [PATCH 4/4] vhost: avoid populate guest memory
2018-02-14 4:01 [dpdk-dev] [PATCH 0/4] some fixes target for 18.05 Jianfeng Tan
` (2 preceding siblings ...)
2018-02-14 4:01 ` [dpdk-dev] [PATCH 3/4] app/testpmd: add option to avoid lock all memory Jianfeng Tan
@ 2018-02-14 4:01 ` Jianfeng Tan
2018-02-19 20:44 ` Maxime Coquelin
2018-02-21 7:35 ` [dpdk-dev] [PATCH 0/4] some fixes target for 18.05 Maxime Coquelin
4 siblings, 1 reply; 23+ messages in thread
From: Jianfeng Tan @ 2018-02-14 4:01 UTC (permalink / raw)
To: dev; +Cc: Jianfeng Tan, maxime.coquelin, yliu
It's not necessary to polulate guest memory from vhost side.
Cc: maxime.coquelin@redhat.com
Cc: yliu@fridaylinux.org
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
lib/librte_vhost/vhost_user.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 90ed211..9bd0391 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -644,6 +644,7 @@ vhost_user_set_mem_table(struct virtio_net *dev, struct VhostUserMsg *pmsg)
uint64_t mmap_offset;
uint64_t alignment;
uint32_t i;
+ int populate;
int fd;
if (dev->mem && !vhost_memory_changed(&memory, dev->mem)) {
@@ -714,8 +715,9 @@ vhost_user_set_mem_table(struct virtio_net *dev, struct VhostUserMsg *pmsg)
}
mmap_size = RTE_ALIGN_CEIL(mmap_size, alignment);
+ populate = (dev->dequeue_zero_copy) ? MAP_POPULATE : 0;
mmap_addr = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
- MAP_SHARED | MAP_POPULATE, fd, 0);
+ MAP_SHARED | populate, fd, 0);
if (mmap_addr == MAP_FAILED) {
RTE_LOG(ERR, VHOST_CONFIG,
--
2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH 4/4] vhost: avoid populate guest memory
2018-02-14 4:01 ` [dpdk-dev] [PATCH 4/4] vhost: avoid populate guest memory Jianfeng Tan
@ 2018-02-19 20:44 ` Maxime Coquelin
2018-02-22 2:42 ` Tan, Jianfeng
2018-03-28 6:56 ` [dpdk-dev] [PATCH v2] " Jianfeng Tan
0 siblings, 2 replies; 23+ messages in thread
From: Maxime Coquelin @ 2018-02-19 20:44 UTC (permalink / raw)
To: Jianfeng Tan, dev; +Cc: yliu
Hi Jianfeng,
On 02/14/2018 05:01 AM, Jianfeng Tan wrote:
> It's not necessary to polulate guest memory from vhost side.
>
> Cc: maxime.coquelin@redhat.com
> Cc: yliu@fridaylinux.org
>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
> lib/librte_vhost/vhost_user.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 90ed211..9bd0391 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -644,6 +644,7 @@ vhost_user_set_mem_table(struct virtio_net *dev, struct VhostUserMsg *pmsg)
> uint64_t mmap_offset;
> uint64_t alignment;
> uint32_t i;
> + int populate;
> int fd;
>
> if (dev->mem && !vhost_memory_changed(&memory, dev->mem)) {
> @@ -714,8 +715,9 @@ vhost_user_set_mem_table(struct virtio_net *dev, struct VhostUserMsg *pmsg)
> }
> mmap_size = RTE_ALIGN_CEIL(mmap_size, alignment);
>
> + populate = (dev->dequeue_zero_copy) ? MAP_POPULATE : 0;
> mmap_addr = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> - MAP_SHARED | MAP_POPULATE, fd, 0);
> + MAP_SHARED | populate, fd, 0);
>
> if (mmap_addr == MAP_FAILED) {
> RTE_LOG(ERR, VHOST_CONFIG,
>
Wouldn't not populating all the guest memory have a bad impact on 0%
acceptable loss use-cases?
Thanks,
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH 4/4] vhost: avoid populate guest memory
2018-02-19 20:44 ` Maxime Coquelin
@ 2018-02-22 2:42 ` Tan, Jianfeng
2018-02-22 8:25 ` Maxime Coquelin
2018-03-28 6:56 ` [dpdk-dev] [PATCH v2] " Jianfeng Tan
1 sibling, 1 reply; 23+ messages in thread
From: Tan, Jianfeng @ 2018-02-22 2:42 UTC (permalink / raw)
To: Maxime Coquelin, dev; +Cc: yliu
Hi Maxime,
> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Tuesday, February 20, 2018 4:45 AM
> To: Tan, Jianfeng; dev@dpdk.org
> Cc: yliu@fridaylinux.org
> Subject: Re: [PATCH 4/4] vhost: avoid populate guest memory
>
> Hi Jianfeng,
>
> On 02/14/2018 05:01 AM, Jianfeng Tan wrote:
> > It's not necessary to polulate guest memory from vhost side.
> >
> > Cc: maxime.coquelin@redhat.com
> > Cc: yliu@fridaylinux.org
> >
> > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> > ---
> > lib/librte_vhost/vhost_user.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> > index 90ed211..9bd0391 100644
> > --- a/lib/librte_vhost/vhost_user.c
> > +++ b/lib/librte_vhost/vhost_user.c
> > @@ -644,6 +644,7 @@ vhost_user_set_mem_table(struct virtio_net *dev,
> struct VhostUserMsg *pmsg)
> > uint64_t mmap_offset;
> > uint64_t alignment;
> > uint32_t i;
> > + int populate;
> > int fd;
> >
> > if (dev->mem && !vhost_memory_changed(&memory, dev->mem))
> {
> > @@ -714,8 +715,9 @@ vhost_user_set_mem_table(struct virtio_net *dev,
> struct VhostUserMsg *pmsg)
> > }
> > mmap_size = RTE_ALIGN_CEIL(mmap_size, alignment);
> >
> > + populate = (dev->dequeue_zero_copy) ? MAP_POPULATE :
> 0;
> > mmap_addr = mmap(NULL, mmap_size, PROT_READ |
> PROT_WRITE,
> > - MAP_SHARED | MAP_POPULATE, fd, 0);
> > + MAP_SHARED | populate, fd, 0);
> >
> > if (mmap_addr == MAP_FAILED) {
> > RTE_LOG(ERR, VHOST_CONFIG,
> >
>
> Wouldn't not populating all the guest memory have a bad impact on 0%
> acceptable loss use-cases?
Yes, it could affect such use case; but we can address that by warming up the system a little bit, can't we?
From a good point of view, it could save the memory for VMs without pre-allocating.
Thanks,
Jianfeng
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH 4/4] vhost: avoid populate guest memory
2018-02-22 2:42 ` Tan, Jianfeng
@ 2018-02-22 8:25 ` Maxime Coquelin
2018-02-22 8:40 ` Tan, Jianfeng
0 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2018-02-22 8:25 UTC (permalink / raw)
To: Tan, Jianfeng, dev; +Cc: yliu
On 02/22/2018 03:42 AM, Tan, Jianfeng wrote:
> Hi Maxime,
>
>> -----Original Message-----
>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>> Sent: Tuesday, February 20, 2018 4:45 AM
>> To: Tan, Jianfeng; dev@dpdk.org
>> Cc: yliu@fridaylinux.org
>> Subject: Re: [PATCH 4/4] vhost: avoid populate guest memory
>>
>> Hi Jianfeng,
>>
>> On 02/14/2018 05:01 AM, Jianfeng Tan wrote:
>>> It's not necessary to polulate guest memory from vhost side.
>>>
>>> Cc: maxime.coquelin@redhat.com
>>> Cc: yliu@fridaylinux.org
>>>
>>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>>> ---
>>> lib/librte_vhost/vhost_user.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
>>> index 90ed211..9bd0391 100644
>>> --- a/lib/librte_vhost/vhost_user.c
>>> +++ b/lib/librte_vhost/vhost_user.c
>>> @@ -644,6 +644,7 @@ vhost_user_set_mem_table(struct virtio_net *dev,
>> struct VhostUserMsg *pmsg)
>>> uint64_t mmap_offset;
>>> uint64_t alignment;
>>> uint32_t i;
>>> + int populate;
>>> int fd;
>>>
>>> if (dev->mem && !vhost_memory_changed(&memory, dev->mem))
>> {
>>> @@ -714,8 +715,9 @@ vhost_user_set_mem_table(struct virtio_net *dev,
>> struct VhostUserMsg *pmsg)
>>> }
>>> mmap_size = RTE_ALIGN_CEIL(mmap_size, alignment);
>>>
>>> + populate = (dev->dequeue_zero_copy) ? MAP_POPULATE :
>> 0;
>>> mmap_addr = mmap(NULL, mmap_size, PROT_READ |
>> PROT_WRITE,
>>> - MAP_SHARED | MAP_POPULATE, fd, 0);
>>> + MAP_SHARED | populate, fd, 0);
>>>
>>> if (mmap_addr == MAP_FAILED) {
>>> RTE_LOG(ERR, VHOST_CONFIG,
>>>
>>
>> Wouldn't not populating all the guest memory have a bad impact on 0%
>> acceptable loss use-cases?
>
> Yes, it could affect such use case; but we can address that by warming up the system a little bit, can't we?
I'm not sure this is a good idea to ask the real user to warm-up the
system.
Also, even with benchmarking, the loss happens when the queues are full,
so it is likely that it happens with buffers not used before, even if
system has been warmed-up.
> From a good point of view, it could save the memory for VMs without pre-allocating.
What could be done is maybe to have an EAL API for mmaping, with an
associated EAL parameter to state whether it want populating or not.
This option would be disabled by default.
Does that sounds reasonable?
Cheers,
Maxime
> Thanks,
> Jianfeng
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH 4/4] vhost: avoid populate guest memory
2018-02-22 8:25 ` Maxime Coquelin
@ 2018-02-22 8:40 ` Tan, Jianfeng
2018-02-22 12:32 ` Maxime Coquelin
0 siblings, 1 reply; 23+ messages in thread
From: Tan, Jianfeng @ 2018-02-22 8:40 UTC (permalink / raw)
To: Maxime Coquelin, dev; +Cc: yliu
> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Thursday, February 22, 2018 4:26 PM
> To: Tan, Jianfeng; dev@dpdk.org
> Cc: yliu@fridaylinux.org
> Subject: Re: [PATCH 4/4] vhost: avoid populate guest memory
>
>
>
> On 02/22/2018 03:42 AM, Tan, Jianfeng wrote:
> > Hi Maxime,
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> >> Sent: Tuesday, February 20, 2018 4:45 AM
> >> To: Tan, Jianfeng; dev@dpdk.org
> >> Cc: yliu@fridaylinux.org
> >> Subject: Re: [PATCH 4/4] vhost: avoid populate guest memory
> >>
> >> Hi Jianfeng,
> >>
> >> On 02/14/2018 05:01 AM, Jianfeng Tan wrote:
> >>> It's not necessary to polulate guest memory from vhost side.
> >>>
> >>> Cc: maxime.coquelin@redhat.com
> >>> Cc: yliu@fridaylinux.org
> >>>
> >>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> >>> ---
> >>> lib/librte_vhost/vhost_user.c | 4 +++-
> >>> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> >>> index 90ed211..9bd0391 100644
> >>> --- a/lib/librte_vhost/vhost_user.c
> >>> +++ b/lib/librte_vhost/vhost_user.c
> >>> @@ -644,6 +644,7 @@ vhost_user_set_mem_table(struct virtio_net
> *dev,
> >> struct VhostUserMsg *pmsg)
> >>> uint64_t mmap_offset;
> >>> uint64_t alignment;
> >>> uint32_t i;
> >>> + int populate;
> >>> int fd;
> >>>
> >>> if (dev->mem && !vhost_memory_changed(&memory, dev->mem))
> >> {
> >>> @@ -714,8 +715,9 @@ vhost_user_set_mem_table(struct virtio_net
> *dev,
> >> struct VhostUserMsg *pmsg)
> >>> }
> >>> mmap_size = RTE_ALIGN_CEIL(mmap_size, alignment);
> >>>
> >>> + populate = (dev->dequeue_zero_copy) ? MAP_POPULATE :
> >> 0;
> >>> mmap_addr = mmap(NULL, mmap_size, PROT_READ |
> >> PROT_WRITE,
> >>> - MAP_SHARED | MAP_POPULATE, fd, 0);
> >>> + MAP_SHARED | populate, fd, 0);
> >>>
> >>> if (mmap_addr == MAP_FAILED) {
> >>> RTE_LOG(ERR, VHOST_CONFIG,
> >>>
> >>
> >> Wouldn't not populating all the guest memory have a bad impact on 0%
> >> acceptable loss use-cases?
> >
> > Yes, it could affect such use case; but we can address that by warming up
> the system a little bit, can't we?
>
> I'm not sure this is a good idea to ask the real user to warm-up the
> system.
>
> Also, even with benchmarking, the loss happens when the queues are full,
> so it is likely that it happens with buffers not used before, even if
> system has been warmed-up.
OK, warm-up is a bad idea here :-)
But if a VM is used for such use case, I think we'd better pre-allocate the memory at QEMU side.
>
> > From a good point of view, it could save the memory for VMs without pre-
> allocating.
>
> What could be done is maybe to have an EAL API for mmaping, with an
> associated EAL parameter to state whether it want populating or not.
> This option would be disabled by default.
>
> Does that sounds reasonable?
If we look for an application-level configuration, it's not necessary to have such a parameter. Refer to the 3rd patch in this series, if we make all (current/future) memory locked, the mmap() syscall will populate the memory.
Thanks,
Jianfeng
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH 4/4] vhost: avoid populate guest memory
2018-02-22 8:40 ` Tan, Jianfeng
@ 2018-02-22 12:32 ` Maxime Coquelin
2018-02-23 3:17 ` Tan, Jianfeng
0 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2018-02-22 12:32 UTC (permalink / raw)
To: Tan, Jianfeng, dev; +Cc: yliu
On 02/22/2018 09:40 AM, Tan, Jianfeng wrote:
>
>
>> -----Original Message-----
>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>> Sent: Thursday, February 22, 2018 4:26 PM
>> To: Tan, Jianfeng; dev@dpdk.org
>> Cc: yliu@fridaylinux.org
>> Subject: Re: [PATCH 4/4] vhost: avoid populate guest memory
>>
>>
>>
>> On 02/22/2018 03:42 AM, Tan, Jianfeng wrote:
>>> Hi Maxime,
>>>
>>>> -----Original Message-----
>>>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>>>> Sent: Tuesday, February 20, 2018 4:45 AM
>>>> To: Tan, Jianfeng; dev@dpdk.org
>>>> Cc: yliu@fridaylinux.org
>>>> Subject: Re: [PATCH 4/4] vhost: avoid populate guest memory
>>>>
>>>> Hi Jianfeng,
>>>>
>>>> On 02/14/2018 05:01 AM, Jianfeng Tan wrote:
>>>>> It's not necessary to polulate guest memory from vhost side.
>>>>>
>>>>> Cc: maxime.coquelin@redhat.com
>>>>> Cc: yliu@fridaylinux.org
>>>>>
>>>>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>>>>> ---
>>>>> lib/librte_vhost/vhost_user.c | 4 +++-
>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
>>>>> index 90ed211..9bd0391 100644
>>>>> --- a/lib/librte_vhost/vhost_user.c
>>>>> +++ b/lib/librte_vhost/vhost_user.c
>>>>> @@ -644,6 +644,7 @@ vhost_user_set_mem_table(struct virtio_net
>> *dev,
>>>> struct VhostUserMsg *pmsg)
>>>>> uint64_t mmap_offset;
>>>>> uint64_t alignment;
>>>>> uint32_t i;
>>>>> + int populate;
>>>>> int fd;
>>>>>
>>>>> if (dev->mem && !vhost_memory_changed(&memory, dev->mem))
>>>> {
>>>>> @@ -714,8 +715,9 @@ vhost_user_set_mem_table(struct virtio_net
>> *dev,
>>>> struct VhostUserMsg *pmsg)
>>>>> }
>>>>> mmap_size = RTE_ALIGN_CEIL(mmap_size, alignment);
>>>>>
>>>>> + populate = (dev->dequeue_zero_copy) ? MAP_POPULATE :
>>>> 0;
>>>>> mmap_addr = mmap(NULL, mmap_size, PROT_READ |
>>>> PROT_WRITE,
>>>>> - MAP_SHARED | MAP_POPULATE, fd, 0);
>>>>> + MAP_SHARED | populate, fd, 0);
>>>>>
>>>>> if (mmap_addr == MAP_FAILED) {
>>>>> RTE_LOG(ERR, VHOST_CONFIG,
>>>>>
>>>>
>>>> Wouldn't not populating all the guest memory have a bad impact on 0%
>>>> acceptable loss use-cases?
>>>
>>> Yes, it could affect such use case; but we can address that by warming up
>> the system a little bit, can't we?
>>
>> I'm not sure this is a good idea to ask the real user to warm-up the
>> system.
>>
>> Also, even with benchmarking, the loss happens when the queues are full,
>> so it is likely that it happens with buffers not used before, even if
>> system has been warmed-up.
>
> OK, warm-up is a bad idea here :-)
>
> But if a VM is used for such use case, I think we'd better pre-allocate the memory at QEMU side.
>
>>
>>> From a good point of view, it could save the memory for VMs without pre-
>> allocating.
>>
>> What could be done is maybe to have an EAL API for mmaping, with an
>> associated EAL parameter to state whether it want populating or not.
>> This option would be disabled by default.
>>
>> Does that sounds reasonable?
>
> If we look for an application-level configuration, it's not necessary to have such a parameter. Refer to the 3rd patch in this series, if we make all (current/future) memory locked, the mmap() syscall will populate the memory.
OK, but in that case it should be documented.
I see OVS has also a parameter to request the memory to be locked, but
it seems not to be the default, so the user could face a change in the
behavior it didn't expect.
Thanks,
Maxime
> Thanks,
> Jianfeng
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH 4/4] vhost: avoid populate guest memory
2018-02-22 12:32 ` Maxime Coquelin
@ 2018-02-23 3:17 ` Tan, Jianfeng
0 siblings, 0 replies; 23+ messages in thread
From: Tan, Jianfeng @ 2018-02-23 3:17 UTC (permalink / raw)
To: Maxime Coquelin, dev; +Cc: yliu
> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Thursday, February 22, 2018 8:33 PM
> To: Tan, Jianfeng; dev@dpdk.org
> Cc: yliu@fridaylinux.org
> Subject: Re: [PATCH 4/4] vhost: avoid populate guest memory
>
>
> >> What could be done is maybe to have an EAL API for mmaping, with an
> >> associated EAL parameter to state whether it want populating or not.
> >> This option would be disabled by default.
> >>
> >> Does that sounds reasonable?
> >
> > If we look for an application-level configuration, it's not necessary to have
> such a parameter. Refer to the 3rd patch in this series, if we make all
> (current/future) memory locked, the mmap() syscall will populate the
> memory.
>
> OK, but in that case it should be documented.
> I see OVS has also a parameter to request the memory to be locked, but
> it seems not to be the default, so the user could face a change in the
> behavior it didn't expect.
Make sense, will send v2 with the note.
Thanks,
Jianfeng
^ permalink raw reply [flat|nested] 23+ messages in thread
* [dpdk-dev] [PATCH v2] vhost: avoid populate guest memory
2018-02-19 20:44 ` Maxime Coquelin
2018-02-22 2:42 ` Tan, Jianfeng
@ 2018-03-28 6:56 ` Jianfeng Tan
2018-03-30 8:21 ` Maxime Coquelin
1 sibling, 1 reply; 23+ messages in thread
From: Jianfeng Tan @ 2018-03-28 6:56 UTC (permalink / raw)
To: dev; +Cc: Jianfeng Tan, maxime.coquelin
It's not necessary to populate guest memory from vhost side unless
zerocopy is enabled or users want better performance.
Update the doc for guest memory requirement clarification.
Cc: maxime.coquelin@redhat.com
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
doc/guides/prog_guide/vhost_lib.rst | 21 +++++++++++++++++++++
doc/guides/sample_app_ug/vhost.rst | 9 ---------
lib/librte_vhost/vhost_user.c | 4 +++-
3 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/doc/guides/prog_guide/vhost_lib.rst b/doc/guides/prog_guide/vhost_lib.rst
index 18227b6..f474736 100644
--- a/doc/guides/prog_guide/vhost_lib.rst
+++ b/doc/guides/prog_guide/vhost_lib.rst
@@ -214,6 +214,27 @@ the vhost device from the data plane.
When the socket connection is closed, vhost will destroy the device.
+Guest memory requirement
+------------------------
+
+* Memory pre-allocation
+
+ For non-zerocopy, guest memory pre-allocation is not a must. This can help
+ save of memory. If users really want the guest memory to be pre-allocated
+ (e.g., for performance reason), we can add option ``-mem-prealloc`` when
+ starting QEMU. Or, we can lock all memory at vhost side which will force
+ memory to be allocated when mmap at vhost side; option --mlockall in
+ ovs-dpdk is an example in hand.
+
+ For zerocopy, we force the VM memory to be pre-allocated at vhost lib when
+ mapping the guest memory; and also we need to lock the memory to prevent
+ pages being swapped out to disk.
+
+* Memory sharing
+
+ Make sure ``share=on`` QEMU option is given. vhost-user will not work with
+ a QEMU version without shared memory mapping.
+
Vhost supported vSwitch reference
---------------------------------
diff --git a/doc/guides/sample_app_ug/vhost.rst b/doc/guides/sample_app_ug/vhost.rst
index a4bdc6a..da161a9 100644
--- a/doc/guides/sample_app_ug/vhost.rst
+++ b/doc/guides/sample_app_ug/vhost.rst
@@ -175,15 +175,6 @@ Common Issues
The command above indicates how many hugepages are free to support QEMU's
allocation request.
-* vhost-user will not work with QEMU without the ``-mem-prealloc`` option
-
- The current implementation works properly only when the guest memory is
- pre-allocated.
-
-* vhost-user will not work with a QEMU version without shared memory mapping:
-
- Make sure ``share=on`` QEMU option is given.
-
* Failed to build DPDK in VM
Make sure "-cpu host" QEMU option is given.
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 90ed211..9bd0391 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -644,6 +644,7 @@ vhost_user_set_mem_table(struct virtio_net *dev, struct VhostUserMsg *pmsg)
uint64_t mmap_offset;
uint64_t alignment;
uint32_t i;
+ int populate;
int fd;
if (dev->mem && !vhost_memory_changed(&memory, dev->mem)) {
@@ -714,8 +715,9 @@ vhost_user_set_mem_table(struct virtio_net *dev, struct VhostUserMsg *pmsg)
}
mmap_size = RTE_ALIGN_CEIL(mmap_size, alignment);
+ populate = (dev->dequeue_zero_copy) ? MAP_POPULATE : 0;
mmap_addr = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
- MAP_SHARED | MAP_POPULATE, fd, 0);
+ MAP_SHARED | populate, fd, 0);
if (mmap_addr == MAP_FAILED) {
RTE_LOG(ERR, VHOST_CONFIG,
--
2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: avoid populate guest memory
2018-03-28 6:56 ` [dpdk-dev] [PATCH v2] " Jianfeng Tan
@ 2018-03-30 8:21 ` Maxime Coquelin
2018-03-30 8:34 ` Maxime Coquelin
0 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2018-03-30 8:21 UTC (permalink / raw)
To: Jianfeng Tan, dev
On 03/28/2018 08:56 AM, Jianfeng Tan wrote:
> It's not necessary to populate guest memory from vhost side unless
> zerocopy is enabled or users want better performance.
>
> Update the doc for guest memory requirement clarification.
>
> Cc: maxime.coquelin@redhat.com
>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
> doc/guides/prog_guide/vhost_lib.rst | 21 +++++++++++++++++++++
> doc/guides/sample_app_ug/vhost.rst | 9 ---------
> lib/librte_vhost/vhost_user.c | 4 +++-
> 3 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/doc/guides/prog_guide/vhost_lib.rst b/doc/guides/prog_guide/vhost_lib.rst
> index 18227b6..f474736 100644
> --- a/doc/guides/prog_guide/vhost_lib.rst
> +++ b/doc/guides/prog_guide/vhost_lib.rst
> @@ -214,6 +214,27 @@ the vhost device from the data plane.
>
> When the socket connection is closed, vhost will destroy the device.
>
> +Guest memory requirement
> +------------------------
> +
> +* Memory pre-allocation
> +
> + For non-zerocopy, guest memory pre-allocation is not a must. This can help
> + save of memory. If users really want the guest memory to be pre-allocated
> + (e.g., for performance reason), we can add option ``-mem-prealloc`` when
> + starting QEMU. Or, we can lock all memory at vhost side which will force
> + memory to be allocated when mmap at vhost side; option --mlockall in
> + ovs-dpdk is an example in hand.
> +
> + For zerocopy, we force the VM memory to be pre-allocated at vhost lib when
> + mapping the guest memory; and also we need to lock the memory to prevent
> + pages being swapped out to disk.
> +
> +* Memory sharing
> +
> + Make sure ``share=on`` QEMU option is given. vhost-user will not work with
> + a QEMU version without shared memory mapping.
> +
nice write-up!
> Vhost supported vSwitch reference
> ---------------------------------
>
> diff --git a/doc/guides/sample_app_ug/vhost.rst b/doc/guides/sample_app_ug/vhost.rst
> index a4bdc6a..da161a9 100644
> --- a/doc/guides/sample_app_ug/vhost.rst
> +++ b/doc/guides/sample_app_ug/vhost.rst
> @@ -175,15 +175,6 @@ Common Issues
> The command above indicates how many hugepages are free to support QEMU's
> allocation request.
>
> -* vhost-user will not work with QEMU without the ``-mem-prealloc`` option
> -
> - The current implementation works properly only when the guest memory is
> - pre-allocated.
> -
> -* vhost-user will not work with a QEMU version without shared memory mapping:
> -
> - Make sure ``share=on`` QEMU option is given.
> -
> * Failed to build DPDK in VM
>
> Make sure "-cpu host" QEMU option is given.
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 90ed211..9bd0391 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -644,6 +644,7 @@ vhost_user_set_mem_table(struct virtio_net *dev, struct VhostUserMsg *pmsg)
> uint64_t mmap_offset;
> uint64_t alignment;
> uint32_t i;
> + int populate;
> int fd;
>
> if (dev->mem && !vhost_memory_changed(&memory, dev->mem)) {
> @@ -714,8 +715,9 @@ vhost_user_set_mem_table(struct virtio_net *dev, struct VhostUserMsg *pmsg)
> }
> mmap_size = RTE_ALIGN_CEIL(mmap_size, alignment);
>
> + populate = (dev->dequeue_zero_copy) ? MAP_POPULATE : 0;
> mmap_addr = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> - MAP_SHARED | MAP_POPULATE, fd, 0);
> + MAP_SHARED | populate, fd, 0);
>
> if (mmap_addr == MAP_FAILED) {
> RTE_LOG(ERR, VHOST_CONFIG,
>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: avoid populate guest memory
2018-03-30 8:21 ` Maxime Coquelin
@ 2018-03-30 8:34 ` Maxime Coquelin
0 siblings, 0 replies; 23+ messages in thread
From: Maxime Coquelin @ 2018-03-30 8:34 UTC (permalink / raw)
To: Jianfeng Tan, dev
On 03/30/2018 10:21 AM, Maxime Coquelin wrote:
>
>
> On 03/28/2018 08:56 AM, Jianfeng Tan wrote:
>> It's not necessary to populate guest memory from vhost side unless
>> zerocopy is enabled or users want better performance.
>>
>> Update the doc for guest memory requirement clarification.
>>
>> Cc: maxime.coquelin@redhat.com
>>
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>> ---
>> doc/guides/prog_guide/vhost_lib.rst | 21 +++++++++++++++++++++
>> doc/guides/sample_app_ug/vhost.rst | 9 ---------
>> lib/librte_vhost/vhost_user.c | 4 +++-
>> 3 files changed, 24 insertions(+), 10 deletions(-)
>>
>> diff --git a/doc/guides/prog_guide/vhost_lib.rst
Applied to dpdk-next-virtio/master
Thanks,
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH 0/4] some fixes target for 18.05
2018-02-14 4:01 [dpdk-dev] [PATCH 0/4] some fixes target for 18.05 Jianfeng Tan
` (3 preceding siblings ...)
2018-02-14 4:01 ` [dpdk-dev] [PATCH 4/4] vhost: avoid populate guest memory Jianfeng Tan
@ 2018-02-21 7:35 ` Maxime Coquelin
4 siblings, 0 replies; 23+ messages in thread
From: Maxime Coquelin @ 2018-02-21 7:35 UTC (permalink / raw)
To: Jianfeng Tan, dev
Hi Jianfeng,
On 02/14/2018 05:01 AM, Jianfeng Tan wrote:
> Some trivial fixes, and a vhost memory optimization to avoid polulating
> guest memory.
>
> Jianfeng Tan (4):
> vhost: remove unused macro
> vhost: avoid function call in data path
> app/testpmd: add option to avoid lock all memory
> vhost: avoid populate guest memory
>
> app/test-pmd/parameters.c | 5 ++++-
> app/test-pmd/testpmd.c | 32 ++++++++++++++++++--------------
> app/test-pmd/testpmd.h | 1 +
> lib/librte_vhost/vhost.c | 13 -------------
> lib/librte_vhost/vhost.h | 15 ++++++++++++---
> lib/librte_vhost/vhost_user.c | 4 +++-
> 6 files changed, 38 insertions(+), 32 deletions(-)
>
I applied patches 1 & 2 for now, while we discuss patch 4.
Patch 3 is not to be picked by me.
Thanks,
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread