[dpdk-dev] [PATCH] kni: add support for core_id param in single threaded mode
> > Allow binding KNI thread to specific core in single threaded mode > > by setting core_id and force_bind config parameters. > > > > Signed-off-by: Vladyslav Buslov > > Acked-by: Ferruh Yigit > > Tested-by: Ferruh Yigit Applied, thanks
[dpdk-dev] [PATCH] kni: add support for core_id param in single threaded mode
On 9/6/2016 3:14 PM, Ferruh Yigit wrote:
> On 9/6/2016 12:25 PM, Vladyslav Buslov wrote:
>> Allow binding KNI thread to specific core in single threaded mode
>> by setting core_id and force_bind config parameters.
>
> Thanks, patch does exactly what we talked, and as I tested it works fine.
>
> 1) There are a few comments, can you please find them inline.
>
> 2) Would you mind adding some document related this new feature?
> Document path is: doc/guides/prog_guide/kernel_nic_interface.rst
>
>>
>> Signed-off-by: Vladyslav Buslov
>> ---
>> lib/librte_eal/linuxapp/kni/kni_misc.c | 48
>> ++
>> 1 file changed, 32 insertions(+), 16 deletions(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c
>> b/lib/librte_eal/linuxapp/kni/kni_misc.c
>> index 59d15ca..5e7cf21 100644
>> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
>> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
>> @@ -30,6 +30,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -100,6 +101,7 @@ static int kni_net_id;
>>
>> struct kni_net {
>> unsigned long device_in_use; /* device in use flag */
>> +struct mutex kni_kthread_lock;
>> struct task_struct *kni_kthread;
>> struct rw_semaphore kni_list_lock;
>> struct list_head kni_list_head;
>> @@ -123,6 +125,9 @@ static int __net_init kni_init_net(struct net *net)
>> /* Clear the bit of device in use */
>> clear_bit(KNI_DEV_IN_USE_BIT_NUM, >device_in_use);
>>
>> +mutex_init(>kni_kthread_lock);
>> +knet->kni_kthread = NULL;
>> +
>> init_rwsem(>kni_list_lock);
>> INIT_LIST_HEAD(>kni_list_head);
>>
>> @@ -139,9 +144,9 @@ static int __net_init kni_init_net(struct net *net)
>>
>> static void __net_exit kni_exit_net(struct net *net)
>> {
>> -#ifndef HAVE_SIMPLIFIED_PERNET_OPERATIONS
>> struct kni_net *knet = net_generic(net, kni_net_id);
>> -
>> +mutex_destroy(>kni_kthread_lock);
>> +#ifndef HAVE_SIMPLIFIED_PERNET_OPERATIONS
>> kfree(knet);
>> #endif
>> }
>> @@ -236,16 +241,9 @@ kni_open(struct inode *inode, struct file *file)
>> return -EBUSY;
>>
>> /* Create kernel thread for single mode */
>> -if (multiple_kthread_on == 0) {
>> +if (multiple_kthread_on == 0)
>> KNI_PRINT("Single kernel thread for all KNI devices\n");
>> -/* Create kernel thread for RX */
>> -knet->kni_kthread = kthread_run(kni_thread_single, (void *)knet,
>> -"kni_single");
>> -if (IS_ERR(knet->kni_kthread)) {
>> -KNI_ERR("Unable to create kernel threaed\n");
>> -return PTR_ERR(knet->kni_kthread);
>> -}
>> -} else
>> +else
>> KNI_PRINT("Multiple kernel thread mode enabled\n");
>
> Can move logs to where threads actually created (kni_ioctl_create)
Thinking twice, moving logs to kni_ioctl_create() can be too verbose
since it has been called multiple times, but we need this log only once.
Keepin in open() cb isn't good option, what do you think moving into
kni_init(), after kni_parse_ktread_mode? I think fits here better since
this is a module param...
[dpdk-dev] [PATCH] kni: add support for core_id param in single threaded mode
On 9/6/2016 3:14 PM, Ferruh Yigit wrote:
> On 9/6/2016 12:25 PM, Vladyslav Buslov wrote:
>> Allow binding KNI thread to specific core in single threaded mode
>> by setting core_id and force_bind config parameters.
>
> Thanks, patch does exactly what we talked, and as I tested it works fine.
>
> 1) There are a few comments, can you please find them inline.
>
> 2) Would you mind adding some document related this new feature?
> Document path is: doc/guides/prog_guide/kernel_nic_interface.rst
>
>>
>> Signed-off-by: Vladyslav Buslov
>> ---
...
*/
>> -if (multiple_kthread_on && dev_info.force_bind &&
>> +if (dev_info.force_bind &&
>> !cpu_online(dev_info.core_id)) {
minor thing, these two lines can join into single line ..
[dpdk-dev] [PATCH] kni: add support for core_id param in single threaded mode
On 9/6/2016 12:25 PM, Vladyslav Buslov wrote:
> Allow binding KNI thread to specific core in single threaded mode
> by setting core_id and force_bind config parameters.
Thanks, patch does exactly what we talked, and as I tested it works fine.
1) There are a few comments, can you please find them inline.
2) Would you mind adding some document related this new feature?
Document path is: doc/guides/prog_guide/kernel_nic_interface.rst
>
> Signed-off-by: Vladyslav Buslov
> ---
> lib/librte_eal/linuxapp/kni/kni_misc.c | 48
> ++
> 1 file changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c
> b/lib/librte_eal/linuxapp/kni/kni_misc.c
> index 59d15ca..5e7cf21 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -100,6 +101,7 @@ static int kni_net_id;
>
> struct kni_net {
> unsigned long device_in_use; /* device in use flag */
> + struct mutex kni_kthread_lock;
> struct task_struct *kni_kthread;
> struct rw_semaphore kni_list_lock;
> struct list_head kni_list_head;
> @@ -123,6 +125,9 @@ static int __net_init kni_init_net(struct net *net)
> /* Clear the bit of device in use */
> clear_bit(KNI_DEV_IN_USE_BIT_NUM, >device_in_use);
>
> + mutex_init(>kni_kthread_lock);
> + knet->kni_kthread = NULL;
> +
> init_rwsem(>kni_list_lock);
> INIT_LIST_HEAD(>kni_list_head);
>
> @@ -139,9 +144,9 @@ static int __net_init kni_init_net(struct net *net)
>
> static void __net_exit kni_exit_net(struct net *net)
> {
> -#ifndef HAVE_SIMPLIFIED_PERNET_OPERATIONS
> struct kni_net *knet = net_generic(net, kni_net_id);
> -
> + mutex_destroy(>kni_kthread_lock);
> +#ifndef HAVE_SIMPLIFIED_PERNET_OPERATIONS
> kfree(knet);
> #endif
> }
> @@ -236,16 +241,9 @@ kni_open(struct inode *inode, struct file *file)
> return -EBUSY;
>
> /* Create kernel thread for single mode */
> - if (multiple_kthread_on == 0) {
> + if (multiple_kthread_on == 0)
> KNI_PRINT("Single kernel thread for all KNI devices\n");
> - /* Create kernel thread for RX */
> - knet->kni_kthread = kthread_run(kni_thread_single, (void *)knet,
> - "kni_single");
> - if (IS_ERR(knet->kni_kthread)) {
> - KNI_ERR("Unable to create kernel threaed\n");
> - return PTR_ERR(knet->kni_kthread);
> - }
> - } else
> + else
> KNI_PRINT("Multiple kernel thread mode enabled\n");
Can move logs to where threads actually created (kni_ioctl_create)
>
> file->private_data = get_net(net);
> @@ -263,9 +261,13 @@ kni_release(struct inode *inode, struct file *file)
>
> /* Stop kernel thread for single mode */
> if (multiple_kthread_on == 0) {
> + mutex_lock(>kni_kthread_lock);
> /* Stop kernel thread */
> - kthread_stop(knet->kni_kthread);
> - knet->kni_kthread = NULL;
> + if (knet->kni_kthread != NULL) {
> + kthread_stop(knet->kni_kthread);
> + knet->kni_kthread = NULL;
> + }
> + mutex_unlock(>kni_kthread_lock);
> }
>
> down_write(>kni_list_lock);
> @@ -415,10 +417,9 @@ kni_ioctl_create(struct net *net,
> }
>
> /**
> - * Check if the cpu core id is valid for binding,
> - * for multiple kernel thread mode.
> + * Check if the cpu core id is valid for binding.
>*/
> - if (multiple_kthread_on && dev_info.force_bind &&
> + if (dev_info.force_bind &&
> !cpu_online(dev_info.core_id)) {
> KNI_ERR("cpu %u is not online\n", dev_info.core_id);
> return -EINVAL;
> @@ -581,6 +582,21 @@ kni_ioctl_create(struct net *net,
> if (dev_info.force_bind)
> kthread_bind(kni->pthread, kni->core_id);
> wake_up_process(kni->pthread);
> + } else {
> + mutex_lock(>kni_kthread_lock);
> + if (knet->kni_kthread == NULL) {
->
> + knet->kni_kthread = kthread_create(kni_thread_single,
> + (void
> *)knet,
> +
> "kni_single");
Syntax of this line is not proper, and I am aware above same call has
this syntax J But let's fix here..
> + if (IS_ERR(knet->kni_kthread)) {
> + kni_dev_remove(kni);
> + return -ECANCELED;
> + }
> + if (dev_info.force_bind)
> +
[dpdk-dev] [PATCH] kni: add support for core_id param in single threaded mode
Ferruh,
Thanks for suggestions.
I'll try to provide new patch this week.
Regards,
Vladyslav
-Original Message-
From: Ferruh Yigit [mailto:ferruh.yi...@intel.com]
Sent: Tuesday, September 06, 2016 5:31 PM
To: Vladyslav Buslov
Cc: dev at dpdk.org
Subject: Re: [dpdk-dev] [PATCH] kni: add support for core_id param in single
threaded mode
On 9/6/2016 3:14 PM, Ferruh Yigit wrote:
> On 9/6/2016 12:25 PM, Vladyslav Buslov wrote:
>> Allow binding KNI thread to specific core in single threaded mode by
>> setting core_id and force_bind config parameters.
>
> Thanks, patch does exactly what we talked, and as I tested it works fine.
>
> 1) There are a few comments, can you please find them inline.
>
> 2) Would you mind adding some document related this new feature?
> Document path is: doc/guides/prog_guide/kernel_nic_interface.rst
>
>>
>> Signed-off-by: Vladyslav Buslov
>> ---
>> lib/librte_eal/linuxapp/kni/kni_misc.c | 48
>> ++
>> 1 file changed, 32 insertions(+), 16 deletions(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c
>> b/lib/librte_eal/linuxapp/kni/kni_misc.c
>> index 59d15ca..5e7cf21 100644
>> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
>> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
>> @@ -30,6 +30,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -100,6 +101,7 @@ static int kni_net_id;
>>
>> struct kni_net {
>> unsigned long device_in_use; /* device in use flag */
>> +struct mutex kni_kthread_lock;
>> struct task_struct *kni_kthread;
>> struct rw_semaphore kni_list_lock;
>> struct list_head kni_list_head;
>> @@ -123,6 +125,9 @@ static int __net_init kni_init_net(struct net *net)
>> /* Clear the bit of device in use */
>> clear_bit(KNI_DEV_IN_USE_BIT_NUM, >device_in_use);
>>
>> +mutex_init(>kni_kthread_lock);
>> +knet->kni_kthread = NULL;
>> +
>> init_rwsem(>kni_list_lock);
>> INIT_LIST_HEAD(>kni_list_head);
>>
>> @@ -139,9 +144,9 @@ static int __net_init kni_init_net(struct net
>> *net)
>>
>> static void __net_exit kni_exit_net(struct net *net) { -#ifndef
>> HAVE_SIMPLIFIED_PERNET_OPERATIONS
>> struct kni_net *knet = net_generic(net, kni_net_id);
>> -
>> +mutex_destroy(>kni_kthread_lock);
>> +#ifndef HAVE_SIMPLIFIED_PERNET_OPERATIONS
>> kfree(knet);
>> #endif
>> }
>> @@ -236,16 +241,9 @@ kni_open(struct inode *inode, struct file *file)
>> return -EBUSY;
>>
>> /* Create kernel thread for single mode */
>> -if (multiple_kthread_on == 0) {
>> +if (multiple_kthread_on == 0)
>> KNI_PRINT("Single kernel thread for all KNI devices\n");
>> -/* Create kernel thread for RX */
>> -knet->kni_kthread = kthread_run(kni_thread_single, (void *)knet,
>> -"kni_single");
>> -if (IS_ERR(knet->kni_kthread)) {
>> -KNI_ERR("Unable to create kernel threaed\n");
>> -return PTR_ERR(knet->kni_kthread);
>> -}
>> -} else
>> +else
>> KNI_PRINT("Multiple kernel thread mode enabled\n");
>
> Can move logs to where threads actually created (kni_ioctl_create)
Thinking twice, moving logs to kni_ioctl_create() can be too verbose since it
has been called multiple times, but we need this log only once.
Keepin in open() cb isn't good option, what do you think moving into
kni_init(), after kni_parse_ktread_mode? I think fits here better since this is
a module param...
[dpdk-dev] [PATCH] kni: add support for core_id param in single threaded mode
Allow binding KNI thread to specific core in single threaded mode
by setting core_id and force_bind config parameters.
Signed-off-by: Vladyslav Buslov
---
lib/librte_eal/linuxapp/kni/kni_misc.c | 48 ++
1 file changed, 32 insertions(+), 16 deletions(-)
diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c
b/lib/librte_eal/linuxapp/kni/kni_misc.c
index 59d15ca..5e7cf21 100644
--- a/lib/librte_eal/linuxapp/kni/kni_misc.c
+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -100,6 +101,7 @@ static int kni_net_id;
struct kni_net {
unsigned long device_in_use; /* device in use flag */
+ struct mutex kni_kthread_lock;
struct task_struct *kni_kthread;
struct rw_semaphore kni_list_lock;
struct list_head kni_list_head;
@@ -123,6 +125,9 @@ static int __net_init kni_init_net(struct net *net)
/* Clear the bit of device in use */
clear_bit(KNI_DEV_IN_USE_BIT_NUM, >device_in_use);
+ mutex_init(>kni_kthread_lock);
+ knet->kni_kthread = NULL;
+
init_rwsem(>kni_list_lock);
INIT_LIST_HEAD(>kni_list_head);
@@ -139,9 +144,9 @@ static int __net_init kni_init_net(struct net *net)
static void __net_exit kni_exit_net(struct net *net)
{
-#ifndef HAVE_SIMPLIFIED_PERNET_OPERATIONS
struct kni_net *knet = net_generic(net, kni_net_id);
-
+ mutex_destroy(>kni_kthread_lock);
+#ifndef HAVE_SIMPLIFIED_PERNET_OPERATIONS
kfree(knet);
#endif
}
@@ -236,16 +241,9 @@ kni_open(struct inode *inode, struct file *file)
return -EBUSY;
/* Create kernel thread for single mode */
- if (multiple_kthread_on == 0) {
+ if (multiple_kthread_on == 0)
KNI_PRINT("Single kernel thread for all KNI devices\n");
- /* Create kernel thread for RX */
- knet->kni_kthread = kthread_run(kni_thread_single, (void *)knet,
- "kni_single");
- if (IS_ERR(knet->kni_kthread)) {
- KNI_ERR("Unable to create kernel threaed\n");
- return PTR_ERR(knet->kni_kthread);
- }
- } else
+ else
KNI_PRINT("Multiple kernel thread mode enabled\n");
file->private_data = get_net(net);
@@ -263,9 +261,13 @@ kni_release(struct inode *inode, struct file *file)
/* Stop kernel thread for single mode */
if (multiple_kthread_on == 0) {
+ mutex_lock(>kni_kthread_lock);
/* Stop kernel thread */
- kthread_stop(knet->kni_kthread);
- knet->kni_kthread = NULL;
+ if (knet->kni_kthread != NULL) {
+ kthread_stop(knet->kni_kthread);
+ knet->kni_kthread = NULL;
+ }
+ mutex_unlock(>kni_kthread_lock);
}
down_write(>kni_list_lock);
@@ -415,10 +417,9 @@ kni_ioctl_create(struct net *net,
}
/**
-* Check if the cpu core id is valid for binding,
-* for multiple kernel thread mode.
+* Check if the cpu core id is valid for binding.
*/
- if (multiple_kthread_on && dev_info.force_bind &&
+ if (dev_info.force_bind &&
!cpu_online(dev_info.core_id)) {
KNI_ERR("cpu %u is not online\n", dev_info.core_id);
return -EINVAL;
@@ -581,6 +582,21 @@ kni_ioctl_create(struct net *net,
if (dev_info.force_bind)
kthread_bind(kni->pthread, kni->core_id);
wake_up_process(kni->pthread);
+ } else {
+ mutex_lock(>kni_kthread_lock);
+ if (knet->kni_kthread == NULL) {
+ knet->kni_kthread = kthread_create(kni_thread_single,
+ (void
*)knet,
+
"kni_single");
+ if (IS_ERR(knet->kni_kthread)) {
+ kni_dev_remove(kni);
+ return -ECANCELED;
+ }
+ if (dev_info.force_bind)
+ kthread_bind(knet->kni_kthread, kni->core_id);
+ wake_up_process(knet->kni_kthread);
+ }
+ mutex_unlock(>kni_kthread_lock);
}
down_write(>kni_list_lock);
--
2.8.3
[dpdk-dev] [PATCH] kni: add support for core_id param in single threaded mode
Allow binding KNI thread to specific core in single threaded mode
by setting core_id and force_bind config parameters.
Signed-off-by: Vladyslav Buslov
---
lib/librte_eal/linuxapp/kni/kni_misc.c | 48 ++
1 file changed, 32 insertions(+), 16 deletions(-)
diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c
b/lib/librte_eal/linuxapp/kni/kni_misc.c
index 59d15ca..5e7cf21 100644
--- a/lib/librte_eal/linuxapp/kni/kni_misc.c
+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -100,6 +101,7 @@ static int kni_net_id;
struct kni_net {
unsigned long device_in_use; /* device in use flag */
+ struct mutex kni_kthread_lock;
struct task_struct *kni_kthread;
struct rw_semaphore kni_list_lock;
struct list_head kni_list_head;
@@ -123,6 +125,9 @@ static int __net_init kni_init_net(struct net *net)
/* Clear the bit of device in use */
clear_bit(KNI_DEV_IN_USE_BIT_NUM, >device_in_use);
+ mutex_init(>kni_kthread_lock);
+ knet->kni_kthread = NULL;
+
init_rwsem(>kni_list_lock);
INIT_LIST_HEAD(>kni_list_head);
@@ -139,9 +144,9 @@ static int __net_init kni_init_net(struct net *net)
static void __net_exit kni_exit_net(struct net *net)
{
-#ifndef HAVE_SIMPLIFIED_PERNET_OPERATIONS
struct kni_net *knet = net_generic(net, kni_net_id);
-
+ mutex_destroy(>kni_kthread_lock);
+#ifndef HAVE_SIMPLIFIED_PERNET_OPERATIONS
kfree(knet);
#endif
}
@@ -236,16 +241,9 @@ kni_open(struct inode *inode, struct file *file)
return -EBUSY;
/* Create kernel thread for single mode */
- if (multiple_kthread_on == 0) {
+ if (multiple_kthread_on == 0)
KNI_PRINT("Single kernel thread for all KNI devices\n");
- /* Create kernel thread for RX */
- knet->kni_kthread = kthread_run(kni_thread_single, (void *)knet,
- "kni_single");
- if (IS_ERR(knet->kni_kthread)) {
- KNI_ERR("Unable to create kernel threaed\n");
- return PTR_ERR(knet->kni_kthread);
- }
- } else
+ else
KNI_PRINT("Multiple kernel thread mode enabled\n");
file->private_data = get_net(net);
@@ -263,9 +261,13 @@ kni_release(struct inode *inode, struct file *file)
/* Stop kernel thread for single mode */
if (multiple_kthread_on == 0) {
+ mutex_lock(>kni_kthread_lock);
/* Stop kernel thread */
- kthread_stop(knet->kni_kthread);
- knet->kni_kthread = NULL;
+ if (knet->kni_kthread != NULL) {
+ kthread_stop(knet->kni_kthread);
+ knet->kni_kthread = NULL;
+ }
+ mutex_unlock(>kni_kthread_lock);
}
down_write(>kni_list_lock);
@@ -415,10 +417,9 @@ kni_ioctl_create(struct net *net,
}
/**
-* Check if the cpu core id is valid for binding,
-* for multiple kernel thread mode.
+* Check if the cpu core id is valid for binding.
*/
- if (multiple_kthread_on && dev_info.force_bind &&
+ if (dev_info.force_bind &&
!cpu_online(dev_info.core_id)) {
KNI_ERR("cpu %u is not online\n", dev_info.core_id);
return -EINVAL;
@@ -581,6 +582,21 @@ kni_ioctl_create(struct net *net,
if (dev_info.force_bind)
kthread_bind(kni->pthread, kni->core_id);
wake_up_process(kni->pthread);
+ } else {
+ mutex_lock(>kni_kthread_lock);
+ if (knet->kni_kthread == NULL) {
+ knet->kni_kthread = kthread_create(kni_thread_single,
+ (void
*)knet,
+
"kni_single");
+ if (IS_ERR(knet->kni_kthread)) {
+ kni_dev_remove(kni);
+ return -ECANCELED;
+ }
+ if (dev_info.force_bind)
+ kthread_bind(knet->kni_kthread, kni->core_id);
+ wake_up_process(knet->kni_kthread);
+ }
+ mutex_unlock(>kni_kthread_lock);
}
down_write(>kni_list_lock);
--
2.8.3
