* [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).