[PATCH 1/2] vhost: make sure vhost fdset-thread created successfully

[PATCH 1/2] vhost: make sure vhost fdset-thread created successfully

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

When first call the 'rte_vhost_driver_start', the
fdset_event_dispatch thread should be created successfully.
Because the vhost uses it to poll socket events for vhost
server or clients. Without it, for example, vhost will not
get the connection event.

This patch returns err code directly when created not successful.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 lib/librte_vhost/socket.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 83befdc..8ca01df 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -831,9 +831,11 @@ struct vhost_device_ops const *
 	if (fdset_tid == 0) {
 		int ret = pthread_create(&fdset_tid, NULL, fdset_event_dispatch,
 				     &vhost_user.fdset);
-		if (ret != 0)
+		if (ret != 0) {
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"failed to create fdset handling thread");
+			return ret;
+		}
 	}
 
 	if (vsocket->is_server)
-- 
1.8.3.1

[PATCH 2/2] vhost: add fdset-event thread name

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

This patch adds the name for vhost fdset thread.
It can help us to know whether the thread is running.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 lib/librte_vhost/socket.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 8ca01df..93175ba 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -820,6 +820,7 @@ struct vhost_device_ops const *
 {
 	struct vhost_user_socket *vsocket;
 	static pthread_t fdset_tid;
+	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 
 	pthread_mutex_lock(&vhost_user.mutex);
 	vsocket = find_vhost_user_socket(path);
@@ -835,6 +836,14 @@ struct vhost_device_ops const *
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"failed to create fdset handling thread");
 			return ret;
+		} else {
+			snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
+				 "vhost-events");
+
+			if (rte_thread_setname(fdset_tid, thread_name)) {
+				RTE_LOG(DEBUG, VHOST_CONFIG,
+					"failed to set vhost-event thread name");
+			}
 		}
 	}
 
-- 
1.8.3.1

Re: [PATCH 1/2] vhost: make sure vhost fdset-thread created successfully

[Tan, Jianfeng]

Hi Xiangxia,

> -----Original Message-----
> From: xiangxia.m.yue@gmail.com [mailto:xiangxia.m.yue@gmail.com]
> Sent: Thursday, March 22, 2018 8:40 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; Tonghao Zhang
> Subject: [PATCH 1/2] vhost: make sure vhost fdset-thread created
> successfully
> 
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> When first call the 'rte_vhost_driver_start', the
> fdset_event_dispatch thread should be created successfully.
> Because the vhost uses it to poll socket events for vhost
> server or clients. Without it, for example, vhost will not
> get the connection event.
> 
> This patch returns err code directly when created not successful.
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>

Except a nit below,

Reviewed-by: Jianfeng Tan <jianfeng.tan@intel.com>

> ---
>  lib/librte_vhost/socket.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 83befdc..8ca01df 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -831,9 +831,11 @@ struct vhost_device_ops const *
>  	if (fdset_tid == 0) {
>  		int ret = pthread_create(&fdset_tid, NULL,
> fdset_event_dispatch,
>  				     &vhost_user.fdset);
> -		if (ret != 0)
> +		if (ret != 0) {
>  			RTE_LOG(ERR, VHOST_CONFIG,
>  				"failed to create fdset handling thread");
> +			return ret;

According to the declaration below, we shall return 0 or -1.

/**
 *
 * Start the vhost-user driver.
 *
 * This function triggers the vhost-user negotiation.
 *
 * @param path
 *  The vhost-user socket file path
 * @return
 *  0 on success, -1 on failure
 */
int rte_vhost_driver_start(const char *path);

> +		}
>  	}
> 
>  	if (vsocket->is_server)
> --
> 1.8.3.1

Re: [PATCH 2/2] vhost: add fdset-event thread name

[Tan, Jianfeng]

> -----Original Message-----
> From: xiangxia.m.yue@gmail.com [mailto:xiangxia.m.yue@gmail.com]
> Sent: Thursday, March 22, 2018 8:40 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; Tonghao Zhang
> Subject: [PATCH 2/2] vhost: add fdset-event thread name
> 
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> This patch adds the name for vhost fdset thread.
> It can help us to know whether the thread is running.
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>

Reviewed-by: Jianfeng Tan <jianfeng.tan@intel.com>

Thanks,
Jianfeng

> ---
>  lib/librte_vhost/socket.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 8ca01df..93175ba 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -820,6 +820,7 @@ struct vhost_device_ops const *
>  {
>  	struct vhost_user_socket *vsocket;
>  	static pthread_t fdset_tid;
> +	char thread_name[RTE_MAX_THREAD_NAME_LEN];
> 
>  	pthread_mutex_lock(&vhost_user.mutex);
>  	vsocket = find_vhost_user_socket(path);
> @@ -835,6 +836,14 @@ struct vhost_device_ops const *
>  			RTE_LOG(ERR, VHOST_CONFIG,
>  				"failed to create fdset handling thread");
>  			return ret;
> +		} else {
> +			snprintf(thread_name,
> RTE_MAX_THREAD_NAME_LEN,
> +				 "vhost-events");
> +
> +			if (rte_thread_setname(fdset_tid, thread_name)) {
> +				RTE_LOG(DEBUG, VHOST_CONFIG,
> +					"failed to set vhost-event thread
> name");
> +			}
>  		}
>  	}
> 
> --
> 1.8.3.1

Re: [PATCH 1/2] vhost: make sure vhost fdset-thread created successfully

[Tonghao Zhang]

On Fri, Mar 23, 2018 at 9:30 AM, Tan, Jianfeng <jianfeng.tan@intel.com> wrote:
> Hi Xiangxia,
>
>> -----Original Message-----
>> From: xiangxia.m.yue@gmail.com [mailto:xiangxia.m.yue@gmail.com]
>> Sent: Thursday, March 22, 2018 8:40 PM
>> To: Tan, Jianfeng
>> Cc: dev@dpdk.org; Tonghao Zhang
>> Subject: [PATCH 1/2] vhost: make sure vhost fdset-thread created
>> successfully
>>
>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>
>> When first call the 'rte_vhost_driver_start', the
>> fdset_event_dispatch thread should be created successfully.
>> Because the vhost uses it to poll socket events for vhost
>> server or clients. Without it, for example, vhost will not
>> get the connection event.
>>
>> This patch returns err code directly when created not successful.
>>
>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> Except a nit below,
>
> Reviewed-by: Jianfeng Tan <jianfeng.tan@intel.com>
>
>> ---
>>  lib/librte_vhost/socket.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
>> index 83befdc..8ca01df 100644
>> --- a/lib/librte_vhost/socket.c
>> +++ b/lib/librte_vhost/socket.c
>> @@ -831,9 +831,11 @@ struct vhost_device_ops const *
>>       if (fdset_tid == 0) {
>>               int ret = pthread_create(&fdset_tid, NULL,
>> fdset_event_dispatch,
>>                                    &vhost_user.fdset);
>> -             if (ret != 0)
>> +             if (ret != 0) {
>>                       RTE_LOG(ERR, VHOST_CONFIG,
>>                               "failed to create fdset handling thread");
>> +                     return ret;
>
> According to the declaration below, we shall return 0 or -1.
I will send v2

> /**
>  *
>  * Start the vhost-user driver.
>  *
>  * This function triggers the vhost-user negotiation.
>  *
>  * @param path
>  *  The vhost-user socket file path
>  * @return
>  *  0 on success, -1 on failure
>  */
> int rte_vhost_driver_start(const char *path);
>
>> +             }
>>       }
>>
>>       if (vsocket->is_server)
>> --
>> 1.8.3.1
>