From c7d1c506efebe8422caaf4baf59bad405d936efb Mon Sep 17 00:00:00 2001 From: Konrad Sztyber Date: Thu, 19 Aug 2021 15:18:04 +0200 Subject: [PATCH] sock/uring: reorder task cancellations in remove_sock When removing a socket from a sock_group, the recv_task should be cancelled last, because it can be sent out while cancelling other tasks (if POLLERR is received). Otherwise, we could end up with outstanding recv requests from a socket removed from a group. Fixes #2112. Signed-off-by: Konrad Sztyber Change-Id: Ic8e24c210541390dd8bdffe8d3bc4e7dd746d4b7 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9239 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Ziye Yang Reviewed-by: Jim Harris Reviewed-by: Tomasz Zawadzki Tested-by: SPDK CI Jenkins --- module/sock/uring/uring.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/module/sock/uring/uring.c b/module/sock/uring/uring.c index 8fcee5ae2..e36541a0d 100644 --- a/module/sock/uring/uring.c +++ b/module/sock/uring/uring.c @@ -1409,6 +1409,16 @@ uring_sock_group_impl_remove_sock(struct spdk_sock_group_impl *_group, } } + if (sock->pollin_task.status != SPDK_URING_SOCK_TASK_NOT_IN_USE) { + _sock_prep_cancel_task(_sock, &sock->pollin_task); + /* Since spdk_sock_group_remove_sock is not asynchronous interface, so + * currently can use a while loop here. */ + while ((sock->pollin_task.status != SPDK_URING_SOCK_TASK_NOT_IN_USE) || + (sock->cancel_task.status != SPDK_URING_SOCK_TASK_NOT_IN_USE)) { + uring_sock_group_impl_poll(_group, 32, NULL); + } + } + if (sock->recv_task.status != SPDK_URING_SOCK_TASK_NOT_IN_USE) { _sock_prep_cancel_task(_sock, &sock->recv_task); /* Since spdk_sock_group_remove_sock is not asynchronous interface, so @@ -1419,15 +1429,11 @@ uring_sock_group_impl_remove_sock(struct spdk_sock_group_impl *_group, } } - if (sock->pollin_task.status != SPDK_URING_SOCK_TASK_NOT_IN_USE) { - _sock_prep_cancel_task(_sock, &sock->pollin_task); - /* Since spdk_sock_group_remove_sock is not asynchronous interface, so - * currently can use a while loop here. */ - while ((sock->pollin_task.status != SPDK_URING_SOCK_TASK_NOT_IN_USE) || - (sock->cancel_task.status != SPDK_URING_SOCK_TASK_NOT_IN_USE)) { - uring_sock_group_impl_poll(_group, 32, NULL); - } - } + /* Make sure the cancelling the tasks above didn't cause sending new requests */ + assert(sock->write_task.status == SPDK_URING_SOCK_TASK_NOT_IN_USE); + assert(sock->pollin_task.status == SPDK_URING_SOCK_TASK_NOT_IN_USE); + assert(sock->recv_task.status == SPDK_URING_SOCK_TASK_NOT_IN_USE); + if (sock->pending_recv) { TAILQ_REMOVE(&group->pending_recv, sock, link); sock->pending_recv = false;