Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-10-26 Thread Frank Ch. Eigler
Hi -

On Thu, Oct 25, 2007 at 03:17:22PM -0400, Mathieu Desnoyers wrote:
> [...]
> Since gcc is required to build the systemtap probes on the development
> marchine, I don't see why it would be much harder to also require prople
> to install drawf ? Or maybe the "crash" tool ?

The crash tool requires the dwarf data to work.  The dwarf data for an
entire kernel (including all the modules) is on the order of hundreds
of megabytes.  The symbol & marker list would be one thousandth the
size.  You can see the deployment attractiveness of the latter.

> I guess you must already need to extract the symbols for your kprobes.
> Do you use kallsyms for this? 

Nope.  /proc/kallsyms is a another run-time-only source of data, and
so is not applicable for off-line (ahead-of-time) mapping.

> I would rather prefer not to implement superfluous built-time data
> extraction in the kernel build system just to make userspace
> simpler. [...]

It is not superfluous, as it would solve a real distribution problem.


- FChE
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-10-26 Thread Frank Ch. Eigler
Hi -

On Thu, Oct 25, 2007 at 03:17:22PM -0400, Mathieu Desnoyers wrote:
 [...]
 Since gcc is required to build the systemtap probes on the development
 marchine, I don't see why it would be much harder to also require prople
 to install drawf ? Or maybe the crash tool ?

The crash tool requires the dwarf data to work.  The dwarf data for an
entire kernel (including all the modules) is on the order of hundreds
of megabytes.  The symbol  marker list would be one thousandth the
size.  You can see the deployment attractiveness of the latter.

 I guess you must already need to extract the symbols for your kprobes.
 Do you use kallsyms for this? 

Nope.  /proc/kallsyms is a another run-time-only source of data, and
so is not applicable for off-line (ahead-of-time) mapping.

 I would rather prefer not to implement superfluous built-time data
 extraction in the kernel build system just to make userspace
 simpler. [...]

It is not superfluous, as it would solve a real distribution problem.


- FChE
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-10-25 Thread Mathieu Desnoyers
* Roland McGrath ([EMAIL PROTECTED]) wrote:
> > I think the main issue with the solution you propose is that it doesn't
> > deal with markers in modules, am I right ?
> 
> My suggestion applies as well to modules as anything else.  
> What "like Module.symvers" means is something like:
> 
> name1 vmlinux %s
> name2 fs/nfs/nfs  %d
> 
> All the modules built by the same kernel build go into this one file.
> 
> Modules packaged separately for the same kernel could provide additional
> files of the same kind.
> 
> > I will soon come with a marker iterator and a module that provides a
> > userspace -and in kernel- interface to enable/disable markers. Actually,
> > I already have the code ready in my LTTng snapshots. I can provide a
> > link if you want to have a look.
> 
> That's clearly straightforward to do given the basic markers data structures.
> 
> It does not address the need for an offline list of markers available in a
> particular kernel build or set of modules that you are not running right now.
> The approach now available for that is grovelling through the markers data
> structures extracted from vmlinux and .ko ELF files offline.  That is more
> work than one should have to do, and has lots of problems with coping with
> different packaging details, etc.
> 

Since gcc is required to build the systemtap probes on the development
marchine, I don't see why it would be much harder to also require prople
to install drawf ? Or maybe the "crash" tool ?

I guess you must already need to extract the symbols for your kprobes.
Do you use kallsyms for this ? The way I see it, you could maybe extract
kallsyms symbols corresponding to the markers data structures quite
easily.

I would rather prefer not to implement superfluous built-time data
extraction in the kernel build system just to make userspace simpler. If
we can leverage what currently exists, that would be better.

Mathieu

> 
> Thanks,
> Roland

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-10-25 Thread Mathieu Desnoyers
* Roland McGrath ([EMAIL PROTECTED]) wrote:
  I think the main issue with the solution you propose is that it doesn't
  deal with markers in modules, am I right ?
 
 My suggestion applies as well to modules as anything else.  
 What like Module.symvers means is something like:
 
 name1 vmlinux %s
 name2 fs/nfs/nfs  %d
 
 All the modules built by the same kernel build go into this one file.
 
 Modules packaged separately for the same kernel could provide additional
 files of the same kind.
 
  I will soon come with a marker iterator and a module that provides a
  userspace -and in kernel- interface to enable/disable markers. Actually,
  I already have the code ready in my LTTng snapshots. I can provide a
  link if you want to have a look.
 
 That's clearly straightforward to do given the basic markers data structures.
 
 It does not address the need for an offline list of markers available in a
 particular kernel build or set of modules that you are not running right now.
 The approach now available for that is grovelling through the markers data
 structures extracted from vmlinux and .ko ELF files offline.  That is more
 work than one should have to do, and has lots of problems with coping with
 different packaging details, etc.
 

Since gcc is required to build the systemtap probes on the development
marchine, I don't see why it would be much harder to also require prople
to install drawf ? Or maybe the crash tool ?

I guess you must already need to extract the symbols for your kprobes.
Do you use kallsyms for this ? The way I see it, you could maybe extract
kallsyms symbols corresponding to the markers data structures quite
easily.

I would rather prefer not to implement superfluous built-time data
extraction in the kernel build system just to make userspace simpler. If
we can leverage what currently exists, that would be better.

Mathieu

 
 Thanks,
 Roland

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-10-15 Thread Roland McGrath
> I think the main issue with the solution you propose is that it doesn't
> deal with markers in modules, am I right ?

My suggestion applies as well to modules as anything else.  
What "like Module.symvers" means is something like:

name1   vmlinux %s
name2   fs/nfs/nfs  %d

All the modules built by the same kernel build go into this one file.

Modules packaged separately for the same kernel could provide additional
files of the same kind.

> I will soon come with a marker iterator and a module that provides a
> userspace -and in kernel- interface to enable/disable markers. Actually,
> I already have the code ready in my LTTng snapshots. I can provide a
> link if you want to have a look.

That's clearly straightforward to do given the basic markers data structures.

It does not address the need for an offline list of markers available in a
particular kernel build or set of modules that you are not running right now.
The approach now available for that is grovelling through the markers data
structures extracted from vmlinux and .ko ELF files offline.  That is more
work than one should have to do, and has lots of problems with coping with
different packaging details, etc.


Thanks,
Roland
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-10-15 Thread Mathieu Desnoyers
* Frank Ch. Eigler ([EMAIL PROTECTED]) wrote:
> Hi -
> 
> I wrote:
> 
> > [...]
> > > The marker metadata must be stored in at least one place in the kernel
> > > image - this just happens to be a convenient one that David Smith's
> > > recent systemtap code used.  Without it, we'd probably have to do a
> > > more complicated search, following the pointers within the __markers
> > > structs.  [...]
> 
> Our team is farther along adapting to this change against 2.6.23-mm1,
> and we have run into a complication.  It's more of a distribution
> issue.
> 
> We would prefer to retain systemtap's capability to build
> instrumentation for a kernel other than the currently running one.
> Such instrumentation can be then copied and run on a distinct machine.
> This has meant relying on development data: make install_headers +
> Makefiles (as packaged by Fedora/RHEL), and to a lesser extent
> separated debugging information.
> 
> Markers are attractive partly because they don't require debugging
> information, so the data needs to be found in an executable image.
> But we prefer not to force the executable image itself to be
> installed, for example because /boot is relatively small.  So we would
> prefer something in between: something small that we can put into the
> development package.
> 
> If there exists sympathy to this problem, Roland McGrath supposes we
> could implement a standardized solution, a file like Module.symvers,
> containing the marker names & format strings extracted at build time.
> Any opinions?
> 

Hi Frank,

I think the main issue with the solution you propose is that it doesn't
deal with markers in modules, am I right ?

I will soon come with a marker iterator and a module that provides a
userspace -and in kernel- interface to enable/disable markers. Actually,
I already have the code ready in my LTTng snapshots. I can provide a
link if you want to have a look.

> 
> PS. I wonder why the marker name/format strings are put into a
> __markers_strings object section at all, considering that the only
> place where that is used again appears to be this code in
> kernel/module.c:
> 
> markersstringsindex = find_sec(hdr, sechdrs, secstrings,
> "__markers_strings");
> 
> and the "markersstringsindex" variable is never used.
> 

Considering that  I want to minimize the impact on the system, I put the
marker strings in their own memory location rather than clobbering the
memory containing the kernel strings (which will likely be used more
often than markers). It makes sure that I don't pollute cachelines
otherwise containing useful kernel strings.

Mathieu

> 
> - FChE

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-10-15 Thread Frank Ch. Eigler
Hi -

I wrote:

> [...]
> > The marker metadata must be stored in at least one place in the kernel
> > image - this just happens to be a convenient one that David Smith's
> > recent systemtap code used.  Without it, we'd probably have to do a
> > more complicated search, following the pointers within the __markers
> > structs.  [...]

Our team is farther along adapting to this change against 2.6.23-mm1,
and we have run into a complication.  It's more of a distribution
issue.

We would prefer to retain systemtap's capability to build
instrumentation for a kernel other than the currently running one.
Such instrumentation can be then copied and run on a distinct machine.
This has meant relying on development data: make install_headers +
Makefiles (as packaged by Fedora/RHEL), and to a lesser extent
separated debugging information.

Markers are attractive partly because they don't require debugging
information, so the data needs to be found in an executable image.
But we prefer not to force the executable image itself to be
installed, for example because /boot is relatively small.  So we would
prefer something in between: something small that we can put into the
development package.

If there exists sympathy to this problem, Roland McGrath supposes we
could implement a standardized solution, a file like Module.symvers,
containing the marker names & format strings extracted at build time.
Any opinions?


PS. I wonder why the marker name/format strings are put into a
__markers_strings object section at all, considering that the only
place where that is used again appears to be this code in
kernel/module.c:

markersstringsindex = find_sec(hdr, sechdrs, secstrings,
"__markers_strings");

and the "markersstringsindex" variable is never used.


- FChE
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-10-15 Thread Frank Ch. Eigler
Hi -

I wrote:

 [...]
  The marker metadata must be stored in at least one place in the kernel
  image - this just happens to be a convenient one that David Smith's
  recent systemtap code used.  Without it, we'd probably have to do a
  more complicated search, following the pointers within the __markers
  structs.  [...]

Our team is farther along adapting to this change against 2.6.23-mm1,
and we have run into a complication.  It's more of a distribution
issue.

We would prefer to retain systemtap's capability to build
instrumentation for a kernel other than the currently running one.
Such instrumentation can be then copied and run on a distinct machine.
This has meant relying on development data: make install_headers +
Makefiles (as packaged by Fedora/RHEL), and to a lesser extent
separated debugging information.

Markers are attractive partly because they don't require debugging
information, so the data needs to be found in an executable image.
But we prefer not to force the executable image itself to be
installed, for example because /boot is relatively small.  So we would
prefer something in between: something small that we can put into the
development package.

If there exists sympathy to this problem, Roland McGrath supposes we
could implement a standardized solution, a file like Module.symvers,
containing the marker names  format strings extracted at build time.
Any opinions?


PS. I wonder why the marker name/format strings are put into a
__markers_strings object section at all, considering that the only
place where that is used again appears to be this code in
kernel/module.c:

markersstringsindex = find_sec(hdr, sechdrs, secstrings,
__markers_strings);

and the markersstringsindex variable is never used.


- FChE
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-10-15 Thread Mathieu Desnoyers
* Frank Ch. Eigler ([EMAIL PROTECTED]) wrote:
 Hi -
 
 I wrote:
 
  [...]
   The marker metadata must be stored in at least one place in the kernel
   image - this just happens to be a convenient one that David Smith's
   recent systemtap code used.  Without it, we'd probably have to do a
   more complicated search, following the pointers within the __markers
   structs.  [...]
 
 Our team is farther along adapting to this change against 2.6.23-mm1,
 and we have run into a complication.  It's more of a distribution
 issue.
 
 We would prefer to retain systemtap's capability to build
 instrumentation for a kernel other than the currently running one.
 Such instrumentation can be then copied and run on a distinct machine.
 This has meant relying on development data: make install_headers +
 Makefiles (as packaged by Fedora/RHEL), and to a lesser extent
 separated debugging information.
 
 Markers are attractive partly because they don't require debugging
 information, so the data needs to be found in an executable image.
 But we prefer not to force the executable image itself to be
 installed, for example because /boot is relatively small.  So we would
 prefer something in between: something small that we can put into the
 development package.
 
 If there exists sympathy to this problem, Roland McGrath supposes we
 could implement a standardized solution, a file like Module.symvers,
 containing the marker names  format strings extracted at build time.
 Any opinions?
 

Hi Frank,

I think the main issue with the solution you propose is that it doesn't
deal with markers in modules, am I right ?

I will soon come with a marker iterator and a module that provides a
userspace -and in kernel- interface to enable/disable markers. Actually,
I already have the code ready in my LTTng snapshots. I can provide a
link if you want to have a look.

 
 PS. I wonder why the marker name/format strings are put into a
 __markers_strings object section at all, considering that the only
 place where that is used again appears to be this code in
 kernel/module.c:
 
 markersstringsindex = find_sec(hdr, sechdrs, secstrings,
 __markers_strings);
 
 and the markersstringsindex variable is never used.
 

Considering that  I want to minimize the impact on the system, I put the
marker strings in their own memory location rather than clobbering the
memory containing the kernel strings (which will likely be used more
often than markers). It makes sure that I don't pollute cachelines
otherwise containing useful kernel strings.

Mathieu

 
 - FChE

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-10-15 Thread Roland McGrath
 I think the main issue with the solution you propose is that it doesn't
 deal with markers in modules, am I right ?

My suggestion applies as well to modules as anything else.  
What like Module.symvers means is something like:

name1   vmlinux %s
name2   fs/nfs/nfs  %d

All the modules built by the same kernel build go into this one file.

Modules packaged separately for the same kernel could provide additional
files of the same kind.

 I will soon come with a marker iterator and a module that provides a
 userspace -and in kernel- interface to enable/disable markers. Actually,
 I already have the code ready in my LTTng snapshots. I can provide a
 link if you want to have a look.

That's clearly straightforward to do given the basic markers data structures.

It does not address the need for an offline list of markers available in a
particular kernel build or set of modules that you are not running right now.
The approach now available for that is grovelling through the markers data
structures extracted from vmlinux and .ko ELF files offline.  That is more
work than one should have to do, and has lots of problems with coping with
different packaging details, etc.


Thanks,
Roland
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-21 Thread Mathieu Desnoyers
* Steven Rostedt ([EMAIL PROTECTED]) wrote:
> On Tue, Sep 18, 2007 at 05:13:25PM -0400, Mathieu Desnoyers wrote:
> > +/*
> > + * Sets the probe callback corresponding to one marker.
> > + */
> > +static int set_marker(struct marker_entry **entry,
> > +   struct __mark_marker *elem)
> > +{
> > +   int ret;
> > +   BUG_ON(strcmp((*entry)->name, elem->name) != 0);
> 
> Can you switch this at least to WARN_ON?  Killing a system with X
> running where the user just sees a freeze is not that nice. But a nasty
> message in dmesg is very noticable.
> 
Sure.

> -- Steve
> 
> > +
> > +   if ((*entry)->format) {
> > +   if (strcmp((*entry)->format, elem->format) != 0) {
> > +   printk(KERN_NOTICE
> > +   "Format mismatch for probe %s "
> > +   "(%s), marker (%s)\n",
> > +   (*entry)->name,
> > +   (*entry)->format,
> > +   elem->format);
> > +   return -EPERM;
> > +   }
> > +   } else {
> > +   ret = marker_set_format(entry, elem->format);
> > +   if (ret)
> > +   return ret;
> > +   }
> > +   elem->call = (*entry)->probe;
> > +   elem->pdata = (*entry)->pdata;
> > +   _immediate_set(elem->state, 1);
> > +   return 0;
> > +}

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-21 Thread Mathieu Desnoyers
* Frank Ch. Eigler ([EMAIL PROTECTED]) wrote:
> Hi -
> 
> On Fri, Sep 21, 2007 at 08:58:19AM -0400, Mathieu Desnoyers wrote:
> > [...]
> > > > Current systemtap marker support code relies on the __markers_strings
> > > > section.
> > > Let users know that in comment above section definition in ld script.
> > [...]
> > /* Markers: strings (used by SystemTAP) */  \
> > [...]
> 
> I did not mean to imply that this was a necessary state of affairs.
> 
> The marker metadata must be stored in at least one place in the kernel
> image - this just happens to be a convenient one that David Smith's
> recent systemtap code used.  Without it, we'd probably have to do a
> more complicated search, following the pointers within the __markers
> structs.  That could work, but it hasn't been built/tested.
> 
> So, this proposed change (removal of this section) would break
> systemtap, and we have to jump through more hoops to make it work
> again.  Is the change worth it?
> 

I guess so. Getting the markers as clean as we can is very important for
kernel inclusion.

Mathieu

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-21 Thread Frank Ch. Eigler
Hi -

On Fri, Sep 21, 2007 at 08:58:19AM -0400, Mathieu Desnoyers wrote:
> [...]
> > > Current systemtap marker support code relies on the __markers_strings
> > > section.
> > Let users know that in comment above section definition in ld script.
> [...]
> /* Markers: strings (used by SystemTAP) */  \
> [...]

I did not mean to imply that this was a necessary state of affairs.

The marker metadata must be stored in at least one place in the kernel
image - this just happens to be a convenient one that David Smith's
recent systemtap code used.  Without it, we'd probably have to do a
more complicated search, following the pointers within the __markers
structs.  That could work, but it hasn't been built/tested.

So, this proposed change (removal of this section) would break
systemtap, and we have to jump through more hoops to make it work
again.  Is the change worth it?

- FChE
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-21 Thread Christoph Hellwig
On Fri, Sep 21, 2007 at 08:58:19AM -0400, Mathieu Desnoyers wrote:
> Ok, I am changing it to:

As I mentioned before pleae just kill this gunk entirely as it's not needed
at all intree.  markers are already getting far too complex, I'd rather
want a simple useable version in now than trying to cater for every possible
use-case.  Once the systemtap people are ready for merging their stuff into
the kernel tree we can cater towards their needs.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-21 Thread Mathieu Desnoyers
* Denys Vlasenko ([EMAIL PROTECTED]) wrote:
> On Wednesday 19 September 2007 14:53, Frank Ch. Eigler wrote:
> > Mathieu Desnoyers <[EMAIL PROTECTED]> writes:
> > 
> > > [...]  Do you think I should also remove the __markers_strings
> > > section from here ?
> > 
> > Current systemtap marker support code relies on the __markers_strings
> > section.
> 
> Let users know that in comment above section definition in ld script.
> 

Ok, I am changing it to:


/* Markers: strings (used by SystemTAP) */  \
__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) { \
*(__markers_strings)\
}   \

Mathieu

> Thanks!
> --
> vda

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-21 Thread Mathieu Desnoyers
* Denys Vlasenko ([EMAIL PROTECTED]) wrote:
 On Wednesday 19 September 2007 14:53, Frank Ch. Eigler wrote:
  Mathieu Desnoyers [EMAIL PROTECTED] writes:
  
   [...]  Do you think I should also remove the __markers_strings
   section from here ?
  
  Current systemtap marker support code relies on the __markers_strings
  section.
 
 Let users know that in comment above section definition in ld script.
 

Ok, I am changing it to:


/* Markers: strings (used by SystemTAP) */  \
__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) { \
*(__markers_strings)\
}   \

Mathieu

 Thanks!
 --
 vda

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-21 Thread Christoph Hellwig
On Fri, Sep 21, 2007 at 08:58:19AM -0400, Mathieu Desnoyers wrote:
 Ok, I am changing it to:

As I mentioned before pleae just kill this gunk entirely as it's not needed
at all intree.  markers are already getting far too complex, I'd rather
want a simple useable version in now than trying to cater for every possible
use-case.  Once the systemtap people are ready for merging their stuff into
the kernel tree we can cater towards their needs.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-21 Thread Frank Ch. Eigler
Hi -

On Fri, Sep 21, 2007 at 08:58:19AM -0400, Mathieu Desnoyers wrote:
 [...]
   Current systemtap marker support code relies on the __markers_strings
   section.
  Let users know that in comment above section definition in ld script.
 [...]
 /* Markers: strings (used by SystemTAP) */  \
 [...]

I did not mean to imply that this was a necessary state of affairs.

The marker metadata must be stored in at least one place in the kernel
image - this just happens to be a convenient one that David Smith's
recent systemtap code used.  Without it, we'd probably have to do a
more complicated search, following the pointers within the __markers
structs.  That could work, but it hasn't been built/tested.

So, this proposed change (removal of this section) would break
systemtap, and we have to jump through more hoops to make it work
again.  Is the change worth it?

- FChE
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-21 Thread Mathieu Desnoyers
* Frank Ch. Eigler ([EMAIL PROTECTED]) wrote:
 Hi -
 
 On Fri, Sep 21, 2007 at 08:58:19AM -0400, Mathieu Desnoyers wrote:
  [...]
Current systemtap marker support code relies on the __markers_strings
section.
   Let users know that in comment above section definition in ld script.
  [...]
  /* Markers: strings (used by SystemTAP) */  \
  [...]
 
 I did not mean to imply that this was a necessary state of affairs.
 
 The marker metadata must be stored in at least one place in the kernel
 image - this just happens to be a convenient one that David Smith's
 recent systemtap code used.  Without it, we'd probably have to do a
 more complicated search, following the pointers within the __markers
 structs.  That could work, but it hasn't been built/tested.
 
 So, this proposed change (removal of this section) would break
 systemtap, and we have to jump through more hoops to make it work
 again.  Is the change worth it?
 

I guess so. Getting the markers as clean as we can is very important for
kernel inclusion.

Mathieu

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-21 Thread Mathieu Desnoyers
* Steven Rostedt ([EMAIL PROTECTED]) wrote:
 On Tue, Sep 18, 2007 at 05:13:25PM -0400, Mathieu Desnoyers wrote:
  +/*
  + * Sets the probe callback corresponding to one marker.
  + */
  +static int set_marker(struct marker_entry **entry,
  +   struct __mark_marker *elem)
  +{
  +   int ret;
  +   BUG_ON(strcmp((*entry)-name, elem-name) != 0);
 
 Can you switch this at least to WARN_ON?  Killing a system with X
 running where the user just sees a freeze is not that nice. But a nasty
 message in dmesg is very noticable.
 
Sure.

 -- Steve
 
  +
  +   if ((*entry)-format) {
  +   if (strcmp((*entry)-format, elem-format) != 0) {
  +   printk(KERN_NOTICE
  +   Format mismatch for probe %s 
  +   (%s), marker (%s)\n,
  +   (*entry)-name,
  +   (*entry)-format,
  +   elem-format);
  +   return -EPERM;
  +   }
  +   } else {
  +   ret = marker_set_format(entry, elem-format);
  +   if (ret)
  +   return ret;
  +   }
  +   elem-call = (*entry)-probe;
  +   elem-pdata = (*entry)-pdata;
  +   _immediate_set(elem-state, 1);
  +   return 0;
  +}

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-20 Thread Steven Rostedt
On Tue, Sep 18, 2007 at 05:13:25PM -0400, Mathieu Desnoyers wrote:
> +/*
> + * Sets the probe callback corresponding to one marker.
> + */
> +static int set_marker(struct marker_entry **entry,
> + struct __mark_marker *elem)
> +{
> + int ret;
> + BUG_ON(strcmp((*entry)->name, elem->name) != 0);

Can you switch this at least to WARN_ON?  Killing a system with X
running where the user just sees a freeze is not that nice. But a nasty
message in dmesg is very noticable.

-- Steve

> +
> + if ((*entry)->format) {
> + if (strcmp((*entry)->format, elem->format) != 0) {
> + printk(KERN_NOTICE
> + "Format mismatch for probe %s "
> + "(%s), marker (%s)\n",
> + (*entry)->name,
> + (*entry)->format,
> + elem->format);
> + return -EPERM;
> + }
> + } else {
> + ret = marker_set_format(entry, elem->format);
> + if (ret)
> + return ret;
> + }
> + elem->call = (*entry)->probe;
> + elem->pdata = (*entry)->pdata;
> + _immediate_set(elem->state, 1);
> + return 0;
> +}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-20 Thread Steven Rostedt
On Tue, Sep 18, 2007 at 05:13:25PM -0400, Mathieu Desnoyers wrote:
 +/*
 + * Sets the probe callback corresponding to one marker.
 + */
 +static int set_marker(struct marker_entry **entry,
 + struct __mark_marker *elem)
 +{
 + int ret;
 + BUG_ON(strcmp((*entry)-name, elem-name) != 0);

Can you switch this at least to WARN_ON?  Killing a system with X
running where the user just sees a freeze is not that nice. But a nasty
message in dmesg is very noticable.

-- Steve

 +
 + if ((*entry)-format) {
 + if (strcmp((*entry)-format, elem-format) != 0) {
 + printk(KERN_NOTICE
 + Format mismatch for probe %s 
 + (%s), marker (%s)\n,
 + (*entry)-name,
 + (*entry)-format,
 + elem-format);
 + return -EPERM;
 + }
 + } else {
 + ret = marker_set_format(entry, elem-format);
 + if (ret)
 + return ret;
 + }
 + elem-call = (*entry)-probe;
 + elem-pdata = (*entry)-pdata;
 + _immediate_set(elem-state, 1);
 + return 0;
 +}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-19 Thread Denys Vlasenko
On Wednesday 19 September 2007 14:53, Frank Ch. Eigler wrote:
> Mathieu Desnoyers <[EMAIL PROTECTED]> writes:
> 
> > [...]  Do you think I should also remove the __markers_strings
> > section from here ?
> 
> Current systemtap marker support code relies on the __markers_strings
> section.

Let users know that in comment above section definition in ld script.

Thanks!
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-19 Thread Frank Ch. Eigler
Mathieu Desnoyers <[EMAIL PROTECTED]> writes:

> [...]  Do you think I should also remove the __markers_strings
> section from here ?

Current systemtap marker support code relies on the __markers_strings
section.

- FChE
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-19 Thread Mathieu Desnoyers
* Mathieu Desnoyers ([EMAIL PROTECTED]) wrote:
> * Denys Vlasenko ([EMAIL PROTECTED]) wrote:
> > On Wednesday 19 September 2007 12:37, Mathieu Desnoyers wrote:
> > > > Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
> > > > ===
> > > > --- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  
> > > > 2007-09-14 10:11:18.0 -0400
> > > > +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-09-14 
> > > > 10:11:31.0 -0400
> > > > @@ -129,6 +133,11 @@
> > > > VMLINUX_SYMBOL(__stop___immediate) = .; 
> > > > \
> > > > }   
> > > > \
> > > > 
> > > > \
> > > > +   /* Markers: strings */  
> > > > \
> > > > +__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) 
> > > > {\
> > > > +   *(__markers_strings)
> > > > \
> > > > +   }   
> > > > \
> > > > +   
> > > > \
> > > > /* Kernel symbol table: strings */  
> > > > \
> > > >  __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) 
> > > > {\
> > > > *(__ksymtab_strings)
> > > > \
> > > [...]
> > > 
> > > Do you think I should also remove the __markers_strings section from here 
> > > ?
> > 
> > Yes.
> > 
> > It will be beneficial if one can read include/asm-generic/vmlinux.lds.h and
> > arch/$ARCH/kernel/vmlinux.lds.S and understand which sections in resulting
> > vmlinux serve what purpose. A comment atop each section explaining
> > its role will be nice. Even more so that not many people are fluent
> > in ld script language.
> > 
> > Currently, one will need to grep around (and not only in kernel tree -
> > you need to read depmod.c source too) in order to understand the role
> > of various sections in vmlinux.
> > 
> > There are dearth of comments in ld scripts, and some sections
> > are created "just because I felt like it". For example, there are
> > ".data.page_aligned" and ".data.percpu" sections - can you
> > easily tell which one has to be a section, and which does not need
> > to be one (can be merged with ".data")? Maybe both must be sections?
> > Or none of them?
> > --
> > vda
> 
> Oh, wait.. I need it in module.c:
> 
> immediateindex = find_sec(hdr, sechdrs, secstrings, "__immediate");
> 
> I'll leave the section there then.
> 
> Mathieu

Sorry, let me take this back. It applies to what is linked in the core
image, but I believe it does not apply to the kernel modules.

Mathieu

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-19 Thread Mathieu Desnoyers
* Denys Vlasenko ([EMAIL PROTECTED]) wrote:
> On Wednesday 19 September 2007 12:37, Mathieu Desnoyers wrote:
> > > Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
> > > ===
> > > --- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h
> > > 2007-09-14 10:11:18.0 -0400
> > > +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h 2007-09-14 
> > > 10:11:31.0 -0400
> > > @@ -129,6 +133,11 @@
> > >   VMLINUX_SYMBOL(__stop___immediate) = .; \
> > >   }   \
> > >   \
> > > + /* Markers: strings */  \
> > > +__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {  
> > > \
> > > + *(__markers_strings)\
> > > + }   
> > > \
> > > + \
> > >   /* Kernel symbol table: strings */  \
> > >  __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {  
> > > \
> > >   *(__ksymtab_strings)\
> > [...]
> > 
> > Do you think I should also remove the __markers_strings section from here ?
> 
> Yes.
> 
> It will be beneficial if one can read include/asm-generic/vmlinux.lds.h and
> arch/$ARCH/kernel/vmlinux.lds.S and understand which sections in resulting
> vmlinux serve what purpose. A comment atop each section explaining
> its role will be nice. Even more so that not many people are fluent
> in ld script language.
> 
> Currently, one will need to grep around (and not only in kernel tree -
> you need to read depmod.c source too) in order to understand the role
> of various sections in vmlinux.
> 
> There are dearth of comments in ld scripts, and some sections
> are created "just because I felt like it". For example, there are
> ".data.page_aligned" and ".data.percpu" sections - can you
> easily tell which one has to be a section, and which does not need
> to be one (can be merged with ".data")? Maybe both must be sections?
> Or none of them?
> --
> vda

Oh, wait.. I need it in module.c:

immediateindex = find_sec(hdr, sechdrs, secstrings, "__immediate");

I'll leave the section there then.

Mathieu
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-19 Thread Denys Vlasenko
On Wednesday 19 September 2007 12:37, Mathieu Desnoyers wrote:
> > Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
> > ===
> > --- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-09-14 
> > 10:11:18.0 -0400
> > +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-09-14 
> > 10:11:31.0 -0400
> > @@ -129,6 +133,11 @@
> > VMLINUX_SYMBOL(__stop___immediate) = .; \
> > }   \
> > \
> > +   /* Markers: strings */  \
> > +__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
> > \
> > +   *(__markers_strings)\
> > +   }   \
> > +   \
> > /* Kernel symbol table: strings */  \
> >  __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
> > \
> > *(__ksymtab_strings)\
> [...]
> 
> Do you think I should also remove the __markers_strings section from here ?

Yes.

It will be beneficial if one can read include/asm-generic/vmlinux.lds.h and
arch/$ARCH/kernel/vmlinux.lds.S and understand which sections in resulting
vmlinux serve what purpose. A comment atop each section explaining
its role will be nice. Even more so that not many people are fluent
in ld script language.

Currently, one will need to grep around (and not only in kernel tree -
you need to read depmod.c source too) in order to understand the role
of various sections in vmlinux.

There are dearth of comments in ld scripts, and some sections
are created "just because I felt like it". For example, there are
".data.page_aligned" and ".data.percpu" sections - can you
easily tell which one has to be a section, and which does not need
to be one (can be merged with ".data")? Maybe both must be sections?
Or none of them?
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-19 Thread Mathieu Desnoyers
* Mathieu Desnoyers ([EMAIL PROTECTED]) wrote:
> The marker activation functions sits in kernel/marker.c. A hash table is used
> to keep track of the registered probes and armed markers, so the markers 
> within
> a newly loaded module that should be active can be activated at module load
> time.
> 
> marker_query has been removed. marker_get_first, marker_get_next and
> marker_release should be used as iterators on the markers.
> 
> Changelog:
> - markers_mutex now nests inside module_mutex rather than the opposite.
> - Iteration on modules is now done in module.c.
> - module_mutex is not exported anymore.
> 
 [...]
> Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
> ===
> --- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h2007-09-14 
> 10:11:18.0 -0400
> +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h 2007-09-14 
> 10:11:31.0 -0400
> @@ -12,7 +12,11 @@
>  /* .data section */
>  #define DATA_DATA\
>   *(.data)\
> - *(.data.init.refok)
> + *(.data.init.refok) \
> + . = ALIGN(8);   \
> + VMLINUX_SYMBOL(__start___markers) = .;  \
> + *(__markers)\
> + VMLINUX_SYMBOL(__stop___markers) = .;
>  
>  #define RO_DATA(align)   
> \
>   . = ALIGN((align)); \
> @@ -129,6 +133,11 @@
>   VMLINUX_SYMBOL(__stop___immediate) = .; \
>   }   \
>   \
> + /* Markers: strings */  \
> +__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {  
> \
> + *(__markers_strings)\
> + }   \
> + \
>   /* Kernel symbol table: strings */  \
>  __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {  
> \
>   *(__ksymtab_strings)\
[...]

Do you think I should also remove the __markers_strings section from here ?

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-19 Thread Mathieu Desnoyers
* Mathieu Desnoyers ([EMAIL PROTECTED]) wrote:
 The marker activation functions sits in kernel/marker.c. A hash table is used
 to keep track of the registered probes and armed markers, so the markers 
 within
 a newly loaded module that should be active can be activated at module load
 time.
 
 marker_query has been removed. marker_get_first, marker_get_next and
 marker_release should be used as iterators on the markers.
 
 Changelog:
 - markers_mutex now nests inside module_mutex rather than the opposite.
 - Iteration on modules is now done in module.c.
 - module_mutex is not exported anymore.
 
 [...]
 Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
 ===
 --- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h2007-09-14 
 10:11:18.0 -0400
 +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h 2007-09-14 
 10:11:31.0 -0400
 @@ -12,7 +12,11 @@
  /* .data section */
  #define DATA_DATA\
   *(.data)\
 - *(.data.init.refok)
 + *(.data.init.refok) \
 + . = ALIGN(8);   \
 + VMLINUX_SYMBOL(__start___markers) = .;  \
 + *(__markers)\
 + VMLINUX_SYMBOL(__stop___markers) = .;
  
  #define RO_DATA(align)   
 \
   . = ALIGN((align)); \
 @@ -129,6 +133,11 @@
   VMLINUX_SYMBOL(__stop___immediate) = .; \
   }   \
   \
 + /* Markers: strings */  \
 +__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {  
 \
 + *(__markers_strings)\
 + }   \
 + \
   /* Kernel symbol table: strings */  \
  __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {  
 \
   *(__ksymtab_strings)\
[...]

Do you think I should also remove the __markers_strings section from here ?

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-19 Thread Denys Vlasenko
On Wednesday 19 September 2007 12:37, Mathieu Desnoyers wrote:
  Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
  ===
  --- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-09-14 
  10:11:18.0 -0400
  +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-09-14 
  10:11:31.0 -0400
  @@ -129,6 +133,11 @@
  VMLINUX_SYMBOL(__stop___immediate) = .; \
  }   \
  \
  +   /* Markers: strings */  \
  +__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
  \
  +   *(__markers_strings)\
  +   }   \
  +   \
  /* Kernel symbol table: strings */  \
   __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
  \
  *(__ksymtab_strings)\
 [...]
 
 Do you think I should also remove the __markers_strings section from here ?

Yes.

It will be beneficial if one can read include/asm-generic/vmlinux.lds.h and
arch/$ARCH/kernel/vmlinux.lds.S and understand which sections in resulting
vmlinux serve what purpose. A comment atop each section explaining
its role will be nice. Even more so that not many people are fluent
in ld script language.

Currently, one will need to grep around (and not only in kernel tree -
you need to read depmod.c source too) in order to understand the role
of various sections in vmlinux.

There are dearth of comments in ld scripts, and some sections
are created just because I felt like it. For example, there are
.data.page_aligned and .data.percpu sections - can you
easily tell which one has to be a section, and which does not need
to be one (can be merged with .data)? Maybe both must be sections?
Or none of them?
--
vda
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-19 Thread Mathieu Desnoyers
* Denys Vlasenko ([EMAIL PROTECTED]) wrote:
 On Wednesday 19 September 2007 12:37, Mathieu Desnoyers wrote:
   Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
   ===
   --- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h
   2007-09-14 10:11:18.0 -0400
   +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h 2007-09-14 
   10:11:31.0 -0400
   @@ -129,6 +133,11 @@
 VMLINUX_SYMBOL(__stop___immediate) = .; \
 }   \
 \
   + /* Markers: strings */  \
   +__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {  
   \
   + *(__markers_strings)\
   + }   
   \
   + \
 /* Kernel symbol table: strings */  \
__ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {  
   \
 *(__ksymtab_strings)\
  [...]
  
  Do you think I should also remove the __markers_strings section from here ?
 
 Yes.
 
 It will be beneficial if one can read include/asm-generic/vmlinux.lds.h and
 arch/$ARCH/kernel/vmlinux.lds.S and understand which sections in resulting
 vmlinux serve what purpose. A comment atop each section explaining
 its role will be nice. Even more so that not many people are fluent
 in ld script language.
 
 Currently, one will need to grep around (and not only in kernel tree -
 you need to read depmod.c source too) in order to understand the role
 of various sections in vmlinux.
 
 There are dearth of comments in ld scripts, and some sections
 are created just because I felt like it. For example, there are
 .data.page_aligned and .data.percpu sections - can you
 easily tell which one has to be a section, and which does not need
 to be one (can be merged with .data)? Maybe both must be sections?
 Or none of them?
 --
 vda

Oh, wait.. I need it in module.c:

immediateindex = find_sec(hdr, sechdrs, secstrings, __immediate);

I'll leave the section there then.

Mathieu
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-19 Thread Mathieu Desnoyers
* Mathieu Desnoyers ([EMAIL PROTECTED]) wrote:
 * Denys Vlasenko ([EMAIL PROTECTED]) wrote:
  On Wednesday 19 September 2007 12:37, Mathieu Desnoyers wrote:
Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  
2007-09-14 10:11:18.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-09-14 
10:11:31.0 -0400
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; 
\
}   
\

\
+   /* Markers: strings */  
\
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) 
{\
+   *(__markers_strings)
\
+   }   
\
+   
\
/* Kernel symbol table: strings */  
\
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) 
{\
*(__ksymtab_strings)
\
   [...]
   
   Do you think I should also remove the __markers_strings section from here 
   ?
  
  Yes.
  
  It will be beneficial if one can read include/asm-generic/vmlinux.lds.h and
  arch/$ARCH/kernel/vmlinux.lds.S and understand which sections in resulting
  vmlinux serve what purpose. A comment atop each section explaining
  its role will be nice. Even more so that not many people are fluent
  in ld script language.
  
  Currently, one will need to grep around (and not only in kernel tree -
  you need to read depmod.c source too) in order to understand the role
  of various sections in vmlinux.
  
  There are dearth of comments in ld scripts, and some sections
  are created just because I felt like it. For example, there are
  .data.page_aligned and .data.percpu sections - can you
  easily tell which one has to be a section, and which does not need
  to be one (can be merged with .data)? Maybe both must be sections?
  Or none of them?
  --
  vda
 
 Oh, wait.. I need it in module.c:
 
 immediateindex = find_sec(hdr, sechdrs, secstrings, __immediate);
 
 I'll leave the section there then.
 
 Mathieu

Sorry, let me take this back. It applies to what is linked in the core
image, but I believe it does not apply to the kernel modules.

Mathieu

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-19 Thread Frank Ch. Eigler
Mathieu Desnoyers [EMAIL PROTECTED] writes:

 [...]  Do you think I should also remove the __markers_strings
 section from here ?

Current systemtap marker support code relies on the __markers_strings
section.

- FChE
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-19 Thread Denys Vlasenko
On Wednesday 19 September 2007 14:53, Frank Ch. Eigler wrote:
 Mathieu Desnoyers [EMAIL PROTECTED] writes:
 
  [...]  Do you think I should also remove the __markers_strings
  section from here ?
 
 Current systemtap marker support code relies on the __markers_strings
 section.

Let users know that in comment above section definition in ld script.

Thanks!
--
vda
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-18 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Changelog:
- markers_mutex now nests inside module_mutex rather than the opposite.
- Iteration on modules is now done in module.c.
- module_mutex is not exported anymore.

Signed-off-by: Mathieu Desnoyers <[EMAIL PROTECTED]>
Acked-by: "Frank Ch. Eigler" <[EMAIL PROTECTED]>
CC: Christoph Hellwig <[EMAIL PROTECTED]>
CC: Rusty Russell <[EMAIL PROTECTED]>
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  175 ++
 include/linux/module.h|   18 +
 kernel/marker.c   |  608 ++
 kernel/module.c   |   66 
 5 files changed, 875 insertions(+), 3 deletions(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-09-14 
10:11:18.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-09-14 
10:11:31.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(8);   \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-09-17 12:43:54.0 
-0400
@@ -0,0 +1,175 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <[EMAIL PROTECTED]>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include 
+#include 
+
+struct module;
+struct __mark_marker;
+
+/**
+ * marker_probe_func - Type of a marker probe function
+ * @mdata: pointer of type struct __mark_marker
+ * @private_data: caller site private data
+ * @fmt: format string
+ * @...: variable argument list
+ *
+ * Type of marker probe functions. They receive the mdata and need to parse the
+ * format string to recover the variable argument list.
+ */
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   void *private_data, const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: "arg1, arg2, arg3".
+*/
+   DEFINE_IMMEDIATE(char, state);  /* Immediate value state. */
+   marker_probe_func *call;/* Probe handler function pointer */
+   void *pdata;/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * "used" attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add unwanted 

[patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-18 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Changelog:
- markers_mutex now nests inside module_mutex rather than the opposite.
- Iteration on modules is now done in module.c.
- module_mutex is not exported anymore.

Signed-off-by: Mathieu Desnoyers [EMAIL PROTECTED]
Acked-by: Frank Ch. Eigler [EMAIL PROTECTED]
CC: Christoph Hellwig [EMAIL PROTECTED]
CC: Rusty Russell [EMAIL PROTECTED]
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  175 ++
 include/linux/module.h|   18 +
 kernel/marker.c   |  608 ++
 kernel/module.c   |   66 
 5 files changed, 875 insertions(+), 3 deletions(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-09-14 
10:11:18.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-09-14 
10:11:31.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(8);   \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-09-17 12:43:54.0 
-0400
@@ -0,0 +1,175 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers [EMAIL PROTECTED]
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include linux/immediate.h
+#include linux/types.h
+
+struct module;
+struct __mark_marker;
+
+/**
+ * marker_probe_func - Type of a marker probe function
+ * @mdata: pointer of type struct __mark_marker
+ * @private_data: caller site private data
+ * @fmt: format string
+ * @...: variable argument list
+ *
+ * Type of marker probe functions. They receive the mdata and need to parse the
+ * format string to recover the variable argument list.
+ */
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   void *private_data, const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: arg1, arg2, arg3.
+*/
+   DEFINE_IMMEDIATE(char, state);  /* Immediate value state. */
+   marker_probe_func *call;/* Probe handler function pointer */
+   void *pdata;/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * used attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add 

[patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-17 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Changelog:
- markers_mutex now nests inside module_mutex rather than the opposite.
- Iteration on modules is now done in module.c.
- module_mutex is not exported anymore.

Signed-off-by: Mathieu Desnoyers <[EMAIL PROTECTED]>
Acked-by: "Frank Ch. Eigler" <[EMAIL PROTECTED]>
CC: Christoph Hellwig <[EMAIL PROTECTED]>
CC: Rusty Russell <[EMAIL PROTECTED]>
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  175 ++
 include/linux/module.h|   18 +
 kernel/marker.c   |  608 ++
 kernel/module.c   |   66 
 5 files changed, 875 insertions(+), 3 deletions(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-09-14 
10:11:18.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-09-14 
10:11:31.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(8);   \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-09-17 12:43:54.0 
-0400
@@ -0,0 +1,175 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <[EMAIL PROTECTED]>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include 
+#include 
+
+struct module;
+struct __mark_marker;
+
+/**
+ * marker_probe_func - Type of a marker probe function
+ * @mdata: pointer of type struct __mark_marker
+ * @private_data: caller site private data
+ * @fmt: format string
+ * @...: variable argument list
+ *
+ * Type of marker probe functions. They receive the mdata and need to parse the
+ * format string to recover the variable argument list.
+ */
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   void *private_data, const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: "arg1, arg2, arg3".
+*/
+   DEFINE_IMMEDIATE(char, state);  /* Immediate value state. */
+   marker_probe_func *call;/* Probe handler function pointer */
+   void *pdata;/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * "used" attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add unwanted 

[patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-09-17 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Changelog:
- markers_mutex now nests inside module_mutex rather than the opposite.
- Iteration on modules is now done in module.c.
- module_mutex is not exported anymore.

Signed-off-by: Mathieu Desnoyers [EMAIL PROTECTED]
Acked-by: Frank Ch. Eigler [EMAIL PROTECTED]
CC: Christoph Hellwig [EMAIL PROTECTED]
CC: Rusty Russell [EMAIL PROTECTED]
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  175 ++
 include/linux/module.h|   18 +
 kernel/marker.c   |  608 ++
 kernel/module.c   |   66 
 5 files changed, 875 insertions(+), 3 deletions(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-09-14 
10:11:18.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-09-14 
10:11:31.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(8);   \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-09-17 12:43:54.0 
-0400
@@ -0,0 +1,175 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers [EMAIL PROTECTED]
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include linux/immediate.h
+#include linux/types.h
+
+struct module;
+struct __mark_marker;
+
+/**
+ * marker_probe_func - Type of a marker probe function
+ * @mdata: pointer of type struct __mark_marker
+ * @private_data: caller site private data
+ * @fmt: format string
+ * @...: variable argument list
+ *
+ * Type of marker probe functions. They receive the mdata and need to parse the
+ * format string to recover the variable argument list.
+ */
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   void *private_data, const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: arg1, arg2, arg3.
+*/
+   DEFINE_IMMEDIATE(char, state);  /* Immediate value state. */
+   marker_probe_func *call;/* Probe handler function pointer */
+   void *pdata;/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * used attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add 

[patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-27 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Changelog:
- markers_mutex now nests inside module_mutex rather than the opposite.

Signed-off-by: Mathieu Desnoyers <[EMAIL PROTECTED]>
Reviewed-by: Christoph Hellwig <[EMAIL PROTECTED]>
Reviewed-by: Rusty Russell <[EMAIL PROTECTED]>
Reviewed-by: "Frank Ch. Eigler" <[EMAIL PROTECTED]>
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  169 +
 include/linux/module.h|5 
 kernel/marker.c   |  699 ++
 kernel/module.c   |   13 
 5 files changed, 896 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-08-25 
14:41:21.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-08-25 
14:41:22.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(8);   \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-08-25 14:41:22.0 
-0400
@@ -0,0 +1,169 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <[EMAIL PROTECTED]>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include 
+#include 
+
+struct module;
+struct __mark_marker;
+
+/**
+ * marker_probe_func - Type of a marker probe function
+ * @mdata: pointer of type struct __mark_marker
+ * @private_data: caller site private data
+ * @fmt: format string
+ * @...: variable argument list
+ *
+ * Type of marker probe functions. They receive the mdata and need to parse the
+ * format string to recover the variable argument list.
+ */
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   void *private_data, const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: "arg1, arg2, arg3".
+*/
+   immediate_char_t state; /* Immediate value state. */
+   marker_probe_func *call;/* Probe handler function pointer */
+   void *pdata;/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * "used" attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add unwanted padding between the beginning of the section and the
+ * structure. Force alignment to 

[patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-27 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Changelog:
- markers_mutex now nests inside module_mutex rather than the opposite.

Signed-off-by: Mathieu Desnoyers [EMAIL PROTECTED]
Reviewed-by: Christoph Hellwig [EMAIL PROTECTED]
Reviewed-by: Rusty Russell [EMAIL PROTECTED]
Reviewed-by: Frank Ch. Eigler [EMAIL PROTECTED]
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  169 +
 include/linux/module.h|5 
 kernel/marker.c   |  699 ++
 kernel/module.c   |   13 
 5 files changed, 896 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-08-25 
14:41:21.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-08-25 
14:41:22.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(8);   \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-08-25 14:41:22.0 
-0400
@@ -0,0 +1,169 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers [EMAIL PROTECTED]
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include linux/immediate.h
+#include linux/types.h
+
+struct module;
+struct __mark_marker;
+
+/**
+ * marker_probe_func - Type of a marker probe function
+ * @mdata: pointer of type struct __mark_marker
+ * @private_data: caller site private data
+ * @fmt: format string
+ * @...: variable argument list
+ *
+ * Type of marker probe functions. They receive the mdata and need to parse the
+ * format string to recover the variable argument list.
+ */
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   void *private_data, const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: arg1, arg2, arg3.
+*/
+   immediate_char_t state; /* Immediate value state. */
+   marker_probe_func *call;/* Probe handler function pointer */
+   void *pdata;/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * used attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add unwanted padding between the beginning of the section and the
+ * structure. Force 

Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-25 Thread Mathieu Desnoyers
* Rusty Russell ([EMAIL PROTECTED]) wrote:
> On Fri, 2007-08-24 at 12:26 -0400, Mathieu Desnoyers wrote:
> > * Rusty Russell ([EMAIL PROTECTED]) wrote:
> > > On Mon, 2007-08-20 at 16:27 -0400, Mathieu Desnoyers wrote:
> > > > +{
> > > > +   struct hlist_head *head;
> > > > +   struct hlist_node *node;
> > > > +   struct marker_entry *e;
> > > > +   size_t len = strlen(name) + 1;
> > > > +   u32 hash = jhash(name, len-1, 0);
> > > > +
> > > > +   head = _table[hash & ((1 << MARKER_HASH_BITS)-1)];
> > > > +   hlist_for_each_entry(e, node, head, hlist) {
> > > > +   if (!strcmp(name, e->name))
> > > > +   return e;
> > > > +   }
> > > > +   return NULL;
> > > > +}
> > > 
> > > OK, don't understand the strlen, len, len-1 dance here?
> > > 
> > 
> > Let's say we have abc\0 for marker name as name input.
> > 
> > len = 3 + 1 = 4 (including \0)
> > hash is done only on the 3 first chars, excluding the \0 (therefore the
> >   len-1 there)
> > 
> > Actually, it's like this only for a matter of consistency between
> > add_marker and remove_marker, which are quite similar, but add_marker
> > needs name_len to include the \0 value. It would be odd to change the
> > logic between the two functions to one including the \0 and the other
> > excluding it.
> 
> Sure, but that doesn't really explain why the code does:
> 
>   size_t len = strlen(name) + 1;
>   u32 hash = jhash(name, len-1, 0);
> 
> Rather than:
> 
>   u32 hash = jhash(name, strlen(name), 0);
> 

Yup, good point. Fixed.

Thanks,

Mathieu

> > Thanks for the review,
> 
> That's fine, just some light reading...
> 
> Cheers,
> Rusty.
> > 
> 

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-25 Thread Rusty Russell
On Fri, 2007-08-24 at 12:26 -0400, Mathieu Desnoyers wrote:
> * Rusty Russell ([EMAIL PROTECTED]) wrote:
> > On Mon, 2007-08-20 at 16:27 -0400, Mathieu Desnoyers wrote:
> > > +{
> > > + struct hlist_head *head;
> > > + struct hlist_node *node;
> > > + struct marker_entry *e;
> > > + size_t len = strlen(name) + 1;
> > > + u32 hash = jhash(name, len-1, 0);
> > > +
> > > + head = _table[hash & ((1 << MARKER_HASH_BITS)-1)];
> > > + hlist_for_each_entry(e, node, head, hlist) {
> > > + if (!strcmp(name, e->name))
> > > + return e;
> > > + }
> > > + return NULL;
> > > +}
> > 
> > OK, don't understand the strlen, len, len-1 dance here?
> > 
> 
> Let's say we have abc\0 for marker name as name input.
> 
> len = 3 + 1 = 4 (including \0)
> hash is done only on the 3 first chars, excluding the \0 (therefore the
>   len-1 there)
> 
> Actually, it's like this only for a matter of consistency between
> add_marker and remove_marker, which are quite similar, but add_marker
> needs name_len to include the \0 value. It would be odd to change the
> logic between the two functions to one including the \0 and the other
> excluding it.

Sure, but that doesn't really explain why the code does:

size_t len = strlen(name) + 1;
u32 hash = jhash(name, len-1, 0);

Rather than:

u32 hash = jhash(name, strlen(name), 0);

> Thanks for the review,

That's fine, just some light reading...

Cheers,
Rusty.
> 

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-25 Thread Rusty Russell
On Fri, 2007-08-24 at 12:26 -0400, Mathieu Desnoyers wrote:
 * Rusty Russell ([EMAIL PROTECTED]) wrote:
  On Mon, 2007-08-20 at 16:27 -0400, Mathieu Desnoyers wrote:
   +{
   + struct hlist_head *head;
   + struct hlist_node *node;
   + struct marker_entry *e;
   + size_t len = strlen(name) + 1;
   + u32 hash = jhash(name, len-1, 0);
   +
   + head = marker_table[hash  ((1  MARKER_HASH_BITS)-1)];
   + hlist_for_each_entry(e, node, head, hlist) {
   + if (!strcmp(name, e-name))
   + return e;
   + }
   + return NULL;
   +}
  
  OK, don't understand the strlen, len, len-1 dance here?
  
 
 Let's say we have abc\0 for marker name as name input.
 
 len = 3 + 1 = 4 (including \0)
 hash is done only on the 3 first chars, excluding the \0 (therefore the
   len-1 there)
 
 Actually, it's like this only for a matter of consistency between
 add_marker and remove_marker, which are quite similar, but add_marker
 needs name_len to include the \0 value. It would be odd to change the
 logic between the two functions to one including the \0 and the other
 excluding it.

Sure, but that doesn't really explain why the code does:

size_t len = strlen(name) + 1;
u32 hash = jhash(name, len-1, 0);

Rather than:

u32 hash = jhash(name, strlen(name), 0);

 Thanks for the review,

That's fine, just some light reading...

Cheers,
Rusty.
 

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-25 Thread Mathieu Desnoyers
* Rusty Russell ([EMAIL PROTECTED]) wrote:
 On Fri, 2007-08-24 at 12:26 -0400, Mathieu Desnoyers wrote:
  * Rusty Russell ([EMAIL PROTECTED]) wrote:
   On Mon, 2007-08-20 at 16:27 -0400, Mathieu Desnoyers wrote:
+{
+   struct hlist_head *head;
+   struct hlist_node *node;
+   struct marker_entry *e;
+   size_t len = strlen(name) + 1;
+   u32 hash = jhash(name, len-1, 0);
+
+   head = marker_table[hash  ((1  MARKER_HASH_BITS)-1)];
+   hlist_for_each_entry(e, node, head, hlist) {
+   if (!strcmp(name, e-name))
+   return e;
+   }
+   return NULL;
+}
   
   OK, don't understand the strlen, len, len-1 dance here?
   
  
  Let's say we have abc\0 for marker name as name input.
  
  len = 3 + 1 = 4 (including \0)
  hash is done only on the 3 first chars, excluding the \0 (therefore the
len-1 there)
  
  Actually, it's like this only for a matter of consistency between
  add_marker and remove_marker, which are quite similar, but add_marker
  needs name_len to include the \0 value. It would be odd to change the
  logic between the two functions to one including the \0 and the other
  excluding it.
 
 Sure, but that doesn't really explain why the code does:
 
   size_t len = strlen(name) + 1;
   u32 hash = jhash(name, len-1, 0);
 
 Rather than:
 
   u32 hash = jhash(name, strlen(name), 0);
 

Yup, good point. Fixed.

Thanks,

Mathieu

  Thanks for the review,
 
 That's fine, just some light reading...
 
 Cheers,
 Rusty.
  
 

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-24 Thread Mathieu Desnoyers
* Rusty Russell ([EMAIL PROTECTED]) wrote:
> On Mon, 2007-08-20 at 16:27 -0400, Mathieu Desnoyers wrote:
> > The marker activation functions sits in kernel/marker.c. A hash table is 
> > used
> > to keep track of the registered probes and armed markers, so the markers 
> > within
> > a newly loaded module that should be active can be activated at module load
> > time.
> 
> Hi Mathieu!
> 
>   Just reading through this patch, a couple of comments:
> 

Hi Rusty,

> > +/* To be used for string format validity checking with gcc */
> > +static inline void __mark_check_format(const char *fmt, ...)
> > +   __attribute__ ((format (printf, 1, 2)));
> > +static inline void __mark_check_format(const char *fmt, ...) { }
> 
> If you place the __attribute__() before the function name, you can do
> this in the definition.
> 

Ok, will fix.

> > ===
> > --- linux-2.6-lttng.orig/kernel/module.c2007-08-10 19:44:18.0 
> > -0400
> > +++ linux-2.6-lttng/kernel/module.c 2007-08-10 23:54:38.0 -0400
> > @@ -1980,6 +1986,10 @@ static struct module *load_module(void _
> > sechdrs[immediateindex].sh_size / 
> > sizeof(*mod->immediate);
> > }
> >  #endif
> > +   if (markersindex)
> > +   sechdrs[markersindex].sh_flags |= SHF_ALLOC;
> > +   if (markersstringsindex)
> > +   sechdrs[markersstringsindex].sh_flags |= SHF_ALLOC;
> >  
> 
> Perhaps I'm missing something, but I don't see why these sections
> wouldn't be SHF_ALLOC already.
> 

Declaring variables with __attribute__((section("__markers_strings")))
will likely put them in an allocated section, you are right. Will fix.

The same applies to immediate values with .section __immediate, \"a\",
@progbits, which is explicitely allocated.

> > mod->unused_syms = (void *)sechdrs[unusedindex].sh_addr;
> > if (unusedcrcindex)
> > @@ -2021,6 +2031,13 @@ static struct module *load_module(void _
> > if (err < 0)
> > goto cleanup;
> > }
> > +#ifdef CONFIG_MARKERS
> > +   if (markersindex) {
> > +   mod->markers = (void *)sechdrs[markersindex].sh_addr;
> > +   mod->num_markers =
> > +   sechdrs[markersindex].sh_size / sizeof(*mod->markers);
> > +   }
> > +#endif
> 
> Because of the wonders of ELF, section 0 has sh_addr and sh_size 0.  So
> the if (markersindex) is unnecessary here.
> 

Ok. Will apply to immediate values too.

> > +/*
> > + * Get marker if the marker is present in the marker hash table.
> > + * Must be called with markers_mutex held.
> > + * Returns NULL if not present.
> > + */
> > +static struct marker_entry *_get_marker(const char *name)
> 
> You seem to really enjoy underscores, yet I'm having trouble
> understanding why this would have an underscore in it.
> 

Just they are internal functiona meant to be called with markers_mutex
held. But I guess having a static prefix and not being exported is
enough. Will remove. I'll just keep
_marker_update_probes/marker_update_probes to differentiate between
locked/non-locked version.

> > +{
> > +   struct hlist_head *head;
> > +   struct hlist_node *node;
> > +   struct marker_entry *e;
> > +   size_t len = strlen(name) + 1;
> > +   u32 hash = jhash(name, len-1, 0);
> > +
> > +   head = _table[hash & ((1 << MARKER_HASH_BITS)-1)];
> > +   hlist_for_each_entry(e, node, head, hlist) {
> > +   if (!strcmp(name, e->name))
> > +   return e;
> > +   }
> > +   return NULL;
> > +}
> 
> OK, don't understand the strlen, len, len-1 dance here?
> 

Let's say we have abc\0 for marker name as name input.

len = 3 + 1 = 4 (including \0)
hash is done only on the 3 first chars, excluding the \0 (therefore the
  len-1 there)

Actually, it's like this only for a matter of consistency between
add_marker and remove_marker, which are quite similar, but add_marker
needs name_len to include the \0 value. It would be odd to change the
logic between the two functions to one including the \0 and the other
excluding it.

> > +/*
> > + * Updates the probe callback corresponding to a range of markers.
> > + * Must be called with markers_mutex held.
> > + */
> > +static void _marker_update_probe_range(
> 
> And yet:
> 
> > +void module_marker_update(struct module *mod)
> > +{
> > +   if (!mod->taints)
> > +   _marker_update_probe_range(mod->markers,
> > +   mod->markers+mod->num_markers, NULL, NULL);
> > +}
> 
> This doesn't hold the markers_mutex.
> 

Since this function is only meant to be called upon module load, before
the module is added to the module list, this marker range cannot be seen
by other calls to marker_update_probe_range. But you are right. It also
protects the hash table, so I should take the mutex there.  Fixing.

Thanks for the review,

Mathieu

> Cheers,
> Rusty.
> 

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole 

Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-24 Thread Mathieu Desnoyers
* Rusty Russell ([EMAIL PROTECTED]) wrote:
 On Mon, 2007-08-20 at 16:27 -0400, Mathieu Desnoyers wrote:
  The marker activation functions sits in kernel/marker.c. A hash table is 
  used
  to keep track of the registered probes and armed markers, so the markers 
  within
  a newly loaded module that should be active can be activated at module load
  time.
 
 Hi Mathieu!
 
   Just reading through this patch, a couple of comments:
 

Hi Rusty,

  +/* To be used for string format validity checking with gcc */
  +static inline void __mark_check_format(const char *fmt, ...)
  +   __attribute__ ((format (printf, 1, 2)));
  +static inline void __mark_check_format(const char *fmt, ...) { }
 
 If you place the __attribute__() before the function name, you can do
 this in the definition.
 

Ok, will fix.

  ===
  --- linux-2.6-lttng.orig/kernel/module.c2007-08-10 19:44:18.0 
  -0400
  +++ linux-2.6-lttng/kernel/module.c 2007-08-10 23:54:38.0 -0400
  @@ -1980,6 +1986,10 @@ static struct module *load_module(void _
  sechdrs[immediateindex].sh_size / 
  sizeof(*mod-immediate);
  }
   #endif
  +   if (markersindex)
  +   sechdrs[markersindex].sh_flags |= SHF_ALLOC;
  +   if (markersstringsindex)
  +   sechdrs[markersstringsindex].sh_flags |= SHF_ALLOC;
   
 
 Perhaps I'm missing something, but I don't see why these sections
 wouldn't be SHF_ALLOC already.
 

Declaring variables with __attribute__((section(__markers_strings)))
will likely put them in an allocated section, you are right. Will fix.

The same applies to immediate values with .section __immediate, \a\,
@progbits, which is explicitely allocated.

  mod-unused_syms = (void *)sechdrs[unusedindex].sh_addr;
  if (unusedcrcindex)
  @@ -2021,6 +2031,13 @@ static struct module *load_module(void _
  if (err  0)
  goto cleanup;
  }
  +#ifdef CONFIG_MARKERS
  +   if (markersindex) {
  +   mod-markers = (void *)sechdrs[markersindex].sh_addr;
  +   mod-num_markers =
  +   sechdrs[markersindex].sh_size / sizeof(*mod-markers);
  +   }
  +#endif
 
 Because of the wonders of ELF, section 0 has sh_addr and sh_size 0.  So
 the if (markersindex) is unnecessary here.
 

Ok. Will apply to immediate values too.

  +/*
  + * Get marker if the marker is present in the marker hash table.
  + * Must be called with markers_mutex held.
  + * Returns NULL if not present.
  + */
  +static struct marker_entry *_get_marker(const char *name)
 
 You seem to really enjoy underscores, yet I'm having trouble
 understanding why this would have an underscore in it.
 

Just they are internal functiona meant to be called with markers_mutex
held. But I guess having a static prefix and not being exported is
enough. Will remove. I'll just keep
_marker_update_probes/marker_update_probes to differentiate between
locked/non-locked version.

  +{
  +   struct hlist_head *head;
  +   struct hlist_node *node;
  +   struct marker_entry *e;
  +   size_t len = strlen(name) + 1;
  +   u32 hash = jhash(name, len-1, 0);
  +
  +   head = marker_table[hash  ((1  MARKER_HASH_BITS)-1)];
  +   hlist_for_each_entry(e, node, head, hlist) {
  +   if (!strcmp(name, e-name))
  +   return e;
  +   }
  +   return NULL;
  +}
 
 OK, don't understand the strlen, len, len-1 dance here?
 

Let's say we have abc\0 for marker name as name input.

len = 3 + 1 = 4 (including \0)
hash is done only on the 3 first chars, excluding the \0 (therefore the
  len-1 there)

Actually, it's like this only for a matter of consistency between
add_marker and remove_marker, which are quite similar, but add_marker
needs name_len to include the \0 value. It would be odd to change the
logic between the two functions to one including the \0 and the other
excluding it.

  +/*
  + * Updates the probe callback corresponding to a range of markers.
  + * Must be called with markers_mutex held.
  + */
  +static void _marker_update_probe_range(
 
 And yet:
 
  +void module_marker_update(struct module *mod)
  +{
  +   if (!mod-taints)
  +   _marker_update_probe_range(mod-markers,
  +   mod-markers+mod-num_markers, NULL, NULL);
  +}
 
 This doesn't hold the markers_mutex.
 

Since this function is only meant to be called upon module load, before
the module is added to the module list, this marker range cannot be seen
by other calls to marker_update_probe_range. But you are right. It also
protects the hash table, so I should take the mutex there.  Fixing.

Thanks for the review,

Mathieu

 Cheers,
 Rusty.
 

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the 

Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-20 Thread Rusty Russell
On Mon, 2007-08-20 at 16:27 -0400, Mathieu Desnoyers wrote:
> The marker activation functions sits in kernel/marker.c. A hash table is used
> to keep track of the registered probes and armed markers, so the markers 
> within
> a newly loaded module that should be active can be activated at module load
> time.

Hi Mathieu!

Just reading through this patch, a couple of comments:

> +/* To be used for string format validity checking with gcc */
> +static inline void __mark_check_format(const char *fmt, ...)
> + __attribute__ ((format (printf, 1, 2)));
> +static inline void __mark_check_format(const char *fmt, ...) { }

If you place the __attribute__() before the function name, you can do
this in the definition.

> ===
> --- linux-2.6-lttng.orig/kernel/module.c  2007-08-10 19:44:18.0 
> -0400
> +++ linux-2.6-lttng/kernel/module.c   2007-08-10 23:54:38.0 -0400
> @@ -1980,6 +1986,10 @@ static struct module *load_module(void _
>   sechdrs[immediateindex].sh_size / 
> sizeof(*mod->immediate);
>   }
>  #endif
> + if (markersindex)
> + sechdrs[markersindex].sh_flags |= SHF_ALLOC;
> + if (markersstringsindex)
> + sechdrs[markersstringsindex].sh_flags |= SHF_ALLOC;
>  

Perhaps I'm missing something, but I don't see why these sections
wouldn't be SHF_ALLOC already.

>   mod->unused_syms = (void *)sechdrs[unusedindex].sh_addr;
>   if (unusedcrcindex)
> @@ -2021,6 +2031,13 @@ static struct module *load_module(void _
>   if (err < 0)
>   goto cleanup;
>   }
> +#ifdef CONFIG_MARKERS
> + if (markersindex) {
> + mod->markers = (void *)sechdrs[markersindex].sh_addr;
> + mod->num_markers =
> + sechdrs[markersindex].sh_size / sizeof(*mod->markers);
> + }
> +#endif

Because of the wonders of ELF, section 0 has sh_addr and sh_size 0.  So
the if (markersindex) is unnecessary here.

> +/*
> + * Get marker if the marker is present in the marker hash table.
> + * Must be called with markers_mutex held.
> + * Returns NULL if not present.
> + */
> +static struct marker_entry *_get_marker(const char *name)

You seem to really enjoy underscores, yet I'm having trouble
understanding why this would have an underscore in it.

> +{
> + struct hlist_head *head;
> + struct hlist_node *node;
> + struct marker_entry *e;
> + size_t len = strlen(name) + 1;
> + u32 hash = jhash(name, len-1, 0);
> +
> + head = _table[hash & ((1 << MARKER_HASH_BITS)-1)];
> + hlist_for_each_entry(e, node, head, hlist) {
> + if (!strcmp(name, e->name))
> + return e;
> + }
> + return NULL;
> +}

OK, don't understand the strlen, len, len-1 dance here?

> +/*
> + * Updates the probe callback corresponding to a range of markers.
> + * Must be called with markers_mutex held.
> + */
> +static void _marker_update_probe_range(

And yet:

> +void module_marker_update(struct module *mod)
> +{
> + if (!mod->taints)
> + _marker_update_probe_range(mod->markers,
> + mod->markers+mod->num_markers, NULL, NULL);
> +}

This doesn't hold the markers_mutex.

Cheers,
Rusty.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-20 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Signed-off-by: Mathieu Desnoyers <[EMAIL PROTECTED]>
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  170 +
 include/linux/module.h|5 
 kernel/marker.c   |  678 ++
 kernel/module.c   |   19 +
 5 files changed, 882 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-08-10 
19:44:17.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-08-10 
19:44:18.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(8);   \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-08-10 23:54:35.0 
-0400
@@ -0,0 +1,170 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <[EMAIL PROTECTED]>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include 
+#include 
+
+struct module;
+struct __mark_marker;
+
+/**
+ * marker_probe_func - Type of a marker probe function
+ * @mdata: pointer of type struct __mark_marker
+ * @private_data: caller site private data
+ * @fmt: format string
+ * @...: variable argument list
+ *
+ * Type of marker probe functions. They receive the mdata and need to parse the
+ * format string to recover the variable argument list.
+ */
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   void *private_data, const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: "arg1, arg2, arg3".
+*/
+   immediate_char_t state; /* Immediate value state. */
+   marker_probe_func *call;/* Probe handler function pointer */
+   void *pdata;/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * "used" attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add unwanted padding between the beginning of the section and the
+ * structure. Force alignment to the same alignment as the section start.
+ */
+#define __trace_mark(generic, name, call_data, format, args...)
\
+   do {\
+   static const 

[patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-20 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Signed-off-by: Mathieu Desnoyers [EMAIL PROTECTED]
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  170 +
 include/linux/module.h|5 
 kernel/marker.c   |  678 ++
 kernel/module.c   |   19 +
 5 files changed, 882 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-08-10 
19:44:17.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-08-10 
19:44:18.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(8);   \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-08-10 23:54:35.0 
-0400
@@ -0,0 +1,170 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers [EMAIL PROTECTED]
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include linux/immediate.h
+#include linux/types.h
+
+struct module;
+struct __mark_marker;
+
+/**
+ * marker_probe_func - Type of a marker probe function
+ * @mdata: pointer of type struct __mark_marker
+ * @private_data: caller site private data
+ * @fmt: format string
+ * @...: variable argument list
+ *
+ * Type of marker probe functions. They receive the mdata and need to parse the
+ * format string to recover the variable argument list.
+ */
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   void *private_data, const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: arg1, arg2, arg3.
+*/
+   immediate_char_t state; /* Immediate value state. */
+   marker_probe_func *call;/* Probe handler function pointer */
+   void *pdata;/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * used attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add unwanted padding between the beginning of the section and the
+ * structure. Force alignment to the same alignment as the section start.
+ */
+#define __trace_mark(generic, name, call_data, format, args...)
\
+   do {\
+ 

Re: [patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-20 Thread Rusty Russell
On Mon, 2007-08-20 at 16:27 -0400, Mathieu Desnoyers wrote:
 The marker activation functions sits in kernel/marker.c. A hash table is used
 to keep track of the registered probes and armed markers, so the markers 
 within
 a newly loaded module that should be active can be activated at module load
 time.

Hi Mathieu!

Just reading through this patch, a couple of comments:

 +/* To be used for string format validity checking with gcc */
 +static inline void __mark_check_format(const char *fmt, ...)
 + __attribute__ ((format (printf, 1, 2)));
 +static inline void __mark_check_format(const char *fmt, ...) { }

If you place the __attribute__() before the function name, you can do
this in the definition.

 ===
 --- linux-2.6-lttng.orig/kernel/module.c  2007-08-10 19:44:18.0 
 -0400
 +++ linux-2.6-lttng/kernel/module.c   2007-08-10 23:54:38.0 -0400
 @@ -1980,6 +1986,10 @@ static struct module *load_module(void _
   sechdrs[immediateindex].sh_size / 
 sizeof(*mod-immediate);
   }
  #endif
 + if (markersindex)
 + sechdrs[markersindex].sh_flags |= SHF_ALLOC;
 + if (markersstringsindex)
 + sechdrs[markersstringsindex].sh_flags |= SHF_ALLOC;
  

Perhaps I'm missing something, but I don't see why these sections
wouldn't be SHF_ALLOC already.

   mod-unused_syms = (void *)sechdrs[unusedindex].sh_addr;
   if (unusedcrcindex)
 @@ -2021,6 +2031,13 @@ static struct module *load_module(void _
   if (err  0)
   goto cleanup;
   }
 +#ifdef CONFIG_MARKERS
 + if (markersindex) {
 + mod-markers = (void *)sechdrs[markersindex].sh_addr;
 + mod-num_markers =
 + sechdrs[markersindex].sh_size / sizeof(*mod-markers);
 + }
 +#endif

Because of the wonders of ELF, section 0 has sh_addr and sh_size 0.  So
the if (markersindex) is unnecessary here.

 +/*
 + * Get marker if the marker is present in the marker hash table.
 + * Must be called with markers_mutex held.
 + * Returns NULL if not present.
 + */
 +static struct marker_entry *_get_marker(const char *name)

You seem to really enjoy underscores, yet I'm having trouble
understanding why this would have an underscore in it.

 +{
 + struct hlist_head *head;
 + struct hlist_node *node;
 + struct marker_entry *e;
 + size_t len = strlen(name) + 1;
 + u32 hash = jhash(name, len-1, 0);
 +
 + head = marker_table[hash  ((1  MARKER_HASH_BITS)-1)];
 + hlist_for_each_entry(e, node, head, hlist) {
 + if (!strcmp(name, e-name))
 + return e;
 + }
 + return NULL;
 +}

OK, don't understand the strlen, len, len-1 dance here?

 +/*
 + * Updates the probe callback corresponding to a range of markers.
 + * Must be called with markers_mutex held.
 + */
 +static void _marker_update_probe_range(

And yet:

 +void module_marker_update(struct module *mod)
 +{
 + if (!mod-taints)
 + _marker_update_probe_range(mod-markers,
 + mod-markers+mod-num_markers, NULL, NULL);
 +}

This doesn't hold the markers_mutex.

Cheers,
Rusty.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-12 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Signed-off-by: Mathieu Desnoyers <[EMAIL PROTECTED]>
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  170 +
 include/linux/module.h|5 
 kernel/marker.c   |  678 ++
 kernel/module.c   |   19 +
 5 files changed, 882 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-08-10 
19:44:17.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-08-10 
19:44:18.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(8);   \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-08-10 23:54:35.0 
-0400
@@ -0,0 +1,170 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <[EMAIL PROTECTED]>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include 
+#include 
+
+struct module;
+struct __mark_marker;
+
+/**
+ * marker_probe_func - Type of a marker probe function
+ * @mdata: pointer of type struct __mark_marker
+ * @private_data: caller site private data
+ * @fmt: format string
+ * @...: variable argument list
+ *
+ * Type of marker probe functions. They receive the mdata and need to parse the
+ * format string to recover the variable argument list.
+ */
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   void *private_data, const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: "arg1, arg2, arg3".
+*/
+   immediate_char_t state; /* Immediate value state. */
+   marker_probe_func *call;/* Probe handler function pointer */
+   void *pdata;/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * "used" attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add unwanted padding between the beginning of the section and the
+ * structure. Force alignment to the same alignment as the section start.
+ */
+#define __trace_mark(generic, name, call_data, format, args...)
\
+   do {\
+   static const 

[patch 1/4] Linux Kernel Markers - Architecture Independent Code

2007-08-12 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Signed-off-by: Mathieu Desnoyers [EMAIL PROTECTED]
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  170 +
 include/linux/module.h|5 
 kernel/marker.c   |  678 ++
 kernel/module.c   |   19 +
 5 files changed, 882 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-08-10 
19:44:17.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-08-10 
19:44:18.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(8);   \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-08-10 23:54:35.0 
-0400
@@ -0,0 +1,170 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers [EMAIL PROTECTED]
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include linux/immediate.h
+#include linux/types.h
+
+struct module;
+struct __mark_marker;
+
+/**
+ * marker_probe_func - Type of a marker probe function
+ * @mdata: pointer of type struct __mark_marker
+ * @private_data: caller site private data
+ * @fmt: format string
+ * @...: variable argument list
+ *
+ * Type of marker probe functions. They receive the mdata and need to parse the
+ * format string to recover the variable argument list.
+ */
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   void *private_data, const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: arg1, arg2, arg3.
+*/
+   immediate_char_t state; /* Immediate value state. */
+   marker_probe_func *call;/* Probe handler function pointer */
+   void *pdata;/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * used attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add unwanted padding between the beginning of the section and the
+ * structure. Force alignment to the same alignment as the section start.
+ */
+#define __trace_mark(generic, name, call_data, format, args...)
\
+   do {\
+ 

[patch 1/4] Linux Kernel Markers, architecture independent code.

2007-07-13 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Signed-off-by: Mathieu Desnoyers <[EMAIL PROTECTED]>
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  136 
 include/linux/module.h|5 
 kernel/marker.c   |  636 ++
 kernel/module.c   |   19 +
 5 files changed, 806 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-07-13 
09:08:46.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-07-13 
10:32:40.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(8);   \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-07-13 10:33:58.0 
-0400
@@ -0,0 +1,136 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <[EMAIL PROTECTED]>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#ifdef __KERNEL__
+
+#include 
+#include 
+
+struct module;
+struct __mark_marker;
+
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: "arg1, arg2, arg3".
+*/
+   immediate_char_t state; /* Immediate value state. */
+   marker_probe_func *call;/* Probe handler function pointer */
+   void *pdata;/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * "used" attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add unwanted padding between the beginning of the section and the
+ * structure. Force alignment to the same alignment as the section start.
+ */
+#define __trace_mark(generic, name, format, args...)   \
+   do {\
+   static const char __mstrtab_name_##name[]   \
+   __attribute__((section("__markers_strings")))   \
+   = #name;\
+   static const char __mstrtab_format_##name[] \
+   __attribute__((section("__markers_strings")))   \
+   = format;

[patch 1/4] Linux Kernel Markers, architecture independent code.

2007-07-13 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Signed-off-by: Mathieu Desnoyers [EMAIL PROTECTED]
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  136 
 include/linux/module.h|5 
 kernel/marker.c   |  636 ++
 kernel/module.c   |   19 +
 5 files changed, 806 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-07-13 
09:08:46.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-07-13 
10:32:40.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(8);   \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-07-13 10:33:58.0 
-0400
@@ -0,0 +1,136 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers [EMAIL PROTECTED]
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#ifdef __KERNEL__
+
+#include linux/immediate.h
+#include linux/types.h
+
+struct module;
+struct __mark_marker;
+
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: arg1, arg2, arg3.
+*/
+   immediate_char_t state; /* Immediate value state. */
+   marker_probe_func *call;/* Probe handler function pointer */
+   void *pdata;/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * used attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add unwanted padding between the beginning of the section and the
+ * structure. Force alignment to the same alignment as the section start.
+ */
+#define __trace_mark(generic, name, format, args...)   \
+   do {\
+   static const char __mstrtab_name_##name[]   \
+   __attribute__((section(__markers_strings)))   \
+   = #name;\
+   static const char __mstrtab_format_##name[] \
+   __attribute__((section(__markers_strings)))   \
+  

[patch 1/4] Linux Kernel Markers, architecture independent code.

2007-07-03 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the armed/disarmed markers, so they can be activated at module
load time.

The marker IDs will be used as unique identifiers for markers. They are
assigned by the marker infrastructure and are used to identify a marker in a
compact representation either in a probe handler and especially in a trace.

Marker group is a parameter given by the probe provider. It can be used, for
instance, to tell which set of buffers the information must be send to when
recording a trace.

Signed-off-by: Mathieu Desnoyers <[EMAIL PROTECTED]>
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  159 +++
 include/linux/module.h|5 
 kernel/Makefile   |1 
 kernel/marker.c   |  801 ++
 kernel/module.c   |   17 
 6 files changed, 993 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-07-03 
12:33:20.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-07-03 
12:33:21.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(32);  \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-07-03 13:03:14.0 
-0400
@@ -0,0 +1,159 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <[EMAIL PROTECTED]>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#ifdef __KERNEL__
+
+#include 
+#include 
+
+struct module;
+struct __mark_marker;
+
+/*
+ * Unique ID assigned to each registered probe.
+ */
+enum marker_id {
+   MARKER_ID_LOAD_MARKER = 0,  /* Static IDs available (range 0-7) */
+   MARKER_ID_HEARTBEAT_32,
+   MARKER_ID_HEARTBEAT_64,
+   MARKER_ID_COMPACT,  /* Compact IDs (range: 8-127)   */
+   MARKER_ID_DYNAMIC,  /* Dynamic IDs (range: 128-65535)   */
+};
+
+/* static ids 0-7 reserved for internal use. */
+#define MARKER_CORE_IDS8
+/* dynamic ids 8-127 reserved for compact events. */
+#define MARKER_COMPACT_IDS 128
+static inline enum marker_id marker_id_type(uint16_t id)
+{
+   if (id < MARKER_CORE_IDS)
+   return (enum marker_id)id;
+   else if (id < MARKER_COMPACT_IDS)
+   return MARKER_ID_COMPACT;
+   else
+   return MARKER_ID_DYNAMIC;
+}
+
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: "arg1, arg2, arg3".
+*/
+   immediate_t state;  /* Immediate value state. */
+   int flags;  /* Flags 

[patch 1/4] Linux Kernel Markers, architecture independent code.

2007-07-03 Thread Mathieu Desnoyers
The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the armed/disarmed markers, so they can be activated at module
load time.

The marker IDs will be used as unique identifiers for markers. They are
assigned by the marker infrastructure and are used to identify a marker in a
compact representation either in a probe handler and especially in a trace.

Marker group is a parameter given by the probe provider. It can be used, for
instance, to tell which set of buffers the information must be send to when
recording a trace.

Signed-off-by: Mathieu Desnoyers [EMAIL PROTECTED]
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h|  159 +++
 include/linux/module.h|5 
 kernel/Makefile   |1 
 kernel/marker.c   |  801 ++
 kernel/module.c   |   17 
 6 files changed, 993 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h  2007-07-03 
12:33:20.0 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h   2007-07-03 
12:33:21.0 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA  \
*(.data)\
-   *(.data.init.refok)
+   *(.data.init.refok) \
+   . = ALIGN(32);  \
+   VMLINUX_SYMBOL(__start___markers) = .;  \
+   *(__markers)\
+   VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align) \
. = ALIGN((align)); \
@@ -129,6 +133,11 @@
VMLINUX_SYMBOL(__stop___immediate) = .; \
}   \
\
+   /* Markers: strings */  \
+__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {
\
+   *(__markers_strings)\
+   }   \
+   \
/* Kernel symbol table: strings */  \
 __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
Index: linux-2.6-lttng/include/linux/marker.h
===
--- /dev/null   1970-01-01 00:00:00.0 +
+++ linux-2.6-lttng/include/linux/marker.h  2007-07-03 13:03:14.0 
-0400
@@ -0,0 +1,159 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers [EMAIL PROTECTED]
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#ifdef __KERNEL__
+
+#include linux/immediate.h
+#include linux/types.h
+
+struct module;
+struct __mark_marker;
+
+/*
+ * Unique ID assigned to each registered probe.
+ */
+enum marker_id {
+   MARKER_ID_LOAD_MARKER = 0,  /* Static IDs available (range 0-7) */
+   MARKER_ID_HEARTBEAT_32,
+   MARKER_ID_HEARTBEAT_64,
+   MARKER_ID_COMPACT,  /* Compact IDs (range: 8-127)   */
+   MARKER_ID_DYNAMIC,  /* Dynamic IDs (range: 128-65535)   */
+};
+
+/* static ids 0-7 reserved for internal use. */
+#define MARKER_CORE_IDS8
+/* dynamic ids 8-127 reserved for compact events. */
+#define MARKER_COMPACT_IDS 128
+static inline enum marker_id marker_id_type(uint16_t id)
+{
+   if (id  MARKER_CORE_IDS)
+   return (enum marker_id)id;
+   else if (id  MARKER_COMPACT_IDS)
+   return MARKER_ID_COMPACT;
+   else
+   return MARKER_ID_DYNAMIC;
+}
+
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+   const char *fmt, ...);
+
+struct __mark_marker {
+   const char *name;   /* Marker name */
+   const char *format; /* Marker format string, describing the
+* variable argument list.
+*/
+   const char *args;   /* List of arguments litteraly transformed
+* into a string: arg1, arg2, arg3.
+*/
+   immediate_t state;  /* Immediate value state. */
+   int flags;