From: Damjan Marion Date: Tue, 15 Apr 2025 07:25:03 +0000 (+0200) Subject: vppinfra: thread safe clib file X-Git-Tag: v25.10-rc0~87 X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;h=f1586f1431032e7fae653a254d9298a3b7dee015;p=vpp.git vppinfra: thread safe clib file Type: improvement Change-Id: I37663f0f600b6fe8c76de25e77d0425a10ef9a21 Signed-off-by: Damjan Marion --- diff --git a/src/plugins/dpdk/device/common.c b/src/plugins/dpdk/device/common.c index d6eed5441b4..5e7961fe0f1 100644 --- a/src/plugins/dpdk/device/common.c +++ b/src/plugins/dpdk/device/common.c @@ -369,8 +369,7 @@ dpdk_setup_interrupts (dpdk_device_t *xd) if (xd->flags & DPDK_DEVICE_FLAG_INT_UNMASKABLE) { clib_file_main_t *fm = &file_main; - clib_file_t *f = - pool_elt_at_index (fm->file_pool, rxq->clib_file_index); + clib_file_t *f = clib_file_get (fm, rxq->clib_file_index); fm->file_update (f, UNIX_FILE_UPDATE_DELETE); } } diff --git a/src/plugins/dpdk/device/device.c b/src/plugins/dpdk/device/device.c index 58ad4fda0d1..cb42bed3de0 100644 --- a/src/plugins/dpdk/device/device.c +++ b/src/plugins/dpdk/device/device.c @@ -729,7 +729,7 @@ dpdk_interface_rx_mode_change (vnet_main_t *vnm, u32 hw_if_index, u32 qid, else if (mode == VNET_HW_IF_RX_MODE_POLLING) { rxq = vec_elt_at_index (xd->rx_queues, qid); - f = pool_elt_at_index (fm->file_pool, rxq->clib_file_index); + f = clib_file_get (fm, rxq->clib_file_index); fm->file_update (f, UNIX_FILE_UPDATE_DELETE); } else if (!(xd->flags & DPDK_DEVICE_FLAG_INT_UNMASKABLE)) @@ -737,7 +737,7 @@ dpdk_interface_rx_mode_change (vnet_main_t *vnm, u32 hw_if_index, u32 qid, else { rxq = vec_elt_at_index (xd->rx_queues, qid); - f = pool_elt_at_index (fm->file_pool, rxq->clib_file_index); + f = clib_file_get (fm, rxq->clib_file_index); fm->file_update (f, UNIX_FILE_UPDATE_ADD); } if (rv) diff --git a/src/plugins/memif/private.h b/src/plugins/memif/private.h index 43455d00522..af82a8bfaa3 100644 --- a/src/plugins/memif/private.h +++ b/src/plugins/memif/private.h @@ -76,7 +76,7 @@ #define memif_file_del(a) \ do \ { \ - memif_log_debug (0, "clib_file_del idx %u", a - file_main.file_pool); \ + memif_log_debug (0, "clib_file_del idx %u", (a)->index); \ clib_file_del (&file_main, a); \ } \ while (0) diff --git a/src/plugins/netmap/netmap.c b/src/plugins/netmap/netmap.c index ebef215eb3b..0d92d03151c 100644 --- a/src/plugins/netmap/netmap.c +++ b/src/plugins/netmap/netmap.c @@ -22,7 +22,7 @@ #include #include -#include +#include #include #include @@ -53,7 +53,7 @@ close_netmap_if (netmap_main_t * nm, netmap_if_t * nif) { if (nif->clib_file_index != ~0) { - clib_file_del (&file_main, file_main.file_pool + nif->clib_file_index); + clib_file_del_by_index (&file_main, nif->clib_file_index); nif->clib_file_index = ~0; } else if (nif->fd > -1) diff --git a/src/vlib/cli.c b/src/vlib/cli.c index d6a96293869..47fba14365d 100644 --- a/src/vlib/cli.c +++ b/src/vlib/cli.c @@ -1226,14 +1226,13 @@ restart_cmd_fn (vlib_main_t * vm, unformat_input_t * input, { vlib_global_main_t *vgm = vlib_get_global_main (); clib_file_main_t *fm = &file_main; - clib_file_t *f; /* environ(7) does not indicate a header for this */ extern char **environ; /* Close all known open files */ - pool_foreach (f, fm->file_pool) - { + pool_foreach_pointer (f, fm->file_pool) + { if (f->file_descriptor > 2) close(f->file_descriptor); } diff --git a/src/vlib/unix/cli.c b/src/vlib/unix/cli.c index 051c5730aed..debe53f8786 100644 --- a/src/vlib/unix/cli.c +++ b/src/vlib/unix/cli.c @@ -1102,7 +1102,7 @@ unix_vlib_cli_output (uword cli_file_index, u8 * buffer, uword buffer_bytes) clib_file_t *uf; cf = pool_elt_at_index (cm->cli_file_pool, cli_file_index); - uf = pool_elt_at_index (fm->file_pool, cf->clib_file_index); + uf = clib_file_get (fm, cf->clib_file_index); if (cf->no_pager || um->cli_pager_buffer_limit == 0 || cf->height == 0) { @@ -1244,7 +1244,7 @@ unix_cli_file_welcome (unix_cli_main_t * cm, unix_cli_file_t * cf) { unix_main_t *um = &unix_main; clib_file_main_t *fm = &file_main; - clib_file_t *uf = pool_elt_at_index (fm->file_pool, cf->clib_file_index); + clib_file_t *uf = clib_file_get (fm, cf->clib_file_index); unix_cli_banner_t *banner; int i, len; @@ -2460,7 +2460,7 @@ static int unix_cli_line_edit (unix_cli_main_t * cm, unix_main_t * um, clib_file_main_t * fm, unix_cli_file_t * cf) { - clib_file_t *uf = pool_elt_at_index (fm->file_pool, cf->clib_file_index); + clib_file_t *uf = clib_file_get (fm, cf->clib_file_index); int i; for (i = 0; i < vec_len (cf->input_vector); i++) @@ -2628,7 +2628,7 @@ more: /* Re-fetch pointer since pool may have moved. */ cf = pool_elt_at_index (cm->cli_file_pool, cli_file_index); - uf = pool_elt_at_index (fm->file_pool, cf->clib_file_index); + uf = clib_file_get (fm, cf->clib_file_index); done: /* reset vector; we'll re-use it later */ @@ -2707,7 +2707,7 @@ unix_cli_kill (unix_cli_main_t * cm, uword cli_file_index) } cf = pool_elt_at_index (cm->cli_file_pool, cli_file_index); - uf = pool_elt_at_index (fm->file_pool, cf->clib_file_index); + uf = clib_file_get (fm, cf->clib_file_index); /* Quit/EOF on stdin means quit program. */ if (uf->file_descriptor == STDIN_FILENO) @@ -3015,7 +3015,7 @@ unix_cli_listen_read_ready (clib_file_t * uf) cf->height = UNIX_CLI_DEFAULT_TERMINAL_HEIGHT; /* Send the telnet options */ - uf = pool_elt_at_index (fm->file_pool, cf->clib_file_index); + uf = clib_file_get (fm, cf->clib_file_index); unix_vlib_cli_output_raw (cf, uf, charmode_option, ARRAY_LEN (charmode_option)); @@ -3050,7 +3050,7 @@ unix_cli_resize_interrupt (int signum) unix_cli_main_t *cm = &unix_cli_main; unix_cli_file_t *cf = pool_elt_at_index (cm->cli_file_pool, cm->stdin_cli_file_index); - clib_file_t *uf = pool_elt_at_index (fm->file_pool, cf->clib_file_index); + clib_file_t *uf = clib_file_get (fm, cf->clib_file_index); struct winsize ws; (void) signum; @@ -3555,15 +3555,14 @@ unix_show_files (vlib_main_t * vm, { clib_error_t *error = 0; clib_file_main_t *fm = &file_main; - clib_file_t *f; char path[PATH_MAX]; u8 *s = 0; vlib_cli_output (vm, "%3s %6s %12s %12s %12s %-32s %s", "FD", "Thread", "Read", "Write", "Error", "File Name", "Description"); - pool_foreach (f, fm->file_pool) - { + pool_foreach_pointer (f, fm->file_pool) + { int rv; s = format (s, "/proc/self/fd/%d%c", f->file_descriptor, 0); rv = readlink((char *) s, path, PATH_MAX - 1); @@ -3713,7 +3712,7 @@ unix_cli_show_cli_sessions (vlib_main_t * vm, { int j = 0; - uf = pool_elt_at_index (fm->file_pool, cf->clib_file_index); + uf = clib_file_get (fm, cf->clib_file_index); table_format_cell (t, i, j++, "%u", cf->process_node_index); table_format_cell (t, i, j++, "%u", uf->file_descriptor); table_format_cell (t, i, j++, "%v", cf->name); diff --git a/src/vlib/unix/input.c b/src/vlib/unix/input.c index 2edc5ecb760..4bc4c8208ed 100644 --- a/src/vlib/unix/input.c +++ b/src/vlib/unix/input.c @@ -67,7 +67,6 @@ static linux_epoll_main_t *linux_epoll_mains = 0; static void linux_epoll_file_update (clib_file_t * f, clib_file_update_type_t update_type) { - clib_file_main_t *fm = &file_main; linux_epoll_main_t *em = vec_elt_at_index (linux_epoll_mains, f->polling_thread_index); struct epoll_event e = { 0 }; @@ -78,7 +77,7 @@ linux_epoll_file_update (clib_file_t * f, clib_file_update_type_t update_type) e.events |= EPOLLOUT; if (f->flags & UNIX_FILE_EVENT_EDGE_TRIGGERED) e.events |= EPOLLET; - e.data.u32 = f - fm->file_pool; + e.data.u32 = f->index; op = -1; @@ -292,8 +291,8 @@ linux_epoll_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node, * deleted file descriptor. We just deal with it and throw away the * events for the corresponding file descriptor. */ - f = fm->file_pool + i; - if (PREDICT_FALSE (pool_is_free (fm->file_pool, f))) + f = clib_file_get (fm, i); + if (PREDICT_FALSE (!f)) { if (e->events & EPOLLIN) { @@ -326,7 +325,7 @@ linux_epoll_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node, /* Make sure f is valid if the file pool moves */ if (pool_is_free_index (fm->file_pool, i)) continue; - f = pool_elt_at_index (fm->file_pool, i); + f = clib_file_get (fm, i); n_errors += errors[n_errors] != 0; } if (e->events & EPOLLOUT) diff --git a/src/vlib/unix/mc_socket.c b/src/vlib/unix/mc_socket.c index 1f3b4e9a8f1..396e442d4fa 100644 --- a/src/vlib/unix/mc_socket.c +++ b/src/vlib/unix/mc_socket.c @@ -827,8 +827,7 @@ static void * catchup_add_pending_output (mc_socket_catchup_t * c, uword n_bytes, u8 * set_output_vector) { - clib_file_t *uf = pool_elt_at_index (file_main.file_pool, - c->clib_file_index); + clib_file_t *uf = clib_file_get (&file_main, c->clib_file_index); u8 *result = 0; if (set_output_vector) diff --git a/src/vlibmemory/socket_api.c b/src/vlibmemory/socket_api.c index 26be8d09522..83b63592d44 100644 --- a/src/vlibmemory/socket_api.c +++ b/src/vlibmemory/socket_api.c @@ -227,7 +227,7 @@ socket_cleanup_pending_remove_registration_cb (u32 *preg_index) clib_file_main_t *fm = &file_main; u32 pending_remove_file_index = vl_api_registration_file_index (rp); - clib_file_t *zf = fm->file_pool + pending_remove_file_index; + clib_file_t *zf = clib_file_get (fm, pending_remove_file_index); clib_file_del (fm, zf); vl_socket_free_registration_index (rp - socket_main.registration_pool); diff --git a/src/vppinfra/file.h b/src/vppinfra/file.h index 71956137665..22d6feee274 100644 --- a/src/vppinfra/file.h +++ b/src/vppinfra/file.h @@ -42,6 +42,7 @@ #include #include +#include #include @@ -53,13 +54,18 @@ typedef struct clib_file /* Unix file descriptor from open/socket. */ u32 file_descriptor; - u32 flags; + u16 flags; #define UNIX_FILE_DATA_AVAILABLE_TO_WRITE (1 << 0) #define UNIX_FILE_EVENT_EDGE_TRIGGERED (1 << 1) + u16 active : 1; + u16 dont_close : 1; + /* polling thread index */ u32 polling_thread_index; + u32 index; + /* Data available for function's use. */ u64 private_data; @@ -85,77 +91,116 @@ typedef enum typedef struct { /* Pool of files to poll for input/output. */ - clib_file_t *file_pool; + clib_file_t **file_pool; + clib_file_t **pending_free; + + u8 lock; void (*file_update) (clib_file_t * file, clib_file_update_type_t update_type); } clib_file_main_t; +always_inline clib_file_t * +clib_file_get (clib_file_main_t *fm, u32 file_index) +{ + if (pool_is_free_index (fm->file_pool, file_index)) + return 0; + return *pool_elt_at_index (fm->file_pool, file_index); +} + always_inline uword -clib_file_add (clib_file_main_t * um, clib_file_t * template) +clib_file_add (clib_file_main_t *fm, clib_file_t *template) { - clib_file_t *f; - pool_get (um->file_pool, f); + clib_file_t *f, **fp; + u32 index; + + f = clib_mem_alloc_aligned (sizeof (clib_file_t), CLIB_CACHE_LINE_BYTES); + + CLIB_SPINLOCK_LOCK (fm->lock); + pool_get (fm->file_pool, fp); + index = fp - fm->file_pool; + fp[0] = f; + CLIB_SPINLOCK_UNLOCK (fm->lock); + f[0] = template[0]; f->read_events = 0; f->write_events = 0; f->error_events = 0; - um->file_update (f, UNIX_FILE_UPDATE_ADD); - return f - um->file_pool; + f->index = index; + fm->file_update (f, UNIX_FILE_UPDATE_ADD); + f->active = 1; + return index; +} + +always_inline void +clib_file_del (clib_file_main_t *fm, clib_file_t *f) +{ + fm->file_update (f, UNIX_FILE_UPDATE_DELETE); + if (f->dont_close == 0) + close ((int) f->file_descriptor); + + CLIB_SPINLOCK_LOCK (fm->lock); + f->active = 0; + vec_add1 (fm->pending_free, f); + pool_put_index (fm->file_pool, f->index); + CLIB_SPINLOCK_UNLOCK (fm->lock); } always_inline void -clib_file_del (clib_file_main_t * um, clib_file_t * f) +clib_file_del_by_index (clib_file_main_t *fm, uword index) { - um->file_update (f, UNIX_FILE_UPDATE_DELETE); - close (f->file_descriptor); - f->file_descriptor = ~0; - vec_free (f->description); - pool_put (um->file_pool, f); + clib_file_t *f = clib_file_get (fm, index); + clib_file_del (fm, f); } always_inline void -clib_file_del_by_index (clib_file_main_t * um, uword index) +clib_file_free_deleted (clib_file_main_t *fm, u32 thread_index) { - clib_file_t *uf; - uf = pool_elt_at_index (um->file_pool, index); - clib_file_del (um, uf); + u32 n_keep = 0; + + if (vec_len (fm->pending_free) == 0) + return; + + CLIB_SPINLOCK_LOCK (fm->lock); + vec_foreach_pointer (f, fm->pending_free) + { + if (f->polling_thread_index == thread_index) + { + vec_free (f->description); + clib_mem_free (f); + } + else + fm->pending_free[n_keep++] = f; + } + vec_set_len (fm->pending_free, n_keep); + CLIB_SPINLOCK_UNLOCK (fm->lock); } always_inline void -clib_file_set_polling_thread (clib_file_main_t * um, uword index, +clib_file_set_polling_thread (clib_file_main_t *fm, uword index, u32 thread_index) { - clib_file_t *f = pool_elt_at_index (um->file_pool, index); - um->file_update (f, UNIX_FILE_UPDATE_DELETE); + clib_file_t *f = clib_file_get (fm, index); + fm->file_update (f, UNIX_FILE_UPDATE_DELETE); f->polling_thread_index = thread_index; - um->file_update (f, UNIX_FILE_UPDATE_ADD); + fm->file_update (f, UNIX_FILE_UPDATE_ADD); } always_inline uword -clib_file_set_data_available_to_write (clib_file_main_t * um, - u32 clib_file_index, - uword is_available) +clib_file_set_data_available_to_write (clib_file_main_t *fm, + u32 clib_file_index, uword is_available) { - clib_file_t *uf = pool_elt_at_index (um->file_pool, clib_file_index); - uword was_available = (uf->flags & UNIX_FILE_DATA_AVAILABLE_TO_WRITE); + clib_file_t *f = clib_file_get (fm, clib_file_index); + uword was_available = (f->flags & UNIX_FILE_DATA_AVAILABLE_TO_WRITE); if ((was_available != 0) != (is_available != 0)) { - uf->flags ^= UNIX_FILE_DATA_AVAILABLE_TO_WRITE; - um->file_update (uf, UNIX_FILE_UPDATE_MODIFY); + f->flags ^= UNIX_FILE_DATA_AVAILABLE_TO_WRITE; + fm->file_update (f, UNIX_FILE_UPDATE_MODIFY); } return was_available != 0; } -always_inline clib_file_t * -clib_file_get (clib_file_main_t * fm, u32 file_index) -{ - if (pool_is_free_index (fm->file_pool, file_index)) - return 0; - return pool_elt_at_index (fm->file_pool, file_index); -} - always_inline clib_error_t * clib_file_write (clib_file_t * f) { @@ -166,11 +211,3 @@ clib_file_write (clib_file_t * f) u8 *clib_file_get_resolved_basename (char *fmt, ...); #endif /* included_clib_file_h */ - -/* - * fd.io coding-style-patch-verification: ON - * - * Local Variables: - * eval: (c-set-style "gnu") - * End: - */