[dpdk-dev] [RFC 4/8] ipc: remove IPC thread for async request

Burakov, Anatoly anatoly.burakov at intel.com
Fri Apr 20 11:03:15 CEST 2018


On 19-Apr-18 4:03 PM, Jianfeng Tan wrote:
> As discussed here, http://dpdk.org/dev/patchwork/patch/36579/,
> we remove IPC threads, rte_mp_handle and rte_mp_handle_async.
> This patch targets to remove thread rte_mp_handle_async.
> 
> Previously, to handle replies for an async request, rte_mp_handle
> wakes up the rte_mp_handle_async thread to process through
> pending_requests.async_cond. Now, we change to handle that in
> rte_mp_handle context directly.
> 
> To handle timeout events, for each async request which is sent,
> we set an alarm for it. If its reply is received before timeout,
> we will cancel the alarm when we handle the reply; otherwise,
> alarm will invoke the async_reply_handle() as the alarm callback.
> 
> Cc: Anatoly Burakov <anatoly.burakov at intel.com>
> 
> Suggested-by: Thomas Monjalon <thomas at monjalon.net>
> Signed-off-by: Jianfeng Tan <jianfeng.tan at intel.com>
> ---

<...>

> @@ -299,9 +300,11 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
>   
>   			if (pending_req->type == REQUEST_TYPE_SYNC)
>   				pthread_cond_signal(&pending_req->sync.cond);
> -			else if (pending_req->type == REQUEST_TYPE_ASYNC)
> -				pthread_cond_signal(
> -					&pending_requests.async_cond);
> +			else if (pending_req->type == REQUEST_TYPE_ASYNC) {
> +				pthread_mutex_unlock(&pending_requests.lock);
> +				async_reply_handle(pending_req);
> +				pthread_mutex_lock(&pending_requests.lock);

There must be a better way to do this than to unlock mutex before 
locking it again :) I haven't looked at implications of this suggestion 
yet, but how about alarm calling a wrapper function that locks the 
mutex, but leave async_reply_handle without any locking whatsoever?

> +			}
>   		} else
>   			RTE_LOG(ERR, EAL, "Drop mp reply: %s\n", msg->name);
>   		pthread_mutex_unlock(&pending_requests.lock);
> @@ -450,115 +453,39 @@ trigger_async_action(struct pending_request *sr)
>   	free(sr->request);
>   }
>   
> -static struct pending_request *
> -check_trigger(struct timespec *ts)

<...>

> +	ts_now.tv_nsec = now.tv_usec * 1000;
> +	ts_now.tv_sec = now.tv_sec;
>   
> -				/* we've triggered a callback, but there may be
> -				 * more, so lock the list and check again.
> -				 */
> -				pthread_mutex_lock(&pending_requests.lock);
> -			}
> -		} while (trigger);
> +	pthread_mutex_lock(&pending_requests.lock);
> +	action = process_async_request(req, &ts_now);
> +	if (action == ACTION_NONE) {
> +		pthread_mutex_unlock(&pending_requests.lock);
> +		return;
>   	}
> +	TAILQ_REMOVE(&pending_requests.requests, req, next);
> +	pthread_mutex_unlock(&pending_requests.lock);
>   
> -	RTE_LOG(ERR, EAL, "ERROR: asynchronous requests disabled\n");
> +	if (action == ACTION_TRIGGER)
> +		trigger_async_action(req);
>   
> -	return NULL;
> +	if (rte_eal_alarm_cancel(async_reply_handle, req) < 0 &&
> +	    rte_errno != EINPROGRESS)
> +		RTE_LOG(ERR, EAL, "Failed to cancel alarm\n");
> +

Perhaps cancel the alarm before triggering callback?

-- 
Thanks,
Anatoly


More information about the dev mailing list