* [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on x86
@ 2020-11-11 15:40 Marvin Liu
2020-11-11 15:45 ` David Marchand
2020-11-12 8:48 ` Maxime Coquelin
0 siblings, 2 replies; 9+ messages in thread
From: Marvin Liu @ 2020-11-11 15:40 UTC (permalink / raw)
To: maxime.coquelin, chenbo.xia; +Cc: dev, Marvin Liu
Data prefetch instruction can preload data into cpu’s hierarchical
cache before data access. Virtio datapath utilized this feature for
data access acceleration. As config RTE_PMD_PACKET_PREFETCH was
discarded, now packet data prefetch is enabled based on architecture.
Signed-off-by: Marvin Liu <yong.liu@intel.com>
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 42c4c9882..0196290a5 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -106,7 +106,7 @@ virtqueue_store_flags_packed(struct vring_packed_desc *dp,
dp->flags = flags;
}
}
-#ifdef RTE_PMD_PACKET_PREFETCH
+#if defined(RTE_ARCH_X86)
#define rte_packet_prefetch(p) rte_prefetch1(p)
#else
#define rte_packet_prefetch(p) do {} while(0)
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on x86
2020-11-11 15:40 [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on x86 Marvin Liu
@ 2020-11-11 15:45 ` David Marchand
2020-11-11 15:53 ` Bruce Richardson
2020-11-12 8:48 ` Maxime Coquelin
1 sibling, 1 reply; 9+ messages in thread
From: David Marchand @ 2020-11-11 15:45 UTC (permalink / raw)
To: David Christensen, Jerin Jacob Kollanukkaran,
Ruifeng Wang (Arm Technology China)
Cc: Maxime Coquelin, Xia, Chenbo, dev, Marvin Liu, Thomas Monjalon
On Wed, Nov 11, 2020 at 4:40 PM Marvin Liu <yong.liu@intel.com> wrote:
>
> Data prefetch instruction can preload data into cpu’s hierarchical
> cache before data access. Virtio datapath utilized this feature for
> data access acceleration. As config RTE_PMD_PACKET_PREFETCH was
> discarded, now packet data prefetch is enabled based on architecture.
IIUC, this config item was set for all architectures under make compilation.
$ git grep RTE_PMD_PACKET_PREFETCH v20.08 config/
v20.08:config/common_base:CONFIG_RTE_PMD_PACKET_PREFETCH=y
Now that we switched to meson, it got lost and this patch only
restores it for x86.
Can other architectures check this?
Thanks.
--
David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on x86
2020-11-11 15:45 ` David Marchand
@ 2020-11-11 15:53 ` Bruce Richardson
2020-11-11 15:56 ` David Marchand
0 siblings, 1 reply; 9+ messages in thread
From: Bruce Richardson @ 2020-11-11 15:53 UTC (permalink / raw)
To: David Marchand
Cc: David Christensen, Jerin Jacob Kollanukkaran,
Ruifeng Wang (Arm Technology China),
Maxime Coquelin, Xia, Chenbo, dev, Marvin Liu, Thomas Monjalon
On Wed, Nov 11, 2020 at 04:45:25PM +0100, David Marchand wrote:
> On Wed, Nov 11, 2020 at 4:40 PM Marvin Liu <yong.liu@intel.com> wrote:
> >
> > Data prefetch instruction can preload data into cpu’s hierarchical
> > cache before data access. Virtio datapath utilized this feature for
> > data access acceleration. As config RTE_PMD_PACKET_PREFETCH was
> > discarded, now packet data prefetch is enabled based on architecture.
>
> IIUC, this config item was set for all architectures under make compilation.
>
> $ git grep RTE_PMD_PACKET_PREFETCH v20.08 config/
> v20.08:config/common_base:CONFIG_RTE_PMD_PACKET_PREFETCH=y
>
> Now that we switched to meson, it got lost and this patch only
> restores it for x86.
> Can other architectures check this?
>
If it was globally enabled before, it probably should just be added to
config/rte_config.h file.
/Bruce
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on x86
2020-11-11 15:53 ` Bruce Richardson
@ 2020-11-11 15:56 ` David Marchand
0 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2020-11-11 15:56 UTC (permalink / raw)
To: Bruce Richardson
Cc: David Christensen, Jerin Jacob Kollanukkaran,
Ruifeng Wang (Arm Technology China),
Maxime Coquelin, Xia, Chenbo, dev, Marvin Liu, Thomas Monjalon
On Wed, Nov 11, 2020 at 4:54 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, Nov 11, 2020 at 04:45:25PM +0100, David Marchand wrote:
> > On Wed, Nov 11, 2020 at 4:40 PM Marvin Liu <yong.liu@intel.com> wrote:
> > >
> > > Data prefetch instruction can preload data into cpu’s hierarchical
> > > cache before data access. Virtio datapath utilized this feature for
> > > data access acceleration. As config RTE_PMD_PACKET_PREFETCH was
> > > discarded, now packet data prefetch is enabled based on architecture.
> >
> > IIUC, this config item was set for all architectures under make compilation.
> >
> > $ git grep RTE_PMD_PACKET_PREFETCH v20.08 config/
> > v20.08:config/common_base:CONFIG_RTE_PMD_PACKET_PREFETCH=y
> >
> > Now that we switched to meson, it got lost and this patch only
> > restores it for x86.
> > Can other architectures check this?
> >
> If it was globally enabled before, it probably should just be added to
> config/rte_config.h file.
>
Restoring globally is the probably the best fix, yes.
I am still surprised nobody but x86 testers caught a perf regression.
--
David marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on x86
2020-11-11 15:40 [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on x86 Marvin Liu
2020-11-11 15:45 ` David Marchand
@ 2020-11-12 8:48 ` Maxime Coquelin
2020-11-12 8:57 ` David Marchand
1 sibling, 1 reply; 9+ messages in thread
From: Maxime Coquelin @ 2020-11-12 8:48 UTC (permalink / raw)
To: Marvin Liu, chenbo.xia; +Cc: dev, Richardson, Bruce
Hi Marvin,
On 11/11/20 4:40 PM, Marvin Liu wrote:
> Data prefetch instruction can preload data into cpu’s hierarchical
> cache before data access. Virtio datapath utilized this feature for
> data access acceleration. As config RTE_PMD_PACKET_PREFETCH was
> discarded, now packet data prefetch is enabled based on architecture.
>
> Signed-off-by: Marvin Liu <yong.liu@intel.com>
>
> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
> index 42c4c9882..0196290a5 100644
> --- a/drivers/net/virtio/virtqueue.h
> +++ b/drivers/net/virtio/virtqueue.h
> @@ -106,7 +106,7 @@ virtqueue_store_flags_packed(struct vring_packed_desc *dp,
> dp->flags = flags;
> }
> }
> -#ifdef RTE_PMD_PACKET_PREFETCH
> +#if defined(RTE_ARCH_X86)
> #define rte_packet_prefetch(p) rte_prefetch1(p)
> #else
> #define rte_packet_prefetch(p) do {} while(0)
>
Thanks for catching this issue.
I agree it should be re-enabled by default, and not only on X86, not
only on Virtio PMD.
AFAICS, prefetch was enabled for all platforms before the switch to
Meson, so I see it as an involuntary change that needs to be reverted.
Then, I think having a possibility to disable it would be nice, so maybe
we should add an option in our build system to do that.
Thanks,
Maxime
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on x86
2020-11-12 8:48 ` Maxime Coquelin
@ 2020-11-12 8:57 ` David Marchand
2020-11-13 1:20 ` Liu, Yong
0 siblings, 1 reply; 9+ messages in thread
From: David Marchand @ 2020-11-12 8:57 UTC (permalink / raw)
To: Marvin Liu, Maxime Coquelin; +Cc: Xia, Chenbo, dev, Richardson, Bruce
On Thu, Nov 12, 2020 at 9:48 AM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
> On 11/11/20 4:40 PM, Marvin Liu wrote:
> > Data prefetch instruction can preload data into cpu’s hierarchical
> > cache before data access. Virtio datapath utilized this feature for
> > data access acceleration. As config RTE_PMD_PACKET_PREFETCH was
> > discarded, now packet data prefetch is enabled based on architecture.
> >
> > Signed-off-by: Marvin Liu <yong.liu@intel.com>
> >
> > diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
> > index 42c4c9882..0196290a5 100644
> > --- a/drivers/net/virtio/virtqueue.h
> > +++ b/drivers/net/virtio/virtqueue.h
> > @@ -106,7 +106,7 @@ virtqueue_store_flags_packed(struct vring_packed_desc *dp,
> > dp->flags = flags;
> > }
> > }
> > -#ifdef RTE_PMD_PACKET_PREFETCH
> > +#if defined(RTE_ARCH_X86)
> > #define rte_packet_prefetch(p) rte_prefetch1(p)
> > #else
> > #define rte_packet_prefetch(p) do {} while(0)
> >
>
> Thanks for catching this issue.
> I agree it should be re-enabled by default, and not only on X86, not
> only on Virtio PMD.
>
> AFAICS, prefetch was enabled for all platforms before the switch to
> Meson, so I see it as an involuntary change that needs to be reverted.
Yes and this will also solve https://patchwork.dpdk.org/patch/83468/.
Thanks.
--
David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on x86
2020-11-12 8:57 ` David Marchand
@ 2020-11-13 1:20 ` Liu, Yong
2020-11-13 7:27 ` David Marchand
0 siblings, 1 reply; 9+ messages in thread
From: Liu, Yong @ 2020-11-13 1:20 UTC (permalink / raw)
To: David Marchand, Maxime Coquelin; +Cc: Xia, Chenbo, dev, Richardson, Bruce
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, November 12, 2020 4:58 PM
> To: Liu, Yong <yong.liu@intel.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; dev <dev@dpdk.org>; Richardson,
> Bruce <bruce.richardson@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on
> x86
>
> On Thu, Nov 12, 2020 at 9:48 AM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
> > On 11/11/20 4:40 PM, Marvin Liu wrote:
> > > Data prefetch instruction can preload data into cpu’s hierarchical
> > > cache before data access. Virtio datapath utilized this feature for
> > > data access acceleration. As config RTE_PMD_PACKET_PREFETCH was
> > > discarded, now packet data prefetch is enabled based on architecture.
> > >
> > > Signed-off-by: Marvin Liu <yong.liu@intel.com>
> > >
> > > diff --git a/drivers/net/virtio/virtqueue.h
> b/drivers/net/virtio/virtqueue.h
> > > index 42c4c9882..0196290a5 100644
> > > --- a/drivers/net/virtio/virtqueue.h
> > > +++ b/drivers/net/virtio/virtqueue.h
> > > @@ -106,7 +106,7 @@ virtqueue_store_flags_packed(struct
> vring_packed_desc *dp,
> > > dp->flags = flags;
> > > }
> > > }
> > > -#ifdef RTE_PMD_PACKET_PREFETCH
> > > +#if defined(RTE_ARCH_X86)
> > > #define rte_packet_prefetch(p) rte_prefetch1(p)
> > > #else
> > > #define rte_packet_prefetch(p) do {} while(0)
> > >
> >
> > Thanks for catching this issue.
> > I agree it should be re-enabled by default, and not only on X86, not
> > only on Virtio PMD.
> >
> > AFAICS, prefetch was enabled for all platforms before the switch to
> > Meson, so I see it as an involuntary change that needs to be reverted.
>
> Yes and this will also solve https://patchwork.dpdk.org/patch/83468/.
> Thanks.
>
Agreed, original patch was intended to recover prefetch configuration in meson build.
Please check http://patchwork.dpdk.org/patch/78451/.
And it leaded a discussion about how to utilize prefetch function optimally.
Due to no conclusion for current position is best for other platforms except x86, now only enable prefetch in virtio + x86.
> --
> David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on x86
2020-11-13 1:20 ` Liu, Yong
@ 2020-11-13 7:27 ` David Marchand
2020-11-13 12:21 ` Liu, Yong
0 siblings, 1 reply; 9+ messages in thread
From: David Marchand @ 2020-11-13 7:27 UTC (permalink / raw)
To: Liu, Yong; +Cc: Maxime Coquelin, Xia, Chenbo, dev, Richardson, Bruce
On Fri, Nov 13, 2020 at 2:20 AM Liu, Yong <yong.liu@intel.com> wrote:
> > Yes and this will also solve https://patchwork.dpdk.org/patch/83468/.
> > Thanks.
> >
>
> Agreed, original patch was intended to recover prefetch configuration in meson build.
> Please check http://patchwork.dpdk.org/patch/78451/.
> And it leaded a discussion about how to utilize prefetch function optimally.
> Due to no conclusion for current position is best for other platforms except x86, now only enable prefetch in virtio + x86.
I disagree.
No conclusion means the best is to restore the previous state, i.e.
enable this option for all platforms.
If later other architectures want to change this, this can revisit.
--
David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on x86
2020-11-13 7:27 ` David Marchand
@ 2020-11-13 12:21 ` Liu, Yong
0 siblings, 0 replies; 9+ messages in thread
From: Liu, Yong @ 2020-11-13 12:21 UTC (permalink / raw)
To: David Marchand, Thomas Monjalon, Honnappa Nagarahalli, stephen
Cc: Maxime Coquelin, Xia, Chenbo, dev, Richardson, Bruce
+ more people into this conversation. IMHO, restore to previous state is the best choice by now.
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, November 13, 2020 3:27 PM
> To: Liu, Yong <yong.liu@intel.com>
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo
> <chenbo.xia@intel.com>; dev <dev@dpdk.org>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on
> x86
>
> On Fri, Nov 13, 2020 at 2:20 AM Liu, Yong <yong.liu@intel.com> wrote:
> > > Yes and this will also solve https://patchwork.dpdk.org/patch/83468/.
> > > Thanks.
> > >
> >
> > Agreed, original patch was intended to recover prefetch configuration in
> meson build.
> > Please check http://patchwork.dpdk.org/patch/78451/.
> > And it leaded a discussion about how to utilize prefetch function
> optimally.
> > Due to no conclusion for current position is best for other platforms
> except x86, now only enable prefetch in virtio + x86.
>
> I disagree.
> No conclusion means the best is to restore the previous state, i.e.
> enable this option for all platforms.
>
> If later other architectures want to change this, this can revisit.
>
>
> --
> David Marchand
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-11-13 12:22 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 15:40 [dpdk-dev] [PATCH] net/virtio: enable packet data prefetch on x86 Marvin Liu
2020-11-11 15:45 ` David Marchand
2020-11-11 15:53 ` Bruce Richardson
2020-11-11 15:56 ` David Marchand
2020-11-12 8:48 ` Maxime Coquelin
2020-11-12 8:57 ` David Marchand
2020-11-13 1:20 ` Liu, Yong
2020-11-13 7:27 ` David Marchand
2020-11-13 12:21 ` Liu, Yong
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).