vppinfra: thread safe clib file

Type: improvement
Change-Id: I37663f0f600b6fe8c76de25e77d0425a10ef9a21
Signed-off-by: Damjan Marion <[email protected]>

diff --git a/src/plugins/dpdk/device/common.c b/src/plugins/dpdk/device/common.c
index d6eed54..5e7961f 100644 (file)
--- 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 58ad4fd..cb42bed 100644 (file)
--- 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 43455d0..af82a8b 100644 (file)
--- 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 ebef215..0d92d03 100644 (file)
--- a/src/plugins/netmap/netmap.c
+++ b/src/plugins/netmap/netmap.c
@@ -22,7 +22,7 @@
 #include <fcntl.h>
 
 #include <vlib/vlib.h>
-#include <vlib/unix/unix.h>
+#include <vlib/file.h>
 #include <vnet/ethernet/ethernet.h>
 
 #include <netmap/net_netmap.h>
@@ -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 d6a9629..47fba14 100644 (file)
--- 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 051c573..debe53f 100644 (file)
--- 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 2edc5ec..4bc4c82 100644 (file)
--- 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 1f3b4e9..396e442 100644 (file)
--- 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 26be8d0..83b6359 100644 (file)
--- 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 7195613..22d6fee 100644 (file)
--- a/src/vppinfra/file.h
+++ b/src/vppinfra/file.h
@@ -42,6 +42,7 @@
 
 #include <vppinfra/socket.h>
 #include <vppinfra/pool.h>
+#include <vppinfra/lock.h>
 #include <termios.h>
 
 
@@ -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:
- */