Linux Trace Toolkit 0.9.5
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
[email protected]
Embedded and Real-Time Linux Expert
===================================================
Hi,
Quoted 2 lines · Karim Yaghmour
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
Hello Roman,
Quoted 4 lines · Roman Zippel
Quoted 2 lines
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 :)
Quoted 3 lines
but I think some small problems are left,
mostly formatting. Please readDocumentation/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.
Quoted 2 lines
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?
Quoted 1 line
Do we really need more usages of
uint32_toruint8_tin 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.
Quoted 2 lines
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.
Quoted 1 line
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:
Quoted 2 lines
/* 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
[email protected]
Embedded and Real-Time Linux Expert
===================================================
Hi,
Quoted 4 lines · Karim Yaghmour
Quoted 2 lines
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.
Quoted 6 lines
Quoted 1 line
Do we really need more usages of
uint32_toruint8_tin 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 foruintXYZ_t.
There is u32 or u8, which is usually used in the kernel. In user space you can map them to whatever you like.
Quoted 11 lines
Quoted 2 lines
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.handdrivers/trace/tracer.c. The variations among architectures intracer.hwill be dropped in any case in the near future. So there's onlytracer.cwho 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.
Quoted 11 lines
Quoted 1 line
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:
Quoted 2 lines
/* 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.
Quoted 7 lines
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.
Quoted 1 line
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