diff mbox series

[v1,1/2] net/virtio: restrict pointer aliasing for NEON vpmd

Message ID 20200611033248.39049-2-joyce.kong@arm.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers show
Series virtio: restrict pointer aliasing for loops vectorization | expand

Checks

Context Check Description
ci/iol-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

Joyce Kong June 11, 2020, 3:32 a.m. UTC
Restrict pointer aliasing to allow the compiler to vectorize loops
more aggressively.

With this patch, a 9.6% improvement is observed in throughput for
the virtio-net PVP case, and a 2.4% perf improvement in throughput
for the virtio-user PVP case. All performance data are measured
under the 0.001% acceptable packet loss with 2 cores on the vhost
side.

Signed-off-by: Joyce Kong <joyce.kong@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
 drivers/net/virtio/virtio_rxtx_simple_neon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Maxime Coquelin June 23, 2020, 8:47 a.m. UTC | #1
On 6/11/20 5:32 AM, Joyce Kong wrote:
> Restrict pointer aliasing to allow the compiler to vectorize loops
> more aggressively.
> 
> With this patch, a 9.6% improvement is observed in throughput for
> the virtio-net PVP case, and a 2.4% perf improvement in throughput
> for the virtio-user PVP case. All performance data are measured
> under the 0.001% acceptable packet loss with 2 cores on the vhost
> side.
> 
> Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
>  drivers/net/virtio/virtio_rxtx_simple_neon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Very nice, we should consider doing the same on other platforms.

> diff --git a/drivers/net/virtio/virtio_rxtx_simple_neon.c b/drivers/net/virtio/virtio_rxtx_simple_neon.c
> index 363e2b330..c08dd51fb 100644
> --- a/drivers/net/virtio/virtio_rxtx_simple_neon.c
> +++ b/drivers/net/virtio/virtio_rxtx_simple_neon.c
> @@ -36,8 +36,8 @@
>   * - nb_pkts < RTE_VIRTIO_DESC_PER_LOOP, just return no packet
>   */
>  uint16_t
> -virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
> -	uint16_t nb_pkts)
> +virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf
> +		**__restrict rx_pkts, uint16_t nb_pkts)

Is __restrict supported by all the compilers?
Wouldn't it be better to introduce a wrapper?

>  {
>  	struct virtnet_rx *rxvq = rx_queue;
>  	struct virtqueue *vq = rxvq->vq;
>
Phil Yang June 23, 2020, 9:05 a.m. UTC | #2
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, June 23, 2020 4:48 PM
> To: Joyce Kong <Joyce.Kong@arm.com>; jerinj@marvell.com;
> zhihong.wang@intel.com; xiaolong.ye@intel.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v1 1/2] net/virtio: restrict pointer aliasing for NEON
> vpmd
>
>
>
> On 6/11/20 5:32 AM, Joyce Kong wrote:
> > Restrict pointer aliasing to allow the compiler to vectorize loops
> > more aggressively.
> >
> > With this patch, a 9.6% improvement is observed in throughput for
> > the virtio-net PVP case, and a 2.4% perf improvement in throughput
> > for the virtio-user PVP case. All performance data are measured
> > under the 0.001% acceptable packet loss with 2 cores on the vhost
> > side.
> >
> > Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> > Reviewed-by: Phil Yang <phil.yang@arm.com>
> > ---
> >  drivers/net/virtio/virtio_rxtx_simple_neon.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
>
> Very nice, we should consider doing the same on other platforms.
>
> > diff --git a/drivers/net/virtio/virtio_rxtx_simple_neon.c
> b/drivers/net/virtio/virtio_rxtx_simple_neon.c
> > index 363e2b330..c08dd51fb 100644
> > --- a/drivers/net/virtio/virtio_rxtx_simple_neon.c
> > +++ b/drivers/net/virtio/virtio_rxtx_simple_neon.c
> > @@ -36,8 +36,8 @@
> >   * - nb_pkts < RTE_VIRTIO_DESC_PER_LOOP, just return no packet
> >   */
> >  uint16_t
> > -virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
> > -uint16_t nb_pkts)
> > +virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf
> > +**__restrict rx_pkts, uint16_t nb_pkts)
>
> Is __restrict supported by all the compilers?
> Wouldn't it be better to introduce a wrapper?

+1 for this.
In my understanding, the __restrict keyword is recognized in C at all language levels.
However, the restrict keyword is recognized in C under compilation with c99.
DPDK uses the restrict qualifier a lot, which might have some issues with some old compilers.
So the wrapper will be useful.

Thanks,
Phil

>
> >  {
> >  struct virtnet_rx *rxvq = rx_queue;
> >  struct virtqueue *vq = rxvq->vq;
> >

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Joyce Kong June 24, 2020, 2:58 a.m. UTC | #3
> -----Original Message-----
> From: Phil Yang <Phil.Yang@arm.com>
> Sent: Tuesday, June 23, 2020 5:06 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Joyce Kong
> <Joyce.Kong@arm.com>; jerinj@marvell.com; zhihong.wang@intel.com;
> xiaolong.ye@intel.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v1 1/2] net/virtio: restrict pointer aliasing for NEON
> vpmd
>
> > -----Original Message-----
> > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Sent: Tuesday, June 23, 2020 4:48 PM
> > To: Joyce Kong <Joyce.Kong@arm.com>; jerinj@marvell.com;
> > zhihong.wang@intel.com; xiaolong.ye@intel.com; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> Ruifeng
> > Wang <Ruifeng.Wang@arm.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [PATCH v1 1/2] net/virtio: restrict pointer aliasing for
> > NEON vpmd
> >
> >
> >
> > On 6/11/20 5:32 AM, Joyce Kong wrote:
> > > Restrict pointer aliasing to allow the compiler to vectorize loops
> > > more aggressively.
> > >
> > > With this patch, a 9.6% improvement is observed in throughput for
> > > the virtio-net PVP case, and a 2.4% perf improvement in throughput
> > > for the virtio-user PVP case. All performance data are measured
> > > under the 0.001% acceptable packet loss with 2 cores on the vhost
> > > side.
> > >
> > > Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> > > Reviewed-by: Phil Yang <phil.yang@arm.com>
> > > ---
> > >  drivers/net/virtio/virtio_rxtx_simple_neon.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Very nice, we should consider doing the same on other platforms.
> >
> > > diff --git a/drivers/net/virtio/virtio_rxtx_simple_neon.c
> > b/drivers/net/virtio/virtio_rxtx_simple_neon.c
> > > index 363e2b330..c08dd51fb 100644
> > > --- a/drivers/net/virtio/virtio_rxtx_simple_neon.c
> > > +++ b/drivers/net/virtio/virtio_rxtx_simple_neon.c
> > > @@ -36,8 +36,8 @@
> > >   * - nb_pkts < RTE_VIRTIO_DESC_PER_LOOP, just return no packet
> > >   */
> > >  uint16_t
> > > -virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
> > > -uint16_t nb_pkts)
> > > +virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **__restrict
> > > +rx_pkts, uint16_t nb_pkts)
> >
> > Is __restrict supported by all the compilers?
> > Wouldn't it be better to introduce a wrapper?
>
> +1 for this.
> In my understanding, the __restrict keyword is recognized in C at all language
> levels.
> However, the restrict keyword is recognized in C under compilation with c99.
> DPDK uses the restrict qualifier a lot, which might have some issues with
> some old compilers.
> So the wrapper will be useful.
>
> Thanks,
> Phil
>

Thanks for the suggestion, I shall introduce a wrapper to support all the compilers
in next version.

> >
> > >  {
> > >  struct virtnet_rx *rxvq = rx_queue;  struct virtqueue *vq =
> > > rxvq->vq;
> > >
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Stephen Hemminger June 24, 2020, 4:16 a.m. UTC | #4
On Wed, 24 Jun 2020 02:58:28 +0000
Joyce Kong <Joyce.Kong@arm.com> wrote:

> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Please fix your email system.
Technically, it is not legal to post to a public mailing list with
such a boilerplate footer.
diff mbox series

Patch

diff --git a/drivers/net/virtio/virtio_rxtx_simple_neon.c b/drivers/net/virtio/virtio_rxtx_simple_neon.c
index 363e2b330..c08dd51fb 100644
--- a/drivers/net/virtio/virtio_rxtx_simple_neon.c
+++ b/drivers/net/virtio/virtio_rxtx_simple_neon.c
@@ -36,8 +36,8 @@ 
  * - nb_pkts < RTE_VIRTIO_DESC_PER_LOOP, just return no packet
  */
 uint16_t
-virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
-	uint16_t nb_pkts)
+virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf
+		**__restrict rx_pkts, uint16_t nb_pkts)
 {
 	struct virtnet_rx *rxvq = rx_queue;
 	struct virtqueue *vq = rxvq->vq;