From: Mathieu Desnoyers Date: Wed, 12 Nov 2014 23:18:32 +0000 (-0500) Subject: Fix: filter attach vs event enable race X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=401137874924a040b3266954aa2067c8d6aedf26 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. This patch should be backported to stable-2.4, stable-2.5, stable-2.6 branches. Signed-off-by: Mathieu Desnoyers Signed-off-by: Jérémie Galarneau --- diff --git a/src/bin/lttng-sessiond/trace-ust.c b/src/bin/lttng-sessiond/trace-ust.c index 1f6fd5273..ac980fd45 100644 --- a/src/bin/lttng-sessiond/trace-ust.c +++ b/src/bin/lttng-sessiond/trace-ust.c @@ -419,6 +419,11 @@ struct ltt_ust_event *trace_ust_create_event(struct lttng_event *ev, ERR("Unknown ust loglevel type (%d)", ev->loglevel_type); goto error_free_event; } + /* + * Fix for enabler race. Enable is now done explicitly by + * sessiond after setting filter. + */ + lue->attr.disabled = 1; /* Same layout. */ lue->filter_expression = filter_expression; diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c index 5ebe9917a..7e4bf9405 100644 --- a/src/bin/lttng-sessiond/ust-app.c +++ b/src/bin/lttng-sessiond/ust-app.c @@ -1446,7 +1446,32 @@ int create_ust_event(struct ust_app *app, struct ust_app_session *ua_sess, } /* If event not enabled, disable it on the tracer */ - if (ua_event->enabled == 0) { + if (ua_event->enabled) { + /* + * We now need to explicitly enable the event, since it + * is now disabled at creation. + */ + ret = enable_ust_event(app, ua_sess, ua_event); + if (ret < 0) { + /* + * If we hit an EPERM, something is wrong with our enable call. If + * we get an EEXIST, there is a problem on the tracer side since we + * just created it. + */ + switch (ret) { + case -LTTNG_UST_ERR_PERM: + /* Code flow problem */ + assert(0); + case -LTTNG_UST_ERR_EXIST: + /* It's OK for our use case. */ + ret = 0; + break; + default: + break; + } + goto error; + } + } else { ret = disable_ust_event(app, ua_sess, ua_event); if (ret < 0) { /*