From bffe9ae3448c6a4a6a267d8b6822b33b8601db0f Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Fri, 22 Dec 2023 12:00:55 -0500 Subject: [PATCH] Statedump improvements - Introduce polling vs agent thread statedump callbacks, - Use uint64_t for tracer key. Signed-off-by: Mathieu Desnoyers --- include/side/trace.h | 20 ++-- src/list.h | 26 +++++ src/side.c | 237 ++++++++++++++++++++++++++++++++++------- src/tracer.c | 12 ++- tests/unit/statedump.c | 3 +- 5 files changed, 243 insertions(+), 55 deletions(-) diff --git a/include/side/trace.h b/include/side/trace.h index c12639a..53d5df8 100644 --- a/include/side/trace.h +++ b/include/side/trace.h @@ -107,18 +107,20 @@ typedef void (*side_tracer_callback_variadic_func)(const struct side_event_descr const struct side_arg_dynamic_struct *var_struct, void *priv); +int side_tracer_request_key(uint64_t *key); + int side_tracer_callback_register(struct side_event_description *desc, side_tracer_callback_func call, - void *priv, void *key); + void *priv, uint64_t key); int side_tracer_callback_variadic_register(struct side_event_description *desc, side_tracer_callback_variadic_func call_variadic, - void *priv, void *key); + void *priv, uint64_t key); int side_tracer_callback_unregister(struct side_event_description *desc, side_tracer_callback_func call, - void *priv, void *key); + void *priv, uint64_t key); int side_tracer_callback_variadic_unregister(struct side_event_description *desc, side_tracer_callback_variadic_func call_variadic, - void *priv, void *key); + void *priv, uint64_t key); enum side_tracer_notification { SIDE_TRACER_NOTIFICATION_INSERT_EVENTS, @@ -183,20 +185,16 @@ void side_statedump_request_notification_unregister( /* Returns true if the handle has pending statedump requests. */ bool side_statedump_poll_pending_requests(struct side_statedump_request_handle *handle); -void side_statedump_run_pending_requests(struct side_statedump_request_handle *handle); +int side_statedump_run_pending_requests(struct side_statedump_request_handle *handle); /* * Request a state dump for tracer callbacks identified with "key". - * Calls the completion callback when the statedump request is fulfilled. */ -void side_tracer_statedump_request(void *key, void (*completion)(void *priv), void *priv); +int side_tracer_statedump_request(uint64_t key); /* * Cancel a statedump request. - * Returns true if the request is cancelled before completion. - * The completion callback is not invoked when a statedump request is - * cancelled. */ -bool side_tracer_statedump_request_cancel(void *key); +int side_tracer_statedump_request_cancel(uint64_t key); /* * Explicit hooks to initialize/finalize the side instrumentation diff --git a/src/list.h b/src/list.h index fc97605..5c1c37a 100644 --- a/src/list.h +++ b/src/list.h @@ -8,6 +8,12 @@ #include "list_types.h" +static inline +void side_list_head_init(struct side_list_head *head) +{ + head->node.next = head->node.prev = &head->node; +} + static inline void side_list_insert_node_tail(struct side_list_head *head, struct side_list_node *node) { @@ -33,6 +39,26 @@ void side_list_remove_node(struct side_list_node *node) node->prev->next = node->next; } +static inline +bool side_list_empty(struct side_list_head *head) +{ + return &head->node == head->node.next; +} + +/* + * Splice "from" list at the beginning of "to" list. + */ +static inline +void side_list_splice(struct side_list_head *from, struct side_list_head *to) +{ + if (side_list_empty(from)) + return; + from->node.next->prev = &to->node; + from->node.prev->next = to->node.next; + to->node.next->prev = from->node.prev; + to->node.next = from->node.next; +} + #define side_list_for_each_entry(_entry, _head, _member) \ for ((_entry) = side_container_of((_head)->node.next, __typeof__(*(_entry)), _member); \ &(_entry)->_member != &(_head)->node; \ diff --git a/src/side.c b/src/side.c index ca48d2c..f7628b0 100644 --- a/src/side.c +++ b/src/side.c @@ -28,10 +28,14 @@ # define SIDE_EVENT_ENABLED_PRIVATE_MASK 0x00FFFFFFUL #endif +#define SIDE_KEY_RESERVED_RANGE_END 0x8 + +/* Key 0x0 is reserved to match all. */ +#define SIDE_KEY_MATCH_ALL 0x0 /* Key 0x1 is reserved for user event. */ -#define SIDE_USER_EVENT_KEY ((void *)0x1UL) +#define SIDE_KEY_USER_EVENT 0x1 /* Key 0x2 is reserved for ptrace. */ -#define SIDE_PTRACE_KEY ((void *)0x2UL) +#define SIDE_KEY_PTRACE 0x2 struct side_events_register_handle { struct side_list_node node; @@ -46,9 +50,17 @@ struct side_tracer_handle { void *priv; }; +struct side_statedump_notification { + struct side_list_node node; + uint64_t key; +}; + struct side_statedump_request_handle { - struct side_list_node node; /* RCU list. */ + struct side_list_node node; /* Statedump request RCU list node. */ + struct side_list_head notification_queue; /* Queue of struct side_statedump_notification */ void (*cb)(void); + char *name; + enum side_statedump_mode mode; }; struct side_callback { @@ -62,7 +74,7 @@ struct side_callback { void *priv); } u; void *priv; - void *key; + uint64_t key; }; static struct side_rcu_gp_state event_rcu_gp, statedump_rcu_gp; @@ -81,15 +93,24 @@ static bool finalized; */ static pthread_mutex_t side_event_lock = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP; static pthread_mutex_t side_statedump_lock = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP; +static pthread_mutex_t side_key_lock = PTHREAD_MUTEX_INITIALIZER; + +/* Dynamic tracer key allocation. */ +static uint64_t side_key_next = SIDE_KEY_RESERVED_RANGE_END; static DEFINE_SIDE_LIST_HEAD(side_events_list); static DEFINE_SIDE_LIST_HEAD(side_tracer_list); + +/* + * The statedump request list is a RCU list to allow the agent thread to + * iterate over this list with a RCU read-side lock. + */ static DEFINE_SIDE_LIST_HEAD(side_statedump_list); /* * Callback filter key for state dump. */ -static __thread void *filter_key; +static __thread uint64_t filter_key = SIDE_KEY_MATCH_ALL; /* * The empty callback has a NULL function callback pointer, which stops @@ -97,6 +118,11 @@ static __thread void *filter_key; */ const char side_empty_callback[sizeof(struct side_callback)]; +side_static_event(side_statedump_begin, "side", "statedump_begin", + SIDE_LOGLEVEL_INFO, side_field_list(side_field_string("name"))); +side_static_event(side_statedump_end, "side", "statedump_end", + SIDE_LOGLEVEL_INFO, side_field_list(side_field_string("name"))); + /* * side_ptrace_hook is a place holder for a debugger breakpoint. * var_struct is NULL if not variadic. @@ -112,7 +138,7 @@ void side_ptrace_hook(const struct side_event_state *event_state __attribute__(( } static -void _side_call(const struct side_event_state *event_state, const struct side_arg_vec *side_arg_vec, void *key) +void _side_call(const struct side_event_state *event_state, const struct side_arg_vec *side_arg_vec, uint64_t key) { struct side_rcu_read_state rcu_read_state; const struct side_event_state_0 *es0; @@ -130,17 +156,16 @@ void _side_call(const struct side_event_state *event_state, const struct side_ar enabled = __atomic_load_n(&es0->enabled, __ATOMIC_RELAXED); if (side_unlikely(enabled & SIDE_EVENT_ENABLED_SHARED_MASK)) { if ((enabled & SIDE_EVENT_ENABLED_SHARED_USER_EVENT_MASK) && - (!key || key == SIDE_USER_EVENT_KEY)) { + (key == SIDE_KEY_MATCH_ALL || key == SIDE_KEY_USER_EVENT)) { // TODO: call kernel write. } if ((enabled & SIDE_EVENT_ENABLED_SHARED_PTRACE_MASK) && - (!key || key == SIDE_PTRACE_KEY)) + (key == SIDE_KEY_MATCH_ALL || key == SIDE_KEY_PTRACE)) side_ptrace_hook(event_state, side_arg_vec, NULL); } side_rcu_read_begin(&event_rcu_gp, &rcu_read_state); for (side_cb = side_rcu_dereference(es0->callbacks); side_cb->u.call != NULL; side_cb++) { - /* A NULL key is always a match. */ - if (key && side_cb->key && side_cb->key != key) + if (key != SIDE_KEY_MATCH_ALL && side_cb->key != SIDE_KEY_MATCH_ALL && side_cb->key != key) continue; side_cb->u.call(es0->desc, side_arg_vec, side_cb->priv); } @@ -149,7 +174,7 @@ void _side_call(const struct side_event_state *event_state, const struct side_ar void side_call(const struct side_event_state *event_state, const struct side_arg_vec *side_arg_vec) { - _side_call(event_state, side_arg_vec, NULL); + _side_call(event_state, side_arg_vec, SIDE_KEY_MATCH_ALL); } void side_statedump_call(const struct side_event_state *event_state, const struct side_arg_vec *side_arg_vec) @@ -161,7 +186,7 @@ static void _side_call_variadic(const struct side_event_state *event_state, const struct side_arg_vec *side_arg_vec, const struct side_arg_dynamic_struct *var_struct, - void *key) + uint64_t key) { struct side_rcu_read_state rcu_read_state; const struct side_event_state_0 *es0; @@ -179,17 +204,16 @@ void _side_call_variadic(const struct side_event_state *event_state, enabled = __atomic_load_n(&es0->enabled, __ATOMIC_RELAXED); if (side_unlikely(enabled & SIDE_EVENT_ENABLED_SHARED_MASK)) { if ((enabled & SIDE_EVENT_ENABLED_SHARED_USER_EVENT_MASK) && - (!key || key == SIDE_USER_EVENT_KEY)) { + (key == SIDE_KEY_MATCH_ALL || key == SIDE_KEY_USER_EVENT)) { // TODO: call kernel write. } if ((enabled & SIDE_EVENT_ENABLED_SHARED_PTRACE_MASK) && - (!key || key == SIDE_PTRACE_KEY)) + (key == SIDE_KEY_MATCH_ALL || key == SIDE_KEY_PTRACE)) side_ptrace_hook(event_state, side_arg_vec, var_struct); } side_rcu_read_begin(&event_rcu_gp, &rcu_read_state); for (side_cb = side_rcu_dereference(es0->callbacks); side_cb->u.call_variadic != NULL; side_cb++) { - /* A NULL key is always a match. */ - if (key && side_cb->key && side_cb->key != key) + if (key != SIDE_KEY_MATCH_ALL && side_cb->key != SIDE_KEY_MATCH_ALL && side_cb->key != key) continue; side_cb->u.call_variadic(es0->desc, side_arg_vec, var_struct, side_cb->priv); } @@ -200,7 +224,7 @@ void side_call_variadic(const struct side_event_state *event_state, const struct side_arg_vec *side_arg_vec, const struct side_arg_dynamic_struct *var_struct) { - _side_call_variadic(event_state, side_arg_vec, var_struct, NULL); + _side_call_variadic(event_state, side_arg_vec, var_struct, SIDE_KEY_MATCH_ALL); } void side_statedump_call_variadic(const struct side_event_state *event_state, @@ -213,7 +237,7 @@ void side_statedump_call_variadic(const struct side_event_state *event_state, static const struct side_callback *side_tracer_callback_lookup( const struct side_event_description *desc, - void *call, void *priv, void *key) + void *call, void *priv, uint64_t key) { struct side_event_state *event_state = side_ptr_get(desc->state); const struct side_event_state_0 *es0; @@ -231,7 +255,7 @@ const struct side_callback *side_tracer_callback_lookup( static int _side_tracer_callback_register(struct side_event_description *desc, - void *call, void *priv, void *key) + void *call, void *priv, uint64_t key) { struct side_event_state *event_state; struct side_callback *old_cb, *new_cb; @@ -292,7 +316,7 @@ unlock: int side_tracer_callback_register(struct side_event_description *desc, side_tracer_callback_func call, - void *priv, void *key) + void *priv, uint64_t key) { if (desc->flags & SIDE_EVENT_FLAG_VARIADIC) return SIDE_ERROR_INVAL; @@ -301,7 +325,7 @@ int side_tracer_callback_register(struct side_event_description *desc, int side_tracer_callback_variadic_register(struct side_event_description *desc, side_tracer_callback_variadic_func call_variadic, - void *priv, void *key) + void *priv, uint64_t key) { if (!(desc->flags & SIDE_EVENT_FLAG_VARIADIC)) return SIDE_ERROR_INVAL; @@ -309,7 +333,7 @@ int side_tracer_callback_variadic_register(struct side_event_description *desc, } static int _side_tracer_callback_unregister(struct side_event_description *desc, - void *call, void *priv, void *key) + void *call, void *priv, uint64_t key) { struct side_event_state *event_state; struct side_callback *old_cb, *new_cb; @@ -366,7 +390,7 @@ unlock: int side_tracer_callback_unregister(struct side_event_description *desc, side_tracer_callback_func call, - void *priv, void *key) + void *priv, uint64_t key) { if (desc->flags & SIDE_EVENT_FLAG_VARIADIC) return SIDE_ERROR_INVAL; @@ -375,7 +399,7 @@ int side_tracer_callback_unregister(struct side_event_description *desc, int side_tracer_callback_variadic_unregister(struct side_event_description *desc, side_tracer_callback_variadic_func call_variadic, - void *priv, void *key) + void *priv, uint64_t key) { if (!(desc->flags & SIDE_EVENT_FLAG_VARIADIC)) return SIDE_ERROR_INVAL; @@ -521,9 +545,40 @@ void side_tracer_event_notification_unregister(struct side_tracer_handle *tracer free(tracer_handle); } -struct side_statedump_request_handle *side_statedump_request_notification_register(void (*statedump_cb)(void)) +/* Called with side_statedump_lock held. */ +static +void queue_statedump_pending(struct side_statedump_request_handle *handle, uint64_t key) +{ + struct side_statedump_notification *notif; + + notif = (struct side_statedump_notification *) calloc(1, sizeof(struct side_statedump_notification)); + if (!notif) + abort(); + notif->key = key; + side_list_insert_node_tail(&handle->notification_queue, ¬if->node); +} + +/* Called with side_statedump_lock held. */ +static +void unqueue_statedump_pending(struct side_statedump_request_handle *handle, uint64_t key) +{ + struct side_statedump_notification *notif, *tmp; + + side_list_for_each_entry_safe(notif, tmp, &handle->notification_queue, node) { + if (key == SIDE_KEY_MATCH_ALL || key == notif->key) { + side_list_remove_node(¬if->node); + free(notif); + } + } +} + +struct side_statedump_request_handle * + side_statedump_request_notification_register(const char *state_name, + void (*statedump_cb)(void), + enum side_statedump_mode mode) { struct side_statedump_request_handle *handle; + char *name; if (finalized) return NULL; @@ -533,21 +588,30 @@ struct side_statedump_request_handle *side_statedump_request_notification_regist * The statedump request notification should not be registered * from a notification callback. */ - assert(filter_key == NULL); + assert(!filter_key); handle = (struct side_statedump_request_handle *) calloc(1, sizeof(struct side_statedump_request_handle)); if (!handle) return NULL; + name = strdup(state_name); + if (!name) + goto name_nomem; handle->cb = statedump_cb; + handle->name = name; + handle->mode = mode; + side_list_head_init(&handle->notification_queue); pthread_mutex_lock(&side_statedump_lock); side_list_insert_node_tail_rcu(&side_statedump_list, &handle->node); + /* Queue statedump pending for all tracers. */ + queue_statedump_pending(handle, SIDE_KEY_MATCH_ALL); pthread_mutex_unlock(&side_statedump_lock); - /* Invoke callback for all tracers. */ - statedump_cb(); - return handle; + +name_nomem: + free(handle); + return NULL; } void side_statedump_request_notification_unregister(struct side_statedump_request_handle *handle) @@ -556,28 +620,123 @@ void side_statedump_request_notification_unregister(struct side_statedump_reques return; if (!initialized) side_init(); - assert(filter_key == NULL); + assert(!filter_key); pthread_mutex_lock(&side_statedump_lock); + unqueue_statedump_pending(handle, SIDE_KEY_MATCH_ALL); side_list_remove_node_rcu(&handle->node); pthread_mutex_unlock(&side_statedump_lock); side_rcu_wait_grace_period(&statedump_rcu_gp); + free(handle->name); free(handle); } -void side_tracer_statedump_request(void *key) +/* Returns true if the handle has pending statedump requests. */ +bool side_statedump_poll_pending_requests(struct side_statedump_request_handle *handle) { - struct side_statedump_request_handle *handle; - struct side_rcu_read_state rcu_read_state; + bool ret; + + if (handle->mode != SIDE_STATEDUMP_MODE_POLLING) + return false; + pthread_mutex_lock(&side_statedump_lock); + ret = !side_list_empty(&handle->notification_queue); + pthread_mutex_unlock(&side_statedump_lock); + return ret; +} +static +void side_statedump_run(struct side_statedump_request_handle *handle, + struct side_statedump_notification *notif) +{ /* Invoke the state dump callback specifically for the tracer key. */ - filter_key = key; - side_rcu_read_begin(&statedump_rcu_gp, &rcu_read_state); - side_list_for_each_entry_rcu(handle, &side_statedump_list, node) - handle->cb(); - side_rcu_read_end(&statedump_rcu_gp, &rcu_read_state); - filter_key = NULL; + filter_key = notif->key; + side_statedump_event_call(side_statedump_begin, + side_arg_list(side_arg_string(handle->name))); + handle->cb(); + side_statedump_event_call(side_statedump_end, + side_arg_list(side_arg_string(handle->name))); + filter_key = SIDE_KEY_MATCH_ALL; +} + +static +void _side_statedump_run_pending_requests(struct side_statedump_request_handle *handle) +{ + struct side_statedump_notification *notif, *tmp; + DEFINE_SIDE_LIST_HEAD(tmp_head); + + pthread_mutex_lock(&side_statedump_lock); + side_list_splice(&handle->notification_queue, &tmp_head); + pthread_mutex_lock(&side_statedump_lock); + + /* We are now sole owner of the tmp_head list. */ + side_list_for_each_entry(notif, &tmp_head, node) + side_statedump_run(handle, notif); + side_list_for_each_entry_safe(notif, tmp, &tmp_head, node) + free(notif); +} + +/* + * Only polling mode state dump handles allow application to explicitly handle the + * pending requests. + */ +int side_statedump_run_pending_requests(struct side_statedump_request_handle *handle) +{ + if (handle->mode != SIDE_STATEDUMP_MODE_POLLING) + return SIDE_ERROR_INVAL; + _side_statedump_run_pending_requests(handle); + return SIDE_ERROR_OK; +} + +/* + * Request a state dump for tracer callbacks identified with "key". + */ +int side_tracer_statedump_request(uint64_t key) +{ + struct side_statedump_request_handle *handle; + + if (key == SIDE_KEY_MATCH_ALL) + return SIDE_ERROR_INVAL; + pthread_mutex_lock(&side_statedump_lock); + side_list_for_each_entry(handle, &side_statedump_list, node) + queue_statedump_pending(handle, key); + pthread_mutex_lock(&side_statedump_lock); + return SIDE_ERROR_OK; +} + +/* + * Cancel a statedump request. + */ +int side_tracer_statedump_request_cancel(uint64_t key) +{ + struct side_statedump_request_handle *handle; + + if (key == SIDE_KEY_MATCH_ALL) + return SIDE_ERROR_INVAL; + pthread_mutex_lock(&side_statedump_lock); + side_list_for_each_entry(handle, &side_statedump_list, node) + unqueue_statedump_pending(handle, key); + pthread_mutex_lock(&side_statedump_lock); + return SIDE_ERROR_OK; +} + +/* + * Tracer keys are represented on 64-bit. Return SIDE_ERROR_NOMEM on + * overflow (which should never happen in practice). + */ +int side_tracer_request_key(uint64_t *key) +{ + int ret = SIDE_ERROR_OK; + + pthread_mutex_lock(&side_key_lock); + if (side_key_next == 0) { + ret = SIDE_ERROR_NOMEM; + goto end; + } + *key = side_key_next++; +end: + pthread_mutex_unlock(&side_key_lock); + return ret; } void side_init(void) diff --git a/src/tracer.c b/src/tracer.c index 1b95afc..1f27032 100644 --- a/src/tracer.c +++ b/src/tracer.c @@ -27,6 +27,8 @@ union int_value { static struct side_tracer_handle *tracer_handle; +static uint64_t tracer_key; + static void tracer_print_struct(const struct side_type *type_desc, const struct side_arg_vec *side_arg_vec); static @@ -2120,21 +2122,21 @@ void tracer_event_notification(enum side_tracer_notification notif, event->nr_side_attr_type - _NR_SIDE_ATTR_TYPE); } if (event->flags & SIDE_EVENT_FLAG_VARIADIC) { - ret = side_tracer_callback_variadic_register(event, tracer_call_variadic, NULL, tracer_handle); + ret = side_tracer_callback_variadic_register(event, tracer_call_variadic, NULL, tracer_key); if (ret) abort(); } else { - ret = side_tracer_callback_register(event, tracer_call, NULL, tracer_handle); + ret = side_tracer_callback_register(event, tracer_call, NULL, tracer_key); if (ret) abort(); } } else { if (event->flags & SIDE_EVENT_FLAG_VARIADIC) { - ret = side_tracer_callback_variadic_unregister(event, tracer_call_variadic, NULL, tracer_handle); + ret = side_tracer_callback_variadic_unregister(event, tracer_call_variadic, NULL, tracer_key); if (ret) abort(); } else { - ret = side_tracer_callback_unregister(event, tracer_call, NULL, tracer_handle); + ret = side_tracer_callback_unregister(event, tracer_call, NULL, tracer_key); if (ret) abort(); } @@ -2148,6 +2150,8 @@ void tracer_init(void); static void tracer_init(void) { + if (side_tracer_request_key(&tracer_key)) + abort(); tracer_handle = side_tracer_event_notification_register(tracer_event_notification, NULL); if (!tracer_handle) abort(); diff --git a/tests/unit/statedump.c b/tests/unit/statedump.c index c2541f0..c33e8a6 100644 --- a/tests/unit/statedump.c +++ b/tests/unit/statedump.c @@ -51,7 +51,8 @@ static void my_constructor(void) static void my_constructor(void) { side_event_description_ptr_init(); - statedump_request_handle = side_statedump_request_notification_register(statedump_cb); + statedump_request_handle = side_statedump_request_notification_register("mystatedump", + statedump_cb, SIDE_STATEDUMP_MODE_AGENT_THREAD); if (!statedump_request_handle) abort(); } -- 2.34.1