diff --git a/include/spdk_internal/sock.h b/include/spdk_internal/sock.h index 13249d007..d32ed0bab 100644 --- a/include/spdk_internal/sock.h +++ b/include/spdk_internal/sock.h @@ -84,6 +84,11 @@ struct spdk_sock_group_impl { STAILQ_ENTRY(spdk_sock_group_impl) link; }; +struct spdk_sock_map { + STAILQ_HEAD(, spdk_sock_placement_id_entry) entries; + pthread_mutex_t mtx; +}; + struct spdk_net_impl { const char *name; int priority; @@ -109,7 +114,7 @@ struct spdk_net_impl { bool (*is_ipv4)(struct spdk_sock *sock); bool (*is_connected)(struct spdk_sock *sock); - int (*get_placement_id)(struct spdk_sock *sock, int *placement_id); + struct spdk_sock_group *(*group_impl_get_optimal)(struct spdk_sock *sock); struct spdk_sock_group_impl *(*group_impl_create)(void); int (*group_impl_add_sock)(struct spdk_sock_group_impl *group, struct spdk_sock *sock); int (*group_impl_remove_sock)(struct spdk_sock_group_impl *group, struct spdk_sock *sock); @@ -300,8 +305,6 @@ spdk_sock_get_placement_id(int fd, enum spdk_placement_mode mode, int *placement } } -extern struct spdk_sock_map g_map; - /** * Insert a group into the placement map. * If the group is already in the map, take a reference. @@ -321,6 +324,11 @@ void spdk_sock_map_release(struct spdk_sock_map *map, int placement_id); int spdk_sock_map_lookup(struct spdk_sock_map *map, int placement_id, struct spdk_sock_group **group); +/** + * Clean up all memory associated with the given map + */ +void spdk_sock_map_cleanup(struct spdk_sock_map *map); + #ifdef __cplusplus } #endif diff --git a/lib/sock/sock.c b/lib/sock/sock.c index 971c9110a..2d0ecc395 100644 --- a/lib/sock/sock.c +++ b/lib/sock/sock.c @@ -52,16 +52,6 @@ struct spdk_sock_placement_id_entry { STAILQ_ENTRY(spdk_sock_placement_id_entry) link; }; -struct spdk_sock_map { - STAILQ_HEAD(, spdk_sock_placement_id_entry) entries; - pthread_mutex_t mtx; -}; - -struct spdk_sock_map g_map = { - .entries = STAILQ_HEAD_INITIALIZER(g_map.entries), - .mtx = PTHREAD_MUTEX_INITIALIZER -}; - int spdk_sock_map_insert(struct spdk_sock_map *map, int placement_id, struct spdk_sock_group *group) { @@ -150,45 +140,26 @@ spdk_sock_map_lookup(struct spdk_sock_map *map, int placement_id, struct spdk_so return rc; } -__attribute((destructor)) static void -sock_map_cleanup(void) +void +spdk_sock_map_cleanup(struct spdk_sock_map *map) { struct spdk_sock_placement_id_entry *entry, *tmp; - pthread_mutex_lock(&g_map.mtx); - STAILQ_FOREACH_SAFE(entry, &g_map.entries, link, tmp) { - STAILQ_REMOVE(&g_map.entries, entry, spdk_sock_placement_id_entry, link); + pthread_mutex_lock(&map->mtx); + STAILQ_FOREACH_SAFE(entry, &map->entries, link, tmp) { + STAILQ_REMOVE(&map->entries, entry, spdk_sock_placement_id_entry, link); free(entry); } - pthread_mutex_unlock(&g_map.mtx); -} - -static int -sock_get_placement_id(struct spdk_sock *sock) -{ - int rc; - int placement_id; - - rc = sock->net_impl->get_placement_id(sock, &placement_id); - if (rc) { - placement_id = -1; - } - - return placement_id; + pthread_mutex_unlock(&map->mtx); } int spdk_sock_get_optimal_sock_group(struct spdk_sock *sock, struct spdk_sock_group **group) { - int placement_id = -1; + assert(group != NULL); - placement_id = sock_get_placement_id(sock); - if (placement_id != -1) { - spdk_sock_map_lookup(&g_map, placement_id, group); - return 0; - } else { - return -1; - } + *group = sock->net_impl->group_impl_get_optimal(sock); + return 0; } int @@ -488,9 +459,6 @@ spdk_sock_group_create(void *ctx) struct spdk_net_impl *impl = NULL; struct spdk_sock_group *group; struct spdk_sock_group_impl *group_impl; - struct spdk_sock_impl_opts sock_opts = {}; - size_t sock_len; - bool enable_incoming_cpu = false; group = calloc(1, sizeof(*group)); if (group == NULL) { @@ -506,22 +474,11 @@ spdk_sock_group_create(void *ctx) TAILQ_INIT(&group_impl->socks); group_impl->net_impl = impl; group_impl->group = group; - - sock_len = sizeof(sock_opts); - spdk_sock_impl_get_opts(impl->name, &sock_opts, &sock_len); - if (sock_opts.enable_placement_id == PLACEMENT_CPU) { - enable_incoming_cpu = true; - } } } group->ctx = ctx; - /* if any net_impl is configured to use SO_INCOMING_CPU, initialize the sock map */ - if (enable_incoming_cpu) { - spdk_sock_map_insert(&g_map, spdk_env_get_current_core(), group); - } - return group; } @@ -540,7 +497,7 @@ spdk_sock_group_add_sock(struct spdk_sock_group *group, struct spdk_sock *sock, spdk_sock_cb cb_fn, void *cb_arg) { struct spdk_sock_group_impl *group_impl = NULL; - int rc, placement_id = 0; + int rc; if (cb_fn == NULL) { errno = EINVAL; @@ -576,15 +533,6 @@ spdk_sock_group_add_sock(struct spdk_sock_group *group, struct spdk_sock *sock, sock->cb_fn = cb_fn; sock->cb_arg = cb_arg; - placement_id = sock_get_placement_id(sock); - if (placement_id != -1) { - rc = spdk_sock_map_insert(&g_map, placement_id, group); - if (rc != 0) { - SPDK_ERRLOG("Failed to insert sock group into map: %d", rc); - /* Do not treat this as an error. The system will continue running. */ - } - } - return 0; } @@ -592,7 +540,7 @@ int spdk_sock_group_remove_sock(struct spdk_sock_group *group, struct spdk_sock *sock) { struct spdk_sock_group_impl *group_impl = NULL; - int rc, placement_id = 0; + int rc; STAILQ_FOREACH_FROM(group_impl, &group->group_impls, link) { if (sock->net_impl == group_impl->net_impl) { @@ -607,11 +555,6 @@ spdk_sock_group_remove_sock(struct spdk_sock_group *group, struct spdk_sock *soc assert(group_impl == sock->group_impl); - placement_id = sock_get_placement_id(sock); - if (placement_id != -1) { - spdk_sock_map_release(&g_map, placement_id); - } - rc = group_impl->net_impl->group_impl_remove_sock(group_impl, sock); if (rc == 0) { TAILQ_REMOVE(&group_impl->socks, sock, link); @@ -693,9 +636,6 @@ spdk_sock_group_close(struct spdk_sock_group **group) { struct spdk_sock_group_impl *group_impl = NULL, *tmp; int rc; - struct spdk_sock_impl_opts sock_opts = {}; - size_t sock_len; - bool enable_incoming_cpu = false; if (*group == NULL) { errno = EBADF; @@ -709,19 +649,6 @@ spdk_sock_group_close(struct spdk_sock_group **group) } } - STAILQ_FOREACH_SAFE(group_impl, &(*group)->group_impls, link, tmp) { - sock_len = sizeof(sock_opts); - spdk_sock_impl_get_opts(group_impl->net_impl->name, &sock_opts, &sock_len); - if (sock_opts.enable_placement_id == PLACEMENT_CPU) { - enable_incoming_cpu = true; - break; - } - } - - if (enable_incoming_cpu) { - spdk_sock_map_release(&g_map, spdk_env_get_current_core()); - } - STAILQ_FOREACH_SAFE(group_impl, &(*group)->group_impls, link, tmp) { rc = group_impl->net_impl->group_impl_close(group_impl); if (rc != 0) { diff --git a/lib/sock/spdk_sock.map b/lib/sock/spdk_sock.map index 14c743a46..64fa372c9 100644 --- a/lib/sock/spdk_sock.map +++ b/lib/sock/spdk_sock.map @@ -46,6 +46,7 @@ spdk_sock_map_insert; spdk_sock_map_release; spdk_sock_map_lookup; + spdk_sock_map_cleanup; local: *; }; diff --git a/module/sock/posix/posix.c b/module/sock/posix/posix.c index a5612ab5c..e013d4211 100644 --- a/module/sock/posix/posix.c +++ b/module/sock/posix/posix.c @@ -45,6 +45,7 @@ #include #endif +#include "spdk/env.h" #include "spdk/log.h" #include "spdk/pipe.h" #include "spdk/sock.h" @@ -92,6 +93,17 @@ static struct spdk_sock_impl_opts g_spdk_posix_sock_impl_opts = { .enable_placement_id = PLACEMENT_NONE, }; +static struct spdk_sock_map g_map = { + .entries = STAILQ_HEAD_INITIALIZER(g_map.entries), + .mtx = PTHREAD_MUTEX_INITIALIZER +}; + +__attribute((destructor)) static void +posix_sock_map_cleanup(void) +{ + spdk_sock_map_cleanup(&g_map); +} + static int get_addr_str(struct sockaddr *sa, char *host, size_t hlen) { @@ -1098,16 +1110,18 @@ posix_sock_is_connected(struct spdk_sock *_sock) return true; } -static int -posix_sock_get_placement_id(struct spdk_sock *_sock, int *placement_id) +static struct spdk_sock_group * +posix_sock_group_impl_get_optimal(struct spdk_sock *_sock) { struct spdk_posix_sock *sock = __posix_sock(_sock); + struct spdk_sock_group *group; - assert(placement_id); + if (sock->placement_id != -1) { + spdk_sock_map_lookup(&g_map, sock->placement_id, &group); + return group; + } - *placement_id = sock->placement_id; - - return 0; + return NULL; } static struct spdk_sock_group_impl * @@ -1135,6 +1149,10 @@ posix_sock_group_impl_create(void) group_impl->fd = fd; TAILQ_INIT(&group_impl->pending_events); + if (g_spdk_posix_sock_impl_opts.enable_placement_id == PLACEMENT_CPU) { + spdk_sock_map_insert(&g_map, spdk_env_get_current_core(), group_impl->base.group); + } + return &group_impl->base; } @@ -1171,6 +1189,14 @@ posix_sock_group_impl_add_sock(struct spdk_sock_group_impl *_group, struct spdk_ TAILQ_INSERT_TAIL(&group->pending_events, sock, link); } + if (sock->placement_id != -1) { + rc = spdk_sock_map_insert(&g_map, sock->placement_id, group->base.group); + if (rc != 0) { + SPDK_ERRLOG("Failed to insert sock group into map: %d", rc); + /* Do not treat this as an error. The system will continue running. */ + } + } + return rc; } @@ -1186,6 +1212,10 @@ posix_sock_group_impl_remove_sock(struct spdk_sock_group_impl *_group, struct sp sock->pending_events = false; } + if (sock->placement_id != -1) { + spdk_sock_map_release(&g_map, sock->placement_id); + } + #if defined(SPDK_EPOLL) struct epoll_event event; @@ -1348,6 +1378,10 @@ posix_sock_group_impl_close(struct spdk_sock_group_impl *_group) struct spdk_posix_sock_group_impl *group = __posix_group_impl(_group); int rc; + if (g_spdk_posix_sock_impl_opts.enable_placement_id == PLACEMENT_CPU) { + spdk_sock_map_release(&g_map, spdk_env_get_current_core()); + } + rc = close(group->fd); free(group); return rc; @@ -1432,7 +1466,7 @@ static struct spdk_net_impl g_posix_net_impl = { .is_ipv6 = posix_sock_is_ipv6, .is_ipv4 = posix_sock_is_ipv4, .is_connected = posix_sock_is_connected, - .get_placement_id = posix_sock_get_placement_id, + .group_impl_get_optimal = posix_sock_group_impl_get_optimal, .group_impl_create = posix_sock_group_impl_create, .group_impl_add_sock = posix_sock_group_impl_add_sock, .group_impl_remove_sock = posix_sock_group_impl_remove_sock, diff --git a/module/sock/uring/uring.c b/module/sock/uring/uring.c index 2a9e04f5c..d088d1a9c 100644 --- a/module/sock/uring/uring.c +++ b/module/sock/uring/uring.c @@ -38,6 +38,7 @@ #include #include "spdk/barrier.h" +#include "spdk/env.h" #include "spdk/log.h" #include "spdk/pipe.h" #include "spdk/sock.h" @@ -106,6 +107,17 @@ static struct spdk_sock_impl_opts g_spdk_uring_sock_impl_opts = { .enable_placement_id = PLACEMENT_NONE, }; +static struct spdk_sock_map g_map = { + .entries = STAILQ_HEAD_INITIALIZER(g_map.entries), + .mtx = PTHREAD_MUTEX_INITIALIZER +}; + +__attribute((destructor)) static void +uring_sock_map_cleanup(void) +{ + spdk_sock_map_cleanup(&g_map); +} + #define SPDK_URING_SOCK_REQUEST_IOV(req) ((struct iovec *)((uint8_t *)req + sizeof(struct spdk_sock_request))) static int @@ -1094,16 +1106,18 @@ uring_sock_is_connected(struct spdk_sock *_sock) return true; } -static int -uring_sock_get_placement_id(struct spdk_sock *_sock, int *placement_id) +static struct spdk_sock_group * +uring_sock_group_impl_get_optimal(struct spdk_sock *_sock) { struct spdk_uring_sock *sock = __uring_sock(_sock); + struct spdk_sock_group *group; - assert(placement_id); + if (sock->placement_id != -1) { + spdk_sock_map_lookup(&g_map, sock->placement_id, &group); + return group; + } - *placement_id = sock->placement_id; - - return 0; + return NULL; } static struct spdk_sock_group_impl * @@ -1127,6 +1141,10 @@ uring_sock_group_impl_create(void) TAILQ_INIT(&group_impl->pending_recv); + if (g_spdk_uring_sock_impl_opts.enable_placement_id == PLACEMENT_CPU) { + spdk_sock_map_insert(&g_map, spdk_env_get_current_core(), group_impl->base.group); + } + return &group_impl->base; } @@ -1136,6 +1154,7 @@ uring_sock_group_impl_add_sock(struct spdk_sock_group_impl *_group, { struct spdk_uring_sock *sock = __uring_sock(_sock); struct spdk_uring_sock_group_impl *group = __uring_group_impl(_group); + int rc; sock->group = group; sock->write_task.sock = sock; @@ -1155,6 +1174,14 @@ uring_sock_group_impl_add_sock(struct spdk_sock_group_impl *_group, TAILQ_INSERT_TAIL(&group->pending_recv, sock, link); } + if (sock->placement_id != -1) { + rc = spdk_sock_map_insert(&g_map, sock->placement_id, group->base.group); + if (rc != 0) { + SPDK_ERRLOG("Failed to insert sock group into map: %d", rc); + /* Do not treat this as an error. The system will continue running. */ + } + } + return 0; } @@ -1236,6 +1263,10 @@ uring_sock_group_impl_remove_sock(struct spdk_sock_group_impl *_group, } assert(sock->pending_recv == false); + if (sock->placement_id != -1) { + spdk_sock_map_release(&g_map, sock->placement_id); + } + sock->group = NULL; return 0; } @@ -1254,6 +1285,10 @@ uring_sock_group_impl_close(struct spdk_sock_group_impl *_group) io_uring_queue_exit(&group->uring); + if (g_spdk_uring_sock_impl_opts.enable_placement_id == PLACEMENT_CPU) { + spdk_sock_map_release(&g_map, spdk_env_get_current_core()); + } + free(group); return 0; } @@ -1346,7 +1381,7 @@ static struct spdk_net_impl g_uring_net_impl = { .is_ipv6 = uring_sock_is_ipv6, .is_ipv4 = uring_sock_is_ipv4, .is_connected = uring_sock_is_connected, - .get_placement_id = uring_sock_get_placement_id, + .group_impl_get_optimal = uring_sock_group_impl_get_optimal, .group_impl_create = uring_sock_group_impl_create, .group_impl_add_sock = uring_sock_group_impl_add_sock, .group_impl_remove_sock = uring_sock_group_impl_remove_sock, diff --git a/test/unit/lib/sock/posix.c/posix_ut.c b/test/unit/lib/sock/posix.c/posix_ut.c index 498a37628..5fe715cb7 100644 --- a/test/unit/lib/sock/posix.c/posix_ut.c +++ b/test/unit/lib/sock/posix.c/posix_ut.c @@ -38,8 +38,16 @@ #include "spdk_cunit.h" +#include "common/lib/test_env.c" #include "sock/posix/posix.c" +DEFINE_STUB(spdk_sock_map_insert, int, (struct spdk_sock_map *map, int placement_id, + struct spdk_sock_group *group), 0); +DEFINE_STUB_V(spdk_sock_map_release, (struct spdk_sock_map *map, int placement_id)); +DEFINE_STUB(spdk_sock_map_lookup, int, (struct spdk_sock_map *map, int placement_id, + struct spdk_sock_group **group), 0); +DEFINE_STUB_V(spdk_sock_map_cleanup, (struct spdk_sock_map *map)); + DEFINE_STUB_V(spdk_net_impl_register, (struct spdk_net_impl *impl, int priority)); DEFINE_STUB(spdk_sock_close, int, (struct spdk_sock **s), 0); diff --git a/test/unit/lib/sock/sock.c/sock_ut.c b/test/unit/lib/sock/sock.c/sock_ut.c index 3e936d9e3..e3968074b 100644 --- a/test/unit/lib/sock/sock.c/sock_ut.c +++ b/test/unit/lib/sock/sock.c/sock_ut.c @@ -290,10 +290,10 @@ spdk_ut_sock_is_connected(struct spdk_sock *_sock) return (sock->peer != NULL); } -static int -spdk_ut_sock_get_placement_id(struct spdk_sock *_sock, int *placement_id) +static struct spdk_sock_group * +spdk_ut_sock_group_impl_get_optimal(struct spdk_sock *_sock) { - return -1; + return NULL; } static struct spdk_sock_group_impl * @@ -371,7 +371,7 @@ static struct spdk_net_impl g_ut_net_impl = { .is_ipv6 = spdk_ut_sock_is_ipv6, .is_ipv4 = spdk_ut_sock_is_ipv4, .is_connected = spdk_ut_sock_is_connected, - .get_placement_id = spdk_ut_sock_get_placement_id, + .group_impl_get_optimal = spdk_ut_sock_group_impl_get_optimal, .group_impl_create = spdk_ut_sock_group_impl_create, .group_impl_add_sock = spdk_ut_sock_group_impl_add_sock, .group_impl_remove_sock = spdk_ut_sock_group_impl_remove_sock, diff --git a/test/unit/lib/sock/uring.c/uring_ut.c b/test/unit/lib/sock/uring.c/uring_ut.c index 4ff9d3719..d4cc893a3 100644 --- a/test/unit/lib/sock/uring.c/uring_ut.c +++ b/test/unit/lib/sock/uring.c/uring_ut.c @@ -38,8 +38,16 @@ #include "spdk_cunit.h" +#include "common/lib/test_env.c" #include "sock/uring/uring.c" +DEFINE_STUB(spdk_sock_map_insert, int, (struct spdk_sock_map *map, int placement_id, + struct spdk_sock_group *group), 0); +DEFINE_STUB_V(spdk_sock_map_release, (struct spdk_sock_map *map, int placement_id)); +DEFINE_STUB(spdk_sock_map_lookup, int, (struct spdk_sock_map *map, int placement_id, + struct spdk_sock_group **group), 0); +DEFINE_STUB_V(spdk_sock_map_cleanup, (struct spdk_sock_map *map)); + DEFINE_STUB_V(spdk_net_impl_register, (struct spdk_net_impl *impl, int priority)); DEFINE_STUB(spdk_sock_close, int, (struct spdk_sock **s), 0); DEFINE_STUB(__io_uring_get_cqe, int, (struct io_uring *ring, struct io_uring_cqe **cqe_ptr,