Commit a03bc556 authored by Ziye Yang's avatar Ziye Yang Committed by Tomasz Zawadzki
Browse files

uring: Fix socket rotation and ordering issue in the pending_recv list.



When we rotate the socket in the list, we did not check whether the uc pointer
is NULL, then we cause the coredump when using uc pointer.

When we add a new socket (with pollin event) into the pending_recv list,
we add it into the end of the pending_recv list, then it delays the execution
of the newly socket, and it can cause the timeout especially when a socket
is in a connection phase.

So the purpose of this patch is:
1 Revise the rotation logic to handle the two cases, i.e., (1)sock is in the
beginning of the list; (2)sock is in the end of list. The purpose is
to avoid NULL pointer access, and efficently handle the exceptional case.

2 When there is new pollin event of the socket, we should add socket in the beginning
of the list. And this can avoid the new socket handling starvation.
Since max poll event num is 32 from upper layer and if we always put the new socket
in the end of the list, then starvation will occur if there are many socket connection events.
Because if we add the new socket into the end of the pending list, we will always handle the
existing socks first, then the later coming socket(with relatively pollin event) will always be
handled late. Then in the sock connection initialization phase, it will consume a relatively
long time, then the upper layer connection based on this socket will cause timeout,.e.g.,

ctrlr.c: 185:nvmf_ctrlr_keep_alive_poll: *NOTICE*: Disconnecting host nqn.2014-08.org.nvmexpress:
uuid:af56cce7-2008-408c-a8a0-9c710857febf from subsystem nqn.2019-02.io.spdk:cnode0 due to
keep alive timeout.
[2021-08-25 20:13:42.201139] ctrlr.c: 579:_nvmf_ctrlr_add_io_qpair:
*ERROR*: Unknown controller ID 0x1

Fixes #2097

Signed-off-by: default avatarZiye Yang <ziye.yang@intel.com>
Change-Id: I171b83ffd800539e86660c7607538e120fbe1a91
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9223


Reviewed-by: default avatarJohn Kariuki <John.K.Kariuki@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent a8c953cd
Loading
Loading
Loading
Loading
+15 −2
Original line number Diff line number Diff line
@@ -1023,7 +1023,7 @@ sock_uring_group_reap(struct spdk_uring_sock_group_impl *group, int max, int max
				if (sock->base.cb_fn != NULL &&
				    sock->pending_recv == false) {
					sock->pending_recv = true;
					TAILQ_INSERT_TAIL(&group->pending_recv, sock, link);
					TAILQ_INSERT_HEAD(&group->pending_recv, sock, link);
				}
			}
			break;
@@ -1093,9 +1093,21 @@ sock_uring_group_reap(struct spdk_uring_sock_group_impl *group, int max, int max

		/* Capture pointers to the elements we need */
		ud = sock;
		uc = TAILQ_PREV(ud, pending_recv_list, link);

		ua = TAILQ_FIRST(&group->pending_recv);
		if (ua == ud) {
			goto end;
		}

		uf = TAILQ_LAST(&group->pending_recv, pending_recv_list);
		if (uf == ud) {
			TAILQ_REMOVE(&group->pending_recv, ud, link);
			TAILQ_INSERT_HEAD(&group->pending_recv, ud, link);
			goto end;
		}

		uc = TAILQ_PREV(ud, pending_recv_list, link);
		assert(uc != NULL);

		/* Break the link between C and D */
		uc->link.tqe_next = NULL;
@@ -1112,6 +1124,7 @@ sock_uring_group_reap(struct spdk_uring_sock_group_impl *group, int max, int max
		ud->link.tqe_prev = &group->pending_recv.tqh_first;
	}

end:
	return count;
}