mbox series

[v2,0/4] vhost: add missing barriers, move prefetching

Message ID 20181219082113.24455-1-maxime.coquelin@redhat.com (mailing list archive)
Headers
Series vhost: add missing barriers, move prefetching |

Message

Maxime Coquelin Dec. 19, 2018, 8:21 a.m. UTC 
  This series adds missing read barriers after reading avail index
for split ring and desc flags for packed ring.

Also, it turns out that some descriptors prefetching are either
badly placed, or useless, last part of the series fixes that.

With the series applied, I get between 0 and 4% gain depending
on the benchmark (testpmd txonly/rxonly/io).

Thanks to Jason for reporting the missing read barriers.

Changes since v1:
=================
- Drop volatile removal patch (Ilya)
- Improve commit messages for RMB patches (Ilya)

Maxime Coquelin (4):
  vhost: enforce avail index and desc read ordering
  vhost: enforce desc flags and content read ordering
  vhost: prefetch descriptor after the read barrier
  vhost: remove useless prefetch for packed ring descriptor

 lib/librte_vhost/virtio_net.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

Comments

Michael S. Tsirkin Dec. 19, 2018, 3:50 p.m. UTC | #1 
On Wed, Dec 19, 2018 at 09:21:09AM +0100, Maxime Coquelin wrote:
> This series adds missing read barriers after reading avail index
> for split ring and desc flags for packed ring.
> 
> Also, it turns out that some descriptors prefetching are either
> badly placed, or useless, last part of the series fixes that.
> 
> With the series applied, I get between 0 and 4% gain depending
> on the benchmark (testpmd txonly/rxonly/io).
> 
> Thanks to Jason for reporting the missing read barriers.

Acked-by: Michael S. Tsirkin <mst@redhat.com>

But I wonder what effect this has on ARM where RMB isn't a NOP.
Ilya do you happen to have any data?

> Changes since v1:
> =================
> - Drop volatile removal patch (Ilya)
> - Improve commit messages for RMB patches (Ilya)
> 
> Maxime Coquelin (4):
>   vhost: enforce avail index and desc read ordering
>   vhost: enforce desc flags and content read ordering
>   vhost: prefetch descriptor after the read barrier
>   vhost: remove useless prefetch for packed ring descriptor
> 
>  lib/librte_vhost/virtio_net.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> -- 
> 2.17.2
Ilya Maximets Dec. 19, 2018, 4:28 p.m. UTC | #2 
On 19.12.2018 18:50, Michael S. Tsirkin wrote:
> On Wed, Dec 19, 2018 at 09:21:09AM +0100, Maxime Coquelin wrote:
>> This series adds missing read barriers after reading avail index
>> for split ring and desc flags for packed ring.
>>
>> Also, it turns out that some descriptors prefetching are either
>> badly placed, or useless, last part of the series fixes that.
>>
>> With the series applied, I get between 0 and 4% gain depending
>> on the benchmark (testpmd txonly/rxonly/io).
>>
>> Thanks to Jason for reporting the missing read barriers.
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> But I wonder what effect this has on ARM where RMB isn't a NOP.
> Ilya do you happen to have any data?

Hi.
My rough testing shows no significant performance difference on ARMv8.

> 
>> Changes since v1:
>> =================
>> - Drop volatile removal patch (Ilya)
>> - Improve commit messages for RMB patches (Ilya)
>>
>> Maxime Coquelin (4):
>>   vhost: enforce avail index and desc read ordering
>>   vhost: enforce desc flags and content read ordering
>>   vhost: prefetch descriptor after the read barrier
>>   vhost: remove useless prefetch for packed ring descriptor
>>
>>  lib/librte_vhost/virtio_net.c | 27 ++++++++++++++++++++++-----
>>  1 file changed, 22 insertions(+), 5 deletions(-)
>>
>> -- 
>> 2.17.2
> 
>
Tiwei Bie Dec. 20, 2018, 4:39 a.m. UTC | #3 
On Wed, Dec 19, 2018 at 09:21:09AM +0100, Maxime Coquelin wrote:
> This series adds missing read barriers after reading avail index
> for split ring and desc flags for packed ring.
> 
> Also, it turns out that some descriptors prefetching are either
> badly placed, or useless, last part of the series fixes that.
> 
> With the series applied, I get between 0 and 4% gain depending
> on the benchmark (testpmd txonly/rxonly/io).
> 
> Thanks to Jason for reporting the missing read barriers.
> 
> Changes since v1:
> =================
> - Drop volatile removal patch (Ilya)
> - Improve commit messages for RMB patches (Ilya)
> 
> Maxime Coquelin (4):
>   vhost: enforce avail index and desc read ordering
>   vhost: enforce desc flags and content read ordering
>   vhost: prefetch descriptor after the read barrier
>   vhost: remove useless prefetch for packed ring descriptor
> 
>  lib/librte_vhost/virtio_net.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)

Acked-by: Tiwei Bie <tiwei.bie@intel.com>
Maxime Coquelin Dec. 20, 2018, 6:19 p.m. UTC | #4 
On 12/19/18 9:21 AM, Maxime Coquelin wrote:
> This series adds missing read barriers after reading avail index
> for split ring and desc flags for packed ring.
> 
> Also, it turns out that some descriptors prefetching are either
> badly placed, or useless, last part of the series fixes that.
> 
> With the series applied, I get between 0 and 4% gain depending
> on the benchmark (testpmd txonly/rxonly/io).
> 
> Thanks to Jason for reporting the missing read barriers.
> 
> Changes since v1:
> =================
> - Drop volatile removal patch (Ilya)
> - Improve commit messages for RMB patches (Ilya)
> 
> Maxime Coquelin (4):
>    vhost: enforce avail index and desc read ordering
>    vhost: enforce desc flags and content read ordering
>    vhost: prefetch descriptor after the read barrier
>    vhost: remove useless prefetch for packed ring descriptor
> 
>   lib/librte_vhost/virtio_net.c | 27 ++++++++++++++++++++++-----
>   1 file changed, 22 insertions(+), 5 deletions(-)
> 

Applied to dpdk-next-virtio.

Maxime