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

lib/sock: Fix the coredump issue in sock_map_realese 



When tested on Linux 5.8 kernel and configure spdk
with debug mode (--enable-debug), and test SPDK NVMe-oF
tcp transport, and we see the coredump in sock_map_release
with the following statements:
	assert(entry->ref > 0);

After debug, I can confirm that the placement_id value got
from the following function (sock->net_impl->get_placement_id)
changes.
It means that: When the sock is added into the poll group
(spdk_sock_group_add_sock), we get the placement_id (named as
Value(begin)); and when the sock is removed from the poll group
(spdk_sock_group_remove_sock), we get the plaemednt_id on
the same sock (named as Vaule(end)). I found that
Value(begin) ! = Value(end).

So our solution is for a socket, we will get placement_id once,
then we can solve this issue.

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


Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
parent 49601c04

include/spdk_internal/sock.h

+1 −0
Original line number Diff line number Diff line
@@ -63,6 +63,7 @@ struct spdk_sock { 
	TAILQ_HEAD(, spdk_sock_request)	queued_reqs;

	TAILQ_HEAD(, spdk_sock_request)	pending_reqs;

	int				queued_iovcnt;

	int				placement_id;



	struct {

		uint8_t		closed		: 1;

lib/sock/sock.c

+23 −7
Original line number Diff line number Diff line
@@ -146,13 +146,29 @@ sock_remove_sock_group_from_map_table(struct spdk_sock_group *group) 


}



int

spdk_sock_get_optimal_sock_group(struct spdk_sock *sock, struct spdk_sock_group **group)

static int

sock_get_placement_id(struct spdk_sock *sock)

{

	int placement_id = 0, rc;

	int rc;

	int placement_id;



	if (!sock->placement_id) {

		rc = sock->net_impl->get_placement_id(sock, &placement_id);

		if (!rc && (placement_id != 0)) {

			sock->placement_id = placement_id;

		}

	}



	return sock->placement_id;

}



int

spdk_sock_get_optimal_sock_group(struct spdk_sock *sock, struct spdk_sock_group **group)

{

	int placement_id;



	placement_id = sock_get_placement_id(sock);

	if (placement_id != 0) {

		sock_map_lookup(placement_id, group);

		return 0;

	} else {
@@ -508,8 +524,8 @@ spdk_sock_group_add_sock(struct spdk_sock_group *group, struct spdk_sock *sock, 
		return -1;

	}



	rc = sock->net_impl->get_placement_id(sock, &placement_id);

	if (!rc && (placement_id != 0)) {

	placement_id = sock_get_placement_id(sock);

	if (placement_id != 0) {

		rc = sock_map_insert(placement_id, group);

		if (rc < 0) {

			return -1;
@@ -557,8 +573,8 @@ spdk_sock_group_remove_sock(struct spdk_sock_group *group, struct spdk_sock *soc 


	assert(group_impl == sock->group_impl);



	rc = sock->net_impl->get_placement_id(sock, &placement_id);

	if (!rc && (placement_id != 0)) {

	placement_id = sock_get_placement_id(sock);

	if (placement_id != 0) {

		sock_map_release(placement_id);

	}