All of lore.kernel.org
 help / color / mirror / Atom feed
* [[RFC PATCH lttng-tools]] Fix: Disable all UST events matching the given name
       [not found] <1441832900-7573-1-git-send-email-jonathan.rajotte-julien@efficios.com>
@ 2015-09-15 20:59 ` Jérémie Galarneau
       [not found] ` <1442350762-10938-1-git-send-email-jeremie.galarneau@efficios.com>
  2015-09-16 18:51 ` [PATCH lttng-tools] Fix: sessiond: disable: match app event by name Jérémie Galarneau
  2 siblings, 0 replies; 3+ messages in thread
From: Jérémie Galarneau @ 2015-09-15 20:59 UTC (permalink / raw)
  To: lttng-dev; +Cc: joraj

The session daemon will only disable the first event
matching the name provided to the disable-event command.
This fix iterates on all events matching the name, ignoring
their filter, loglevel and exclusion list.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
---
 src/bin/lttng-sessiond/ust-app.c | 78 +++++++++++++++++++++++++++++-----------
 1 file changed, 57 insertions(+), 21 deletions(-)

diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
index 4066b06..02f6153 100644
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -170,6 +170,34 @@ no_match:
 }
 
 /*
+ * Match function for the hash table lookup.
+ *
+ * Matches an ust app event based on its name only.
+ */
+static int ht_match_ust_app_event_by_name(struct cds_lfht_node *node,
+		const void *_key)
+{
+	struct ust_app_event *event;
+	const char *name;
+
+	assert(node);
+	assert(_key);
+
+	event = caa_container_of(node, struct ust_app_event, node.node);
+	name = _key;
+
+	if (strncmp(event->attr.name, name, sizeof(event->attr.name))) {
+		goto no_match;
+	}
+
+	/* Match. */
+	return 1;
+
+no_match:
+	return 0;
+}
+
+/*
  * Unique add of an ust app event in the given ht. This uses the custom
  * ht_match_ust_app_event match function and the event name as hash.
  */
@@ -3873,11 +3901,9 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
 {
 	int ret = 0;
 	struct lttng_ht_iter iter, uiter;
-	struct lttng_ht_node_str *ua_chan_node, *ua_event_node;
+	struct lttng_ht_node_str *ua_chan_node;
 	struct ust_app *app;
-	struct ust_app_session *ua_sess;
 	struct ust_app_channel *ua_chan;
-	struct ust_app_event *ua_event;
 
 	DBG("UST app disabling event %s for all apps in channel "
 			"%s for session id %" PRIu64,
@@ -3887,10 +3913,13 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
 
 	/* For all registered applications */
 	cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, pid_n.node) {
+		struct ust_app_session *ua_sess;
+		struct cds_lfht_node *ua_event_node;
+
 		if (!app->compatible) {
 			/*
-			 * TODO: In time, we should notice the caller of this error by
-			 * telling him that this is a version error.
+			 * TODO: In time, we should notice the caller of this
+			 * error by telling him that this is a version error.
 			 */
 			continue;
 		}
@@ -3901,28 +3930,35 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
 		}
 
 		/* Lookup channel in the ust app session */
-		lttng_ht_lookup(ua_sess->channels, (void *)uchan->name, &uiter);
+		lttng_ht_lookup(ua_sess->channels, (void *) uchan->name,
+				&uiter);
 		ua_chan_node = lttng_ht_iter_get_node_str(&uiter);
-		if (ua_chan_node == NULL) {
+		if (!ua_chan_node) {
 			DBG2("Channel %s not found in session id %" PRIu64 " for app pid %d."
-					"Skipping", uchan->name, usess->id, app->pid);
+					"Skipping", uchan->name, usess->id,
+					app->pid);
 			continue;
 		}
 		ua_chan = caa_container_of(ua_chan_node, struct ust_app_channel, node);
 
-		lttng_ht_lookup(ua_chan->events, (void *)uevent->attr.name, &uiter);
-		ua_event_node = lttng_ht_iter_get_node_str(&uiter);
-		if (ua_event_node == NULL) {
-			DBG2("Event %s not found in channel %s for app pid %d."
-					"Skipping", uevent->attr.name, uchan->name, app->pid);
-			continue;
-		}
-		ua_event = caa_container_of(ua_event_node, struct ust_app_event, node);
-
-		ret = disable_ust_app_event(ua_sess, ua_event, app);
-		if (ret < 0) {
-			/* XXX: Report error someday... */
-			continue;
+		cds_lfht_for_each_duplicate(ua_chan->events->ht,
+				ua_chan->events->hash_fct(uevent->attr.name,
+					lttng_ht_seed),
+				ht_match_ust_app_event_by_name,
+				(void *) uevent->attr.name, &uiter.iter,
+				ua_event_node) {
+			struct ust_app_event *ua_event;
+
+			ua_event = caa_container_of(ua_event_node,
+					struct ust_app_event, node);
+			ret = disable_ust_app_event(ua_sess, ua_event, app);
+			if (ret < 0) {
+				DBG("Failed to disable event %s in channel %s "
+						"of app pid %d.",
+						uevent->attr.name,
+						uchan->name, app->pid);
+				continue;
+			}
 		}
 	}
 
-- 
2.5.2


_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [[RFC PATCH lttng-tools]] Fix: Disable all UST events matching the given name
       [not found] ` <1442350762-10938-1-git-send-email-jeremie.galarneau@efficios.com>
@ 2015-09-15 21:04   ` Jérémie Galarneau
  0 siblings, 0 replies; 3+ messages in thread
From: Jérémie Galarneau @ 2015-09-15 21:04 UTC (permalink / raw)
  To: lttng-dev; +Cc: Jonathan Rajotte-Julien

As discussed with Jonathan, while the original proposed patch would be
the "correct" fix, the disable-event command does not accept a filter,
loglevel and exclusion list. This makes it impossible to achieve a
perfect match such as what find_ust_app_event() would provide.
However, the current code is still incorrect as it only disables the
first event matching the provided name.

This patch iterates on all matches and disables all events matching
the name provided in the UST domain.

Jia, any chance you can test this on your end?

Thanks,
Jérémie

On Tue, Sep 15, 2015 at 4:59 PM, Jérémie Galarneau
<jeremie.galarneau@efficios.com> wrote:
> The session daemon will only disable the first event
> matching the name provided to the disable-event command.
> This fix iterates on all events matching the name, ignoring
> their filter, loglevel and exclusion list.
>
> Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
> ---
>  src/bin/lttng-sessiond/ust-app.c | 78 +++++++++++++++++++++++++++++-----------
>  1 file changed, 57 insertions(+), 21 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
> index 4066b06..02f6153 100644
> --- a/src/bin/lttng-sessiond/ust-app.c
> +++ b/src/bin/lttng-sessiond/ust-app.c
> @@ -170,6 +170,34 @@ no_match:
>  }
>
>  /*
> + * Match function for the hash table lookup.
> + *
> + * Matches an ust app event based on its name only.
> + */
> +static int ht_match_ust_app_event_by_name(struct cds_lfht_node *node,
> +               const void *_key)
> +{
> +       struct ust_app_event *event;
> +       const char *name;
> +
> +       assert(node);
> +       assert(_key);
> +
> +       event = caa_container_of(node, struct ust_app_event, node.node);
> +       name = _key;
> +
> +       if (strncmp(event->attr.name, name, sizeof(event->attr.name))) {
> +               goto no_match;
> +       }
> +
> +       /* Match. */
> +       return 1;
> +
> +no_match:
> +       return 0;
> +}
> +
> +/*
>   * Unique add of an ust app event in the given ht. This uses the custom
>   * ht_match_ust_app_event match function and the event name as hash.
>   */
> @@ -3873,11 +3901,9 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
>  {
>         int ret = 0;
>         struct lttng_ht_iter iter, uiter;
> -       struct lttng_ht_node_str *ua_chan_node, *ua_event_node;
> +       struct lttng_ht_node_str *ua_chan_node;
>         struct ust_app *app;
> -       struct ust_app_session *ua_sess;
>         struct ust_app_channel *ua_chan;
> -       struct ust_app_event *ua_event;
>
>         DBG("UST app disabling event %s for all apps in channel "
>                         "%s for session id %" PRIu64,
> @@ -3887,10 +3913,13 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
>
>         /* For all registered applications */
>         cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, pid_n.node) {
> +               struct ust_app_session *ua_sess;
> +               struct cds_lfht_node *ua_event_node;
> +
>                 if (!app->compatible) {
>                         /*
> -                        * TODO: In time, we should notice the caller of this error by
> -                        * telling him that this is a version error.
> +                        * TODO: In time, we should notice the caller of this
> +                        * error by telling him that this is a version error.
>                          */
>                         continue;
>                 }
> @@ -3901,28 +3930,35 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
>                 }
>
>                 /* Lookup channel in the ust app session */
> -               lttng_ht_lookup(ua_sess->channels, (void *)uchan->name, &uiter);
> +               lttng_ht_lookup(ua_sess->channels, (void *) uchan->name,
> +                               &uiter);
>                 ua_chan_node = lttng_ht_iter_get_node_str(&uiter);
> -               if (ua_chan_node == NULL) {
> +               if (!ua_chan_node) {
>                         DBG2("Channel %s not found in session id %" PRIu64 " for app pid %d."
> -                                       "Skipping", uchan->name, usess->id, app->pid);
> +                                       "Skipping", uchan->name, usess->id,
> +                                       app->pid);
>                         continue;
>                 }
>                 ua_chan = caa_container_of(ua_chan_node, struct ust_app_channel, node);
>
> -               lttng_ht_lookup(ua_chan->events, (void *)uevent->attr.name, &uiter);
> -               ua_event_node = lttng_ht_iter_get_node_str(&uiter);
> -               if (ua_event_node == NULL) {
> -                       DBG2("Event %s not found in channel %s for app pid %d."
> -                                       "Skipping", uevent->attr.name, uchan->name, app->pid);
> -                       continue;
> -               }
> -               ua_event = caa_container_of(ua_event_node, struct ust_app_event, node);
> -
> -               ret = disable_ust_app_event(ua_sess, ua_event, app);
> -               if (ret < 0) {
> -                       /* XXX: Report error someday... */
> -                       continue;
> +               cds_lfht_for_each_duplicate(ua_chan->events->ht,
> +                               ua_chan->events->hash_fct(uevent->attr.name,
> +                                       lttng_ht_seed),
> +                               ht_match_ust_app_event_by_name,
> +                               (void *) uevent->attr.name, &uiter.iter,
> +                               ua_event_node) {
> +                       struct ust_app_event *ua_event;
> +
> +                       ua_event = caa_container_of(ua_event_node,
> +                                       struct ust_app_event, node);
> +                       ret = disable_ust_app_event(ua_sess, ua_event, app);
> +                       if (ret < 0) {
> +                               DBG("Failed to disable event %s in channel %s "
> +                                               "of app pid %d.",
> +                                               uevent->attr.name,
> +                                               uchan->name, app->pid);
> +                               continue;
> +                       }
>                 }
>         }
>
> --
> 2.5.2
>



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH lttng-tools] Fix: sessiond: disable: match app event by name
       [not found] <1441832900-7573-1-git-send-email-jonathan.rajotte-julien@efficios.com>
  2015-09-15 20:59 ` [[RFC PATCH lttng-tools]] Fix: Disable all UST events matching the given name Jérémie Galarneau
       [not found] ` <1442350762-10938-1-git-send-email-jeremie.galarneau@efficios.com>
@ 2015-09-16 18:51 ` Jérémie Galarneau
  2 siblings, 0 replies; 3+ messages in thread
From: Jérémie Galarneau @ 2015-09-16 18:51 UTC (permalink / raw)
  To: Jonathan Rajotte; +Cc: lttng-dev, Jeremie Galarneau

Merged, thanks!

Jérémie

On Wed, Sep 9, 2015 at 5:08 PM, Jonathan Rajotte
<jonathan.rajotte-julien@efficios.com> wrote:
> The use of a simple lookup and match on event name is insufficient
> to identify the corresponding ust app event.
>
> Fixes #914
>
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
> ---
>  src/bin/lttng-sessiond/ust-app.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
> index 4066b06..53a6f93 100644
> --- a/src/bin/lttng-sessiond/ust-app.c
> +++ b/src/bin/lttng-sessiond/ust-app.c
> @@ -3873,7 +3873,7 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
>  {
>         int ret = 0;
>         struct lttng_ht_iter iter, uiter;
> -       struct lttng_ht_node_str *ua_chan_node, *ua_event_node;
> +       struct lttng_ht_node_str *ua_chan_node;
>         struct ust_app *app;
>         struct ust_app_session *ua_sess;
>         struct ust_app_channel *ua_chan;
> @@ -3910,14 +3910,14 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
>                 }
>                 ua_chan = caa_container_of(ua_chan_node, struct ust_app_channel, node);
>
> -               lttng_ht_lookup(ua_chan->events, (void *)uevent->attr.name, &uiter);
> -               ua_event_node = lttng_ht_iter_get_node_str(&uiter);
> -               if (ua_event_node == NULL) {
> +               ua_event = find_ust_app_event(ua_chan->events, uevent->attr.name,
> +                               uevent->filter, uevent->attr.loglevel,
> +                               uevent->exclusion);
> +               if (ua_event == NULL) {
>                         DBG2("Event %s not found in channel %s for app pid %d."
>                                         "Skipping", uevent->attr.name, uchan->name, app->pid);
>                         continue;
>                 }
> -               ua_event = caa_container_of(ua_event_node, struct ust_app_event, node);
>
>                 ret = disable_ust_app_event(ua_sess, ua_event, app);
>                 if (ret < 0) {
> --
> 2.1.4
>



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-09-16 18:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1441832900-7573-1-git-send-email-jonathan.rajotte-julien@efficios.com>
2015-09-15 20:59 ` [[RFC PATCH lttng-tools]] Fix: Disable all UST events matching the given name Jérémie Galarneau
     [not found] ` <1442350762-10938-1-git-send-email-jeremie.galarneau@efficios.com>
2015-09-15 21:04   ` Jérémie Galarneau
2015-09-16 18:51 ` [PATCH lttng-tools] Fix: sessiond: disable: match app event by name Jérémie Galarneau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.