From ff40d8f1b2f4efcf05f21934b423dce8aba8b652 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Tue, 11 Aug 2020 22:05:28 -0700 Subject: vcl: mt detection and cleanup Type: improvement Signed-off-by: Florin Coras Change-Id: I521c110fd4d7445bd585c96d4c768f16a0a7d3b8 --- src/vcl/ldp.c | 7 ++- src/vcl/vcl_cfg.c | 7 ++- src/vcl/vcl_locked.c | 160 +++++++++++++++++++++++++++++++------------------- src/vcl/vcl_private.h | 2 +- 4 files changed, 109 insertions(+), 67 deletions(-) diff --git a/src/vcl/ldp.c b/src/vcl/ldp.c index fddf45cd502..a4b66feedfc 100644 --- a/src/vcl/ldp.c +++ b/src/vcl/ldp.c @@ -2438,9 +2438,10 @@ ldp_epoll_pwait_eventfd (int epfd, struct epoll_event *events, return -1; } - if (vls_mt_wrk_supported ()) - if (PREDICT_FALSE (vppcom_worker_index () == ~0)) - vls_register_vcl_worker (); + /* Make sure the vcl worker is valid. Could be that epoll fd was created on + * one thread but it is now used on another */ + if (PREDICT_FALSE (vppcom_worker_index () == ~0)) + vls_register_vcl_worker (); ldpw = ldp_worker_get_current (); if (epfd == ldpw->vcl_mq_epfd) diff --git a/src/vcl/vcl_cfg.c b/src/vcl/vcl_cfg.c index a94c874532f..f7e271bbdc9 100644 --- a/src/vcl/vcl_cfg.c +++ b/src/vcl/vcl_cfg.c @@ -498,10 +498,11 @@ vppcom_cfg_read_file (char *conf_fname) VCFG_DBG (0, "VCL<%d>: configured tls-engine %u (0x%x)", getpid (), vcl_cfg->tls_engine, vcl_cfg->tls_engine); } - else if (unformat (line_input, "multi-thread")) + else if (unformat (line_input, "multi-thread-workers")) { - vcl_cfg->mt_supported = 1; - VCFG_DBG (0, "VCL<%d>: configured with multithread", getpid ()); + vcl_cfg->mt_wrk_supported = 1; + VCFG_DBG (0, "VCL<%d>: configured with multithread workers", + getpid ()); } else if (unformat (line_input, "}")) { diff --git a/src/vcl/vcl_locked.c b/src/vcl/vcl_locked.c index 02da8cd2cf5..da4522aed70 100644 --- a/src/vcl/vcl_locked.c +++ b/src/vcl/vcl_locked.c @@ -169,28 +169,28 @@ vls_shared_data_pool_runlock (void) } static inline void -vls_table_rlock (void) +vls_mt_table_rlock (void) { if (vlsl->vls_mt_n_threads > 1) clib_rwlock_reader_lock (&vlsm->vls_table_lock); } static inline void -vls_table_runlock (void) +vls_mt_table_runlock (void) { if (vlsl->vls_mt_n_threads > 1) clib_rwlock_reader_unlock (&vlsm->vls_table_lock); } static inline void -vls_table_wlock (void) +vls_mt_table_wlock (void) { if (vlsl->vls_mt_n_threads > 1) clib_rwlock_writer_lock (&vlsm->vls_table_lock); } static inline void -vls_table_wunlock (void) +vls_mt_table_wunlock (void) { if (vlsl->vls_mt_n_threads > 1) clib_rwlock_writer_unlock (&vlsm->vls_table_lock); @@ -214,6 +214,10 @@ static void vls_mt_add (void) { vlsl->vls_mt_n_threads += 1; + + /* If multi-thread workers are supported, for each new thread register a new + * vcl worker with vpp. Otherwise, all threads use the same vcl worker, so + * update the vcl worker's thread local worker index variable */ if (vls_mt_wrk_supported ()) vls_register_vcl_worker (); else @@ -282,7 +286,7 @@ vls_to_sh_tu (vcl_locked_session_t * vls) { vcl_session_handle_t sh; sh = vls_to_sh (vls); - vls_table_runlock (); + vls_mt_table_runlock (); return sh; } @@ -323,7 +327,7 @@ vls_alloc (vcl_session_handle_t sh) vls_worker_t *wrk = vls_worker_get_current (); vcl_locked_session_t *vls; - vls_table_wlock (); + vls_mt_table_wlock (); pool_get_zero (wrk->vls_pool, vls); vls->session_index = vppcom_session_index (sh); @@ -340,7 +344,7 @@ vls_alloc (vcl_session_handle_t sh) } clib_spinlock_init (&vls->lock); - vls_table_wunlock (); + vls_mt_table_wunlock (); return vls->vls_index; } @@ -380,10 +384,10 @@ static vcl_locked_session_t * vls_get_w_dlock (vls_handle_t vlsh) { vcl_locked_session_t *vls; - vls_table_rlock (); + vls_mt_table_rlock (); vls = vls_get_and_lock (vlsh); if (!vls) - vls_table_runlock (); + vls_mt_table_runlock (); return vls; } @@ -391,17 +395,17 @@ static inline void vls_get_and_unlock (vls_handle_t vlsh) { vcl_locked_session_t *vls; - vls_table_rlock (); + vls_mt_table_rlock (); vls = vls_get (vlsh); vls_unlock (vls); - vls_table_runlock (); + vls_mt_table_runlock (); } static inline void vls_dunlock (vcl_locked_session_t * vls) { vls_unlock (vls); - vls_table_runlock (); + vls_mt_table_runlock (); } static vcl_locked_session_t * @@ -448,9 +452,9 @@ vls_session_index_to_vlsh (uint32_t session_index) { vls_handle_t vlsh; - vls_table_rlock (); + vls_mt_table_rlock (); vlsh = vls_si_to_vlsh (session_index); - vls_table_runlock (); + vls_mt_table_runlock (); return vlsh; } @@ -826,8 +830,15 @@ vls_mt_rel_locks (int locks_acq) vls_mt_create_unlock (); } +static inline u8 +vls_mt_session_should_migrate (vcl_locked_session_t * vls) +{ + return (vls_mt_wrk_supported () + && vls->worker_index != vcl_get_worker_index ()); +} + static void -vls_session_migrate (vcl_locked_session_t * vls) +vls_mt_session_migrate (vcl_locked_session_t * vls) { u32 wrk_index = vcl_get_worker_index (); vcl_worker_t *wrk; @@ -835,11 +846,12 @@ vls_session_migrate (vcl_locked_session_t * vls) vcl_session_t *session; uword *p; - if (!vls_mt_wrk_supported ()) - return; + ASSERT (vls_mt_wrk_supported () && vls->worker_index != wrk_index); - if (PREDICT_TRUE (vls->worker_index == wrk_index)) - return; + /* + * VCL session on current vcl worker already allocated. Update current + * owner worker and index and return + */ if ((p = hash_get (vls->vcl_wrk_index_to_session_index, wrk_index))) { vls->worker_index = wrk_index; @@ -847,7 +859,11 @@ vls_session_migrate (vcl_locked_session_t * vls) return; } - /* migrate from orignal vls */ + /* + * Ask vcl worker that owns the original vcl session to clone it into + * current vcl worker session pool + */ + if (!(p = hash_get (vls->vcl_wrk_index_to_session_index, vls->owner_vcl_wrk_index))) { @@ -888,19 +904,30 @@ vls_session_migrate (vcl_locked_session_t * vls) } } -#define vls_mt_guard(_vls, _op) \ - int _locks_acq = 0; \ - if (PREDICT_FALSE (vcl_get_worker_index () == ~0)) \ - vls_mt_add (); \ - if (PREDICT_FALSE (_vls && vls_mt_wrk_supported () && \ - ((vcl_locked_session_t *)_vls)->worker_index != \ - vcl_get_worker_index ())) \ - vls_session_migrate (_vls); \ - if (PREDICT_FALSE (vlsl->vls_mt_n_threads > 1)) \ - vls_mt_acq_locks (_vls, _op, &_locks_acq); \ +static inline void +vls_mt_detect (void) +{ + if (PREDICT_FALSE (vcl_get_worker_index () == ~0)) + vls_mt_add (); +} -#define vls_mt_unguard() \ - if (PREDICT_FALSE (_locks_acq)) \ +#define vls_mt_guard(_vls, _op) \ + int _locks_acq = 0; \ + if (vls_mt_wrk_supported ()) \ + { \ + if (PREDICT_FALSE (_vls \ + && ((vcl_locked_session_t *)_vls)->worker_index != \ + vcl_get_worker_index ())) \ + vls_mt_session_migrate (_vls); \ + } \ + else \ + { \ + if (PREDICT_FALSE (vlsl->vls_mt_n_threads > 1)) \ + vls_mt_acq_locks (_vls, _op, &_locks_acq); \ + } \ + +#define vls_mt_unguard() \ + if (PREDICT_FALSE (_locks_acq)) \ vls_mt_rel_locks (_locks_acq) int @@ -909,6 +936,7 @@ vls_write (vls_handle_t vlsh, void *buf, size_t nbytes) vcl_locked_session_t *vls; int rv; + vls_mt_detect (); if (!(vls = vls_get_w_dlock (vlsh))) return VPPCOM_EBADFD; @@ -925,6 +953,7 @@ vls_write_msg (vls_handle_t vlsh, void *buf, size_t nbytes) vcl_locked_session_t *vls; int rv; + vls_mt_detect (); if (!(vls = vls_get_w_dlock (vlsh))) return VPPCOM_EBADFD; vls_mt_guard (vls, VLS_MT_OP_WRITE); @@ -941,6 +970,7 @@ vls_sendto (vls_handle_t vlsh, void *buf, int buflen, int flags, vcl_locked_session_t *vls; int rv; + vls_mt_detect (); if (!(vls = vls_get_w_dlock (vlsh))) return VPPCOM_EBADFD; vls_mt_guard (vls, VLS_MT_OP_WRITE); @@ -956,6 +986,7 @@ vls_read (vls_handle_t vlsh, void *buf, size_t nbytes) vcl_locked_session_t *vls; int rv; + vls_mt_detect (); if (!(vls = vls_get_w_dlock (vlsh))) return VPPCOM_EBADFD; vls_mt_guard (vls, VLS_MT_OP_READ); @@ -972,6 +1003,7 @@ vls_recvfrom (vls_handle_t vlsh, void *buffer, uint32_t buflen, int flags, vcl_locked_session_t *vls; int rv; + vls_mt_detect (); if (!(vls = vls_get_w_dlock (vlsh))) return VPPCOM_EBADFD; vls_mt_guard (vls, VLS_MT_OP_READ); @@ -988,12 +1020,11 @@ vls_attr (vls_handle_t vlsh, uint32_t op, void *buffer, uint32_t * buflen) vcl_locked_session_t *vls; int rv; - if (PREDICT_FALSE (vcl_get_worker_index () == ~0)) - vls_mt_add (); - + vls_mt_detect (); if (!(vls = vls_get_w_dlock (vlsh))) return VPPCOM_EBADFD; - vls_session_migrate (vls); + if (vls_mt_session_should_migrate (vls)) + vls_mt_session_migrate (vls); rv = vppcom_session_attr (vls_to_sh_tu (vls), op, buffer, buflen); vls_get_and_unlock (vlsh); return rv; @@ -1005,6 +1036,7 @@ vls_bind (vls_handle_t vlsh, vppcom_endpt_t * ep) vcl_locked_session_t *vls; int rv; + vls_mt_detect (); if (!(vls = vls_get_w_dlock (vlsh))) return VPPCOM_EBADFD; rv = vppcom_session_bind (vls_to_sh_tu (vls), ep); @@ -1018,6 +1050,7 @@ vls_listen (vls_handle_t vlsh, int q_len) vcl_locked_session_t *vls; int rv; + vls_mt_detect (); if (!(vls = vls_get_w_dlock (vlsh))) return VPPCOM_EBADFD; vls_mt_guard (vls, VLS_MT_OP_XPOLL); @@ -1033,6 +1066,7 @@ vls_connect (vls_handle_t vlsh, vppcom_endpt_t * server_ep) vcl_locked_session_t *vls; int rv; + vls_mt_detect (); if (!(vls = vls_get_w_dlock (vlsh))) return VPPCOM_EBADFD; vls_mt_guard (vls, VLS_MT_OP_XPOLL); @@ -1093,6 +1127,7 @@ vls_accept (vls_handle_t listener_vlsh, vppcom_endpt_t * ep, int flags) vcl_locked_session_t *vls; int sh; + vls_mt_detect (); if (!(vls = vls_get_w_dlock (listener_vlsh))) return VPPCOM_EBADFD; if (vcl_n_workers () > 1) @@ -1115,6 +1150,7 @@ vls_create (uint8_t proto, uint8_t is_nonblocking) vcl_session_handle_t sh; vls_handle_t vlsh; + vls_mt_detect (); vls_mt_guard (0, VLS_MT_OP_SPOOL); sh = vppcom_session_create (proto, is_nonblocking); vls_mt_unguard (); @@ -1129,21 +1165,20 @@ vls_create (uint8_t proto, uint8_t is_nonblocking) } static void -vls_migrate_session_close (vcl_locked_session_t * vls) +vls_mt_session_cleanup (vcl_locked_session_t * vls) { - u32 session_index, wrk_index; + u32 session_index, wrk_index, current_vcl_wrk; vcl_worker_t *wrk = vcl_worker_get_current (); - if (!vls_mt_wrk_supported ()) - return; + ASSERT (vls_mt_wrk_supported ()); + + current_vcl_wrk = vcl_get_worker_index (); /* *INDENT-OFF* */ hash_foreach (wrk_index, session_index, vls->vcl_wrk_index_to_session_index, ({ - if (vcl_get_worker_index () != wrk_index) - { - vls_send_session_cleanup_rpc (wrk, wrk_index, session_index); - } + if (current_vcl_wrk != wrk_index) + vls_send_session_cleanup_rpc (wrk, wrk_index, session_index); })); /* *INDENT-ON* */ hash_free (vls->vcl_wrk_index_to_session_index); @@ -1155,12 +1190,13 @@ vls_close (vls_handle_t vlsh) vcl_locked_session_t *vls; int rv; - vls_table_wlock (); + vls_mt_detect (); + vls_mt_table_wlock (); vls = vls_get_and_lock (vlsh); if (!vls) { - vls_table_wunlock (); + vls_mt_table_wunlock (); return VPPCOM_EBADFD; } @@ -1171,12 +1207,13 @@ vls_close (vls_handle_t vlsh) else rv = vppcom_session_close (vls_to_sh (vls)); - vls_migrate_session_close (vls); + if (vls_mt_wrk_supported ()) + vls_mt_session_cleanup (vls); vls_free (vls); vls_mt_unguard (); - vls_table_wunlock (); + vls_mt_table_wunlock (); return rv; } @@ -1187,8 +1224,7 @@ vls_epoll_create (void) vcl_session_handle_t sh; vls_handle_t vlsh; - if (PREDICT_FALSE (vcl_get_worker_index () == ~0)) - vls_mt_add (); + vls_mt_detect (); sh = vppcom_epoll_create (); if (sh == INVALID_SESSION_ID) @@ -1225,26 +1261,30 @@ vls_epoll_ctl (vls_handle_t ep_vlsh, int op, vls_handle_t vlsh, vcl_session_handle_t ep_sh, sh; int rv; - vls_table_rlock (); + vls_mt_detect (); + vls_mt_table_rlock (); ep_vls = vls_get_and_lock (ep_vlsh); vls = vls_get_and_lock (vlsh); - vls_session_migrate (ep_vls); + + if (vls_mt_session_should_migrate (ep_vls)) + vls_mt_session_migrate (ep_vls); + ep_sh = vls_to_sh (ep_vls); sh = vls_to_sh (vls); if (PREDICT_FALSE (!vlsl->epoll_mp_check)) vls_epoll_ctl_mp_checks (vls, op); - vls_table_runlock (); + vls_mt_table_runlock (); rv = vppcom_epoll_ctl (ep_sh, op, sh, event); - vls_table_rlock (); + vls_mt_table_rlock (); ep_vls = vls_get (ep_vlsh); vls = vls_get (vlsh); vls_unlock (vls); vls_unlock (ep_vls); - vls_table_runlock (); + vls_mt_table_runlock (); return rv; } @@ -1255,6 +1295,7 @@ vls_epoll_wait (vls_handle_t ep_vlsh, struct epoll_event *events, vcl_locked_session_t *vls; int rv; + vls_mt_detect (); if (!(vls = vls_get_w_dlock (ep_vlsh))) return VPPCOM_EBADFD; vls_mt_guard (0, VLS_MT_OP_XPOLL); @@ -1303,6 +1344,7 @@ vls_select (int n_bits, vcl_si_set * read_map, vcl_si_set * write_map, { int rv; + vls_mt_detect (); vls_mt_guard (0, VLS_MT_OP_XPOLL); if (PREDICT_FALSE (!vlsl->select_mp_check)) vls_select_mp_checks (read_map); @@ -1595,8 +1637,7 @@ vls_send_clone_and_share_rpc (vcl_worker_t * wrk, wrk->rpc_done = 0; ret = vcl_send_worker_rpc (dst_wrk_index, rpc, sizeof (data)); - VDBG (1, - "send session clone of worker (session): %u (%u) -> %u (%u), ret=%d", + VDBG (1, "send session clone to wrk (session): %u (%u) -> %u (%u), ret=%d", dst_wrk_index, msg->session_index, msg->origin_vcl_wrk, msg->origin_session_index, ret); while (!ret && !wrk->rpc_done) @@ -1621,8 +1662,7 @@ vls_send_session_cleanup_rpc (vcl_worker_t * wrk, wrk->rpc_done = 0; ret = vcl_send_worker_rpc (dst_wrk_index, rpc, sizeof (data)); - VDBG (1, - "send session cleanup of worker (session): %u (%u) from %u (), ret=%d", + VDBG (1, "send session cleanup to wrk (session): %u (%u) from %u, ret=%d", dst_wrk_index, msg->session_index, msg->origin_vcl_wrk, ret); while (!ret && !wrk->rpc_done) ; @@ -1661,7 +1701,7 @@ vls_use_eventfd (void) unsigned char vls_mt_wrk_supported (void) { - return vcm->cfg.mt_supported; + return vcm->cfg.mt_wrk_supported; } int diff --git a/src/vcl/vcl_private.h b/src/vcl/vcl_private.h index b873ad994a6..231d7ebcbcf 100644 --- a/src/vcl/vcl_private.h +++ b/src/vcl/vcl_private.h @@ -218,7 +218,7 @@ typedef struct vppcom_cfg_t_ u8 *vpp_api_socket_name; u8 *vpp_api_chroot; u32 tls_engine; - u8 mt_supported; + u8 mt_wrk_supported; } vppcom_cfg_t; void vppcom_cfg (vppcom_cfg_t * vcl_cfg); -- cgit 1.2.3-korg