X-Git-Url: https://git.liburcu.org/?a=blobdiff_plain;f=src%2Fbin%2Flttng-sessiond%2Faction-executor.c;h=1908ce58d547a43ce3aadb2527ec66a5f0072114;hb=cfb1642dfc940ec63ec86e0721f0f5cdf5a79fe7;hp=cbcd08b5f84c0047cf14ae6b601a8b158a574c25;hpb=702f26c8641ee4554c629d13b9367b1e93f96e31;p=lttng-tools.git diff --git a/src/bin/lttng-sessiond/action-executor.c b/src/bin/lttng-sessiond/action-executor.c index cbcd08b5f..1908ce58d 100644 --- a/src/bin/lttng-sessiond/action-executor.c +++ b/src/bin/lttng-sessiond/action-executor.c @@ -16,8 +16,8 @@ #include #include #include -#include -#include +#include +#include #include #include #include @@ -77,7 +77,7 @@ struct action_work_item { * The actions to be executed with their respective execution context. * See struct `action_work_subitem`. */ - struct lttng_dynamic_array *subitems; + struct lttng_dynamic_array subitems; /* Execution context data */ struct lttng_trigger *trigger; @@ -135,7 +135,7 @@ static int action_executor_snapshot_session_handler( struct action_executor *executor, const struct action_work_item *, struct action_work_subitem *); -static int action_executor_group_handler(struct action_executor *executor, +static int action_executor_list_handler(struct action_executor *executor, const struct action_work_item *, struct action_work_subitem *); static int action_executor_generic_handler(struct action_executor *executor, @@ -148,7 +148,7 @@ static const action_executor_handler action_executors[] = { [LTTNG_ACTION_TYPE_STOP_SESSION] = action_executor_stop_session_handler, [LTTNG_ACTION_TYPE_ROTATE_SESSION] = action_executor_rotate_session_handler, [LTTNG_ACTION_TYPE_SNAPSHOT_SESSION] = action_executor_snapshot_session_handler, - [LTTNG_ACTION_TYPE_GROUP] = action_executor_group_handler, + [LTTNG_ACTION_TYPE_LIST] = action_executor_list_handler, }; /* Forward declaration */ @@ -206,7 +206,16 @@ static const char *get_trigger_name(const struct lttng_trigger *trigger) enum lttng_trigger_status trigger_status; trigger_status = lttng_trigger_get_name(trigger, &trigger_name); - assert(trigger_status == LTTNG_TRIGGER_STATUS_OK); + switch (trigger_status) { + case LTTNG_TRIGGER_STATUS_OK: + break; + case LTTNG_TRIGGER_STATUS_UNSET: + trigger_name = "(anonymous)"; + break; + default: + trigger_name = "(failed to get name)"; + break; + } return trigger_name; } @@ -292,7 +301,7 @@ static int action_executor_start_session_handler( * existed. If not skip the action altogether. */ if (!item->context.session_id.is_set) { - DBG("Session `%s` was not present at the moment the work item was enqueued for %s` action of trigger `%s`", + DBG("Session `%s` was not present at the moment the work item was enqueued for `%s` action of trigger `%s`", session_name, get_action_name(action), get_trigger_name(work_item->trigger)); lttng_action_increase_execution_failure_count(action); @@ -316,18 +325,18 @@ static int action_executor_start_session_handler( if (session->id != LTTNG_OPTIONAL_GET(item->context.session_id)) { DBG("Session id for session `%s` (id: %" PRIu64 " is not the same that was sampled (id: %" PRIu64 - " at the moment the work item was enqueued for %s` action of trigger `%s`", + " at the moment the work item was enqueued for `%s` action of trigger `%s`", session_name, session->id, LTTNG_OPTIONAL_GET(item->context.session_id), get_action_name(action), get_trigger_name(work_item->trigger)); ret = 0; - goto error_unlock_list; + goto error_put_session; } session_lock(session); if (!is_trigger_allowed_for_session(work_item->trigger, session)) { - goto error_dispose_session; + goto error_unlock_session; } cmd_ret = cmd_start_trace(session); @@ -348,8 +357,9 @@ static int action_executor_start_session_handler( break; } -error_dispose_session: +error_unlock_session: session_unlock(session); +error_put_session: session_put(session); error_unlock_list: session_unlock_list(); @@ -383,7 +393,7 @@ static int action_executor_stop_session_handler( * existed. If not, skip the action altogether. */ if (!item->context.session_id.is_set) { - DBG("Session `%s` was not present at the moment the work item was enqueued for %s` action of trigger `%s`", + DBG("Session `%s` was not present at the moment the work item was enqueued for `%s` action of trigger `%s`", session_name, get_action_name(action), get_trigger_name(work_item->trigger)); lttng_action_increase_execution_failure_count(action); @@ -408,18 +418,18 @@ static int action_executor_stop_session_handler( if (session->id != LTTNG_OPTIONAL_GET(item->context.session_id)) { DBG("Session id for session `%s` (id: %" PRIu64 " is not the same that was sampled (id: %" PRIu64 - " at the moment the work item was enqueued for %s` action of trigger `%s`", + " at the moment the work item was enqueued for `%s` action of trigger `%s`", session_name, session->id, LTTNG_OPTIONAL_GET(item->context.session_id), get_action_name(action), get_trigger_name(work_item->trigger)); ret = 0; - goto error_unlock_list; + goto error_put_session; } session_lock(session); if (!is_trigger_allowed_for_session(work_item->trigger, session)) { - goto error_dispose_session; + goto error_unlock_session; } cmd_ret = cmd_stop_trace(session); @@ -440,8 +450,9 @@ static int action_executor_stop_session_handler( break; } -error_dispose_session: +error_unlock_session: session_unlock(session); +error_put_session: session_put(session); error_unlock_list: session_unlock_list(); @@ -475,7 +486,7 @@ static int action_executor_rotate_session_handler( * existed. If not, skip the action altogether. */ if (!item->context.session_id.is_set) { - DBG("Session `%s` was not present at the moment the work item was enqueued for %s` action of trigger `%s`", + DBG("Session `%s` was not present at the moment the work item was enqueued for `%s` action of trigger `%s`", session_name, get_action_name(action), get_trigger_name(work_item->trigger)); lttng_action_increase_execution_failure_count(action); @@ -500,18 +511,18 @@ static int action_executor_rotate_session_handler( if (session->id != LTTNG_OPTIONAL_GET(item->context.session_id)) { DBG("Session id for session `%s` (id: %" PRIu64 " is not the same that was sampled (id: %" PRIu64 - " at the moment the work item was enqueued for %s` action of trigger `%s`", + " at the moment the work item was enqueued for `%s` action of trigger `%s`", session_name, session->id, LTTNG_OPTIONAL_GET(item->context.session_id), get_action_name(action), get_trigger_name(work_item->trigger)); ret = 0; - goto error_unlock_list; + goto error_put_session; } session_lock(session); if (!is_trigger_allowed_for_session(work_item->trigger, session)) { - goto error_dispose_session; + goto error_unlock_session; } cmd_ret = cmd_rotate_session(session, NULL, false, @@ -539,8 +550,9 @@ static int action_executor_rotate_session_handler( break; } -error_dispose_session: +error_unlock_session: session_unlock(session); +error_put_session: session_put(session); error_unlock_list: session_unlock_list(); @@ -570,7 +582,7 @@ static int action_executor_snapshot_session_handler( * existed. If not, skip the action altogether. */ if (!item->context.session_id.is_set) { - DBG("Session was not present at the moment the work item was enqueued for %s` action of trigger `%s`", + DBG("Session was not present at the moment the work item was enqueued for `%s` action of trigger `%s`", get_action_name(action), get_trigger_name(work_item->trigger)); lttng_action_increase_execution_failure_count(action); @@ -614,18 +626,18 @@ static int action_executor_snapshot_session_handler( if (session->id != LTTNG_OPTIONAL_GET(item->context.session_id)) { DBG("Session id for session `%s` (id: %" PRIu64 " is not the same that was sampled (id: %" PRIu64 - " at the moment the work item was enqueued for %s` action of trigger `%s`", + " at the moment the work item was enqueued for `%s` action of trigger `%s`", session_name, session->id, LTTNG_OPTIONAL_GET(item->context.session_id), get_action_name(action), get_trigger_name(work_item->trigger)); ret = 0; - goto error_unlock_list; + goto error_put_session; } session_lock(session); if (!is_trigger_allowed_for_session(work_item->trigger, session)) { - goto error_dispose_session; + goto error_unlock_session; } cmd_ret = cmd_snapshot_record(session, snapshot_output, 0); @@ -642,8 +654,9 @@ static int action_executor_snapshot_session_handler( break; } -error_dispose_session: +error_unlock_session: session_unlock(session); +error_put_session: session_put(session); error_unlock_list: session_unlock_list(); @@ -651,11 +664,11 @@ end: return ret; } -static int action_executor_group_handler(struct action_executor *executor, +static int action_executor_list_handler(struct action_executor *executor, const struct action_work_item *work_item, struct action_work_subitem *item) { - ERR("Execution of a group action by the action executor should never occur"); + ERR("Execution of a list action by the action executor should never occur"); abort(); } @@ -698,11 +711,11 @@ static int action_work_item_execute(struct action_executor *executor, DBG("Starting execution of action work item %" PRIu64 " of trigger `%s`", work_item->id, get_trigger_name(work_item->trigger)); - count = lttng_dynamic_array_get_count(work_item->subitems); + count = lttng_dynamic_array_get_count(&work_item->subitems); for (i = 0; i < count; i++) { struct action_work_subitem *item; - item = lttng_dynamic_array_get_element(work_item->subitems, i); + item = lttng_dynamic_array_get_element(&work_item->subitems, i); ret = action_executor_generic_handler( executor, work_item, item); if (ret) { @@ -720,7 +733,7 @@ static void action_work_item_destroy(struct action_work_item *work_item) lttng_trigger_put(work_item->trigger); lttng_evaluation_destroy(work_item->evaluation); notification_client_list_put(work_item->client_list); - lttng_dynamic_array_reset(work_item->subitems); + lttng_dynamic_array_reset(&work_item->subitems); free(work_item); } @@ -739,7 +752,7 @@ static void *action_executor_thread(void *_data) DBG("Entering work execution loop"); pthread_mutex_lock(&executor->work.lock); while (!executor->should_quit) { - int ret; + int ret = 0; struct action_work_item *work_item; health_code_update(); @@ -772,23 +785,13 @@ static void *action_executor_thread(void *_data) uid_t trigger_owner_uid; enum lttng_trigger_status trigger_status; - trigger_status = lttng_trigger_get_name( - work_item->trigger, &trigger_name); - switch (trigger_status) { - case LTTNG_TRIGGER_STATUS_OK: - break; - case LTTNG_TRIGGER_STATUS_UNSET: - trigger_name = "(unset)"; - break; - default: - abort(); - } + trigger_name = get_trigger_name(work_item->trigger); trigger_status = lttng_trigger_get_owner_uid( work_item->trigger, &trigger_owner_uid); assert(trigger_status == LTTNG_TRIGGER_STATUS_OK); - DBG("Work item skipped since the associated trigger is no longer registered: work item id = %" PRIu64 ", trigger name = '%s', trigger owner uid = %d", + DBG("Work item skipped since the associated trigger is no longer registered: work item id = %" PRIu64 ", trigger name = `%s`, trigger owner uid = %d", work_item->id, trigger_name, (int) trigger_owner_uid); ret = 0; @@ -906,30 +909,9 @@ enum action_executor_status action_executor_enqueue_trigger( const uint64_t work_item_id = executor->next_work_item_id++; struct action_work_item *work_item; bool signal = false; - struct lttng_dynamic_array *subitems = NULL; assert(trigger); - /* Build the array of action work subitems for the passed trigger. */ - subitems = zmalloc(sizeof(*subitems)); - if (!subitems) { - PERROR("Failed to allocate action executor subitems array: trigger name = `%s`", - get_trigger_name(trigger)); - executor_status = ACTION_EXECUTOR_STATUS_ERROR; - goto error_unlock; - } - - lttng_dynamic_array_init(subitems, sizeof(struct action_work_subitem), - action_work_subitem_destructor); - - ret = populate_subitem_array_from_trigger(trigger, subitems); - if (ret) { - ERR("Failed to populate work item sub items on behalf of trigger: trigger name = `%s`", - get_trigger_name(trigger)); - executor_status = ACTION_EXECUTOR_STATUS_ERROR; - goto error_unlock; - } - pthread_mutex_lock(&executor->work.lock); /* Check for queue overflow. */ if (executor->work.pending_count >= MAX_QUEUED_WORK_COUNT) { @@ -942,7 +924,7 @@ enum action_executor_status action_executor_enqueue_trigger( work_item = zmalloc(sizeof(*work_item)); if (!work_item) { - PERROR("Failed to allocate action executor work item: trigger name = '%s'", + PERROR("Failed to allocate action executor work item: trigger name = `%s`", get_trigger_name(trigger)); executor_status = ACTION_EXECUTOR_STATUS_ERROR; goto error_unlock; @@ -958,8 +940,6 @@ enum action_executor_status action_executor_enqueue_trigger( *work_item = (typeof(*work_item)){ .id = work_item_id, - /* Ownership transferred to the work item. */ - .subitems = subitems, .trigger = trigger, /* Ownership transferred to the work item. */ .evaluation = evaluation, @@ -973,7 +953,21 @@ enum action_executor_status action_executor_enqueue_trigger( }; evaluation = NULL; - subitems = NULL; + + /* Build the array of action work subitems for the passed trigger. */ + lttng_dynamic_array_init(&work_item->subitems, + sizeof(struct action_work_subitem), + action_work_subitem_destructor); + + ret = populate_subitem_array_from_trigger( + trigger, &work_item->subitems); + if (ret) { + ERR("Failed to populate work item sub items on behalf of trigger: trigger name = `%s`", + get_trigger_name(trigger)); + executor_status = ACTION_EXECUTOR_STATUS_ERROR; + goto error_unlock; + } + cds_list_add_tail(&work_item->list_node, &executor->work.list); executor->work.pending_count++; DBG("Enqueued action for trigger: trigger name = `%s`, work item id = %" PRIu64, @@ -984,20 +978,16 @@ error_unlock: if (signal) { pthread_cond_signal(&executor->work.cond); } - pthread_mutex_unlock(&executor->work.lock); + pthread_mutex_unlock(&executor->work.lock); lttng_evaluation_destroy(evaluation); - if (subitems) { - lttng_dynamic_array_reset(subitems); - free(subitems); - } return executor_status; } static int add_action_to_subitem_array(struct lttng_action *action, struct lttng_dynamic_array *subitems) { - int ret; + int ret = 0; enum lttng_action_type type = lttng_action_get_type(action); const char *session_name = NULL; enum lttng_action_status status; @@ -1011,7 +1001,7 @@ static int add_action_to_subitem_array(struct lttng_action *action, assert(action); assert(subitems); - if (type == LTTNG_ACTION_TYPE_GROUP) { + if (type == LTTNG_ACTION_TYPE_LIST) { unsigned int count, i; status = lttng_action_list_get_count(action, &count); @@ -1032,7 +1022,7 @@ static int add_action_to_subitem_array(struct lttng_action *action, /* * Go directly to the end since there is no need to add the - * group action by itself to the subitems array. + * list action by itself to the subitems array. */ goto end; } @@ -1061,7 +1051,7 @@ static int add_action_to_subitem_array(struct lttng_action *action, action, &session_name); assert(status == LTTNG_ACTION_STATUS_OK); break; - case LTTNG_ACTION_TYPE_GROUP: + case LTTNG_ACTION_TYPE_LIST: case LTTNG_ACTION_TYPE_UNKNOWN: /* Fallthrough */ default: @@ -1077,17 +1067,26 @@ static int add_action_to_subitem_array(struct lttng_action *action, * simplicity and consistency. */ if (session_name != NULL) { - struct ltt_session *session = NULL; + uint64_t session_id; - session_lock_list(); - session = session_find_by_name(session_name); - if (session) { + /* + * Instantaneous sampling of the session id if present. + * + * This method is preferred over `sessiond_find_by_name` then + * fetching the session'd id since `sessiond_find_by_name` + * requires the session list lock to be taken. + * + * Taking the session list lock can lead to a deadlock + * between the action executor and the notification thread + * (caller of add_action_to_subitem_array). It is okay if the + * session state changes between the enqueuing time and the + * execution time. The execution context is validated at + * execution time. + */ + if (sample_session_id_by_name(session_name, &session_id)) { LTTNG_OPTIONAL_SET(&subitem.context.session_id, - session->id); - session_put(session); + session_id); } - - session_unlock_list(); } /* Get a reference to the action. */