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 <mathieu.desnoyers@polymtl.ca>
Reviewed-by: Christoph Hellwig <hch@infradead.org>
Reviewed-by: Rusty Russell <rusty@rustcorp.com.au>
Reviewed-by: "Frank Ch. Eigler" <fche@redhat.com>
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h            |  169 +++++++++
 include/linux/module.h            |    5 
 kernel/marker.c                   |  699 ++++++++++++++++++++++++++++++++++++++
 kernel/module.c                   |   11 
 5 files changed, 894 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-09-06 15:06:36.000000000 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h	2007-09-06 15:07:28.000000000 -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) - ...
[ message continues ]

Documentation/SubmittingPatches recognizes Signed-off-by: and
Acked-by:.  Nothing about Reviewed-by.  Quote:

  Acked-by: is not as formal as Signed-off-by:.  It is a record that the acker
  has at least reviewed the patch and has indicated acceptance.

Reviewed-by: carries no such connotation or indication IMO.







hm, so if the module is tainted, markers won't be applied to it?
That's OK IMO.  However, I tend to think that something that
modifies code on the fly (like kprobes or text edit) should set some
flag somewhere so that when the system crashes, we can know that
code was modified.  This could be done via taints flags or some other


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-

[ message continues ]

Yeah.  We will start introducing Reviewed-by: (I haven't yet quite worked
out how yet) but it will be a quite formal thing and it would be something
which the reviewer explicitly provided.  For now, let's please stick with
acked-by (if those individuals did indeed send an acked-by)
-

[ message continues ]

Thanks.  I look forward to the explanation of Reviewed-by, what it
means, and how it differs from Acked-by.


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-

[ message continues ]

As long as you're at it,  the only tag *I* seem to need is 'Tested-By:' :)
[ message continues ]

Yep, and that's a good one IMO.

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-

[ message continues ]

This was proposed by Andrew and discussed at the Kernel Summit; the
basic idea is that it is a formal indication that the person has done
a *full* review of the patch (a few random comments from the local
whitespace police don't count), and is willing to vouch that the patch
is correct, safe, extremely unlikely to cause regressions, etc.  If
the patch does need to be reverted or fixed because it was buggy, then
both the original submitter and the reviewer would bear responsibility
and subsystem maintainers might take that into account when assessing
the reputations of the submitter and reviewer in the future when
deciding whether or not to accept a patch.

Basically, some people seem to be using "Acked-by" to mean, "seems
good to me", without necessarily doing a full review of the patch, and
instead of trying to change the meaning of "Acked-by", to have a new
sign off which is a bit more explicitly about what it means.  (Hmmm,
thinking about it afterwards, maybe "Vouched-by:" would be even
better....)

There was some thought about negative attention (i.e., "public
mockery") given to people who sign off on a patch via Reviewed-by:
that subsequently turns out to be buggy or cause a regression, but the
concern with that is that we have enough trouble finding people to
review patches, and we wouldn't want to scare off reviewers.  But it
would be fair to say that the consequences of reviewing patches
successfully or unsuccessfully would naturally impact people's
reputations.

There was also some discussion about whether or not patches would not
be accepted at all without a Reviewed-by, but that probably won't
happen initially.  The general consensus was to gently ease into it
and see how well it works first.

        					- Ted
-

[ message continues ]

Anybody got a proposed scheme for the case where somebody like myself
who is *not* a member of the Maintainer Cabal has looked at a patch, and
found a valid show-stopper that's bigger than just whitespace (breaks on
64-bit, locking issues, etc), or other commentary that *should* be addressed
before it gets merged?  I'd like *some* way to tag a patch with "I had an
issue with V1, but the author addressed it to my satisfaction in V2"....

(Note that includes "the author convinced me the patch was right and I was
wrong"...)
[ message continues ]

I think that'd be Reviewed-By.  While you are not part of the smokey room
cabal you have shown technical expertise in various areas so it seems
perfectly fine to have reviewed-by from you.  The fix vs a previous version
should probably be just in the text with a paragraph ala:

Issue blah in a previous version as found by Valdis Kletnieks has been fixed
by doing foo.
-

[ message continues ]

> > Anybody got a proposed scheme for the case where somebody like myself
 > > who is *not* a member of the Maintainer Cabal has looked at a patch, and
 > > found a valid show-stopper that's bigger than just whitespace (breaks on
 > > 64-bit, locking issues, etc), or other commentary that *should* be addressed
 > > before it gets merged?  I'd like *some* way to tag a patch with "I had an
 > > issue with V1, but the author addressed it to my satisfaction in V2"....

 > I think that'd be Reviewed-By.  While you are not part of the smokey room
 > cabal you have shown technical expertise in various areas so it seems
 > perfectly fine to have reviewed-by from you.  The fix vs a previous version
 > should probably be just in the text with a paragraph ala:

 > Issue blah in a previous version as found by Valdis Kletnieks has been fixed
 > by doing foo.

At ksummit Andrew also mentioned including a link to the relevant
mailing list discussion too, and I think this would be a good example
of when that would be useful.

 - R.
-

[ message continues ]

* Randy Dunlap (randy.dunlap@oracle.com) wrote:

I could, but it's really a bug: if this condition happens, there would
be an inconsistency in the hash table get_marker() function. Therefore I
prefer to yell if this happens rather than simply telling the caller
that the entry does not exist.

And actually, marker_update_probe_range ignores errors from set_marker
(there is even a comment saying that in the code) :

                if (mark_entry && mark_entry->refcount) {
                        set_marker(&mark_entry, iter);
                        /*
                         * ignore error, continue
                         */

It is done on purpose: if a marker does not have the right arguments, it
will simply be skipped and a printk warning will be emitted. I prefer
this approach rather than stopping marker activation/disactivation as
soon as a bad marker is found.



I am not aware of discussions about this. But then we would have to do
the same for paravirt, alternatives, kprobes and immediate values.

As soon as the kernel code of a crashing kernel may have been modified
in memory, the image of the crashed kernel should be used instead of the
compiled image to analyze the crash. But code patching becomes so common
that we will have to start using core dumps more and more.

Yes, your idea makes sense. printking that kind of information in a OOPS
would be useful IMO.

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
-

[ message continues ]