2021-06-10 06:10:19

by Ian Rogers

[permalink] [raw]
Subject: [RFC PATCH] libtraceevent: Increase libtraceevent logging when verbose

libtraceevent has added more levels of debug printout and with changes
like:
https://lore.kernel.org/linux-trace-devel/[email protected]
previously generated output like "registering plugin" is no longer
displayed. This change makes it so that if perf's verbose debug output
is enabled then the debug and info libtraceevent messages can be
displayed.
As this API isn't present in the deprecated tools version of
libtracevent I'm uploading this as an RFC.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/debug.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 2c06abf6dcd2..700828e92fe1 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -24,6 +24,7 @@
#include "util/parse-sublevel-options.h"

#include <linux/ctype.h>
+#include <traceevent/event-parse.h>

int verbose;
int debug_peo_args;
@@ -228,6 +229,13 @@ int perf_debug_option(const char *str)
/* Allow only verbose value in range (0, 10), otherwise set 0. */
verbose = (verbose < 0) || (verbose > 10) ? 0 : verbose;

+ if (verbose == 1)
+ tep_set_loglevel(TEP_LOG_INFO);
+ else if (verbose == 2)
+ tep_set_loglevel(TEP_LOG_DEBUG);
+ else if (verbose >= 3)
+ tep_set_loglevel(TEP_LOG_ALL);
+
return 0;
}

--
2.32.0.272.g935e593368-goog


2021-06-10 14:43:00

by Steven Rostedt

[permalink] [raw]
Subject: Re: [RFC PATCH] libtraceevent: Increase libtraceevent logging when verbose

On Wed, 9 Jun 2021 23:06:43 -0700
Ian Rogers <[email protected]> wrote:

> libtraceevent has added more levels of debug printout and with changes
> like:
> https://lore.kernel.org/linux-trace-devel/[email protected]
> previously generated output like "registering plugin" is no longer
> displayed. This change makes it so that if perf's verbose debug output
> is enabled then the debug and info libtraceevent messages can be
> displayed.
> As this API isn't present in the deprecated tools version of
> libtracevent I'm uploading this as an RFC.

Thanks Ian,

We need to start porting perf to using the upstream libtraceevent
library. I think the best way to do that is what we did with trace-cmd.
That is to have the make files check if the minimum version of
libtraceevent is installed, and if so, use that instead of the local
version. If it is not installed, produce a message encouraging the
developer to install the upstream libtraceevent and warn that it will
be using a deprecated older versino, then build the deprecated local
version. After some time, we could simply remove it and make it a
dependency, but I want to do that when all the main distros being used
have it.

Currently its in the latest Debian, Ubuntu, and Fedora. I also believe
its in SUSE but have not checked. It's in Fedora 34, but it doesn't
appear to be in Fedora 33. As that's not too old, I don't think we
should make it a dependency as of yet.

-- Steve

2021-06-10 18:53:30

by Ian Rogers

[permalink] [raw]
Subject: Re: [RFC PATCH] libtraceevent: Increase libtraceevent logging when verbose

On Thu, Jun 10, 2021 at 7:39 AM Steven Rostedt <[email protected]> wrote:
>
> On Wed, 9 Jun 2021 23:06:43 -0700
> Ian Rogers <[email protected]> wrote:
>
> > libtraceevent has added more levels of debug printout and with changes
> > like:
> > https://lore.kernel.org/linux-trace-devel/[email protected]
> > previously generated output like "registering plugin" is no longer
> > displayed. This change makes it so that if perf's verbose debug output
> > is enabled then the debug and info libtraceevent messages can be
> > displayed.
> > As this API isn't present in the deprecated tools version of
> > libtracevent I'm uploading this as an RFC.
>
> Thanks Ian,
>
> We need to start porting perf to using the upstream libtraceevent
> library. I think the best way to do that is what we did with trace-cmd.
> That is to have the make files check if the minimum version of
> libtraceevent is installed, and if so, use that instead of the local
> version. If it is not installed, produce a message encouraging the
> developer to install the upstream libtraceevent and warn that it will
> be using a deprecated older versino, then build the deprecated local
> version. After some time, we could simply remove it and make it a
> dependency, but I want to do that when all the main distros being used
> have it.
>
> Currently its in the latest Debian, Ubuntu, and Fedora. I also believe
> its in SUSE but have not checked. It's in Fedora 34, but it doesn't
> appear to be in Fedora 33. As that's not too old, I don't think we
> should make it a dependency as of yet.
>
> -- Steve

Thanks! Is there a way to do something like:

#if LIBTRACE_EVENT_API > 123
...
#endif

so that we can make sure perf is taking advantage of the improvements
in the libtraceevent API?

Ian

2021-06-10 19:11:46

by Jiri Olsa

[permalink] [raw]
Subject: Re: [RFC PATCH] libtraceevent: Increase libtraceevent logging when verbose

On Thu, Jun 10, 2021 at 10:39:27AM -0400, Steven Rostedt wrote:
> On Wed, 9 Jun 2021 23:06:43 -0700
> Ian Rogers <[email protected]> wrote:
>
> > libtraceevent has added more levels of debug printout and with changes
> > like:
> > https://lore.kernel.org/linux-trace-devel/[email protected]
> > previously generated output like "registering plugin" is no longer
> > displayed. This change makes it so that if perf's verbose debug output
> > is enabled then the debug and info libtraceevent messages can be
> > displayed.
> > As this API isn't present in the deprecated tools version of
> > libtracevent I'm uploading this as an RFC.
>
> Thanks Ian,
>
> We need to start porting perf to using the upstream libtraceevent
> library. I think the best way to do that is what we did with trace-cmd.
> That is to have the make files check if the minimum version of
> libtraceevent is installed, and if so, use that instead of the local
> version. If it is not installed, produce a message encouraging the
> developer to install the upstream libtraceevent and warn that it will
> be using a deprecated older versino, then build the deprecated local
> version. After some time, we could simply remove it and make it a
> dependency, but I want to do that when all the main distros being used
> have it.

Michael did the libtraceevent detection and dynamic linking support:
https://lore.kernel.org/linux-perf-users/[email protected]/

I think we should have that in Fedora/RHEL rpms already, or it's on the way.

The detection code could be change to contain things we need.

jirka

>
> Currently its in the latest Debian, Ubuntu, and Fedora. I also believe
> its in SUSE but have not checked. It's in Fedora 34, but it doesn't
> appear to be in Fedora 33. As that's not too old, I don't think we
> should make it a dependency as of yet.
>
> -- Steve
>

2021-06-10 19:34:08

by Steven Rostedt

[permalink] [raw]
Subject: Re: [RFC PATCH] libtraceevent: Increase libtraceevent logging when verbose

On Thu, 10 Jun 2021 11:50:00 -0700
Ian Rogers <[email protected]> wrote:

> #if LIBTRACE_EVENT_API > 123
> ...
> #endif
>
> so that we can make sure perf is taking advantage of the improvements
> in the libtraceevent API?

Yes, and trace-cmd did that.

In the Makefile:

LIBTRACEEVENT_MIN_VERSION = 1.2.3
TEST_LIBTRACEEVENT = $(shell sh -c "$(PKG_CONFIG) --atleast-version $(LIBTRACEEVENT_MIN_VERSION) libtraceevent > /dev/null 2>&1 && echo y")

ifeq ("$(TEST_LIBTRACEEVENT)", "y")
CFLAGS += -DHAVE_TRACEVEVENT_API
endif

Then you can use in perf

#ifdef HAVE_TRACEEVENT_API
...
#endif

That's just one example of what you could do.

-- Steve

2021-06-10 19:50:44

by Steven Rostedt

[permalink] [raw]
Subject: Re: [RFC PATCH] libtraceevent: Increase libtraceevent logging when verbose

On Thu, 10 Jun 2021 21:08:32 +0200
Jiri Olsa <[email protected]> wrote:

> Michael did the libtraceevent detection and dynamic linking support:
> https://lore.kernel.org/linux-perf-users/[email protected]/

Oh cool! I missed this.

>
> I think we should have that in Fedora/RHEL rpms already, or it's on the way.
>
> The detection code could be change to contain things we need.

Sounds great!

But the next thing we should use is libtracefs, to simplify some of the
other logic, like finding the event files, and even reading the
error_log file when a kprobe_event fails.

-- Steve

2021-06-10 20:03:30

by Steven Rostedt

[permalink] [raw]
Subject: Re: [RFC PATCH] libtraceevent: Increase libtraceevent logging when verbose

On Thu, 10 Jun 2021 21:08:32 +0200
Jiri Olsa <[email protected]> wrote:

> Michael did the libtraceevent detection and dynamic linking support:
> https://lore.kernel.org/linux-perf-users/[email protected]/
>
> I think we should have that in Fedora/RHEL rpms already, or it's on the way.
>
> The detection code could be change to contain things we need.

Or it can also be changed to export the version too?

LIBTRACEEVENT_VERSION = $(shell pkg-config --modversion libtraceevent | perl -e '$ver=<>; @v=split /,/,$ver; printf "%d\n", $v[0] * 255 * 255 | $v[1] * 255 | $v[2];')

-DLIBTRACEFS_VERSION=$(LIBTRACEEVENT_VERSION)

Then in C have:

#define VERSION(a,b,c) ((a)*255*255|(b)*255|(c))

#if LIBTRACEEVENT_VERSION >= VERSION(1,2,3)

To test if the libtraceevent feature is available?

-- Steve

2021-06-10 20:52:08

by Jiri Olsa

[permalink] [raw]
Subject: Re: [RFC PATCH] libtraceevent: Increase libtraceevent logging when verbose

On Thu, Jun 10, 2021 at 03:59:15PM -0400, Steven Rostedt wrote:
> On Thu, 10 Jun 2021 21:08:32 +0200
> Jiri Olsa <[email protected]> wrote:
>
> > Michael did the libtraceevent detection and dynamic linking support:
> > https://lore.kernel.org/linux-perf-users/[email protected]/
> >
> > I think we should have that in Fedora/RHEL rpms already, or it's on the way.
> >
> > The detection code could be change to contain things we need.
>
> Or it can also be changed to export the version too?
>
> LIBTRACEEVENT_VERSION = $(shell pkg-config --modversion libtraceevent | perl -e '$ver=<>; @v=split /,/,$ver; printf "%d\n", $v[0] * 255 * 255 | $v[1] * 255 | $v[2];')
>
> -DLIBTRACEFS_VERSION=$(LIBTRACEEVENT_VERSION)

so the feature detection just gives out on/off answers,
but we can easily put above to perf's makefile

jirka

>
> Then in C have:
>
> #define VERSION(a,b,c) ((a)*255*255|(b)*255|(c))
>
> #if LIBTRACEEVENT_VERSION >= VERSION(1,2,3)
>
> To test if the libtraceevent feature is available?
>
> -- Steve
>