From dda8e0418f578764bb10a53d1c545ef4497c5359 Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Fri, 28 Apr 2023 23:52:06 +0200 Subject: [PATCH 01/13] Support MPI tests This commit introduces the possibility to run MPI tests from ctest. Signed-off-by: Alessandro Pellegrini --- src/CMakeLists.txt | 6 +++--- test/CMakeLists.txt | 11 +++++++++-- test/tests/integration/phold.c | 2 +- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 1dfee262..fcc43808 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -34,8 +34,8 @@ endif() # Build the core library add_library(rscore STATIC ${rscore_srcs}) -target_compile_definitions(rscore PRIVATE ROOTSIM_VERSION="${PROJECT_VERSION}") -target_include_directories(rscore PRIVATE .) +target_compile_definitions(rscore PRIVATE ROOTSIM_VERSION= "${PROJECT_VERSION}") +target_include_directories(rscore PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/include/) target_link_libraries(rscore ${CMAKE_THREAD_LIBS_INIT} ${EXTRA_LIBS}) if(NOT DISABLE_MPI) @@ -44,5 +44,5 @@ if(NOT DISABLE_MPI) target_link_libraries(rscore ${MPI_C_LIBRARIES}) endif() -install(FILES ROOT-Sim.h DESTINATION include) +install(DIRECTORY include/ DESTINATION include) install(TARGETS rscore LIBRARY DESTINATION lib) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index d5ca002b..4154afd6 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -6,11 +6,11 @@ add_library(test_framework_lib STATIC framework/test.c framework/thread.c ) -target_include_directories(test_framework_lib PRIVATE ../src .) +target_include_directories(test_framework_lib PRIVATE ${CMAKE_SOURCE_DIR}/src ${CMAKE_SOURCE_DIR}/src/include ${CMAKE_CURRENT_SOURCE_DIR}) function(test_program name) add_executable(test_${name} ${ARGN}) - target_include_directories(test_${name} PRIVATE ../src .) + target_include_directories(test_${name} PRIVATE ${CMAKE_SOURCE_DIR}/src ${CMAKE_SOURCE_DIR}/src/include ${CMAKE_CURRENT_SOURCE_DIR}) target_link_libraries(test_${name} test_framework_lib rscore) # FIXME this doesn't need to be in all the tests add_test(NAME test_${name} COMMAND test_${name}) set_tests_properties(test_${name} PROPERTIES TIMEOUT 60) @@ -21,6 +21,11 @@ function(test_program_xf name) set_tests_properties(test_${name} PROPERTIES WILL_FAIL TRUE) endfunction() +function(test_program_mpi name) + test_program(${name} ${ARGN}) + set_property(TARGET test_${name} PROPERTY CROSSCOMPILING_EMULATOR "${MPIEXEC_EXECUTABLE};${MPIEXEC_NUMPROC_FLAG};2") +endfunction() + # Test framework tests test_program(self_main framework/self-tests/stubs.c framework/self-tests/main.c) test_program_xf(self_fail_assert framework/self-tests/stubs.c framework/self-tests/fail_assert.c) @@ -61,4 +66,6 @@ test_program(sync tests/core/sync.c) # Integration tests test_program(correctness_serial tests/integration/correctness/serial.c tests/integration/correctness/application.c tests/integration/correctness/functions.c tests/integration/correctness/output_256.c) test_program(correctness_parallel tests/integration/correctness/parallel.c tests/integration/correctness/application.c tests/integration/correctness/functions.c tests/integration/correctness/output_256.c) +test_program_mpi(correctness_parallel_mpi tests/integration/correctness/parallel.c tests/integration/correctness/application.c tests/integration/correctness/functions.c tests/integration/correctness/output_256.c) test_program(phold tests/integration/phold.c) +test_program_mpi(phold_mpi tests/integration/phold.c) diff --git a/test/tests/integration/phold.c b/test/tests/integration/phold.c index 5459b25b..9509ad2b 100644 --- a/test/tests/integration/phold.c +++ b/test/tests/integration/phold.c @@ -20,7 +20,7 @@ #endif #ifndef NUM_THREADS -#define NUM_THREADS 0 +#define NUM_THREADS 2 #endif #define EVENT 1 From d72d41f201b1ecdbb94301b1077c80412d888061 Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Fri, 28 Apr 2023 23:55:20 +0200 Subject: [PATCH 02/13] Move initialization code to RootsimInit RootsimInit() is the function that is supposed to initialize the simulation. Yet, some of the initialization was taking place in RootsimRun(). This commit moves most of the initialization code into RootsimInit(). Signed-off-by: Alessandro Pellegrini --- src/init.c | 33 ++++++++++++++-------------- src/parallel/parallel.c | 4 +--- src/parallel/parallel.h | 1 + src/serial/serial.c | 4 +--- src/serial/serial.h | 1 + test/tests/log/rootsim_stats_test.py | 2 +- test/tests/log/stats.c | 4 ++-- 7 files changed, 24 insertions(+), 25 deletions(-) diff --git a/src/init.c b/src/init.c index 817e206f..b0213200 100644 --- a/src/init.c +++ b/src/init.c @@ -11,12 +11,9 @@ #include #include #include -#include #include #include -#include - #include #include @@ -110,16 +107,28 @@ int RootsimInit(const struct simulation_configuration *conf) log_init(global_config.logfile); - if (global_config.serial) - global_config.n_threads = 1; - else if (global_config.n_threads == 0) - global_config.n_threads = thread_cores_count(); - if(global_config.termination_time == 0) global_config.termination_time = SIMTIME_MAX; + logger(LOG_INFO, "Initializing %s simulation", global_config.serial ? "serial" : "parallel"); + if(global_config.serial) { + global_config.n_threads = 1; + serial_simulation_init(); + } else { + if(global_config.n_threads == 0) { + global_config.n_threads = thread_cores_count(); + } + mpi_global_init(NULL, NULL); + parallel_global_init(); + } + configuration_done = true; + if(global_config.log_level < LOG_SILENT && !rid && !nid) { + print_logo(); + print_config(); + } + return 0; } @@ -138,14 +147,6 @@ int RootsimRun(void) if(!configuration_done) return -1; - if(!global_config.serial) - mpi_global_init(NULL, NULL); - - if(global_config.log_level < LOG_SILENT && !rid) { - print_logo(); - print_config(); - } - if(global_config.serial) { ret = serial_simulation(); } else { diff --git a/src/parallel/parallel.c b/src/parallel/parallel.c index 01e32a32..1923e3f4 100644 --- a/src/parallel/parallel.c +++ b/src/parallel/parallel.c @@ -82,7 +82,7 @@ static thrd_ret_t THREAD_CALL_CONV parallel_thread_run(void *rid_arg) return THREAD_RET_SUCCESS; } -static void parallel_global_init(void) +void parallel_global_init(void) { stats_global_init(); lp_global_init(); @@ -100,8 +100,6 @@ static void parallel_global_fini(void) int parallel_simulation(void) { - logger(LOG_INFO, "Initializing parallel simulation"); - parallel_global_init(); stats_global_time_take(STATS_GLOBAL_INIT_END); thr_id_t thrs[global_config.n_threads]; diff --git a/src/parallel/parallel.h b/src/parallel/parallel.h index 38290b7e..3ec3ed5f 100644 --- a/src/parallel/parallel.h +++ b/src/parallel/parallel.h @@ -9,3 +9,4 @@ #pragma once extern int parallel_simulation(void); +extern void parallel_global_init(void); diff --git a/src/serial/serial.c b/src/serial/serial.c index 99f8ce02..0115a777 100644 --- a/src/serial/serial.c +++ b/src/serial/serial.c @@ -20,7 +20,7 @@ static heap_declare(struct lp_msg *) queue; /** * @brief Initialize the serial simulation environment */ -static void serial_simulation_init(void) +void serial_simulation_init(void) { stats_global_init(); stats_init(); @@ -145,8 +145,6 @@ int serial_simulation(void) { int ret; - logger(LOG_INFO, "Initializing serial simulation"); - serial_simulation_init(); stats_global_time_take(STATS_GLOBAL_INIT_END); stats_global_time_take(STATS_GLOBAL_EVENTS_START); diff --git a/src/serial/serial.h b/src/serial/serial.h index 5e721deb..a59776ec 100644 --- a/src/serial/serial.h +++ b/src/serial/serial.h @@ -10,6 +10,7 @@ #include +extern void serial_simulation_init(void); extern int serial_simulation(void); extern void ScheduleNewEvent_serial(lp_id_t receiver, simtime_t timestamp, unsigned event_type, const void *payload, unsigned payload_size); diff --git a/test/tests/log/rootsim_stats_test.py b/test/tests/log/rootsim_stats_test.py index baf23911..55ac20f5 100644 --- a/test/tests/log/rootsim_stats_test.py +++ b/test/tests/log/rootsim_stats_test.py @@ -73,7 +73,7 @@ def test_stats_file(base_name, expected): if __name__ == "__main__": rs_script_path, bin_folder = test_init() stats_regex = regex_get() - test_stats_file("empty_stats", ["NZ", "1", "2", "0", "0", "0", "0", "0", "0", "0", "0.00", "0.00", "100.00", "0", + test_stats_file("empty_stats", ["NZ", "1", "2", "123", "0", "0", "0", "0", "0", "0", "0.00", "0.00", "100.00", "0", "0", "0", "0", "0.0", "0", "0.0", "0", "NZ"]) test_stats_file("single_gvt_stats", ["NZ", "1", "2", "16", "0", "0", "0", "0", "0", "0", "0.00", "0.00", "100.00", "0", "0", "0", "0", "0.0", "1", "0.0", "NZ", "NZ"]) diff --git a/test/tests/log/stats.c b/test/tests/log/stats.c index 8bfb1e6d..97befa34 100644 --- a/test/tests/log/stats.c +++ b/test/tests/log/stats.c @@ -87,8 +87,7 @@ int stats_measures_test(_unused void *arg) static void stats_subsystem_test(const char *name, test_fn thread_fn) { - conf.stats_file = name; - RootsimInit(&conf); + global_config.stats_file = name; stats_global_init(); test_parallel("Testing statistics module", thread_fn, NULL, N_THREADS); stats_global_fini(); @@ -96,6 +95,7 @@ static void stats_subsystem_test(const char *name, test_fn thread_fn) int main(void) { + RootsimInit(&conf); stats_subsystem_test("empty_stats", stats_empty_test); n_lps_node = 16; stats_subsystem_test("single_gvt_stats", stats_single_gvt_test); From 262f20e8b331ab335e848ba13bc8196c3815879e Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Fri, 28 Apr 2023 23:59:39 +0200 Subject: [PATCH 03/13] Introduce configurable control messages In a recent PR, we have phased out some libraries that were providing to models functionalities that are not necessarily relevant to the core. For proper development of these libraries, anyhow, they may need to interact with the internals of the core, or extend part of their functionalities. Years ago we discussed the possibility to introduce a sort of SDK to allow integrating additional layer over the core. This commit is a first step in that direction. Here, we are introducing the possibility to register handlers from higher-level libraries, that are associated with proper control messages that a library can exchange across nodes. In this way, in a distributed simulation, a library can synchronize the various nodes to configure the different instances. I am not particularly happy with the reduced size of the payload, but changing this would require to rework the antimessage extraction logic which is something I don't want to do now. For the use case I am running on, the current payload is sufficient. Signed-off-by: Alessandro Pellegrini --- src/distributed/control_msg.c | 63 ++++++++++++++++++++++++++++- src/distributed/control_msg.h | 21 ++++++++-- src/distributed/mpi.c | 59 +++++++++++++++++---------- src/distributed/mpi.h | 5 ++- src/{ => include}/ROOT-Sim.h | 0 src/include/ROOT-Sim/sdk.h | 46 +++++++++++++++++++++ src/mm/buddy/multi.c | 4 +- test/CMakeLists.txt | 3 ++ test/tests/old_tests/gvt/gvt_test.c | 4 +- test/tests/sdk/control_msg.c | 56 +++++++++++++++++++++++++ test/tests/sdk/control_msg.h | 7 ++++ test/tests/sdk/distributed.c | 21 ++++++++++ test/tests/sdk/parallel.c | 17 ++++++++ test/tests/sdk/serial.c | 17 ++++++++ 14 files changed, 290 insertions(+), 33 deletions(-) rename src/{ => include}/ROOT-Sim.h (100%) create mode 100644 src/include/ROOT-Sim/sdk.h create mode 100644 test/tests/sdk/control_msg.c create mode 100644 test/tests/sdk/control_msg.h create mode 100644 test/tests/sdk/distributed.c create mode 100644 test/tests/sdk/parallel.c create mode 100644 test/tests/sdk/serial.c diff --git a/src/distributed/control_msg.c b/src/distributed/control_msg.c index e14f5512..48f3af2a 100644 --- a/src/distributed/control_msg.c +++ b/src/distributed/control_msg.c @@ -8,13 +8,59 @@ * SPDX-FileCopyrightText: 2008-2022 HPDCS Group * SPDX-License-Identifier: GPL-3.0-only */ +#include #include +#include + +struct library_handler { + unsigned control_msg_id; + control_msg_handler_t handler; +}; + +static struct library_handler *library_handlers = NULL; +static size_t library_handlers_capacity = 0; +static size_t library_handlers_size = 0; +static int next_control_msg_id = MSG_CTRL_DEFAULT_END; + +int control_msg_register_handler(control_msg_handler_t handler) +{ + int ret = next_control_msg_id; + + if(unlikely(library_handlers == NULL)) { + library_handlers_capacity = INITIAL_HANDLERS_CAPACITY; + library_handlers = malloc(library_handlers_capacity * sizeof(*library_handlers)); + } + + if(library_handlers_size == library_handlers_capacity) { + library_handlers_capacity *= 2; + library_handlers = realloc(library_handlers, library_handlers_capacity * sizeof(*library_handlers)); + } + library_handlers[library_handlers_size].control_msg_id = next_control_msg_id++; + library_handlers[library_handlers_size++].handler = handler; + + return ret; +} + +/** + * @brief Invoke a library handler + * @param code the control message ID + * @param payload the payload of the control message + */ +void invoke_library_handler(unsigned code, const void *payload) +{ + for(size_t i = 0; i < library_handlers_size; i++) { + if(library_handlers[i].control_msg_id == code) { + library_handlers[i].handler(code, payload); + return; + } + } +} /** * @brief Handle a received control message * @param ctrl the tag of the received control message */ -void control_msg_process(enum msg_ctrl_code ctrl) +void control_msg_process(unsigned ctrl, void *payload) { switch(ctrl) { case MSG_CTRL_GVT_START: @@ -27,6 +73,19 @@ void control_msg_process(enum msg_ctrl_code ctrl) termination_on_ctrl_msg(); break; default: - __builtin_unreachable(); + invoke_library_handler(ctrl, payload); + } +} + +void control_msg_broadcast(unsigned ctrl, const void *payload, size_t size) +{ + if(size > CONTROL_MSG_PAYLOAD_SIZE) { + logger(LOG_FATAL, "The payload of the control message is too large"); + abort(); + } + if(global_config.serial || n_nodes == 1) { + invoke_library_handler(ctrl, payload); + } else { + mpi_library_control_msg_broadcast(ctrl, payload, size); } } diff --git a/src/distributed/control_msg.h b/src/distributed/control_msg.h index 4c55ca81..54c44321 100644 --- a/src/distributed/control_msg.h +++ b/src/distributed/control_msg.h @@ -10,17 +10,32 @@ */ #pragma once +#include #include #include +#include "include/ROOT-Sim/sdk.h" + +#define INITIAL_HANDLERS_CAPACITY 4 /// A control message MPI tag value -enum msg_ctrl_code { +enum default_msg_ctrl_code { /// Used by the master to start a new gvt reduction operation MSG_CTRL_GVT_START = 1, /// Used by slaves to signal their completion of the gvt protocol MSG_CTRL_GVT_DONE, /// Used in broadcast to signal that local LPs can terminate - MSG_CTRL_TERMINATION + MSG_CTRL_TERMINATION, + /// Marker to signal the end of the enum + MSG_CTRL_DEFAULT_END +}; + +struct control_msg { + /// The control message code + unsigned ctrl_code; + /// The payload of the control message + char payload[CONTROL_MSG_PAYLOAD_SIZE]; }; -extern void control_msg_process(enum msg_ctrl_code ctrl); +_Static_assert(sizeof(struct control_msg) < msg_remote_anti_size(), "Invalid control message payload size"); + +extern void control_msg_process(unsigned ctrl, void *payload); diff --git a/src/distributed/mpi.c b/src/distributed/mpi.c index 84abdd90..53e3e070 100644 --- a/src/distributed/mpi.c +++ b/src/distributed/mpi.c @@ -9,6 +9,8 @@ * SPDX-FileCopyrightText: 2008-2022 HPDCS Group * SPDX-License-Identifier: GPL-3.0-only */ +#include + #include #include @@ -17,18 +19,10 @@ #include -enum { - RS_MSG_TAG = 0, +enum { RS_MSG_TAG = 0, RS_DATA_TAG }; -/// Array of control codes values to be able to get their address for MPI_Send() -static const enum msg_ctrl_code ctrl_msgs[] = { - [MSG_CTRL_GVT_START] = MSG_CTRL_GVT_START, - [MSG_CTRL_GVT_DONE] = MSG_CTRL_GVT_DONE, - [MSG_CTRL_TERMINATION] = MSG_CTRL_TERMINATION -}; - /// The MPI request associated with the non blocking scatter gather collective static MPI_Request reduce_sum_scatter_req = MPI_REQUEST_NULL; /// The MPI request associated with the non blocking all reduce collective @@ -136,11 +130,31 @@ void mpi_remote_anti_msg_send(struct lp_msg *msg, nid_t dest_nid) MPI_Request_free(&req); } + +/** + * @brief Sends a library control message to all the nodes, including self + * @param ctrl the control message to send + */ +void mpi_library_control_msg_broadcast(unsigned ctrl, const void *payload, size_t size) +{ + nid_t i = n_nodes; + struct control_msg msg = {.ctrl_code = ctrl, {0}}; + if(unlikely(payload != NULL)) { + memcpy(&msg.payload, payload, size); + } + + while(i--) { + MPI_Request req; + MPI_Isend(&msg, sizeof(msg), MPI_BYTE, i, RS_MSG_TAG, MPI_COMM_WORLD, &req); + MPI_Request_free(&req); + } +} + /** * @brief Sends a platform control message to all the nodes, including self * @param ctrl the control message to send */ -void mpi_control_msg_broadcast(enum msg_ctrl_code ctrl) +void mpi_control_msg_broadcast(unsigned ctrl) { nid_t i = n_nodes; while(i--) { @@ -153,10 +167,11 @@ void mpi_control_msg_broadcast(enum msg_ctrl_code ctrl) * @param ctrl the control message to send * @param dest the id of the destination node */ -void mpi_control_msg_send_to(enum msg_ctrl_code ctrl, nid_t dest) +void mpi_control_msg_send_to(enum default_msg_ctrl_code ctrl, nid_t dest) { MPI_Request req; - MPI_Isend(&ctrl_msgs[ctrl], sizeof(*ctrl_msgs), MPI_BYTE, dest, RS_MSG_TAG, MPI_COMM_WORLD, &req); + struct control_msg msg = {.ctrl_code = ctrl, {0}}; + MPI_Isend(&msg, sizeof(msg), MPI_BYTE, dest, RS_MSG_TAG, MPI_COMM_WORLD, &req); MPI_Request_free(&req); } @@ -165,8 +180,8 @@ void mpi_control_msg_send_to(enum msg_ctrl_code ctrl, nid_t dest) * each one of them. * * This routine checks, using the MPI probing mechanism, for new remote messages and it handles them accordingly. - * Control messages are handled by the respective platform handler. Simulation messages are unpacked and put in the - * queue. Anti-messages are matched and accordingly processed by the message map. + * Control messages are handled by the respective platform handler or library handler. Simulation messages are unpacked + * and put in the queue. Anti-messages are matched and accordingly processed by the message map. */ void mpi_remote_msg_handle(void) { @@ -184,10 +199,10 @@ void mpi_remote_msg_handle(void) MPI_Get_count(&status, MPI_BYTE, &size); struct lp_msg *msg; if(unlikely(size <= (int)msg_remote_anti_size())) { - if(unlikely(size == sizeof(enum msg_ctrl_code))) { - enum msg_ctrl_code c; - MPI_Mrecv(&c, sizeof(c), MPI_BYTE, &mpi_msg, MPI_STATUS_IGNORE); - control_msg_process(c); + if(unlikely(size == sizeof(struct control_msg))) { + struct control_msg ctrl; + MPI_Mrecv(&ctrl, sizeof(ctrl), MPI_BYTE, &mpi_msg, MPI_STATUS_IGNORE); + control_msg_process(ctrl.ctrl_code, ctrl.payload); continue; } msg = msg_allocator_alloc(0); @@ -231,10 +246,10 @@ void mpi_remote_msg_drain(void) int size; MPI_Get_count(&status, MPI_BYTE, &size); - if(unlikely(size == sizeof(enum msg_ctrl_code))) { - enum msg_ctrl_code c; - MPI_Mrecv(&c, sizeof(c), MPI_BYTE, &mpi_msg, MPI_STATUS_IGNORE); - control_msg_process(c); + if(unlikely(size == sizeof(struct control_msg))) { + struct control_msg ctrl; + MPI_Mrecv(&ctrl, sizeof(ctrl), MPI_BYTE, &mpi_msg, MPI_STATUS_IGNORE); + control_msg_process(ctrl.ctrl_code, ctrl.payload); continue; } diff --git a/src/distributed/mpi.h b/src/distributed/mpi.h index 4d78cda2..6ffe1add 100644 --- a/src/distributed/mpi.h +++ b/src/distributed/mpi.h @@ -24,8 +24,9 @@ extern void mpi_global_fini(void); extern void mpi_remote_msg_send(struct lp_msg *msg, nid_t dest_nid); extern void mpi_remote_anti_msg_send(struct lp_msg *msg, nid_t dest_nid); -extern void mpi_control_msg_broadcast(enum msg_ctrl_code ctrl); -extern void mpi_control_msg_send_to(enum msg_ctrl_code ctrl, nid_t dest); +extern void mpi_library_control_msg_broadcast(unsigned ctrl, const void *payload, size_t size); +extern void mpi_control_msg_broadcast(unsigned ctrl); +extern void mpi_control_msg_send_to(unsigned ctrl, nid_t dest); extern void mpi_remote_msg_handle(void); extern void mpi_remote_msg_drain(void); diff --git a/src/ROOT-Sim.h b/src/include/ROOT-Sim.h similarity index 100% rename from src/ROOT-Sim.h rename to src/include/ROOT-Sim.h diff --git a/src/include/ROOT-Sim/sdk.h b/src/include/ROOT-Sim/sdk.h new file mode 100644 index 00000000..dc1acf8d --- /dev/null +++ b/src/include/ROOT-Sim/sdk.h @@ -0,0 +1,46 @@ +/** + * @file sdk.h + * + * @brief ROOT-Sim header for internal library development + * + * This header is intended to be used by ROOT-Sim library developers only. + * It defines all the symbols which are needed to develop a library to be + * used by simulation models. + * + * SPDX-FileCopyrightText: 2008-2022 HPDCS Group + * SPDX-License-Identifier: GPL-3.0-only + */ +#pragma once + +#include + +/// The size of the maximum payload of a control message +#define CONTROL_MSG_PAYLOAD_SIZE 16 + +// The type of control message handlers that libraries can register +typedef void (*control_msg_handler_t)(unsigned ctrl_msg_id, const void *payload); + +/** + * @brief Register a new control message handler + * + * This function registers a control message handler for internal library use. + * When a handler is registered, a control message ID is automatically allocated + * and returned to the caller. + * The library function should then use that ID to send control messages to the + * various MPI ranks. The size of the payloads of such control messages is fixed. + * + * @param handler the handler to register + * @return the ID of the registered control message ID + */ +extern int control_msg_register_handler(control_msg_handler_t handler); + +/** + * @brief Send a control message to all simulation nodes. + * + * This function sends a control message to all nodes of the simulation. + * The corresponding handler must be registered first via control_msg_register_handler(). + * + * @param ctrl the control message ID, as returned by control_msg_register_handler() + * @param payload the payload of the control message, must be of size CONTROL_MSG_PAYLOAD_SIZE at most + */ +extern void control_msg_broadcast(unsigned ctrl, const void *payload, size_t size); diff --git a/src/mm/buddy/multi.c b/src/mm/buddy/multi.c index 44d14acb..a16972da 100644 --- a/src/mm/buddy/multi.c +++ b/src/mm/buddy/multi.c @@ -6,9 +6,9 @@ * SPDX-FileCopyrightText: 2008-2022 HPDCS Group * SPDX-License-Identifier: GPL-3.0-only */ -#include +#include -#include +#include #include #include #include diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 4154afd6..c81f3f1d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -48,6 +48,9 @@ test_program(load tests/core/load.c) test_program(bitmap tests/datatypes/bitmap.c) test_program(mm tests/mm/buddy.c tests/mm/buddy_hard.c tests/mm/parallel.c tests/mm/main.c) test_program(termination tests/gvt/termination.c) +test_program(control_msg_serial tests/sdk/control_msg.c tests/sdk/serial.c) +test_program(control_msg_parallel tests/sdk/control_msg.c tests/sdk/parallel.c) +test_program_mpi(control_msg_distributed tests/sdk/control_msg.c tests/sdk/distributed.c) # Test the statistics subsystem test_program(stats tests/log/stats.c) diff --git a/test/tests/old_tests/gvt/gvt_test.c b/test/tests/old_tests/gvt/gvt_test.c index 25c3b1bd..81b4d32b 100644 --- a/test/tests/old_tests/gvt/gvt_test.c +++ b/test/tests/old_tests/gvt/gvt_test.c @@ -6,10 +6,10 @@ * SPDX-FileCopyrightText: 2008-2022 HPDCS Group * SPDX-License-Identifier: GPL-3.0-only */ -#include "test.h" +#include #include -#include "gvt/gvt.h" +#include #define N_THREADS 3 diff --git a/test/tests/sdk/control_msg.c b/test/tests/sdk/control_msg.c new file mode 100644 index 00000000..bada679d --- /dev/null +++ b/test/tests/sdk/control_msg.c @@ -0,0 +1,56 @@ +#include + +#include +#include +#include + +#include +#include + +#include "control_msg.h" + +static long value = 1234L; + +static unsigned ctrl_msg_id1 = 0; +static unsigned ctrl_msg_id2 = 0; + +static unsigned short inv1 = 0; +static unsigned short inv2 = 0; + +void handler(unsigned ctrl_msg_id, const void *payload) +{ + assert(*(long *)payload == value); + + if(ctrl_msg_id == ctrl_msg_id1) { + inv1++; + } else if(ctrl_msg_id == ctrl_msg_id2) { + inv2++; + } else { + assert(false); + } +} + +int test_ctrl_msg(bool distributed) +{ + ctrl_msg_id1 = control_msg_register_handler(handler); + ctrl_msg_id2 = control_msg_register_handler(handler); + + assert(ctrl_msg_id1 == MSG_CTRL_DEFAULT_END); + assert(ctrl_msg_id2 > MSG_CTRL_DEFAULT_END); + + control_msg_broadcast(ctrl_msg_id1, &value, sizeof(value)); + control_msg_broadcast(ctrl_msg_id2, &value, sizeof(value)); + + if(distributed) { + mpi_remote_msg_handle(); + } + + assert(inv1 != 0); + assert(inv2 != 0); + + return inv1 != inv2; +} + +void ProcessEvent(_unused lp_id_t me, _unused simtime_t now, _unused unsigned event_type, _unused const void *content, + _unused unsigned size, _unused void *s) +{} diff --git a/test/tests/sdk/control_msg.h b/test/tests/sdk/control_msg.h new file mode 100644 index 00000000..a03cba9c --- /dev/null +++ b/test/tests/sdk/control_msg.h @@ -0,0 +1,7 @@ +#pragma once + +#include + +extern int test_ctrl_msg(bool distributed); +extern void ProcessEvent(_unused lp_id_t me, _unused simtime_t now, _unused unsigned event_type, + _unused const void *content, _unused unsigned size, _unused void *s); diff --git a/test/tests/sdk/distributed.c b/test/tests/sdk/distributed.c new file mode 100644 index 00000000..6bbcb2ea --- /dev/null +++ b/test/tests/sdk/distributed.c @@ -0,0 +1,21 @@ +#include + +#include + +#include "control_msg.h" + +struct simulation_configuration conf = { + .lps = 16, + .n_threads = 1, + .serial = false, + .dispatcher = (ProcessEvent_t)1, + .committed = (CanEnd_t)1, +}; + +int main(void) +{ + RootsimInit(&conf); + int ret = test_ctrl_msg(true); + mpi_global_fini(); + return ret; +} diff --git a/test/tests/sdk/parallel.c b/test/tests/sdk/parallel.c new file mode 100644 index 00000000..dedda70f --- /dev/null +++ b/test/tests/sdk/parallel.c @@ -0,0 +1,17 @@ +#include + +#include "control_msg.h" + +struct simulation_configuration conf = { + .lps = 16, + .n_threads = 0, + .serial = false, + .dispatcher = (ProcessEvent_t)1, + .committed = (CanEnd_t)1, +}; + +int main(void) +{ + RootsimInit(&conf); + return test_ctrl_msg(false); +} diff --git a/test/tests/sdk/serial.c b/test/tests/sdk/serial.c new file mode 100644 index 00000000..187c877c --- /dev/null +++ b/test/tests/sdk/serial.c @@ -0,0 +1,17 @@ +#include + +#include "control_msg.h" + +struct simulation_configuration conf = { + .lps = 16, + .n_threads = 0, + .serial = true, + .dispatcher = (ProcessEvent_t)ProcessEvent, + .committed = (CanEnd_t)1, +}; + +int main(void) +{ + RootsimInit(&conf); + return test_ctrl_msg(false); +} From 96db962955e0046471ee1b00c6df62f2d16ac50a Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Sat, 29 Apr 2023 00:27:31 +0200 Subject: [PATCH 04/13] Fix wrong initialization of ROOTSIM_VERSION A stale space was messing up everything. Signed-off-by: Alessandro Pellegrini --- src/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index fcc43808..b476d286 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -34,7 +34,7 @@ endif() # Build the core library add_library(rscore STATIC ${rscore_srcs}) -target_compile_definitions(rscore PRIVATE ROOTSIM_VERSION= "${PROJECT_VERSION}") +target_compile_definitions(rscore PRIVATE ROOTSIM_VERSION="${PROJECT_VERSION}") target_include_directories(rscore PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/include/) target_link_libraries(rscore ${CMAKE_THREAD_LIBS_INIT} ${EXTRA_LIBS}) From bff2ff3abcfe0b4ce2e2beac8816b67b424593dd Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Sat, 29 Apr 2023 01:25:18 +0200 Subject: [PATCH 05/13] Move library control messages to a separate tag In this way, we don't enlarge the size of GVT control messages which are used much more often. Signed-off-by: Alessandro Pellegrini --- src/distributed/control_msg.c | 12 +++--- src/distributed/control_msg.h | 15 ++++--- src/distributed/mpi.c | 73 +++++++++++++++++++++++------------ src/distributed/mpi.h | 4 +- test/tests/sdk/control_msg.c | 4 +- 5 files changed, 65 insertions(+), 43 deletions(-) diff --git a/src/distributed/control_msg.c b/src/distributed/control_msg.c index 48f3af2a..91a555d6 100644 --- a/src/distributed/control_msg.c +++ b/src/distributed/control_msg.c @@ -20,7 +20,7 @@ struct library_handler { static struct library_handler *library_handlers = NULL; static size_t library_handlers_capacity = 0; static size_t library_handlers_size = 0; -static int next_control_msg_id = MSG_CTRL_DEFAULT_END; +static int next_control_msg_id = FIRST_LIBRARY_CONTROL_MSG_ID; int control_msg_register_handler(control_msg_handler_t handler) { @@ -54,13 +54,15 @@ void invoke_library_handler(unsigned code, const void *payload) return; } } + assert(false); + __builtin_unreachable(); } /** * @brief Handle a received control message * @param ctrl the tag of the received control message */ -void control_msg_process(unsigned ctrl, void *payload) +void control_msg_process(enum platform_ctrl_msg_code ctrl) { switch(ctrl) { case MSG_CTRL_GVT_START: @@ -73,16 +75,12 @@ void control_msg_process(unsigned ctrl, void *payload) termination_on_ctrl_msg(); break; default: - invoke_library_handler(ctrl, payload); + __builtin_unreachable(); } } void control_msg_broadcast(unsigned ctrl, const void *payload, size_t size) { - if(size > CONTROL_MSG_PAYLOAD_SIZE) { - logger(LOG_FATAL, "The payload of the control message is too large"); - abort(); - } if(global_config.serial || n_nodes == 1) { invoke_library_handler(ctrl, payload); } else { diff --git a/src/distributed/control_msg.h b/src/distributed/control_msg.h index 54c44321..e861937f 100644 --- a/src/distributed/control_msg.h +++ b/src/distributed/control_msg.h @@ -16,26 +16,25 @@ #include "include/ROOT-Sim/sdk.h" #define INITIAL_HANDLERS_CAPACITY 4 +#define FIRST_LIBRARY_CONTROL_MSG_ID 1 /// A control message MPI tag value -enum default_msg_ctrl_code { +enum platform_ctrl_msg_code { /// Used by the master to start a new gvt reduction operation MSG_CTRL_GVT_START = 1, /// Used by slaves to signal their completion of the gvt protocol MSG_CTRL_GVT_DONE, /// Used in broadcast to signal that local LPs can terminate - MSG_CTRL_TERMINATION, - /// Marker to signal the end of the enum - MSG_CTRL_DEFAULT_END + MSG_CTRL_TERMINATION }; -struct control_msg { +/// A control message that can be user by higher-level libraries to synchronize actions +struct library_ctrl_msg { /// The control message code unsigned ctrl_code; /// The payload of the control message char payload[CONTROL_MSG_PAYLOAD_SIZE]; }; -_Static_assert(sizeof(struct control_msg) < msg_remote_anti_size(), "Invalid control message payload size"); - -extern void control_msg_process(unsigned ctrl, void *payload); +extern void control_msg_process(enum platform_ctrl_msg_code ctrl); +extern void invoke_library_handler(unsigned code, const void *payload); diff --git a/src/distributed/mpi.c b/src/distributed/mpi.c index b6598f22..894f588b 100644 --- a/src/distributed/mpi.c +++ b/src/distributed/mpi.c @@ -20,7 +20,15 @@ #include enum { RS_MSG_TAG = 0, - RS_DATA_TAG + RS_DATA_TAG, + RS_CTRL_TAG +}; + +/// Array of control codes values to be able to get their address for MPI_Send() +static const enum platform_ctrl_msg_code ctrl_msgs[] = { + [MSG_CTRL_GVT_START] = MSG_CTRL_GVT_START, + [MSG_CTRL_GVT_DONE] = MSG_CTRL_GVT_DONE, + [MSG_CTRL_TERMINATION] = MSG_CTRL_TERMINATION }; /// The MPI request associated with the non blocking scatter gather collective @@ -138,14 +146,14 @@ void mpi_remote_anti_msg_send(struct lp_msg *msg, nid_t dest_nid) void mpi_library_control_msg_broadcast(unsigned ctrl, const void *payload, size_t size) { nid_t i = n_nodes; - struct control_msg msg = {.ctrl_code = ctrl, {0}}; + struct library_ctrl_msg msg = {.ctrl_code = ctrl, {0}}; if(unlikely(payload != NULL)) { memcpy(&msg.payload, payload, size); } while(i--) { MPI_Request req; - MPI_Isend(&msg, sizeof(msg), MPI_BYTE, i, RS_MSG_TAG, MPI_COMM_WORLD, &req); + MPI_Isend(&msg, sizeof(msg), MPI_BYTE, i, RS_CTRL_TAG, MPI_COMM_WORLD, &req); MPI_Request_free(&req); } } @@ -154,7 +162,7 @@ void mpi_library_control_msg_broadcast(unsigned ctrl, const void *payload, size_ * @brief Sends a platform control message to all the nodes, including self * @param ctrl the control message to send */ -void mpi_control_msg_broadcast(unsigned ctrl) +void mpi_control_msg_broadcast(enum platform_ctrl_msg_code ctrl) { nid_t i = n_nodes; while(i--) { @@ -167,11 +175,10 @@ void mpi_control_msg_broadcast(unsigned ctrl) * @param ctrl the control message to send * @param dest the id of the destination node */ -void mpi_control_msg_send_to(enum default_msg_ctrl_code ctrl, nid_t dest) +void mpi_control_msg_send_to(enum platform_ctrl_msg_code ctrl, nid_t dest) { MPI_Request req; - struct control_msg msg = {.ctrl_code = ctrl, {0}}; - MPI_Isend(&msg, sizeof(msg), MPI_BYTE, dest, RS_MSG_TAG, MPI_COMM_WORLD, &req); + MPI_Isend(&ctrl_msgs[ctrl], sizeof(*ctrl_msgs), MPI_BYTE, dest, RS_MSG_TAG, MPI_COMM_WORLD, &req); MPI_Request_free(&req); } @@ -185,13 +192,22 @@ void mpi_control_msg_send_to(enum default_msg_ctrl_code ctrl, nid_t dest) */ void mpi_remote_msg_handle(void) { - while(1) { - int pending; - MPI_Message mpi_msg; - MPI_Status status; + int pending; + MPI_Message mpi_msg; + MPI_Status status; - MPI_Improbe(MPI_ANY_SOURCE, RS_MSG_TAG, MPI_COMM_WORLD, &pending, &mpi_msg, &status); + while(true) { + MPI_Improbe(MPI_ANY_SOURCE, RS_CTRL_TAG, MPI_COMM_WORLD, &pending, &mpi_msg, &status); + if(likely(!pending)) + break; + + struct library_ctrl_msg ctrl_msg; + MPI_Mrecv(&ctrl_msg, sizeof(ctrl_msg), MPI_BYTE, &mpi_msg, MPI_STATUS_IGNORE); + invoke_library_handler(ctrl_msg.ctrl_code, &ctrl_msg.payload); + } + while(true) { + MPI_Improbe(MPI_ANY_SOURCE, RS_MSG_TAG, MPI_COMM_WORLD, &pending, &mpi_msg, &status); if(!pending) return; @@ -199,10 +215,10 @@ void mpi_remote_msg_handle(void) MPI_Get_count(&status, MPI_BYTE, &size); struct lp_msg *msg; if(unlikely(size <= (int)msg_remote_anti_size())) { - if(unlikely(size == sizeof(struct control_msg))) { - struct control_msg ctrl; - MPI_Mrecv(&ctrl, sizeof(ctrl), MPI_BYTE, &mpi_msg, MPI_STATUS_IGNORE); - control_msg_process(ctrl.ctrl_code, ctrl.payload); + if(unlikely(size == sizeof(enum platform_ctrl_msg_code))) { + enum platform_ctrl_msg_code c; + MPI_Mrecv(&c, sizeof(c), MPI_BYTE, &mpi_msg, MPI_STATUS_IGNORE); + control_msg_process(c); continue; } msg = msg_allocator_alloc(0); @@ -230,14 +246,23 @@ void mpi_remote_msg_handle(void) */ void mpi_remote_msg_drain(void) { + int pending; + MPI_Message mpi_msg; + MPI_Status status; struct lp_msg *msg = NULL; int msg_size = 0; - while(1) { - int pending; - MPI_Message mpi_msg; - MPI_Status status; + while(true) { + MPI_Improbe(MPI_ANY_SOURCE, RS_CTRL_TAG, MPI_COMM_WORLD, &pending, &mpi_msg, &status); + if(likely(!pending)) + break; + + struct library_ctrl_msg ctrl_msg; + MPI_Mrecv(&ctrl_msg, sizeof(ctrl_msg), MPI_BYTE, &mpi_msg, MPI_STATUS_IGNORE); + invoke_library_handler(ctrl_msg.ctrl_code, &ctrl_msg.payload); + } + while(true) { MPI_Improbe(MPI_ANY_SOURCE, RS_MSG_TAG, MPI_COMM_WORLD, &pending, &mpi_msg, &status); if(!pending) @@ -246,10 +271,10 @@ void mpi_remote_msg_drain(void) int size; MPI_Get_count(&status, MPI_BYTE, &size); - if(unlikely(size == sizeof(struct control_msg))) { - struct control_msg ctrl; - MPI_Mrecv(&ctrl, sizeof(ctrl), MPI_BYTE, &mpi_msg, MPI_STATUS_IGNORE); - control_msg_process(ctrl.ctrl_code, ctrl.payload); + if(unlikely(size == sizeof(enum platform_ctrl_msg_code))) { + enum platform_ctrl_msg_code c; + MPI_Mrecv(&c, sizeof(c), MPI_BYTE, &mpi_msg, MPI_STATUS_IGNORE); + control_msg_process(c); continue; } diff --git a/src/distributed/mpi.h b/src/distributed/mpi.h index 6ffe1add..e094140b 100644 --- a/src/distributed/mpi.h +++ b/src/distributed/mpi.h @@ -25,8 +25,8 @@ extern void mpi_remote_msg_send(struct lp_msg *msg, nid_t dest_nid); extern void mpi_remote_anti_msg_send(struct lp_msg *msg, nid_t dest_nid); extern void mpi_library_control_msg_broadcast(unsigned ctrl, const void *payload, size_t size); -extern void mpi_control_msg_broadcast(unsigned ctrl); -extern void mpi_control_msg_send_to(unsigned ctrl, nid_t dest); +extern void mpi_control_msg_broadcast(enum platform_ctrl_msg_code ctrl); +extern void mpi_control_msg_send_to(enum platform_ctrl_msg_code ctrl, nid_t dest); extern void mpi_remote_msg_handle(void); extern void mpi_remote_msg_drain(void); diff --git a/test/tests/sdk/control_msg.c b/test/tests/sdk/control_msg.c index bada679d..ca055303 100644 --- a/test/tests/sdk/control_msg.c +++ b/test/tests/sdk/control_msg.c @@ -35,8 +35,8 @@ int test_ctrl_msg(bool distributed) ctrl_msg_id1 = control_msg_register_handler(handler); ctrl_msg_id2 = control_msg_register_handler(handler); - assert(ctrl_msg_id1 == MSG_CTRL_DEFAULT_END); - assert(ctrl_msg_id2 > MSG_CTRL_DEFAULT_END); + assert(ctrl_msg_id1 == FIRST_LIBRARY_CONTROL_MSG_ID); + assert(ctrl_msg_id2 > FIRST_LIBRARY_CONTROL_MSG_ID); control_msg_broadcast(ctrl_msg_id1, &value, sizeof(value)); control_msg_broadcast(ctrl_msg_id2, &value, sizeof(value)); From 9e39ee483c7e2a0c102f46ae012e50b677967cae Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Sat, 29 Apr 2023 01:40:33 +0200 Subject: [PATCH 06/13] REUSE compliance The new test files were not bearing a copyright notice. Signed-off-by: Alessandro Pellegrini --- test/tests/sdk/control_msg.c | 8 ++++++++ test/tests/sdk/control_msg.h | 8 ++++++++ test/tests/sdk/distributed.c | 8 ++++++++ test/tests/sdk/parallel.c | 8 ++++++++ test/tests/sdk/serial.c | 8 ++++++++ 5 files changed, 40 insertions(+) diff --git a/test/tests/sdk/control_msg.c b/test/tests/sdk/control_msg.c index ca055303..2aa7faf2 100644 --- a/test/tests/sdk/control_msg.c +++ b/test/tests/sdk/control_msg.c @@ -1,3 +1,11 @@ +/** + * @file test/tests/sdk/control_msg.c + * + * @brief Test: Higher-level control messages + * + * SPDX-FileCopyrightText: 2008-2022 HPDCS Group + * SPDX-License-Identifier: GPL-3.0-only + */ #include #include diff --git a/test/tests/sdk/control_msg.h b/test/tests/sdk/control_msg.h index a03cba9c..0762e89b 100644 --- a/test/tests/sdk/control_msg.h +++ b/test/tests/sdk/control_msg.h @@ -1,3 +1,11 @@ +/** +* @file test/tests/sdk/control_msg.h +* +* @brief Test: Higher-level control messages +* +* SPDX-FileCopyrightText: 2008-2022 HPDCS Group +* SPDX-License-Identifier: GPL-3.0-only + */ #pragma once #include diff --git a/test/tests/sdk/distributed.c b/test/tests/sdk/distributed.c index 6bbcb2ea..98468157 100644 --- a/test/tests/sdk/distributed.c +++ b/test/tests/sdk/distributed.c @@ -1,3 +1,11 @@ +/** + * @file test/tests/sdk/distributed.c + * + * @brief Test: Higher-level control messages (MPI test) + * + * SPDX-FileCopyrightText: 2008-2022 HPDCS Group + * SPDX-License-Identifier: GPL-3.0-only + */ #include #include diff --git a/test/tests/sdk/parallel.c b/test/tests/sdk/parallel.c index dedda70f..5f474b08 100644 --- a/test/tests/sdk/parallel.c +++ b/test/tests/sdk/parallel.c @@ -1,3 +1,11 @@ +/** +* @file test/tests/sdk/parallel.c +* +* @brief Test: Higher-level control messages (parallel test) +* +* SPDX-FileCopyrightText: 2008-2022 HPDCS Group +* SPDX-License-Identifier: GPL-3.0-only +*/ #include #include "control_msg.h" diff --git a/test/tests/sdk/serial.c b/test/tests/sdk/serial.c index 187c877c..28532a3e 100644 --- a/test/tests/sdk/serial.c +++ b/test/tests/sdk/serial.c @@ -1,3 +1,11 @@ +/** +* @file test/tests/sdk/distributed.c +* +* @brief Test: Higher-level control messages (sequential test) +* +* SPDX-FileCopyrightText: 2008-2022 HPDCS Group +* SPDX-License-Identifier: GPL-3.0-only +*/ #include #include "control_msg.h" From 4fd97cb6e6379ada88763d7c82f8820ad26fbfb6 Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Thu, 4 May 2023 09:18:26 +0200 Subject: [PATCH 07/13] Non-lazy initialization of control messages An explicit initialization function is invoked. Signed-off-by: Alessandro Pellegrini --- src/CMakeLists.txt | 2 +- src/{distributed => core}/control_msg.c | 26 +++++++++++++++++-------- src/{distributed => core}/control_msg.h | 8 +++++--- src/distributed/mpi.h | 2 +- src/parallel/parallel.c | 2 ++ src/serial/serial.c | 3 +++ test/tests/sdk/control_msg.c | 2 +- 7 files changed, 31 insertions(+), 14 deletions(-) rename src/{distributed => core}/control_msg.c (82%) rename src/{distributed => core}/control_msg.h (88%) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a075b170..ab326306 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -8,7 +8,7 @@ set(rscore_srcs init.c core/sync.c datatypes/msg_queue.c - distributed/control_msg.c + core/control_msg.c gvt/fossil.c gvt/gvt.c gvt/termination.c diff --git a/src/distributed/control_msg.c b/src/core/control_msg.c similarity index 82% rename from src/distributed/control_msg.c rename to src/core/control_msg.c index da54ea14..3a4d989b 100644 --- a/src/distributed/control_msg.c +++ b/src/core/control_msg.c @@ -8,9 +8,9 @@ * SPDX-FileCopyrightText: 2008-2023 HPDCS Group * SPDX-License-Identifier: GPL-3.0-only */ -#include -#include -#include +#include "include/ROOT-Sim/sdk.h" +#include "control_msg.h" +#include "distributed/mpi.h" struct library_handler { unsigned control_msg_id; @@ -22,18 +22,28 @@ static size_t library_handlers_capacity = 0; static size_t library_handlers_size = 0; static int next_control_msg_id = FIRST_LIBRARY_CONTROL_MSG_ID; +void control_msg_init(void) +{ + library_handlers_capacity = INITIAL_HANDLERS_CAPACITY; + library_handlers = malloc(library_handlers_capacity * sizeof(*library_handlers)); +} + +void control_msg_fini(void) +{ + free(library_handlers); +} + int control_msg_register_handler(control_msg_handler_t handler) { int ret = next_control_msg_id; - if(unlikely(library_handlers == NULL)) { - library_handlers_capacity = INITIAL_HANDLERS_CAPACITY; - library_handlers = malloc(library_handlers_capacity * sizeof(*library_handlers)); - } - if(library_handlers_size == library_handlers_capacity) { library_handlers_capacity *= 2; library_handlers = realloc(library_handlers, library_handlers_capacity * sizeof(*library_handlers)); + if(unlikely(library_handlers == NULL)) { + logger(LOG_FATAL, "Error registering external library handler!"); + abort(); + } } library_handlers[library_handlers_size].control_msg_id = next_control_msg_id++; library_handlers[library_handlers_size++].handler = handler; diff --git a/src/distributed/control_msg.h b/src/core/control_msg.h similarity index 88% rename from src/distributed/control_msg.h rename to src/core/control_msg.h index fe87528e..8ae01354 100644 --- a/src/distributed/control_msg.h +++ b/src/core/control_msg.h @@ -10,9 +10,9 @@ */ #pragma once -#include -#include -#include +#include "lp/msg.h" +#include "gvt/gvt.h" +#include "gvt/termination.h" #include "include/ROOT-Sim/sdk.h" #define INITIAL_HANDLERS_CAPACITY 4 @@ -36,5 +36,7 @@ struct library_ctrl_msg { char payload[CONTROL_MSG_PAYLOAD_SIZE]; }; +extern void control_msg_init(void); +extern void control_msg_fini(void); extern void control_msg_process(enum platform_ctrl_msg_code ctrl); extern void invoke_library_handler(unsigned code, const void *payload); diff --git a/src/distributed/mpi.h b/src/distributed/mpi.h index fd402b31..86bcf82d 100644 --- a/src/distributed/mpi.h +++ b/src/distributed/mpi.h @@ -15,7 +15,7 @@ */ #pragma once -#include +#include "core/control_msg.h" #include extern void mpi_global_init(int *argc_p, char ***argv_p); diff --git a/src/parallel/parallel.c b/src/parallel/parallel.c index dd4797c9..cece86b5 100644 --- a/src/parallel/parallel.c +++ b/src/parallel/parallel.c @@ -89,10 +89,12 @@ void parallel_global_init(void) msg_queue_global_init(); termination_global_init(); gvt_global_init(); + control_msg_init(); } static void parallel_global_fini(void) { + control_msg_fini(); msg_queue_global_fini(); lp_global_fini(); stats_global_fini(); diff --git a/src/serial/serial.c b/src/serial/serial.c index 7649a3ed..65b00f4a 100644 --- a/src/serial/serial.c +++ b/src/serial/serial.c @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -26,6 +27,7 @@ void serial_simulation_init(void) stats_init(); msg_allocator_init(); heap_init(queue); + control_msg_init(); lps = mm_alloc(sizeof(*lps) * global_config.lps); memset(lps, 0, sizeof(*lps) * global_config.lps); @@ -71,6 +73,7 @@ static void serial_simulation_fini(void) mm_free(lps); + control_msg_fini(); heap_fini(queue); msg_allocator_fini(); stats_global_fini(); diff --git a/test/tests/sdk/control_msg.c b/test/tests/sdk/control_msg.c index 2aa7faf2..bd852d55 100644 --- a/test/tests/sdk/control_msg.c +++ b/test/tests/sdk/control_msg.c @@ -9,7 +9,7 @@ #include #include -#include +#include "core/control_msg.h" #include #include From 02f4a325ae151e4145475860f3190a9f7bbdb95a Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Thu, 4 May 2023 09:34:06 +0200 Subject: [PATCH 08/13] Improve documentation and code cleanup Minor changes to the code to improve docs and cleanup Signed-off-by: Alessandro Pellegrini --- src/core/control_msg.c | 15 ++++++++++++++- src/core/control_msg.h | 7 ++++++- src/distributed/mpi.c | 6 ++++++ src/include/ROOT-Sim/sdk.h | 6 ++---- test/tests/sdk/control_msg.c | 11 +++++++++-- 5 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/core/control_msg.c b/src/core/control_msg.c index 3a4d989b..c34739d0 100644 --- a/src/core/control_msg.c +++ b/src/core/control_msg.c @@ -1,5 +1,5 @@ /** - * @file distributed/control_msg.c + * @file core/control_msg.c * * @brief MPI remote control messages module * @@ -12,22 +12,35 @@ #include "control_msg.h" #include "distributed/mpi.h" +/// This structure is used to store the handlers for the control messages struct library_handler { + /// The control message id unsigned control_msg_id; + /// The handler for the control message control_msg_handler_t handler; }; +/// The array of handlers for the control messages static struct library_handler *library_handlers = NULL; +/// The capacity of the array of handlers for the control messages static size_t library_handlers_capacity = 0; +/// The current number of used slots in the array of handlers for the control messages static size_t library_handlers_size = 0; +/// The next control message ID static int next_control_msg_id = FIRST_LIBRARY_CONTROL_MSG_ID; +/** + * @brief Initialize the control message module + */ void control_msg_init(void) { library_handlers_capacity = INITIAL_HANDLERS_CAPACITY; library_handlers = malloc(library_handlers_capacity * sizeof(*library_handlers)); } +/** + * @brief Finalize the control message module + */ void control_msg_fini(void) { free(library_handlers); diff --git a/src/core/control_msg.h b/src/core/control_msg.h index 8ae01354..2aa59243 100644 --- a/src/core/control_msg.h +++ b/src/core/control_msg.h @@ -1,5 +1,5 @@ /** - * @file distributed/control_msg.h + * @file core/control_msg.h * * @brief MPI remote control messages header * @@ -15,9 +15,14 @@ #include "gvt/termination.h" #include "include/ROOT-Sim/sdk.h" +/// The initial size of the handlers dynamic vector #define INITIAL_HANDLERS_CAPACITY 4 +/// The first control message ID that will be returned to the external libraries #define FIRST_LIBRARY_CONTROL_MSG_ID 1 +/// The size of the maximum payload of a control message +#define CONTROL_MSG_PAYLOAD_SIZE 16 + /// A control message MPI tag value enum platform_ctrl_msg_code { /// Used by the master to start a new gvt reduction operation diff --git a/src/distributed/mpi.c b/src/distributed/mpi.c index c267fa7c..40ca7f41 100644 --- a/src/distributed/mpi.c +++ b/src/distributed/mpi.c @@ -142,12 +142,18 @@ void mpi_remote_anti_msg_send(struct lp_msg *msg, nid_t dest_nid) /** * @brief Sends a library control message to all the nodes, including self * @param ctrl the control message to send + * @param payload the payload to send with the message + * @param size the size of the payload */ void mpi_library_control_msg_broadcast(unsigned ctrl, const void *payload, size_t size) { nid_t i = n_nodes; struct library_ctrl_msg msg = {.ctrl_code = ctrl, {0}}; if(unlikely(payload != NULL)) { + if(unlikely(size >= CONTROL_MSG_PAYLOAD_SIZE)) { + logger(LOG_FATAL, "Payload too big for a library control message"); + abort(); + } memcpy(&msg.payload, payload, size); } diff --git a/src/include/ROOT-Sim/sdk.h b/src/include/ROOT-Sim/sdk.h index dc1acf8d..e23892d2 100644 --- a/src/include/ROOT-Sim/sdk.h +++ b/src/include/ROOT-Sim/sdk.h @@ -14,10 +14,7 @@ #include -/// The size of the maximum payload of a control message -#define CONTROL_MSG_PAYLOAD_SIZE 16 - -// The type of control message handlers that libraries can register +/// The type of control message handlers that libraries can register typedef void (*control_msg_handler_t)(unsigned ctrl_msg_id, const void *payload); /** @@ -42,5 +39,6 @@ extern int control_msg_register_handler(control_msg_handler_t handler); * * @param ctrl the control message ID, as returned by control_msg_register_handler() * @param payload the payload of the control message, must be of size CONTROL_MSG_PAYLOAD_SIZE at most + * @param size the size of the payload */ extern void control_msg_broadcast(unsigned ctrl, const void *payload, size_t size); diff --git a/test/tests/sdk/control_msg.c b/test/tests/sdk/control_msg.c index bd852d55..b40af109 100644 --- a/test/tests/sdk/control_msg.c +++ b/test/tests/sdk/control_msg.c @@ -25,9 +25,16 @@ static unsigned ctrl_msg_id2 = 0; static unsigned short inv1 = 0; static unsigned short inv2 = 0; -void handler(unsigned ctrl_msg_id, const void *payload) +#ifdef NDEBUG +#define UNUSED _unused +#else +#define UNUSED +#endif + + +void handler(unsigned ctrl_msg_id, UNUSED const void *payload) { - assert(*(long *)payload == value); + assert(*(const long *)payload == value); if(ctrl_msg_id == ctrl_msg_id1) { inv1++; From 637425561bd7ec97e5fcfa38d8d585e058d2c4d0 Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Thu, 4 May 2023 11:54:14 +0200 Subject: [PATCH 09/13] Improve code for linters Some code reworking to make linters happy. Signed-off-by: Alessandro Pellegrini --- src/core/control_msg.c | 57 +++++++++++++++++++++++++++++------------- src/core/control_msg.h | 6 +++-- src/include/ROOT-Sim.h | 4 +-- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/src/core/control_msg.c b/src/core/control_msg.c index c34739d0..1c3698b5 100644 --- a/src/core/control_msg.c +++ b/src/core/control_msg.c @@ -34,8 +34,21 @@ static int next_control_msg_id = FIRST_LIBRARY_CONTROL_MSG_ID; */ void control_msg_init(void) { - library_handlers_capacity = INITIAL_HANDLERS_CAPACITY; - library_handlers = malloc(library_handlers_capacity * sizeof(*library_handlers)); +#ifndef NDEBUG + if(library_handlers != NULL) { + logger(LOG_WARN, "Trying to reinitialize control message module, ignoring!"); + return; + } +#endif + size_t size = INITIAL_HANDLERS_CAPACITY * sizeof(*library_handlers); + void *ptr = malloc(size); + if (ptr != NULL) { + library_handlers_capacity = INITIAL_HANDLERS_CAPACITY; + library_handlers = ptr; + } else { + logger(LOG_FATAL, "Error initializing control message module!"); + abort(); + } } /** @@ -48,22 +61,25 @@ void control_msg_fini(void) int control_msg_register_handler(control_msg_handler_t handler) { - int ret = next_control_msg_id; - - if(library_handlers_size == library_handlers_capacity) { - library_handlers_capacity *= 2; - library_handlers = realloc(library_handlers, library_handlers_capacity * sizeof(*library_handlers)); - if(unlikely(library_handlers == NULL)) { - logger(LOG_FATAL, "Error registering external library handler!"); - abort(); - } + if (library_handlers_size >= library_handlers_capacity) { + size_t new_capacity = library_handlers_capacity * 2; + struct library_handler *new_handlers = realloc(library_handlers, new_capacity * sizeof(*new_handlers)); + if (new_handlers == NULL) { + logger(LOG_FATAL, "Error registering external library handler!"); + abort(); + } + library_handlers = new_handlers; + library_handlers_capacity = new_capacity; } - library_handlers[library_handlers_size].control_msg_id = next_control_msg_id++; - library_handlers[library_handlers_size++].handler = handler; + int ret = next_control_msg_id++; + library_handlers[library_handlers_size].control_msg_id = ret; + library_handlers[library_handlers_size].handler = handler; + library_handlers_size++; return ret; } + /** * @brief Invoke a library handler * @param code the control message ID @@ -71,16 +87,17 @@ int control_msg_register_handler(control_msg_handler_t handler) */ void invoke_library_handler(unsigned code, const void *payload) { - for(size_t i = 0; i < library_handlers_size; i++) { - if(library_handlers[i].control_msg_id == code) { + for (size_t i = 0; i < library_handlers_size; i++) { + if (library_handlers[i].control_msg_id == code) { library_handlers[i].handler(code, payload); return; } } - assert(false); - __builtin_unreachable(); + logger(LOG_FATAL, "No library handler registered for code %u", code); + abort(); } + /** * @brief Handle a received control message * @param ctrl the tag of the received control message @@ -98,13 +115,17 @@ void control_msg_process(enum platform_ctrl_msg_code ctrl) termination_on_ctrl_msg(); break; default: +#ifndef NDEBUG + logger(LOG_WARN, "Unknown control message code %d", ctrl); +#else __builtin_unreachable(); +#endif } } void control_msg_broadcast(unsigned ctrl, const void *payload, size_t size) { - if(global_config.serial || n_nodes == 1) { + if(global_config.serial || (n_nodes == 1)) { invoke_library_handler(ctrl, payload); } else { mpi_library_control_msg_broadcast(ctrl, payload, size); diff --git a/src/core/control_msg.h b/src/core/control_msg.h index 2aa59243..a7fa3123 100644 --- a/src/core/control_msg.h +++ b/src/core/control_msg.h @@ -18,7 +18,7 @@ /// The initial size of the handlers dynamic vector #define INITIAL_HANDLERS_CAPACITY 4 /// The first control message ID that will be returned to the external libraries -#define FIRST_LIBRARY_CONTROL_MSG_ID 1 +#define FIRST_LIBRARY_CONTROL_MSG_ID 0 /// The size of the maximum payload of a control message #define CONTROL_MSG_PAYLOAD_SIZE 16 @@ -34,11 +34,13 @@ enum platform_ctrl_msg_code { }; /// A control message that can be user by higher-level libraries to synchronize actions +/* cppcheck-suppress misra-c2012-2.4 + * The payload is anything used specified, and the size is checked in the code */ struct library_ctrl_msg { /// The control message code unsigned ctrl_code; /// The payload of the control message - char payload[CONTROL_MSG_PAYLOAD_SIZE]; + unsigned char payload[CONTROL_MSG_PAYLOAD_SIZE]; }; extern void control_msg_init(void); diff --git a/src/include/ROOT-Sim.h b/src/include/ROOT-Sim.h index 9b4c2503..01656333 100644 --- a/src/include/ROOT-Sim.h +++ b/src/include/ROOT-Sim.h @@ -99,8 +99,8 @@ enum log_level { LOG_INFO, //!< The logging level reserved to useful runtime messages LOG_WARN, //!< The logging level reserved to unexpected, non deal breaking conditions LOG_ERROR, //!< The logging level reserved to unexpected, problematic conditions - LOG_FATAL, //!< The logging level reserved to unexpected, fatal conditions - LOG_SILENT //!< Emit no message during the simulation + LOG_FATAL, //!< The logging level reserved to unexpected, fatal conditions + LOG_SILENT //!< Emit no message during the simulation }; /// A set of configurable values used by other modules From c383281ce28edad2d351aa47060554c311b76be8 Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Thu, 4 May 2023 12:30:15 +0200 Subject: [PATCH 10/13] Fixing tests Some work to fix tests: check if we have already free'd the handlers, and don't force binding on phold, to let mpi test run. Signed-off-by: Alessandro Pellegrini --- src/core/control_msg.c | 8 +++++--- test/tests/integration/phold.c | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/core/control_msg.c b/src/core/control_msg.c index 1c3698b5..bc0ca241 100644 --- a/src/core/control_msg.c +++ b/src/core/control_msg.c @@ -34,12 +34,11 @@ static int next_control_msg_id = FIRST_LIBRARY_CONTROL_MSG_ID; */ void control_msg_init(void) { -#ifndef NDEBUG if(library_handlers != NULL) { logger(LOG_WARN, "Trying to reinitialize control message module, ignoring!"); return; } -#endif + size_t size = INITIAL_HANDLERS_CAPACITY * sizeof(*library_handlers); void *ptr = malloc(size); if (ptr != NULL) { @@ -56,7 +55,10 @@ void control_msg_init(void) */ void control_msg_fini(void) { - free(library_handlers); + if(library_handlers != NULL) { + free(library_handlers); + library_handlers = NULL; + } } int control_msg_register_handler(control_msg_handler_t handler) diff --git a/test/tests/integration/phold.c b/test/tests/integration/phold.c index c8a1cac3..ff13bbf6 100644 --- a/test/tests/integration/phold.c +++ b/test/tests/integration/phold.c @@ -20,7 +20,7 @@ #endif #ifndef NUM_THREADS -#define NUM_THREADS 2 +#define NUM_THREADS 0 #endif #define EVENT 1 @@ -105,7 +105,7 @@ struct simulation_configuration conf = { .log_level = LOG_INFO, .stats_file = "phold", .ckpt_interval = 0, - .core_binding = true, + .core_binding = false, .serial = false, .dispatcher = ProcessEvent, .committed = CanEnd, From ee9275a2973cc2ff8f0fc524afb6c488e2cf5a1c Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Fri, 5 May 2023 15:05:10 +0200 Subject: [PATCH 11/13] Export include path to external libraries Otherwise this doesn't work! Signed-off-by: Alessandro Pellegrini --- src/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ab326306..d3c662a0 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -35,7 +35,7 @@ endif() add_library(rscore STATIC ${rscore_srcs}) target_compile_definitions(rscore PRIVATE ROOTSIM_VERSION="${PROJECT_VERSION}") -target_include_directories(rscore PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/include/) +target_include_directories(rscore PUBLIC ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/include/) target_link_libraries(rscore ${CMAKE_THREAD_LIBS_INIT} ${EXTRA_LIBS}) if(NOT DISABLE_MPI) From 415577a06812fc210b93a3cffa32988b64079b50 Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Sat, 6 May 2023 05:53:31 +0200 Subject: [PATCH 12/13] Prevent running tests and installing libs/headers if used as lib When importing using FetchContent, it is now possible to use the IMPORT_AS_LIB switch to prevent generating test targets and avoid installib headers/binaries when directly importing as a library. Signed-off-by: Alessandro Pellegrini --- CMakeLists.txt | 12 +++++++----- src/CMakeLists.txt | 6 ++++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index edf4f524..1a70b947 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -47,9 +47,11 @@ endif() add_subdirectory(src) -# Run the tests -enable_testing() -add_subdirectory(test) +if(NOT IMPORT_AS_LIB) + # Run the tests + enable_testing() + add_subdirectory(test) -# Generate and inspect documentation -add_subdirectory(docs) + # Generate and inspect documentation + add_subdirectory(docs) +endif() diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index d3c662a0..b434e90a 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -44,5 +44,7 @@ if(NOT DISABLE_MPI) target_link_libraries(rscore ${MPI_C_LIBRARIES}) endif() -install(DIRECTORY include/ DESTINATION include) -install(TARGETS rscore LIBRARY DESTINATION lib) +if(NOT IMPORT_AS_LIB) + install(DIRECTORY include/ DESTINATION include) + install(TARGETS rscore LIBRARY DESTINATION lib) +endif() From 937f9fc7e85d4aa4aecbde1ad8ee6af47733aeaf Mon Sep 17 00:00:00 2001 From: Alessandro Pellegrini Date: Wed, 24 May 2023 14:03:17 +0200 Subject: [PATCH 13/13] Export logging facility to the SDK Libraries may leverage the internal logging facilities, which are therefore exported in this commit. Signed-off-by: Alessandro Pellegrini --- src/include/ROOT-Sim/sdk.h | 11 +++++++++++ src/log/log.c | 3 ++- src/log/log.h | 11 +---------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/include/ROOT-Sim/sdk.h b/src/include/ROOT-Sim/sdk.h index e23892d2..f748c3ea 100644 --- a/src/include/ROOT-Sim/sdk.h +++ b/src/include/ROOT-Sim/sdk.h @@ -12,6 +12,7 @@ */ #pragma once +#include #include /// The type of control message handlers that libraries can register @@ -42,3 +43,13 @@ extern int control_msg_register_handler(control_msg_handler_t handler); * @param size the size of the payload */ extern void control_msg_broadcast(unsigned ctrl, const void *payload, size_t size); + + +extern void vlogger(enum log_level level, char *file, unsigned line, const char *fmt, ...); + +/** + * @brief Produce a log message + * @param level the logging level associated to the message + * @param ... the format string followed by its arguments if needed + */ +#define logger(level, ...) vlogger(level, __FILE__, __LINE__, __VA_ARGS__) diff --git a/src/log/log.c b/src/log/log.c index 7ae3b447..22f49b29 100644 --- a/src/log/log.c +++ b/src/log/log.c @@ -11,9 +11,10 @@ #include #include +#include #include -#include #include +#include /// The file to write logging information to static FILE *logfile = NULL; diff --git a/src/log/log.h b/src/log/log.h index 76d5d5b2..b53917db 100644 --- a/src/log/log.h +++ b/src/log/log.h @@ -10,15 +10,6 @@ */ #pragma once -#include - -extern void vlogger(enum log_level level, char *file, unsigned line, const char *fmt, ...); - -/** - * @brief Produce a log message - * @param level the logging level associated to the message - * @param ... the format string followed by its arguments if needed - */ -#define logger(level, ...) vlogger(level, __FILE__, __LINE__, __VA_ARGS__) +#include extern void log_init(FILE *file);