From e1607820a5d01053f509a0ae7617c94cac084dbb Mon Sep 17 00:00:00 2001 From: "Samuel K. Gutierrez" Date: Tue, 23 Jul 2024 21:36:49 -0600 Subject: [PATCH] Cleanup some more code. Signed-off-by: Samuel K. Gutierrez --- src/qvi-rmi.cc | 105 ++++++++++++++--------------------------------- src/qvi-rmi.h | 11 ----- src/qvi-scope.cc | 48 ++++++---------------- src/qvi-scope.h | 7 ---- src/qvi-task.cc | 32 +++++++-------- src/qvi-task.h | 16 +++++--- src/qvi-utils.h | 6 +-- 7 files changed, 73 insertions(+), 152 deletions(-) diff --git a/src/qvi-rmi.cc b/src/qvi-rmi.cc index eb11f7c..537790c 100644 --- a/src/qvi-rmi.cc +++ b/src/qvi-rmi.cc @@ -141,7 +141,6 @@ typedef enum qvi_rpc_funid_e { FID_TASK_SET_CPUBIND_FROM_CPUSET, FID_OBJ_TYPE_DEPTH, FID_GET_NOBJS_IN_CPUSET, - FID_GET_OBJ_TYPE_IN_CPUSET, FID_GET_DEVICE_IN_CPUSET, FID_SCOPE_GET_INTRINSIC_HWPOOL } qvi_rpc_funid_t; @@ -581,31 +580,6 @@ rpc_ssi_get_nobjs_in_cpuset( return qvrc; } -static int -rpc_ssi_get_obj_type_in_cpuset( - qvi_rmi_server_t *server, - qvi_msg_header_t *hdr, - void *input, - qvi_bbuff_t **output -) { - int npieces = 0; - hwloc_cpuset_t cpuset = nullptr; - int qvrc = qvi_bbuff_rmi_unpack( - input, &npieces, &cpuset - ); - if (qvrc != QV_SUCCESS) return qvrc; - - qv_hw_obj_type_t target_obj; - const int rpcrc = qvi_hwloc_get_obj_type_in_cpuset( - server->config.hwloc, npieces, cpuset, &target_obj - ); - - qvrc = rpc_pack(output, hdr->fid, rpcrc, target_obj); - - hwloc_bitmap_free(cpuset); - return qvrc; -} - static int rpc_ssi_get_device_in_cpuset( qvi_rmi_server_t *server, @@ -713,21 +687,19 @@ rpc_ssi_scope_get_intrinsic_hwpool( } /** - * Maps a given qvi_rpc_funid_t to a given function pointer. Must be kept in - * sync with qvi_rpc_funid_t. + * Maps a given qvi_rpc_funid_t to a given function pointer. */ -static const qvi_rpc_fun_ptr_t rpc_dispatch_table[] = { - rpc_ssi_invalid, - rpc_ssi_shutdown, - rpc_ssi_hello, - rpc_ssi_gbye, - rpc_ssi_task_get_cpubind, - rpc_ssi_task_set_cpubind_from_cpuset, - rpc_ssi_obj_type_depth, - rpc_ssi_get_nobjs_in_cpuset, - rpc_ssi_get_obj_type_in_cpuset, - rpc_ssi_get_device_in_cpuset, - rpc_ssi_scope_get_intrinsic_hwpool +static const std::map rpc_dispatch_table = { + {FID_INVALID, rpc_ssi_invalid}, + {FID_SERVER_SHUTDOWN, rpc_ssi_shutdown}, + {FID_HELLO, rpc_ssi_hello}, + {FID_GBYE, rpc_ssi_gbye}, + {FID_TASK_GET_CPUBIND, rpc_ssi_task_get_cpubind}, + {FID_TASK_SET_CPUBIND_FROM_CPUSET, rpc_ssi_task_set_cpubind_from_cpuset}, + {FID_OBJ_TYPE_DEPTH, rpc_ssi_obj_type_depth}, + {FID_GET_NOBJS_IN_CPUSET, rpc_ssi_get_nobjs_in_cpuset}, + {FID_GET_DEVICE_IN_CPUSET, rpc_ssi_get_device_in_cpuset}, + {FID_SCOPE_GET_INTRINSIC_HWPOOL, rpc_ssi_scope_get_intrinsic_hwpool} }; static int @@ -744,8 +716,15 @@ server_rpc_dispatch( const size_t trim = unpack_msg_header(data, &hdr); void *body = data_trim(data, trim); + auto fidfunp = rpc_dispatch_table.find(hdr.fid); + if (qvi_unlikely(fidfunp == rpc_dispatch_table.end())) { + qvi_log_error("Unknown function ID ({}) in RPC. Aborting.", hdr.fid); + rc = QV_ERR_RPC; + goto out; + } + qvi_bbuff_t *res; - rc = rpc_dispatch_table[hdr.fid](server, &hdr, body, &res); + rc = fidfunp->second(server, &hdr, body, &res); if (rc != QV_SUCCESS && rc != QV_SUCCESS_SHUTDOWN) { cstr_t ers = "RPC dispatch failed"; qvi_log_error("{} with rc={} ({})", ers, rc, qv_strerr(rc)); @@ -767,24 +746,24 @@ static void * server_go( void *data ) { - qvi_rmi_server_t *server = (qvi_rmi_server_t *)data; + qvi_rmi_server_t *const server = (qvi_rmi_server_t *)data; void *zworksock = zsocket_create_and_connect( server->zctx, ZMQ_REP, ZINPROC_ADDR ); - if (!zworksock) return nullptr; + if (qvi_unlikely(!zworksock)) return nullptr; int rc, bsent, bsentt = 0; - volatile bool active = true; + volatile std::atomic active{true}; do { zmq_msg_t mrx, mtx; rc = zmsg_recv(zworksock, &mrx); - if (rc != QV_SUCCESS) break; + if (qvi_unlikely(rc != QV_SUCCESS)) break; rc = server_rpc_dispatch(server, &mrx, &mtx); - if (rc != QV_SUCCESS && rc != QV_SUCCESS_SHUTDOWN) break; - if (rc == QV_SUCCESS_SHUTDOWN) active = false; + if (qvi_unlikely(rc != QV_SUCCESS && rc != QV_SUCCESS_SHUTDOWN)) break; + if (qvi_unlikely(rc == QV_SUCCESS_SHUTDOWN)) active = false; rc = zmsg_send(zworksock, &mtx, &bsent); - if (rc != QV_SUCCESS) break; + if (qvi_unlikely(rc != QV_SUCCESS)) break; bsentt += bsent; } while(active); #if QVI_DEBUG_MODE == 1 @@ -956,16 +935,16 @@ qvi_rmi_client_connect( client->zsock = zsocket_create_and_connect( client->zctx, ZMQ_REQ, url.c_str() ); - if (!client->zsock) return QV_ERR_MSG; + if (qvi_unlikely(!client->zsock)) return QV_ERR_MSG; int rc = hello_handshake(client); - if (rc != QV_SUCCESS) return rc; + if (qvi_unlikely(rc != QV_SUCCESS)) return rc; rc = qvi_hwloc_topology_init( client->config.hwloc, client->config.hwtopo_path.c_str() ); - if (rc != QV_SUCCESS) return rc; + if (qvi_unlikely(rc != QV_SUCCESS)) return rc; return qvi_hwloc_topology_load(client->config.hwloc); } @@ -1097,30 +1076,6 @@ qvi_rmi_get_nobjs_in_cpuset( return rpcrc; } -int -qvi_rmi_get_obj_type_in_cpuset( - qvi_rmi_client_t *client, - int npieces, - hwloc_const_cpuset_t cpuset, - qv_hw_obj_type_t *target_obj -) { - int qvrc = rpc_req( - client->zsock, - FID_GET_OBJ_TYPE_IN_CPUSET, - npieces, - cpuset - ); - if (qvrc != QV_SUCCESS) return qvrc; - - // Should be set by rpc_rep, so assume an error. - int rpcrc = QV_ERR_MSG; - qvrc = rpc_rep(client->zsock, &rpcrc, target_obj); - if (qvrc != QV_SUCCESS) return qvrc; - - return rpcrc; -} - - int qvi_rmi_get_device_in_cpuset( qvi_rmi_client_t *client, diff --git a/src/qvi-rmi.h b/src/qvi-rmi.h index 6ec340c..4c91f68 100644 --- a/src/qvi-rmi.h +++ b/src/qvi-rmi.h @@ -151,17 +151,6 @@ qvi_rmi_get_nobjs_in_cpuset( int *nobjs ); -/** - * - */ -int -qvi_rmi_get_obj_type_in_cpuset( - qvi_rmi_client_t *client, - int npieces, - hwloc_const_cpuset_t cpuset, - qv_hw_obj_type_t *target_obj -); - /** * */ diff --git a/src/qvi-scope.cc b/src/qvi-scope.cc index b9b94a3..89aed06 100644 --- a/src/qvi-scope.cc +++ b/src/qvi-scope.cc @@ -112,7 +112,6 @@ struct qvi_scope_split_agg_s { for (auto &hwpool : hwpools) { qvi_delete(&hwpool); } - hwpools.clear(); } /** * Minimally initializes instance. @@ -205,18 +204,6 @@ get_nobjs_in_hwpool( return QV_SUCCESS; } -static int -get_obj_type_in_hwpool( - qvi_rmi_client_t *rmi, - qvi_hwpool_s *hwpool, - int npieces, - qv_hw_obj_type_t *obj -) { - return qvi_rmi_get_obj_type_in_cpuset( - rmi, npieces, hwpool->get_cpuset().cdata(), obj - ); -} - template static int gather_values( @@ -1127,10 +1114,10 @@ qvi_scope_thsplit( int *kcolors, uint_t k, qv_hw_obj_type_t maybe_obj_type, - qv_scope_t ***kchildren + qv_scope_t ***thchildren ) { - if (k == 0 || !kchildren) return QV_ERR_INVLD_ARG; - *kchildren = nullptr; + if (k == 0 || !thchildren) return QV_ERR_INVLD_ARG; + *thchildren = nullptr; const uint_t group_size = k; qvi_scope_split_agg_s splitagg{}; @@ -1174,7 +1161,7 @@ qvi_scope_thsplit( if (rc != QV_SUCCESS) return rc; // Now populate the children. - qv_scope_t **ikchildren = new qv_scope_t*[group_size]; + qv_scope_t **ithchildren = new qv_scope_t *[group_size]; qvi_group_t *thgroup = nullptr; // Split off from our parent group. This call is called from a context in @@ -1182,7 +1169,7 @@ qvi_scope_thsplit( // new thread group for each child. rc = parent->group->thsplit(group_size, &thgroup); if (rc != QV_SUCCESS) { - qvi_scope_thfree(&ikchildren, group_size); + qvi_scope_thfree(&ithchildren, group_size); return rc; } @@ -1210,15 +1197,17 @@ qvi_scope_thsplit( break; } thgroup->retain(); - ikchildren[i] = child; + ithchildren[i] = child; } if (rc != QV_SUCCESS) { - qvi_scope_thfree(&ikchildren, k); + qvi_scope_thfree(&ithchildren, k); } - // Subtract one to account for the parent's implicit retain during - // construct. - thgroup->release(); - *kchildren = ikchildren; + else { + // Subtract one to account for the parent's + // implicit retain during construct. + thgroup->release(); + } + *thchildren = ithchildren; return rc; } @@ -1309,17 +1298,6 @@ qvi_scope_nobjs( ); } -int -qvi_scope_obj_type( - qv_scope_t *scope, - int npieces, - qv_hw_obj_type_t *obj -) { - return get_obj_type_in_hwpool( - scope->group->task()->rmi(), scope->hwpool, npieces, obj - ); -} - int qvi_scope_get_device_id( qv_scope_t *scope, diff --git a/src/qvi-scope.h b/src/qvi-scope.h index 519c187..64c9d64 100644 --- a/src/qvi-scope.h +++ b/src/qvi-scope.h @@ -162,13 +162,6 @@ qvi_scope_nobjs( int *n ); -int -qvi_scope_obj_type( - qv_scope_t *scope, - int npieces, - qv_hw_obj_type_t *obj -); - int qvi_scope_get_device_id( qv_scope_t *scope, diff --git a/src/qvi-task.cc b/src/qvi-task.cc index e9ef950..57cc37a 100644 --- a/src/qvi-task.cc +++ b/src/qvi-task.cc @@ -33,49 +33,49 @@ qvi_task_s::connect_to_server(void) qvi_log_error("{}", qvi_conn_ers()); return rc; } - return qvi_rmi_client_connect(myrmi, url); + return qvi_rmi_client_connect(m_rmi, url); } int -qvi_task_s::bind_stack_init(void) +qvi_task_s::init_bind_stack(void) { // Cache current binding. hwloc_cpuset_t current_bind = nullptr; const int rc = qvi_rmi_task_get_cpubind( - myrmi, mytid(), ¤t_bind + m_rmi, mytid(), ¤t_bind ); if (qvi_unlikely(rc != QV_SUCCESS)) return rc; - mystack.push(qvi_hwloc_bitmap_s(current_bind)); + m_stack.push(qvi_hwloc_bitmap_s(current_bind)); hwloc_bitmap_free(current_bind); return rc; } qvi_task_s::qvi_task_s(void) { - int rc = qvi_rmi_client_new(&myrmi); + int rc = qvi_rmi_client_new(&m_rmi); if (qvi_unlikely(rc != QV_SUCCESS)) throw qvi_runtime_error(); // Connect to our server. rc = connect_to_server(); if (qvi_unlikely(rc != QV_SUCCESS)) throw qvi_runtime_error(); // Initialize our bind stack. - rc = bind_stack_init(); + rc = init_bind_stack(); if (qvi_unlikely(rc != QV_SUCCESS)) throw qvi_runtime_error(); } qvi_task_s::~qvi_task_s(void) { - while (!mystack.empty()) { - mystack.pop(); + while (!m_stack.empty()) { + m_stack.pop(); } - qvi_rmi_client_free(&myrmi); + qvi_rmi_client_free(&m_rmi); } qvi_rmi_client_t * qvi_task_s::rmi(void) { - assert(myrmi); - return myrmi; + assert(m_rmi); + return m_rmi; } int @@ -86,21 +86,21 @@ qvi_task_s::bind_push( qvi_hwloc_bitmap_s bitmap_copy(cpuset); // Change policy const int rc = qvi_rmi_task_set_cpubind_from_cpuset( - myrmi, mytid(), bitmap_copy.cdata() + m_rmi, mytid(), bitmap_copy.cdata() ); if (qvi_unlikely(rc != QV_SUCCESS)) return rc; // Push bitmap onto stack. - mystack.push(bitmap_copy); + m_stack.push(bitmap_copy); return rc; } int qvi_task_s::bind_pop(void) { - mystack.pop(); + m_stack.pop(); return qvi_rmi_task_set_cpubind_from_cpuset( - myrmi, mytid(), mystack.top().cdata() + m_rmi, mytid(), m_stack.top().cdata() ); } @@ -108,7 +108,7 @@ int qvi_task_s::bind_top( hwloc_cpuset_t *dest ) { - return qvi_hwloc_bitmap_dup(mystack.top().cdata(), dest); + return qvi_hwloc_bitmap_dup(m_stack.top().cdata(), dest); } /* diff --git a/src/qvi-task.h b/src/qvi-task.h index c328a9a..23fb101 100644 --- a/src/qvi-task.h +++ b/src/qvi-task.h @@ -25,15 +25,15 @@ using qvi_task_bind_stack_t = std::stack; struct qvi_task_s { private: /** Client-side connection to the RMI. */ - qvi_rmi_client_t *myrmi = nullptr; + qvi_rmi_client_t *m_rmi = nullptr; /** The task's bind stack. */ - qvi_task_bind_stack_t mystack; + qvi_task_bind_stack_t m_stack; /** Connects to the RMI server. */ int connect_to_server(void); /** Initializes the bind stack. */ int - bind_stack_init(void); + init_bind_stack(void); public: /** Returns the caller's thread ID. */ static pid_t @@ -47,12 +47,18 @@ struct qvi_task_s { /** Returns a pointer to the task's RMI. */ qvi_rmi_client_t * rmi(void); - /** Changes the task's affinity. */ + /** + * Changes the task's affinity based on the provided cpuset. + * Also stores the cpuset to the top of the task's bind stack. + */ int bind_push( hwloc_const_cpuset_t cpuset ); - /** */ + /** + * Removes the cpuset from the top of the bind stack + * and changes the task's affinity to that value. + */ int bind_pop(void); /** Returns the task's current cpuset. */ diff --git a/src/qvi-utils.h b/src/qvi-utils.h index caf0577..479f4dd 100644 --- a/src/qvi-utils.h +++ b/src/qvi-utils.h @@ -72,9 +72,9 @@ qvi_delete( ) { if (!t) return; T *it = *t; - if (!it) goto out; - delete it; -out: + if (it) { + delete it; + } *t = nullptr; }