From 91194d7fa0e565cfb0f8dd4a122f1ee54565ba97 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 12 Nov 2014 18:08:04 -0500 Subject: [PATCH] Fix: filter attach vs event enable race In order to correctly handle the use-case where events are enabled _after_ trace is started, and _after_ applications are already being traced, the event should be created in a "disabled" state, so that it does not trace events until its filter is attached. This fix needs to be done both in lttng-tools and lttng-ust. In order to keep ABI compatibility between tools and ust within a stable release cycle, we introduce a new "disabled" within struct lttng_ust_event padding (previously zeroed). Newer LTTng-UST checks this flag, and fallback on the old racy behavior (enabling the event on creation) if it is unset. Therefore, old session daemon works with newer lttng-ust of the same stable release, and vice-versa. However, building lttng-tools requires an upgraded lttng-ust, which contains the communication protocol with the new "disabled" field. Signed-off-by: Mathieu Desnoyers --- include/lttng/ust-abi.h | 3 ++- liblttng-ust-ctl/ustctl.c | 1 + liblttng-ust/lttng-events.c | 11 ++++++++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/include/lttng/ust-abi.h b/include/lttng/ust-abi.h index 8c2469e4..797a06b6 100644 --- a/include/lttng/ust-abi.h +++ b/include/lttng/ust-abi.h @@ -98,7 +98,7 @@ struct lttng_ust_stream { */ } LTTNG_PACKED; -#define LTTNG_UST_EVENT_PADDING1 16 +#define LTTNG_UST_EVENT_PADDING1 15 #define LTTNG_UST_EVENT_PADDING2 (LTTNG_UST_SYM_NAME_LEN + 32) struct lttng_ust_event { enum lttng_ust_instrumentation instrumentation; @@ -106,6 +106,7 @@ struct lttng_ust_event { enum lttng_ust_loglevel_type loglevel_type; int loglevel; /* value, -1: all */ + char disabled; char padding[LTTNG_UST_EVENT_PADDING1]; /* Per instrumentation type configuration */ diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c index 49ae3e6c..195f29c0 100644 --- a/liblttng-ust-ctl/ustctl.c +++ b/liblttng-ust-ctl/ustctl.c @@ -200,6 +200,7 @@ int ustctl_create_event(int sock, struct lttng_ust_event *ev, lum.u.event.instrumentation = ev->instrumentation; lum.u.event.loglevel_type = ev->loglevel_type; lum.u.event.loglevel = ev->loglevel; + lum.u.event.disabled = ev->disabled; ret = ustcomm_send_app_cmd(sock, &lum, &lur); if (ret) { free(event_data); diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c index 285b98e8..8bdb743d 100644 --- a/liblttng-ust/lttng-events.c +++ b/liblttng-ust/lttng-events.c @@ -746,7 +746,16 @@ struct lttng_enabler *lttng_enabler_create(enum lttng_enabler_type type, sizeof(enabler->event_param)); enabler->chan = chan; /* ctx left NULL */ - enabler->enabled = 1; + /* + * The "disable" event create comm field has been added to fix a + * race between event creation (of a started trace) and enabling + * filtering. New session daemon always set the "disable" field + * to 1, and are aware that they need to explicitly enable the + * event. Older session daemon (within same ABI) leave it at 0, + * and therefore we need to enable it here, keeping the original + * racy behavior. + */ + enabler->enabled = !event_param->disabled; cds_list_add(&enabler->node, &enabler->chan->session->enablers_head); lttng_session_lazy_sync_enablers(enabler->chan->session); return enabler; -- 2.34.1