* [dpdk-dev] [PATCH] vhost: make vhost lockless enqueue configurable
@ 2015-04-29 11:29 Huawei Xie
2015-04-29 11:38 ` Panu Matilainen
0 siblings, 1 reply; 5+ messages in thread
From: Huawei Xie @ 2015-04-29 11:29 UTC (permalink / raw)
To: dev
vhost enabled vSwitch could have their own thread-safe vring enqueue policy.
Add the RTE_LIBRTE_VHOST_LOCKLESS_ENQ macro for vhost lockless enqueue.
Turn it off by default.
Signed-off-by: Huawei Xie <huawei.xie@intel.com>
---
config/common_linuxapp | 1 +
lib/librte_vhost/vhost_rxtx.c | 24 +++++++++++++++++++++++-
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/config/common_linuxapp b/config/common_linuxapp
index 0078dc9..7f59499 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -421,6 +421,7 @@ CONFIG_RTE_KNI_VHOST_DEBUG_TX=n
#
CONFIG_RTE_LIBRTE_VHOST=n
CONFIG_RTE_LIBRTE_VHOST_USER=y
+CONFIG_RTE_LIBRTE_VHOST_LOCKLESS_ENQ=n
CONFIG_RTE_LIBRTE_VHOST_DEBUG=n
#
diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 510ffe8..475be6e 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -80,7 +80,11 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
* they need to be reserved.
*/
do {
+#ifdef RTE_LIBRTE_VHOST_LOCKESS_ENQ
res_base_idx = vq->last_used_idx_res;
+#else
+ res_base_idx = vq->last_used_idx;
+#endif
avail_idx = *((volatile uint16_t *)&vq->avail->idx);
free_entries = (avail_idx - res_base_idx);
@@ -92,10 +96,15 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
return 0;
res_end_idx = res_base_idx + count;
+
+#ifdef RTE_LIBRTE_VHOST_LOCKLESS_ENQ
/* vq->last_used_idx_res is atomically updated. */
- /* TODO: Allow to disable cmpset if no concurrency in application. */
success = rte_atomic16_cmpset(&vq->last_used_idx_res,
res_base_idx, res_end_idx);
+#else
+ /* last_used_idx_res isn't used. */
+ success = 1;
+#endif
} while (unlikely(success == 0));
res_cur_idx = res_base_idx;
LOG_DEBUG(VHOST_DATA, "(%"PRIu64") Current Index %d| End Index %d\n",
@@ -171,9 +180,11 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
rte_compiler_barrier();
+#ifdef RTE_LIBRTE_VHOST_LOCKLESS_ENQ
/* Wait until it's our turn to add our buffer to the used ring. */
while (unlikely(vq->last_used_idx != res_base_idx))
rte_pause();
+#endif
*(volatile uint16_t *)&vq->used->idx += count;
vq->last_used_idx = res_end_idx;
@@ -422,11 +433,15 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
uint16_t i, id;
do {
+#ifdef RTE_LIBRTE_VHOST_LOCKLESS_ENQ
/*
* As many data cores may want access to available
* buffers, they need to be reserved.
*/
res_base_idx = vq->last_used_idx_res;
+#else
+ res_base_idx = vq->last_used_idx;
+#endif
res_cur_idx = res_base_idx;
do {
@@ -459,10 +474,15 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
}
} while (pkt_len > secure_len);
+#ifdef RTE_LIBRTE_VHOST_LOCKLESS_ENQ
/* vq->last_used_idx_res is atomically updated. */
success = rte_atomic16_cmpset(&vq->last_used_idx_res,
res_base_idx,
res_cur_idx);
+#else
+ /* last_used_idx_res isn't used. */
+ success = 1;
+#endif
} while (success == 0);
id = res_base_idx;
@@ -495,12 +515,14 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
rte_compiler_barrier();
+#ifdef RTE_LIBRTE_VHOST_LOCKLESS_ENQ
/*
* Wait until it's our turn to add our buffer
* to the used ring.
*/
while (unlikely(vq->last_used_idx != res_base_idx))
rte_pause();
+#endif
*(volatile uint16_t *)&vq->used->idx += entry_success;
vq->last_used_idx = res_end_idx;
--
1.8.1.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: make vhost lockless enqueue configurable
2015-04-29 11:29 [dpdk-dev] [PATCH] vhost: make vhost lockless enqueue configurable Huawei Xie
@ 2015-04-29 11:38 ` Panu Matilainen
2015-04-29 11:56 ` Thomas Monjalon
0 siblings, 1 reply; 5+ messages in thread
From: Panu Matilainen @ 2015-04-29 11:38 UTC (permalink / raw)
To: dev
On 04/29/2015 02:29 PM, Huawei Xie wrote:
> vhost enabled vSwitch could have their own thread-safe vring enqueue policy.
> Add the RTE_LIBRTE_VHOST_LOCKLESS_ENQ macro for vhost lockless enqueue.
> Turn it off by default.
>
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> ---
> config/common_linuxapp | 1 +
> lib/librte_vhost/vhost_rxtx.c | 24 +++++++++++++++++++++++-
> 2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/config/common_linuxapp b/config/common_linuxapp
> index 0078dc9..7f59499 100644
> --- a/config/common_linuxapp
> +++ b/config/common_linuxapp
> @@ -421,6 +421,7 @@ CONFIG_RTE_KNI_VHOST_DEBUG_TX=n
> #
> CONFIG_RTE_LIBRTE_VHOST=n
> CONFIG_RTE_LIBRTE_VHOST_USER=y
> +CONFIG_RTE_LIBRTE_VHOST_LOCKLESS_ENQ=n
> CONFIG_RTE_LIBRTE_VHOST_DEBUG=n
>
> #
> diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
> index 510ffe8..475be6e 100644
> --- a/lib/librte_vhost/vhost_rxtx.c
> +++ b/lib/librte_vhost/vhost_rxtx.c
> @@ -80,7 +80,11 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
> * they need to be reserved.
> */
> do {
> +#ifdef RTE_LIBRTE_VHOST_LOCKESS_ENQ
> res_base_idx = vq->last_used_idx_res;
> +#else
> + res_base_idx = vq->last_used_idx;
> +#endif
These things should be runtime configurable, not build options. Please
do not assume everybody builds DPDK separately for each and every
application that might ever be.
- Panu -
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: make vhost lockless enqueue configurable
2015-04-29 11:38 ` Panu Matilainen
@ 2015-04-29 11:56 ` Thomas Monjalon
2015-04-29 18:07 ` Flavio Leitner
2015-05-21 8:49 ` Xie, Huawei
0 siblings, 2 replies; 5+ messages in thread
From: Thomas Monjalon @ 2015-04-29 11:56 UTC (permalink / raw)
To: Panu Matilainen; +Cc: dev
2015-04-29 13:38 GMT+02:00 Panu Matilainen <pmatilai@redhat.com>:
> On 04/29/2015 02:29 PM, Huawei Xie wrote:
>>
>> vhost enabled vSwitch could have their own thread-safe vring enqueue
>> policy.
>> Add the RTE_LIBRTE_VHOST_LOCKLESS_ENQ macro for vhost lockless enqueue.
>> Turn it off by default.
>>
>> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
>> ---
>> config/common_linuxapp | 1 +
>> lib/librte_vhost/vhost_rxtx.c | 24 +++++++++++++++++++++++-
>> 2 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/config/common_linuxapp b/config/common_linuxapp
>> index 0078dc9..7f59499 100644
>> --- a/config/common_linuxapp
>> +++ b/config/common_linuxapp
>> @@ -421,6 +421,7 @@ CONFIG_RTE_KNI_VHOST_DEBUG_TX=n
>> #
>> CONFIG_RTE_LIBRTE_VHOST=n
>> CONFIG_RTE_LIBRTE_VHOST_USER=y
>> +CONFIG_RTE_LIBRTE_VHOST_LOCKLESS_ENQ=n
>> CONFIG_RTE_LIBRTE_VHOST_DEBUG=n
[...]
>
> These things should be runtime configurable, not build options. Please do
> not assume everybody builds DPDK separately for each and every application
> that might ever be.
+1
Adding new build options must be exceptions and very well justified.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: make vhost lockless enqueue configurable
2015-04-29 11:56 ` Thomas Monjalon
@ 2015-04-29 18:07 ` Flavio Leitner
2015-05-21 8:49 ` Xie, Huawei
1 sibling, 0 replies; 5+ messages in thread
From: Flavio Leitner @ 2015-04-29 18:07 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
On Wed, 29 Apr 2015 13:56:58 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> 2015-04-29 13:38 GMT+02:00 Panu Matilainen <pmatilai@redhat.com>:
> > On 04/29/2015 02:29 PM, Huawei Xie wrote:
> >>
> >> vhost enabled vSwitch could have their own thread-safe vring
> >> enqueue policy.
> >> Add the RTE_LIBRTE_VHOST_LOCKLESS_ENQ macro for vhost lockless
> >> enqueue. Turn it off by default.
> >>
> >> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> >> ---
> >> config/common_linuxapp | 1 +
> >> lib/librte_vhost/vhost_rxtx.c | 24 +++++++++++++++++++++++-
> >> 2 files changed, 24 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/config/common_linuxapp b/config/common_linuxapp
> >> index 0078dc9..7f59499 100644
> >> --- a/config/common_linuxapp
> >> +++ b/config/common_linuxapp
> >> @@ -421,6 +421,7 @@ CONFIG_RTE_KNI_VHOST_DEBUG_TX=n
> >> #
> >> CONFIG_RTE_LIBRTE_VHOST=n
> >> CONFIG_RTE_LIBRTE_VHOST_USER=y
> >> +CONFIG_RTE_LIBRTE_VHOST_LOCKLESS_ENQ=n
> >> CONFIG_RTE_LIBRTE_VHOST_DEBUG=n
> [...]
> >
> > These things should be runtime configurable, not build options.
> > Please do not assume everybody builds DPDK separately for each and
> > every application that might ever be.
>
> +1
> Adding new build options must be exceptions and very well justified.
+1, that's specially hard when dpdk is packaged and distributed on
distros like Fedora/RHEL.
fbl
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: make vhost lockless enqueue configurable
2015-04-29 11:56 ` Thomas Monjalon
2015-04-29 18:07 ` Flavio Leitner
@ 2015-05-21 8:49 ` Xie, Huawei
1 sibling, 0 replies; 5+ messages in thread
From: Xie, Huawei @ 2015-05-21 8:49 UTC (permalink / raw)
To: Thomas Monjalon, Panu Matilainen; +Cc: dev
On 4/29/2015 7:57 PM, Thomas Monjalon wrote:
> 2015-04-29 13:38 GMT+02:00 Panu Matilainen <pmatilai@redhat.com>:
>> On 04/29/2015 02:29 PM, Huawei Xie wrote:
>>> vhost enabled vSwitch could have their own thread-safe vring enqueue
>>> policy.
>>> Add the RTE_LIBRTE_VHOST_LOCKLESS_ENQ macro for vhost lockless enqueue.
>>> Turn it off by default.
>>>
>>> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
>>> ---
>>> config/common_linuxapp | 1 +
>>> lib/librte_vhost/vhost_rxtx.c | 24 +++++++++++++++++++++++-
>>> 2 files changed, 24 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/config/common_linuxapp b/config/common_linuxapp
>>> index 0078dc9..7f59499 100644
>>> --- a/config/common_linuxapp
>>> +++ b/config/common_linuxapp
>>> @@ -421,6 +421,7 @@ CONFIG_RTE_KNI_VHOST_DEBUG_TX=n
>>> #
>>> CONFIG_RTE_LIBRTE_VHOST=n
>>> CONFIG_RTE_LIBRTE_VHOST_USER=y
>>> +CONFIG_RTE_LIBRTE_VHOST_LOCKLESS_ENQ=n
>>> CONFIG_RTE_LIBRTE_VHOST_DEBUG=n
> [...]
>> These things should be runtime configurable, not build options. Please do
>> not assume everybody builds DPDK separately for each and every application
>> that might ever be.
> +1
> Adding new build options must be exceptions and very well justified.
Thomas:
Though we could make per queue/device run time configurable, i feel it
might not make sense that per queue/device has different behavior.
To ease the implementation, i would provide an API to configure the
global lock less behavior, and by default, lockess enqueue will be disabled.
Thoughts?
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-05-21 8:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-29 11:29 [dpdk-dev] [PATCH] vhost: make vhost lockless enqueue configurable Huawei Xie
2015-04-29 11:38 ` Panu Matilainen
2015-04-29 11:56 ` Thomas Monjalon
2015-04-29 18:07 ` Flavio Leitner
2015-05-21 8:49 ` Xie, Huawei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).