[lttng-dev] [PATCH lttng-tools 1/3] Fix: disable kernel event based on name and event type

Jérémie Galarneau jeremie.galarneau at efficios.com
Mon Sep 21 15:00:54 EDT 2015


On Thu, Sep 17, 2015 at 4:37 PM, Jonathan Rajotte
<jonathan.rajotte-julien at efficios.com> wrote:
> The -a argument is interpreted as a zero-length event name
> instead of '*' which is actually a valid wildcard event
> name by itself. This simplify how a disable action is handled.
>
> The event type can now be passed as argument
> and is a new criteria while disabling kernel events.
> The default is to disable for all event type.
>
> UST and agent domain do not support yet disabling by event
> type.
>
> e.g:
>         # Only disable kernel event of type tracepoint.
>         lttng disable -a -k --tracepoint
>
>         # Only disable the event with name '*' and type syscall.
>         lttng disable -k '*' --syscall
>
>         # Disable all kernel event of all type.
>         lttng disable -a -k
>
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
>  src/bin/lttng-sessiond/cmd.c                       | 45 +++++-----
>  src/bin/lttng-sessiond/event.c                     | 65 +++++---------
>  src/bin/lttng-sessiond/event.h                     |  5 +-
>  src/bin/lttng/commands/disable_events.c            | 98 ++++++++++++++++++----
>  src/lib/lttng-ctl/lttng-ctl.c                      |  4 -
>  .../ust/python-logging/test_python_logging.in      |  2 +-
>  6 files changed, 121 insertions(+), 98 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
> index 7878a05..5a1cc7a 100644
> --- a/src/bin/lttng-sessiond/cmd.c
> +++ b/src/bin/lttng-sessiond/cmd.c
> @@ -1225,32 +1225,22 @@ int cmd_disable_event(struct ltt_session *session,
>
>                 switch (event->type) {
>                 case LTTNG_EVENT_ALL:
> -                       ret = event_kernel_disable_event_all(kchan);
> -                       if (ret != LTTNG_OK) {
> -                               goto error_unlock;
> -                       }
> -                       break;
> -               case LTTNG_EVENT_TRACEPOINT:    /* fall-through */
> +               case LTTNG_EVENT_TRACEPOINT:
>                 case LTTNG_EVENT_SYSCALL:
> -                       if (!strcmp(event_name, "*")) {
> -                               ret = event_kernel_disable_event_type(kchan,
> -                                       event->type);
> +               case LTTNG_EVENT_PROBE:
> +               case LTTNG_EVENT_FUNCTION:
> +               case LTTNG_EVENT_FUNCTION_ENTRY:/* fall-through */
> +                       if (event_name[0] == '\0') {
> +                               ret = event_kernel_disable_event(kchan,
> +                                       NULL, event->type);
>                         } else {
>                                 ret = event_kernel_disable_event(kchan,
> -                                       event_name);
> +                                       event_name, event->type);
>                         }
>                         if (ret != LTTNG_OK) {
>                                 goto error_unlock;
>                         }
>                         break;
> -               case LTTNG_EVENT_PROBE:
> -               case LTTNG_EVENT_FUNCTION:
> -               case LTTNG_EVENT_FUNCTION_ENTRY:
> -                       ret = event_kernel_disable_event(kchan, event_name);
> -                       if (ret != LTTNG_OK) {
> -                               goto error_unlock;
> -                       }
> -                       break;
>                 default:
>                         ret = LTTNG_ERR_UNK;
>                         goto error_unlock;
> @@ -1273,7 +1263,7 @@ int cmd_disable_event(struct ltt_session *session,
>
>                 /*
>                  * If a non-default channel has been created in the
> -                * session, explicitely require that -c chan_name needs
> +                * session, explicitly require that -c chan_name needs
>                  * to be provided.
>                  */
>                 if (usess->has_non_default_channel && channel_name[0] == '\0') {
> @@ -1290,10 +1280,12 @@ int cmd_disable_event(struct ltt_session *session,
>
>                 switch (event->type) {
>                 case LTTNG_EVENT_ALL:
> -                       if (strlen(event->name) == 1 &&
> -                                       !strncmp(event->name, "*", 1)) {
> -                               ret = event_ust_disable_all_tracepoints(usess,
> -                                               uchan);
> +                       /*
> +                        * An empty event name means that everything
> +                        * should be disabled.
> +                        */
> +                       if (event->name[0] == '\0') {
> +                               ret = event_ust_disable_all_tracepoints(usess, uchan);
>                         } else {
>                                 ret = event_ust_disable_tracepoint(usess, uchan,
>                                                 event_name);

Please move this hunk and the following one to separate patches as
they fix the same problem for user space and agent domains. This
commit's title implies that it only addresses the issue in the kernel
domain.

> @@ -1333,8 +1325,11 @@ int cmd_disable_event(struct ltt_session *session,
>                         ret = -LTTNG_ERR_UST_EVENT_NOT_FOUND;
>                         goto error_unlock;
>                 }
> -               /* The wild card * means that everything should be disabled. */
> -               if (strncmp(event->name, "*", 1) == 0 && strlen(event->name) == 1) {
> +               /*
> +                * An empty event name means that everything
> +                * should be disabled.
> +                */
> +               if (event->name[0] == '\0') {
>                         ret = event_agent_disable_all(usess, agt);
>                 } else {
>                         ret = event_agent_disable(usess, agt, event_name);
> diff --git a/src/bin/lttng-sessiond/event.c b/src/bin/lttng-sessiond/event.c
> index 13f09a5..a53d4e9 100644
> --- a/src/bin/lttng-sessiond/event.c
> +++ b/src/bin/lttng-sessiond/event.c
> @@ -61,45 +61,15 @@ static void add_unique_ust_event(struct lttng_ht *ht,
>  }
>
>  /*
> - * Disable kernel tracepoint event for a channel from the kernel session.
> + * Disable kernel tracepoint events for a channel from the kernel session of
> + * a specified event_name and event type.
> + * On type LTTNG_EVENT_ALL all events with event_name are disabled.
> + * If event_name if NULL all events of the specified type are disabled.

if NULL -> is NULL

>   */
>  int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
> -               char *event_name)
> -{
> -       int ret;
> -       struct ltt_kernel_event *kevent;
> -
> -       assert(kchan);
> -
> -       kevent = trace_kernel_get_event_by_name(event_name, kchan,
> -                       LTTNG_EVENT_ALL);
> -       if (kevent == NULL) {
> -               ret = LTTNG_ERR_NO_EVENT;
> -               goto error;
> -       }
> -
> -       ret = kernel_disable_event(kevent);
> -       if (ret < 0) {
> -               ret = LTTNG_ERR_KERN_DISABLE_FAIL;
> -               goto error;
> -       }
> -
> -       DBG("Kernel event %s disable for channel %s.",
> -                       kevent->event->name, kchan->channel->name);
> -
> -       ret = LTTNG_OK;
> -
> -error:
> -       return ret;
> -}
> -
> -/*
> - * Disable kernel tracepoint events for a channel from the kernel session.
> - */
> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
> -               enum lttng_event_type type)
> +               char *event_name, enum lttng_event_type type)
>  {
> -       int ret;
> +       int ret, error = 0, found = 0;
>         struct ltt_kernel_event *kevent;
>
>         assert(kchan);
> @@ -108,22 +78,25 @@ int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>         cds_list_for_each_entry(kevent, &kchan->events_list.head, list) {
>                 if (type != LTTNG_EVENT_ALL && kevent->type != type)
>                         continue;
> +               if (event_name != NULL && strcmp(event_name, kevent->event->name)) {
> +                       continue;
> +               }
> +               found++;
>                 ret = kernel_disable_event(kevent);
>                 if (ret < 0) {
> -                       /* We continue disabling the rest */
> +                       error = 1;
>                         continue;
>                 }
>         }
> -       ret = LTTNG_OK;
> -       return ret;
> -}
> +       DBG("Disable kernel event: found %d events with name: %s and type: %d",
> +                       found, event_name, type);

event_name could be NULL here.

event_name ? event_name : "NULL"

>
> -/*
> - * Disable all kernel event for a channel from the kernel session.
> - */
> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan)
> -{
> -       return event_kernel_disable_event_type(kchan, LTTNG_EVENT_ALL);
> +       if (!found && event_name != NULL) {
> +               error = 1;
> +       }
> +
> +       ret = error ? LTTNG_ERR_KERN_DISABLE_FAIL : LTTNG_OK;
> +       return ret;

This should distinguish between:

1) event found, but we failed to disable it (LTTNG_ERR_KERN_DISABLE_FAIL)
2) event_name != NULL && !found (LTTNG_ERR_NO_EVENT, which it returned before)

>  }
>
>  /*
> diff --git a/src/bin/lttng-sessiond/event.h b/src/bin/lttng-sessiond/event.h
> index 7c5231d..45dd1fc 100644
> --- a/src/bin/lttng-sessiond/event.h
> +++ b/src/bin/lttng-sessiond/event.h
> @@ -23,10 +23,7 @@
>  struct agent;
>
>  int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
> -               char *event_name);
> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
> -               enum lttng_event_type type);
> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan);
> +               char *event_name, enum lttng_event_type event_type);
>
>  int event_kernel_enable_event(struct ltt_kernel_channel *kchan,
>                 struct lttng_event *event, char *filter_expression,
> diff --git a/src/bin/lttng/commands/disable_events.c b/src/bin/lttng/commands/disable_events.c
> index c6ec85f..bcbc78d 100644
> --- a/src/bin/lttng/commands/disable_events.c
> +++ b/src/bin/lttng/commands/disable_events.c
> @@ -44,7 +44,12 @@ static int opt_event_type;
>  enum {
>         OPT_HELP = 1,
>         OPT_USERSPACE,
> -       OPT_SYSCALL,
> +       OPT_TYPE_SYSCALL,
> +       OPT_TYPE_TRACEPOINT,
> +       OPT_TYPE_PROBE,
> +       OPT_TYPE_FUNCTION,
> +       OPT_TYPE_FUNCTION_ENTRY,
> +       OPT_TYPE_ALL,
>         OPT_LIST_OPTIONS,
>  };
>
> @@ -61,8 +66,24 @@ static struct poptOption long_options[] = {
>         {"log4j",          'l', POPT_ARG_VAL, &opt_log4j, 1, 0, 0},
>         {"python",         'p', POPT_ARG_VAL, &opt_python, 1, 0, 0},
>         {"kernel",         'k', POPT_ARG_VAL, &opt_kernel, 1, 0, 0},
> -       {"syscall",        0,   POPT_ARG_NONE, 0, OPT_SYSCALL, 0, 0},
> +       {"syscall",          0, POPT_ARG_NONE, 0, OPT_TYPE_SYSCALL, 0, 0},
> +       {"probe",            0, POPT_ARG_NONE, 0, OPT_TYPE_PROBE, 0, 0},
> +       {"tracepoint",       0, POPT_ARG_NONE, 0, OPT_TYPE_TRACEPOINT, 0, 0},
> +       {"function",         0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION, 0, 0},
> +       {"all",              0, POPT_ARG_NONE, 0, OPT_TYPE_ALL, 0, 0},
> +#if 0
> +       /*
> +        * Function entry exits as a lttng_event_type but is never used on
> +        * enable
> +        */
> +       {"function-entry",   0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION_ENTRY, 0, 0},
> +
> +       /* Not implemented yet */
> +       {"userspace",      'u', POPT_ARG_STRING | POPT_ARGFLAG_OPTIONAL, &opt_cmd_name, OPT_USERSPACE, 0, 0},
> +       {"pid",            'p', POPT_ARG_INT, &opt_pid, 0, 0, 0},
> +#else
>         {"userspace",      'u', POPT_ARG_NONE, 0, OPT_USERSPACE, 0, 0},
> +#endif

I have removed #ifdef-ed out code from disable-event in

commit 3fd691fc09199be5a52009a391b22379477aa6e7
Author: Jérémie Galarneau <jeremie.galarneau at efficios.com>
Date:   Tue Sep 15 13:00:40 2015 -0400

    Remove dead code from disable-event command

    Signed-off-by: Jérémie Galarneau <jeremie.galarneau at efficios.com>

function-entry instrumentation is simply not supported, so let's
mention it in the switch case that handles the instrumentation type,
but not add compiled-out code.

>         {"list-options", 0, POPT_ARG_NONE, NULL, OPT_LIST_OPTIONS, NULL, NULL},
>         {0, 0, 0, 0, 0, 0, 0}
>  };
> @@ -86,8 +107,19 @@ static void usage(FILE *ofp)
>         fprintf(ofp, "  -l, --log4j              Apply to Java application using LOG4j\n");
>         fprintf(ofp, "  -p, --python             Apply to Python application using logging\n");
>         fprintf(ofp, "\n");
> -       fprintf(ofp, "Event options:\n");
> +       fprintf(ofp, "Event type options (Only supported with kernel domain):\n");
> +       fprintf(ofp, "      --all                All event type (default)\n");

type -> types

> +       fprintf(ofp, "      --tracepoint         Tracepoint event\n");
>         fprintf(ofp, "      --syscall            System call event\n");
> +       fprintf(ofp, "      --probe              Probe event\n");
> +       fprintf(ofp, "      --function           Function event\n");
> +#if 0
> +       /*
> +        * Function entry exits as a lttng_event_type but is never used on
> +        * enable
> +        */
> +       fprintf(ofp, "      --function-entry     Function-entry event\n");
> +#endif

Same here; no ifdef-ed out code.

>         fprintf(ofp, "\n");
>  }
>
> @@ -103,6 +135,27 @@ const char *print_raw_channel_name(const char *name)
>         return name ? : "<default>";
>  }
>
> +static
> +const char *print_event_type(const enum lttng_event_type ev_type)
> +{
> +       switch (ev_type) {
> +       case LTTNG_EVENT_ALL:
> +               return "any";
> +       case LTTNG_EVENT_TRACEPOINT:
> +               return "tracepoint";
> +       case LTTNG_EVENT_PROBE:
> +               return "probe";
> +       case LTTNG_EVENT_FUNCTION:
> +               return "function";
> +       case LTTNG_EVENT_FUNCTION_ENTRY:
> +               return "function entry";
> +       case LTTNG_EVENT_SYSCALL:
> +               return "syscall";
> +       default:
> +               return "";
> +       }
> +}
> +
>  /* Mi print a partial event.
>   * enabled is 0 or 1
>   * success is 0 or 1
> @@ -213,14 +266,8 @@ static int disable_events(char *session_name)
>         /* Set default loglevel to any/unknown */
>         event.loglevel = -1;
>
> -       switch (opt_event_type) {
> -       case LTTNG_EVENT_SYSCALL:
> -               event.type = LTTNG_EVENT_SYSCALL;
> -               break;
> -       default:
> -               event.type = LTTNG_EVENT_ALL;
> -               break;
> -       }
> +       /* opt_event_type contain the event type to disable at this point */
> +       event.type = opt_event_type;
>
>         if (opt_disable_all) {
>                 command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
> @@ -232,9 +279,9 @@ static int disable_events(char *session_name)
>                 } else {
>                         enabled = 0;
>                         success = 1;
> -                       MSG("All %s %s are disabled in channel %s",
> +                       MSG("All %s events of type %s are disabled in channel %s",
>                                         get_domain_str(dom.type),
> -                                       opt_event_type == LTTNG_EVENT_SYSCALL ? "system calls" : "events",
> +                                       print_event_type(opt_event_type),
>                                         print_channel_name(channel_name));
>                 }
>
> @@ -255,9 +302,9 @@ static int disable_events(char *session_name)
>                         event.name[sizeof(event.name) - 1] = '\0';
>                         command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
>                         if (command_ret < 0) {
> -                               ERR("%s %s: %s (channel %s, session %s)",
> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "System call" : "Event",
> +                               ERR("%s of type %s : %s (channel %s, session %s)",
>                                                 event_name,
> +                                               print_event_type(opt_event_type),
>                                                 lttng_strerror(command_ret),
>                                                 command_ret == -LTTNG_ERR_NEED_CHANNEL_NAME
>                                                         ? print_raw_channel_name(channel_name)
> @@ -271,10 +318,10 @@ static int disable_events(char *session_name)
>                                  */
>                                 enabled = 1;
>                         } else {
> -                               MSG("%s %s %s disabled in channel %s for session %s",
> +                               MSG("%s %s of type %s disabled in channel %s for session %s",
>                                                 get_domain_str(dom.type),
> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "system call" : "event",
>                                                 event_name,
> +                                               print_event_type(opt_event_type),
>                                                 print_channel_name(channel_name),
>                                                 session_name);
>                                 success = 1;
> @@ -341,9 +388,24 @@ int cmd_disable_events(int argc, const char **argv)
>                 case OPT_USERSPACE:
>                         opt_userspace = 1;
>                         break;
> -               case OPT_SYSCALL:
> +               case OPT_TYPE_SYSCALL:
>                         opt_event_type = LTTNG_EVENT_SYSCALL;
>                         break;
> +               case OPT_TYPE_TRACEPOINT:
> +                       opt_event_type = LTTNG_EVENT_TRACEPOINT;
> +                       break;
> +               case OPT_TYPE_PROBE:
> +                       opt_event_type = LTTNG_EVENT_PROBE;
> +                       break;
> +               case OPT_TYPE_FUNCTION:
> +                       opt_event_type = LTTNG_EVENT_FUNCTION;
> +                       break;
> +               case OPT_TYPE_FUNCTION_ENTRY:
> +                       opt_event_type = LTTNG_EVENT_FUNCTION_ENTRY;
> +                       break;

We shouldn't allow this (FUNCTION and FUNCTION_ENTRY) for non-kernel
domains as their support is not implemented.

> +               case OPT_TYPE_ALL:
> +                       opt_event_type = LTTNG_EVENT_ALL;
> +                       break;
>                 case OPT_LIST_OPTIONS:
>                         list_cmd_options(stdout, long_options);
>                         goto end;
> diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
> index 9cbfef5..1007326 100644
> --- a/src/lib/lttng-ctl/lttng-ctl.c
> +++ b/src/lib/lttng-ctl/lttng-ctl.c
> @@ -1091,10 +1091,6 @@ int lttng_disable_event_ext(struct lttng_handle *handle,
>         }
>
>         lsm.cmd_type = LTTNG_DISABLE_EVENT;
> -       if (ev->name[0] == '\0') {
> -               /* Disable all events */
> -               lttng_ctl_copy_string(ev->name, "*", sizeof(ev->name));
> -       }
>
>         lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain);
>         /* FIXME: copying non-packed struct to packed struct. */
> diff --git a/tests/regression/ust/python-logging/test_python_logging.in b/tests/regression/ust/python-logging/test_python_logging.in
> index 8aff408..d5a9b80 100755
> --- a/tests/regression/ust/python-logging/test_python_logging.in
> +++ b/tests/regression/ust/python-logging/test_python_logging.in
> @@ -351,7 +351,7 @@ function test_python_disable_all()
>         enable_python_lttng_event $SESSION_NAME $EVENT_NAME
>         enable_python_lttng_event $SESSION_NAME $EVENT_NAME2
>
> -       disable_python_lttng_event $SESSION_NAME '*'
> +       disable_python_lttng_event $SESSION_NAME -a

Please submit a separate patch for this.

Thanks!
Jérémie

>
>         start_lttng_tracing_ok $SESSION_NAME
>
> --
> 2.1.4
>



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



More information about the lttng-dev mailing list