[ANNOUNCE] Linux Trace Toolkit 0.9.5

[ANNOUNCE] Linux Trace Toolkit 0.9.5

[Karim Yaghmour]

LTT 0.9.5 has now been released.

A very large number of enhancements have been added since 0.9.4. This
is a summary:
o S/390 port 
o SuperH port 
o MIPS port 
o Cross-platform trace reading capabilities 
o Updated RTAI support 
o User-space events (LibUserTrace) 
o Binary traces accessible through user-space library (LibLTT) 
o Visualizer enhancements 
o Dynamic modification of trace masks 
o Autoconf/autobuild build system integration 

For the full detail of the additions, see the news section of the
project's website.

Since LTT is close to 3 years old, it has become a large body of
software. To facilitate easy understanding of what LTT does and what
it doesn't, I've added a "features" description on the project's
front page: http://www.opersys.com/LTT/index.html#features

The list of events traced LTT had been previously documented in
some of the articles I presented about the tool. In order to
facilitate access, I've added an online version of the events
list: http://www.opersys.com/LTT/trace-points.html

As I said earlier, a 2.5.x patch is available and LTT is ready to
be integrated into the 2.5 series.

LTT's website is: http://www.opersys.com/LTT

Cheers,

Karim

===================================================
                 Karim Yaghmour
               karim@opersys.com
      Embedded and Real-Time Linux Expert
===================================================

Re: [ANNOUNCE] Linux Trace Toolkit 0.9.5

[Roman Zippel]

Hi,

On Wed, 24 Apr 2002, Karim Yaghmour wrote:

> As I said earlier, a 2.5.x patch is available and LTT is ready to
> be integrated into the 2.5 series.

I'd really like to see it go in, but I think some small problems are left,
mostly formatting. Please read Documentation/CodingStyle.
Please use tabs for indentation and not spaces.
You should consider using more inline functions, instead of lots of "do
{...} while(0)" macros.
Do we really need more usages of uint32_t or uint8_t in the kernel?
Instead of using lots of "#ifdef __arch__" you should move this into
<asm/trace.h>.
Comments are nice, but IMO your code does a bit too much of it, e.g.:

  /* Everything is OK */
  return 0;

or

  /* Unlock the table for reading */
  read_unlock(&custom_list_lock);

bye, Roman

Re: [ANNOUNCE] Linux Trace Toolkit 0.9.5

[Karim Yaghmour]

Hello Roman,

Roman Zippel wrote:
> > As I said earlier, a 2.5.x patch is available and LTT is ready to
> > be integrated into the 2.5 series.
> 
> I'd really like to see it go in,

Thanks :)

> but I think some small problems are left,
> mostly formatting. Please read Documentation/CodingStyle.
> Please use tabs for indentation and not spaces.

I'm aware of the formatting issues. I will correct the patch to conform with
the kernel's coding style.

> You should consider using more inline functions, instead of lots of "do
> {...} while(0)" macros.

Sure, but what particular advantages do you see in this case?

> Do we really need more usages of uint32_t or uint8_t in the kernel?

I'd really like to avoid using them, but I don't see any other way
since the binary traces have to be readable on multiple machine
architectures and, indeed, multiple OSes. pid_t, for example, isn't
a universally agreed upon type, hence the need for uintXYZ_t.

> Instead of using lots of "#ifdef __arch__" you should move this into
> <asm/trace.h>.

I hadn't thought of it, thanks. Actually, there are only 2 files needing
this, drivers/trace/tracer.h and drivers/trace/tracer.c. The variations
among architectures in tracer.h will be dropped in any case in the near
future. So there's only tracer.c who has a portion of isolated code which
has lots of "#ifdef __arch__". Given that this code is very isolated (i.e.
in only one segment of one function) and there will never be any need for
it to propagate elsewhere in the trace code, is it worth it to create an
<asm/trace.h> which will only have 2 lines for most archs?

There's no problem in doing this, but I just want to make sure I'm not
contributing to creating more files in the kernel source which won't
be of use to anyone else.

> Comments are nice, but IMO your code does a bit too much of it, e.g.:

I understand your point of view, but there is a rational behind this
way of coding. The purpose is to have the code and the explanation in
the same place. So to come back to the snippet you were refering to:

>   /* Everything is OK */
>   return 0;

I often see return's without explanations. The question is then: what
does the value returned mean? This is what this comment is about.

But I don't want to get into a religious fight over code commentary.
The point is, I've been maintaining the kernel and user-space code this
way for a while and it's paid off (i.e. most people reading the code
understand it the first way through). If the comments weren't in sync
with the code I'd agree that comments are dangerous, but this isn't
the case so I don't see that there is any harm in this level of
commenting.

BTW, what about an m68k port? ;)

Cheers,

Karim

===================================================
                 Karim Yaghmour
               karim@opersys.com
      Embedded and Real-Time Linux Expert
===================================================

Re: [ANNOUNCE] Linux Trace Toolkit 0.9.5

[Roman Zippel]

Hi,

Karim Yaghmour wrote:

> > You should consider using more inline functions, instead of lots of "do
> > {...} while(0)" macros.
> 
> Sure, but what particular advantages do you see in this case?

Type safety, it's also easier to read and edit.

> > Do we really need more usages of uint32_t or uint8_t in the kernel?
> 
> I'd really like to avoid using them, but I don't see any other way
> since the binary traces have to be readable on multiple machine
> architectures and, indeed, multiple OSes. pid_t, for example, isn't
> a universally agreed upon type, hence the need for uintXYZ_t.

There is u32 or u8, which is usually used in the kernel. In user space
you can map them to whatever you like.

> > Instead of using lots of "#ifdef __arch__" you should move this into
> > <asm/trace.h>.
> 
> I hadn't thought of it, thanks. Actually, there are only 2 files needing
> this, drivers/trace/tracer.h and drivers/trace/tracer.c. The variations
> among architectures in tracer.h will be dropped in any case in the near
> future. So there's only tracer.c who has a portion of isolated code which
> has lots of "#ifdef __arch__". Given that this code is very isolated (i.e.
> in only one segment of one function) and there will never be any need for
> it to propagate elsewhere in the trace code, is it worth it to create an
> <asm/trace.h> which will only have 2 lines for most archs?

It might be useful for other things, e.g. for using something more
efficient than do_gettimeofday.

> > Comments are nice, but IMO your code does a bit too much of it, e.g.:
> 
> I understand your point of view, but there is a rational behind this
> way of coding. The purpose is to have the code and the explanation in
> the same place. So to come back to the snippet you were refering to:
> 
> >   /* Everything is OK */
> >   return 0;
> 
> I often see return's without explanations. The question is then: what
> does the value returned mean? This is what this comment is about.

You already have this above the function and it describes the return
value at single place and not all over the function.

> But I don't want to get into a religious fight over code commentary.
> The point is, I've been maintaining the kernel and user-space code this
> way for a while and it's paid off (i.e. most people reading the code
> understand it the first way through). If the comments weren't in sync
> with the code I'd agree that comments are dangerous, but this isn't
> the case so I don't see that there is any harm in this level of
> commenting.

IMO most comments are too obvious, I think it would be useful to
describe the more general things (e.g. like locking strategies) at a
single place and comments in functions should concentrate more on the
nonobvious stuff.

> BTW, what about an m68k port? ;)

As soon as I need it. :)

BTW to make it easier for other people to review your patch, I suggest
you include a direct link next time. Splitting the patch in a core part
and tracepoints part also helps reading the patch.

bye, Roman