[dpdk-dev] [PATCH] eal/ipc: stop async IPC loop on callback request
Burakov, Anatoly
anatoly.burakov at intel.com
Tue Apr 10 16:17:14 CEST 2018
On 10-Apr-18 2:53 PM, Tan, Jianfeng wrote:
>
>
> On 4/10/2018 6:03 PM, Anatoly Burakov wrote:
>> EAL did not stop processing further asynchronous requests on
>> encountering a request that should trigger the callback. This
>> resulted in erasing valid requests but not triggering them.
>
> That means one wakeup could process multiple replies, and following
> process_async_request() will erase some valid requests?
Yes.
>
>>
>> Fix this by stopping the loop once we have a request that we
>> can trigger. Also, remove unnecessary check for trigger
>> request being NULL.
>>
>> Fixes: f05e26051c15 ("eal: add IPC asynchronous request")
>> Cc: anatoly.burakov at intel.com
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>
>
>
>> ---
>> lib/librte_eal/common/eal_common_proc.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/librte_eal/common/eal_common_proc.c
>> b/lib/librte_eal/common/eal_common_proc.c
>> index f98622f..1ea3b58 100644
>> --- a/lib/librte_eal/common/eal_common_proc.c
>> +++ b/lib/librte_eal/common/eal_common_proc.c
>> @@ -510,11 +510,11 @@ async_reply_handle(void *arg __rte_unused)
>> TAILQ_REMOVE(&pending_requests.requests,
>> sr, next);
>> free(sr);
>> - } else if (action == ACTION_TRIGGER &&
>> - trigger == NULL) {
>> + } else if (action == ACTION_TRIGGER) {
>> TAILQ_REMOVE(&pending_requests.requests,
>> sr, next);
>> trigger = sr;
>> + break;
>
> If I understand it correctly above, break here, we will trigger an async
> action, and then go back to sleep with some ready requests not handled?
> Seems that we shall unlock, process, and lock here. Right?
Well, we won't go back to sleep - we'll just loop around and come back.
See eal_common_proc.c:472:
/* sometimes, we don't even wait */
if (sr->reply_received) {
nowait = true;
break;
}
Followed by line 478:
if (nowait)
ret = 0;
Followed by line 495:
if (ret == 0 || ret == ETIMEDOUT) {
So, having messages with replies already in the queue will cause wait to
be cancelled. It's not much slower than unlocking, triggering, and
locking again.
However, if you're OK with lock-loop-unlock-trigger-lock-loop-unlock-...
sequence until we run out of triggers, then sure, i can add that.
>
> Thanks,
> Jianfeng
>
>> }
>> }
>> }
>
>
--
Thanks,
Anatoly
More information about the dev
mailing list