* [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option @ 2014-11-07 11:00 Marc Sune 2015-02-10 11:59 ` Marc Sune 0 siblings, 1 reply; 11+ messages in thread From: Marc Sune @ 2014-11-07 11:00 UTC (permalink / raw) To: dev This patch introduces CONFIG_RTE_KNI_PREEMPT flag. When set to 'no', KNI kernel thread(s) do not call schedule_timeout_interruptible(), which improves overall KNI performance at the expense of CPU cycles (polling). Default values is 'yes', maintaining the same behaviour as of now. Signed-off-by: Marc Sune <marc.sune@bisdn.de> --- config/common_linuxapp | 1 + lib/librte_eal/linuxapp/kni/kni_misc.c | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/config/common_linuxapp b/config/common_linuxapp index 57b61c9..24b529d 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -380,6 +380,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y # Compile librte_kni # CONFIG_RTE_LIBRTE_KNI=y +CONFIG_RTE_KNI_PREEMPT=y CONFIG_RTE_KNI_KO_DEBUG=n CONFIG_RTE_KNI_VHOST=n CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c index ba77776..e7e6c27 100644 --- a/lib/librte_eal/linuxapp/kni/kni_misc.c +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c @@ -229,9 +229,11 @@ kni_thread_single(void *unused) } } up_read(&kni_list_lock); +#ifdef RTE_KNI_PREEMPT /* reschedule out for a while */ schedule_timeout_interruptible(usecs_to_jiffies( \ KNI_KTHREAD_RESCHEDULE_INTERVAL)); +#endif } return 0; @@ -252,8 +254,10 @@ kni_thread_multiple(void *param) #endif kni_net_poll_resp(dev); } +#ifdef RTE_KNI_PREEMPT schedule_timeout_interruptible(usecs_to_jiffies( \ KNI_KTHREAD_RESCHEDULE_INTERVAL)); +#endif } return 0; -- 1.7.10.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option 2014-11-07 11:00 [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option Marc Sune @ 2015-02-10 11:59 ` Marc Sune 2015-02-10 12:02 ` Bruce Richardson 2015-02-10 13:24 ` Bruce Richardson 0 siblings, 2 replies; 11+ messages in thread From: Marc Sune @ 2015-02-10 11:59 UTC (permalink / raw) To: dev, Zhang, Helin This patch of Nov 2014 hasn't been yet ACKed/NACKed. Could someone please give some quick feedback? Thanks marc On 07/11/14 12:00, Marc Sune wrote: > This patch introduces CONFIG_RTE_KNI_PREEMPT flag. When set to 'no', KNI > kernel thread(s) do not call schedule_timeout_interruptible(), which improves > overall KNI performance at the expense of CPU cycles (polling). > > Default values is 'yes', maintaining the same behaviour as of now. > > Signed-off-by: Marc Sune <marc.sune@bisdn.de> > --- > config/common_linuxapp | 1 + > lib/librte_eal/linuxapp/kni/kni_misc.c | 4 ++++ > 2 files changed, 5 insertions(+) > > diff --git a/config/common_linuxapp b/config/common_linuxapp > index 57b61c9..24b529d 100644 > --- a/config/common_linuxapp > +++ b/config/common_linuxapp > @@ -380,6 +380,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y > # Compile librte_kni > # > CONFIG_RTE_LIBRTE_KNI=y > +CONFIG_RTE_KNI_PREEMPT=y > CONFIG_RTE_KNI_KO_DEBUG=n > CONFIG_RTE_KNI_VHOST=n > CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 > diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c > index ba77776..e7e6c27 100644 > --- a/lib/librte_eal/linuxapp/kni/kni_misc.c > +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c > @@ -229,9 +229,11 @@ kni_thread_single(void *unused) > } > } > up_read(&kni_list_lock); > +#ifdef RTE_KNI_PREEMPT > /* reschedule out for a while */ > schedule_timeout_interruptible(usecs_to_jiffies( \ > KNI_KTHREAD_RESCHEDULE_INTERVAL)); > +#endif > } > > return 0; > @@ -252,8 +254,10 @@ kni_thread_multiple(void *param) > #endif > kni_net_poll_resp(dev); > } > +#ifdef RTE_KNI_PREEMPT > schedule_timeout_interruptible(usecs_to_jiffies( \ > KNI_KTHREAD_RESCHEDULE_INTERVAL)); > +#endif > } > > return 0; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option 2015-02-10 11:59 ` Marc Sune @ 2015-02-10 12:02 ` Bruce Richardson 2015-02-10 12:21 ` Marc Sune 2015-02-10 13:24 ` Bruce Richardson 1 sibling, 1 reply; 11+ messages in thread From: Bruce Richardson @ 2015-02-10 12:02 UTC (permalink / raw) To: Marc Sune; +Cc: dev On Tue, Feb 10, 2015 at 12:59:29PM +0100, Marc Sune wrote: > This patch of Nov 2014 hasn't been yet ACKed/NACKed. Could someone please > give some quick feedback? > > Thanks > marc > Idea is good, any chance it could be added as a run-time rather than compile-time option? /Bruce > On 07/11/14 12:00, Marc Sune wrote: > >This patch introduces CONFIG_RTE_KNI_PREEMPT flag. When set to 'no', KNI > >kernel thread(s) do not call schedule_timeout_interruptible(), which improves > >overall KNI performance at the expense of CPU cycles (polling). > > > >Default values is 'yes', maintaining the same behaviour as of now. > > > >Signed-off-by: Marc Sune <marc.sune@bisdn.de> > >--- > > config/common_linuxapp | 1 + > > lib/librte_eal/linuxapp/kni/kni_misc.c | 4 ++++ > > 2 files changed, 5 insertions(+) > > > >diff --git a/config/common_linuxapp b/config/common_linuxapp > >index 57b61c9..24b529d 100644 > >--- a/config/common_linuxapp > >+++ b/config/common_linuxapp > >@@ -380,6 +380,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y > > # Compile librte_kni > > # > > CONFIG_RTE_LIBRTE_KNI=y > >+CONFIG_RTE_KNI_PREEMPT=y > > CONFIG_RTE_KNI_KO_DEBUG=n > > CONFIG_RTE_KNI_VHOST=n > > CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 > >diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c > >index ba77776..e7e6c27 100644 > >--- a/lib/librte_eal/linuxapp/kni/kni_misc.c > >+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c > >@@ -229,9 +229,11 @@ kni_thread_single(void *unused) > > } > > } > > up_read(&kni_list_lock); > >+#ifdef RTE_KNI_PREEMPT > > /* reschedule out for a while */ > > schedule_timeout_interruptible(usecs_to_jiffies( \ > > KNI_KTHREAD_RESCHEDULE_INTERVAL)); > >+#endif > > } > > return 0; > >@@ -252,8 +254,10 @@ kni_thread_multiple(void *param) > > #endif > > kni_net_poll_resp(dev); > > } > >+#ifdef RTE_KNI_PREEMPT > > schedule_timeout_interruptible(usecs_to_jiffies( \ > > KNI_KTHREAD_RESCHEDULE_INTERVAL)); > >+#endif > > } > > return 0; > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option 2015-02-10 12:02 ` Bruce Richardson @ 2015-02-10 12:21 ` Marc Sune 2015-02-10 13:22 ` Bruce Richardson 0 siblings, 1 reply; 11+ messages in thread From: Marc Sune @ 2015-02-10 12:21 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev On 10/02/15 13:02, Bruce Richardson wrote: > On Tue, Feb 10, 2015 at 12:59:29PM +0100, Marc Sune wrote: >> This patch of Nov 2014 hasn't been yet ACKed/NACKed. Could someone please >> give some quick feedback? >> >> Thanks >> marc >> > Idea is good, any chance it could be added as a run-time rather than > compile-time option? It is also an option. I wasn't really thinking someone would want to change this behaviour at runtime. If we think it is worth, I can have a closer look on it. Any other opinions on this? If we would go for a runtime flag, we would either have to add a config parameter to rte_kni_init() or add a specific call to turn on/off this knob, depending on whether it is sufficient to change this behaviour at bootstrapping time, or we want to also change it during 'operation'. In either case we would require some communication, probably via ioctl(), from user-space to kernel space. Thanks Marc > > /Bruce > >> On 07/11/14 12:00, Marc Sune wrote: >>> This patch introduces CONFIG_RTE_KNI_PREEMPT flag. When set to 'no', KNI >>> kernel thread(s) do not call schedule_timeout_interruptible(), which improves >>> overall KNI performance at the expense of CPU cycles (polling). >>> >>> Default values is 'yes', maintaining the same behaviour as of now. >>> >>> Signed-off-by: Marc Sune <marc.sune@bisdn.de> >>> --- >>> config/common_linuxapp | 1 + >>> lib/librte_eal/linuxapp/kni/kni_misc.c | 4 ++++ >>> 2 files changed, 5 insertions(+) >>> >>> diff --git a/config/common_linuxapp b/config/common_linuxapp >>> index 57b61c9..24b529d 100644 >>> --- a/config/common_linuxapp >>> +++ b/config/common_linuxapp >>> @@ -380,6 +380,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y >>> # Compile librte_kni >>> # >>> CONFIG_RTE_LIBRTE_KNI=y >>> +CONFIG_RTE_KNI_PREEMPT=y >>> CONFIG_RTE_KNI_KO_DEBUG=n >>> CONFIG_RTE_KNI_VHOST=n >>> CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 >>> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c >>> index ba77776..e7e6c27 100644 >>> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c >>> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c >>> @@ -229,9 +229,11 @@ kni_thread_single(void *unused) >>> } >>> } >>> up_read(&kni_list_lock); >>> +#ifdef RTE_KNI_PREEMPT >>> /* reschedule out for a while */ >>> schedule_timeout_interruptible(usecs_to_jiffies( \ >>> KNI_KTHREAD_RESCHEDULE_INTERVAL)); >>> +#endif >>> } >>> return 0; >>> @@ -252,8 +254,10 @@ kni_thread_multiple(void *param) >>> #endif >>> kni_net_poll_resp(dev); >>> } >>> +#ifdef RTE_KNI_PREEMPT >>> schedule_timeout_interruptible(usecs_to_jiffies( \ >>> KNI_KTHREAD_RESCHEDULE_INTERVAL)); >>> +#endif >>> } >>> return 0; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option 2015-02-10 12:21 ` Marc Sune @ 2015-02-10 13:22 ` Bruce Richardson 2015-02-10 18:06 ` Marc Sune 0 siblings, 1 reply; 11+ messages in thread From: Bruce Richardson @ 2015-02-10 13:22 UTC (permalink / raw) To: Marc Sune; +Cc: dev On Tue, Feb 10, 2015 at 01:21:55PM +0100, Marc Sune wrote: > > On 10/02/15 13:02, Bruce Richardson wrote: > >On Tue, Feb 10, 2015 at 12:59:29PM +0100, Marc Sune wrote: > >>This patch of Nov 2014 hasn't been yet ACKed/NACKed. Could someone please > >>give some quick feedback? > >> > >>Thanks > >>marc > >> > >Idea is good, any chance it could be added as a run-time rather than > >compile-time option? > > It is also an option. I wasn't really thinking someone would want to change > this behaviour at runtime. If we think it is worth, I can have a closer look > on it. Any other opinions on this? > > If we would go for a runtime flag, we would either have to add a config > parameter to rte_kni_init() or add a specific call to turn on/off this knob, > depending on whether it is sufficient to change this behaviour at > bootstrapping time, or we want to also change it during 'operation'. In > either case we would require some communication, probably via ioctl(), from > user-space to kernel space. > > Thanks > Marc > Yes, I can't see it needing to be changed much at runtime, however, we may need to take account of those who are using precompiled DPDK libs. For those using prebuilt DPDK libs, they won't have any ability to modify compile-time values. However, that being said, I have no particular objection to taking this change in as-is for now. It only adds something, rather than taking it away. Regards, /Bruce ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option 2015-02-10 13:22 ` Bruce Richardson @ 2015-02-10 18:06 ` Marc Sune 0 siblings, 0 replies; 11+ messages in thread From: Marc Sune @ 2015-02-10 18:06 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev On 10/02/15 14:22, Bruce Richardson wrote: > On Tue, Feb 10, 2015 at 01:21:55PM +0100, Marc Sune wrote: >> On 10/02/15 13:02, Bruce Richardson wrote: >>> On Tue, Feb 10, 2015 at 12:59:29PM +0100, Marc Sune wrote: >>>> This patch of Nov 2014 hasn't been yet ACKed/NACKed. Could someone please >>>> give some quick feedback? >>>> >>>> Thanks >>>> marc >>>> >>> Idea is good, any chance it could be added as a run-time rather than >>> compile-time option? >> It is also an option. I wasn't really thinking someone would want to change >> this behaviour at runtime. If we think it is worth, I can have a closer look >> on it. Any other opinions on this? >> >> If we would go for a runtime flag, we would either have to add a config >> parameter to rte_kni_init() or add a specific call to turn on/off this knob, >> depending on whether it is sufficient to change this behaviour at >> bootstrapping time, or we want to also change it during 'operation'. In >> either case we would require some communication, probably via ioctl(), from >> user-space to kernel space. >> >> Thanks >> Marc >> > Yes, I can't see it needing to be changed much at runtime, however, we may need > to take account of those who are using precompiled DPDK libs. For those using > prebuilt DPDK libs, they won't have any ability to modify compile-time values. I see the point. So it should be enough to improve rte_kni_init() with an extra argument, but this means add some additional ioctl() calls, as far as I see. > > However, that being said, I have no particular objection to taking this change > in as-is for now. It only adds something, rather than taking it away. Yes, we can improve it in the future, I have no time right now. Thanks marc > > Regards, > /Bruce > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option 2015-02-10 11:59 ` Marc Sune 2015-02-10 12:02 ` Bruce Richardson @ 2015-02-10 13:24 ` Bruce Richardson 2015-02-11 1:54 ` Zhang, Helin 1 sibling, 1 reply; 11+ messages in thread From: Bruce Richardson @ 2015-02-10 13:24 UTC (permalink / raw) To: Marc Sune; +Cc: dev On Tue, Feb 10, 2015 at 12:59:29PM +0100, Marc Sune wrote: > This patch of Nov 2014 hasn't been yet ACKed/NACKed. Could someone please > give some quick feedback? > > Thanks > marc > > On 07/11/14 12:00, Marc Sune wrote: > >This patch introduces CONFIG_RTE_KNI_PREEMPT flag. When set to 'no', KNI > >kernel thread(s) do not call schedule_timeout_interruptible(), which improves > >overall KNI performance at the expense of CPU cycles (polling). > > > >Default values is 'yes', maintaining the same behaviour as of now. > > > >Signed-off-by: Marc Sune <marc.sune@bisdn.de> Although a better option would be to have a runtime setting, this is still an improvement over what we have. Acked-by: Bruce Richardson <bruce.richardson@intel.com> > >--- > > config/common_linuxapp | 1 + > > lib/librte_eal/linuxapp/kni/kni_misc.c | 4 ++++ > > 2 files changed, 5 insertions(+) > > > >diff --git a/config/common_linuxapp b/config/common_linuxapp > >index 57b61c9..24b529d 100644 > >--- a/config/common_linuxapp > >+++ b/config/common_linuxapp > >@@ -380,6 +380,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y > > # Compile librte_kni > > # > > CONFIG_RTE_LIBRTE_KNI=y > >+CONFIG_RTE_KNI_PREEMPT=y > > CONFIG_RTE_KNI_KO_DEBUG=n > > CONFIG_RTE_KNI_VHOST=n > > CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 > >diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c > >index ba77776..e7e6c27 100644 > >--- a/lib/librte_eal/linuxapp/kni/kni_misc.c > >+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c > >@@ -229,9 +229,11 @@ kni_thread_single(void *unused) > > } > > } > > up_read(&kni_list_lock); > >+#ifdef RTE_KNI_PREEMPT > > /* reschedule out for a while */ > > schedule_timeout_interruptible(usecs_to_jiffies( \ > > KNI_KTHREAD_RESCHEDULE_INTERVAL)); > >+#endif > > } > > return 0; > >@@ -252,8 +254,10 @@ kni_thread_multiple(void *param) > > #endif > > kni_net_poll_resp(dev); > > } > >+#ifdef RTE_KNI_PREEMPT > > schedule_timeout_interruptible(usecs_to_jiffies( \ > > KNI_KTHREAD_RESCHEDULE_INTERVAL)); > >+#endif > > } > > return 0; > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option 2015-02-10 13:24 ` Bruce Richardson @ 2015-02-11 1:54 ` Zhang, Helin 2015-02-11 12:26 ` Marc Sune 0 siblings, 1 reply; 11+ messages in thread From: Zhang, Helin @ 2015-02-11 1:54 UTC (permalink / raw) To: Richardson, Bruce, Marc Sune; +Cc: dev > -----Original Message----- > From: Richardson, Bruce > Sent: Tuesday, February 10, 2015 9:24 PM > To: Marc Sune > Cc: dev@dpdk.org; Zhang, Helin > Subject: Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration > option > > On Tue, Feb 10, 2015 at 12:59:29PM +0100, Marc Sune wrote: > > This patch of Nov 2014 hasn't been yet ACKed/NACKed. Could someone > > please give some quick feedback? > > > > Thanks > > marc > > > > On 07/11/14 12:00, Marc Sune wrote: > > >This patch introduces CONFIG_RTE_KNI_PREEMPT flag. When set to 'no', > > >KNI kernel thread(s) do not call schedule_timeout_interruptible(), > > >which improves overall KNI performance at the expense of CPU cycles > (polling). > > > > > >Default values is 'yes', maintaining the same behaviour as of now. > > > > > >Signed-off-by: Marc Sune <marc.sune@bisdn.de> > > Although a better option would be to have a runtime setting, this is still an > improvement over what we have. > > Acked-by: Bruce Richardson <bruce.richardson@intel.com> > > > >--- > > > config/common_linuxapp | 1 + > > > lib/librte_eal/linuxapp/kni/kni_misc.c | 4 ++++ > > > 2 files changed, 5 insertions(+) > > > > > >diff --git a/config/common_linuxapp b/config/common_linuxapp index > > >57b61c9..24b529d 100644 > > >--- a/config/common_linuxapp > > >+++ b/config/common_linuxapp > > >@@ -380,6 +380,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y > > > # Compile librte_kni > > > # > > > CONFIG_RTE_LIBRTE_KNI=y > > >+CONFIG_RTE_KNI_PREEMPT=y > > > CONFIG_RTE_KNI_KO_DEBUG=n > > > CONFIG_RTE_KNI_VHOST=n > > > CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 > > >diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c > > >b/lib/librte_eal/linuxapp/kni/kni_misc.c > > >index ba77776..e7e6c27 100644 > > >--- a/lib/librte_eal/linuxapp/kni/kni_misc.c > > >+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c > > >@@ -229,9 +229,11 @@ kni_thread_single(void *unused) > > > } > > > } > > > up_read(&kni_list_lock); > > >+#ifdef RTE_KNI_PREEMPT > > > /* reschedule out for a while */ > > > schedule_timeout_interruptible(usecs_to_jiffies( \ > > > KNI_KTHREAD_RESCHEDULE_INTERVAL)); > > >+#endif > > > } > > > return 0; > > >@@ -252,8 +254,10 @@ kni_thread_multiple(void *param) > > > #endif > > > kni_net_poll_resp(dev); > > > } > > >+#ifdef RTE_KNI_PREEMPT > > > schedule_timeout_interruptible(usecs_to_jiffies( \ > > > KNI_KTHREAD_RESCHEDULE_INTERVAL)); > > >+#endif > > > } > > > return 0; > > As Bruce indicated, it would be better to do that at runtime, we can add a config in struct rte_kni_conf, which will be copied to struct rte_kni_device_info, then kernel space will know the configuration. This way, we can enable/disable PREEMPT during KNI instance allocation time. Anyway, it can be done now or later. Regards, Helin ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option 2015-02-11 1:54 ` Zhang, Helin @ 2015-02-11 12:26 ` Marc Sune 2015-02-11 14:27 ` Bruce Richardson 0 siblings, 1 reply; 11+ messages in thread From: Marc Sune @ 2015-02-11 12:26 UTC (permalink / raw) To: Zhang, Helin, Richardson, Bruce; +Cc: dev On 11/02/15 02:54, Zhang, Helin wrote: > >> -----Original Message----- >> From: Richardson, Bruce >> Sent: Tuesday, February 10, 2015 9:24 PM >> To: Marc Sune >> Cc: dev@dpdk.org; Zhang, Helin >> Subject: Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration >> option >> >> On Tue, Feb 10, 2015 at 12:59:29PM +0100, Marc Sune wrote: >>> This patch of Nov 2014 hasn't been yet ACKed/NACKed. Could someone >>> please give some quick feedback? >>> >>> Thanks >>> marc >>> >>> On 07/11/14 12:00, Marc Sune wrote: >>>> This patch introduces CONFIG_RTE_KNI_PREEMPT flag. When set to 'no', >>>> KNI kernel thread(s) do not call schedule_timeout_interruptible(), >>>> which improves overall KNI performance at the expense of CPU cycles >> (polling). >>>> Default values is 'yes', maintaining the same behaviour as of now. >>>> >>>> Signed-off-by: Marc Sune <marc.sune@bisdn.de> >> Although a better option would be to have a runtime setting, this is still an >> improvement over what we have. >> >> Acked-by: Bruce Richardson <bruce.richardson@intel.com> >> >>>> --- >>>> config/common_linuxapp | 1 + >>>> lib/librte_eal/linuxapp/kni/kni_misc.c | 4 ++++ >>>> 2 files changed, 5 insertions(+) >>>> >>>> diff --git a/config/common_linuxapp b/config/common_linuxapp index >>>> 57b61c9..24b529d 100644 >>>> --- a/config/common_linuxapp >>>> +++ b/config/common_linuxapp >>>> @@ -380,6 +380,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y >>>> # Compile librte_kni >>>> # >>>> CONFIG_RTE_LIBRTE_KNI=y >>>> +CONFIG_RTE_KNI_PREEMPT=y >>>> CONFIG_RTE_KNI_KO_DEBUG=n >>>> CONFIG_RTE_KNI_VHOST=n >>>> CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 >>>> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c >>>> b/lib/librte_eal/linuxapp/kni/kni_misc.c >>>> index ba77776..e7e6c27 100644 >>>> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c >>>> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c >>>> @@ -229,9 +229,11 @@ kni_thread_single(void *unused) >>>> } >>>> } >>>> up_read(&kni_list_lock); >>>> +#ifdef RTE_KNI_PREEMPT >>>> /* reschedule out for a while */ >>>> schedule_timeout_interruptible(usecs_to_jiffies( \ >>>> KNI_KTHREAD_RESCHEDULE_INTERVAL)); >>>> +#endif >>>> } >>>> return 0; >>>> @@ -252,8 +254,10 @@ kni_thread_multiple(void *param) >>>> #endif >>>> kni_net_poll_resp(dev); >>>> } >>>> +#ifdef RTE_KNI_PREEMPT >>>> schedule_timeout_interruptible(usecs_to_jiffies( \ >>>> KNI_KTHREAD_RESCHEDULE_INTERVAL)); >>>> +#endif >>>> } >>>> return 0; > As Bruce indicated, it would be better to do that at runtime, we can > add a config in struct rte_kni_conf, which will be copied to > struct rte_kni_device_info, then kernel space will know the configuration. > This way, we can enable/disable PREEMPT during KNI instance allocation time. As I said before, I see the point on having it at runtime and I agree. However, rte_kni_device_info is a struct that is per interface, not for the entire KNI subsystem, so it is kind of abusing to add a flag there when only will be used once, at bootstrap. To do it "properly" we should create a new ioctl() call IMHO. > Anyway, it can be done now or later. So, do we integrate the current patch, as acked by Bruce before? Marc > > Regards, > Helin > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option 2015-02-11 12:26 ` Marc Sune @ 2015-02-11 14:27 ` Bruce Richardson 2015-02-12 9:25 ` Marc Sune 0 siblings, 1 reply; 11+ messages in thread From: Bruce Richardson @ 2015-02-11 14:27 UTC (permalink / raw) To: Marc Sune; +Cc: dev On Wed, Feb 11, 2015 at 01:26:41PM +0100, Marc Sune wrote: > > On 11/02/15 02:54, Zhang, Helin wrote: > > > >>-----Original Message----- > >>From: Richardson, Bruce > >>Sent: Tuesday, February 10, 2015 9:24 PM > >>To: Marc Sune > >>Cc: dev@dpdk.org; Zhang, Helin > >>Subject: Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration > >>option > >> > >>On Tue, Feb 10, 2015 at 12:59:29PM +0100, Marc Sune wrote: > >>>This patch of Nov 2014 hasn't been yet ACKed/NACKed. Could someone > >>>please give some quick feedback? > >>> > >>>Thanks > >>>marc > >>> > >>>On 07/11/14 12:00, Marc Sune wrote: > >>>>This patch introduces CONFIG_RTE_KNI_PREEMPT flag. When set to 'no', > >>>>KNI kernel thread(s) do not call schedule_timeout_interruptible(), > >>>>which improves overall KNI performance at the expense of CPU cycles > >>(polling). > >>>>Default values is 'yes', maintaining the same behaviour as of now. > >>>> > >>>>Signed-off-by: Marc Sune <marc.sune@bisdn.de> > >>Although a better option would be to have a runtime setting, this is still an > >>improvement over what we have. > >> > >>Acked-by: Bruce Richardson <bruce.richardson@intel.com> > >> > >>>>--- > >>>> config/common_linuxapp | 1 + > >>>> lib/librte_eal/linuxapp/kni/kni_misc.c | 4 ++++ > >>>> 2 files changed, 5 insertions(+) > >>>> > >>>>diff --git a/config/common_linuxapp b/config/common_linuxapp index > >>>>57b61c9..24b529d 100644 > >>>>--- a/config/common_linuxapp > >>>>+++ b/config/common_linuxapp > >>>>@@ -380,6 +380,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y > >>>> # Compile librte_kni > >>>> # > >>>> CONFIG_RTE_LIBRTE_KNI=y > >>>>+CONFIG_RTE_KNI_PREEMPT=y > >>>> CONFIG_RTE_KNI_KO_DEBUG=n > >>>> CONFIG_RTE_KNI_VHOST=n > >>>> CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 > >>>>diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c > >>>>b/lib/librte_eal/linuxapp/kni/kni_misc.c > >>>>index ba77776..e7e6c27 100644 > >>>>--- a/lib/librte_eal/linuxapp/kni/kni_misc.c > >>>>+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c > >>>>@@ -229,9 +229,11 @@ kni_thread_single(void *unused) > >>>> } > >>>> } > >>>> up_read(&kni_list_lock); > >>>>+#ifdef RTE_KNI_PREEMPT > >>>> /* reschedule out for a while */ > >>>> schedule_timeout_interruptible(usecs_to_jiffies( \ > >>>> KNI_KTHREAD_RESCHEDULE_INTERVAL)); > >>>>+#endif > >>>> } > >>>> return 0; > >>>>@@ -252,8 +254,10 @@ kni_thread_multiple(void *param) > >>>> #endif > >>>> kni_net_poll_resp(dev); > >>>> } > >>>>+#ifdef RTE_KNI_PREEMPT > >>>> schedule_timeout_interruptible(usecs_to_jiffies( \ > >>>> KNI_KTHREAD_RESCHEDULE_INTERVAL)); > >>>>+#endif > >>>> } > >>>> return 0; > >As Bruce indicated, it would be better to do that at runtime, we can > >add a config in struct rte_kni_conf, which will be copied to > >struct rte_kni_device_info, then kernel space will know the configuration. > >This way, we can enable/disable PREEMPT during KNI instance allocation time. > > As I said before, I see the point on having it at runtime and I agree. > > However, rte_kni_device_info is a struct that is per interface, not for the > entire KNI subsystem, so it is kind of abusing to add a flag there when only > will be used once, at bootstrap. To do it "properly" we should create a new > ioctl() call IMHO. > > >Anyway, it can be done now or later. > > So, do we integrate the current patch, as acked by Bruce before? > > Marc I'd like to see it go in, as a step forward, rather than not having it at all because it's not quite perfect. One suggestion might be to have the compile time setting be "CONFIG_RTE_KNI_PREEMPT_DEFAULT" rather than just "CONFIG_RTE_KNI_PREEMPT". This would mean that we would not need to remove the setting later if we do add a runtime option, as the setting only specified the default value rather than the final value. :-) /Bruce ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option 2015-02-11 14:27 ` Bruce Richardson @ 2015-02-12 9:25 ` Marc Sune 0 siblings, 0 replies; 11+ messages in thread From: Marc Sune @ 2015-02-12 9:25 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev On 11/02/15 15:27, Bruce Richardson wrote: > On Wed, Feb 11, 2015 at 01:26:41PM +0100, Marc Sune wrote: >> On 11/02/15 02:54, Zhang, Helin wrote: >>>> -----Original Message----- >>>> From: Richardson, Bruce >>>> Sent: Tuesday, February 10, 2015 9:24 PM >>>> To: Marc Sune >>>> Cc: dev@dpdk.org; Zhang, Helin >>>> Subject: Re: [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration >>>> option >>>> >>>> On Tue, Feb 10, 2015 at 12:59:29PM +0100, Marc Sune wrote: >>>>> This patch of Nov 2014 hasn't been yet ACKed/NACKed. Could someone >>>>> please give some quick feedback? >>>>> >>>>> Thanks >>>>> marc >>>>> >>>>> On 07/11/14 12:00, Marc Sune wrote: >>>>>> This patch introduces CONFIG_RTE_KNI_PREEMPT flag. When set to 'no', >>>>>> KNI kernel thread(s) do not call schedule_timeout_interruptible(), >>>>>> which improves overall KNI performance at the expense of CPU cycles >>>> (polling). >>>>>> Default values is 'yes', maintaining the same behaviour as of now. >>>>>> >>>>>> Signed-off-by: Marc Sune <marc.sune@bisdn.de> >>>> Although a better option would be to have a runtime setting, this is still an >>>> improvement over what we have. >>>> >>>> Acked-by: Bruce Richardson <bruce.richardson@intel.com> >>>> >>>>>> --- >>>>>> config/common_linuxapp | 1 + >>>>>> lib/librte_eal/linuxapp/kni/kni_misc.c | 4 ++++ >>>>>> 2 files changed, 5 insertions(+) >>>>>> >>>>>> diff --git a/config/common_linuxapp b/config/common_linuxapp index >>>>>> 57b61c9..24b529d 100644 >>>>>> --- a/config/common_linuxapp >>>>>> +++ b/config/common_linuxapp >>>>>> @@ -380,6 +380,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=y >>>>>> # Compile librte_kni >>>>>> # >>>>>> CONFIG_RTE_LIBRTE_KNI=y >>>>>> +CONFIG_RTE_KNI_PREEMPT=y >>>>>> CONFIG_RTE_KNI_KO_DEBUG=n >>>>>> CONFIG_RTE_KNI_VHOST=n >>>>>> CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 >>>>>> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c >>>>>> b/lib/librte_eal/linuxapp/kni/kni_misc.c >>>>>> index ba77776..e7e6c27 100644 >>>>>> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c >>>>>> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c >>>>>> @@ -229,9 +229,11 @@ kni_thread_single(void *unused) >>>>>> } >>>>>> } >>>>>> up_read(&kni_list_lock); >>>>>> +#ifdef RTE_KNI_PREEMPT >>>>>> /* reschedule out for a while */ >>>>>> schedule_timeout_interruptible(usecs_to_jiffies( \ >>>>>> KNI_KTHREAD_RESCHEDULE_INTERVAL)); >>>>>> +#endif >>>>>> } >>>>>> return 0; >>>>>> @@ -252,8 +254,10 @@ kni_thread_multiple(void *param) >>>>>> #endif >>>>>> kni_net_poll_resp(dev); >>>>>> } >>>>>> +#ifdef RTE_KNI_PREEMPT >>>>>> schedule_timeout_interruptible(usecs_to_jiffies( \ >>>>>> KNI_KTHREAD_RESCHEDULE_INTERVAL)); >>>>>> +#endif >>>>>> } >>>>>> return 0; >>> As Bruce indicated, it would be better to do that at runtime, we can >>> add a config in struct rte_kni_conf, which will be copied to >>> struct rte_kni_device_info, then kernel space will know the configuration. >>> This way, we can enable/disable PREEMPT during KNI instance allocation time. >> As I said before, I see the point on having it at runtime and I agree. >> >> However, rte_kni_device_info is a struct that is per interface, not for the >> entire KNI subsystem, so it is kind of abusing to add a flag there when only >> will be used once, at bootstrap. To do it "properly" we should create a new >> ioctl() call IMHO. >> >>> Anyway, it can be done now or later. >> So, do we integrate the current patch, as acked by Bruce before? >> >> Marc > I'd like to see it go in, as a step forward, rather than not having it at all > because it's not quite perfect. > > One suggestion might be to have the compile time setting be > "CONFIG_RTE_KNI_PREEMPT_DEFAULT" rather than just "CONFIG_RTE_KNI_PREEMPT". This > would mean that we would not need to remove the setting later if we do add a > runtime option, as the setting only specified the default value rather than > the final value. :-) Ok. I can do this small change and sent v2 if we plan to integrate it and no one else wants to work on the runtime approach. I have no time right now to do a runtime setting patch. Marc > > /Bruce > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-02-12 9:25 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-11-07 11:00 [dpdk-dev] [PATCH] Adding RTE_KNI_PREEMPT configuration option Marc Sune 2015-02-10 11:59 ` Marc Sune 2015-02-10 12:02 ` Bruce Richardson 2015-02-10 12:21 ` Marc Sune 2015-02-10 13:22 ` Bruce Richardson 2015-02-10 18:06 ` Marc Sune 2015-02-10 13:24 ` Bruce Richardson 2015-02-11 1:54 ` Zhang, Helin 2015-02-11 12:26 ` Marc Sune 2015-02-11 14:27 ` Bruce Richardson 2015-02-12 9:25 ` Marc Sune
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).