LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [patch 0/4] Markers updates for 2.6.25-rc6
@ 2008-03-20 0:27 Mathieu Desnoyers
2008-03-20 0:27 ` [patch 1/4] Markers - depends on not PREEMPT_RCU Mathieu Desnoyers
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Mathieu Desnoyers @ 2008-03-20 0:27 UTC (permalink / raw)
To: akpm, linux-kernel
Hi Andrew,
Here are a few updates for the kernel markers. One is particularity important :
the combined use of preempt_disable/enable() and call_rcu/rcu_barrier does not
mix well with PREEMPT_RCU. I added a depends !PREEMPT_RCU to the markers to this
configuration is not reached.
The proper fix will imply to implement call_sched and sched_barrier, but I don't
think it will appear before 2.6.25.
It applies on top of 2.6.25-rc6 on top of the make-marker_debug-static.patch
already in your tree in th following order :
markers-depends-on-not-preempt-rcu.patch
markers-add-comments.patch
markers-remove-access-once.patch
markers-support-for-proprietary-modules.patch
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [patch 1/4] Markers - depends on not PREEMPT_RCU
2008-03-20 0:27 [patch 0/4] Markers updates for 2.6.25-rc6 Mathieu Desnoyers
@ 2008-03-20 0:27 ` Mathieu Desnoyers
2008-03-20 19:03 ` Ingo Molnar
2008-03-20 0:27 ` [patch 2/4] Markers - Update preempt_disable. call_rcu, rcu_barrier comments Mathieu Desnoyers
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Mathieu Desnoyers @ 2008-03-20 0:27 UTC (permalink / raw)
To: akpm, linux-kernel
Cc: Mathieu Desnoyers, Christoph Hellwig, Andrew Morton, Mike Mason,
Dipankar Sarma, David Smith, Paul E. McKenney, Steven Rostedt,
Adrian Bunk
[-- Attachment #1: markers-depends-on-not-preempt-rcu.patch --]
[-- Type: text/plain, Size: 1303 bytes --]
Markers do not mix well with CONFIG_PREEMPT_RCU because it uses
preempt_disable/enable() and not rcu_read_lock/unlock for minimal
intrusiveness. We would need call_sched and sched_barrier primitives.
This should be in for 2.6.25.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Christoph Hellwig <hch@infradead.org>
CC: Andrew Morton <akpm@osdl.org>
CC: Mike Mason <mmlnx@us.ibm.com>
CC: Dipankar Sarma <dipankar@in.ibm.com>
CC: David Smith <dsmith@redhat.com>
CC: "Paul E. McKenney" <paulmck@us.ibm.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Adrian Bunk <adrian.bunk@movial.fi>
---
init/Kconfig | 1 +
1 file changed, 1 insertion(+)
Index: linux-2.6-lttng/init/Kconfig
===================================================================
--- linux-2.6-lttng.orig/init/Kconfig 2008-03-19 18:26:47.000000000 -0400
+++ linux-2.6-lttng/init/Kconfig 2008-03-19 18:27:43.000000000 -0400
@@ -742,6 +742,7 @@ config PROFILING
config MARKERS
bool "Activate markers"
+ depends on !PREEMPT_RCU
help
Place an empty function call at each marker site. Can be
dynamically changed for a probe function.
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
^ permalink raw reply [flat|nested] 17+ messages in thread
* [patch 2/4] Markers - Update preempt_disable. call_rcu, rcu_barrier comments.
2008-03-20 0:27 [patch 0/4] Markers updates for 2.6.25-rc6 Mathieu Desnoyers
2008-03-20 0:27 ` [patch 1/4] Markers - depends on not PREEMPT_RCU Mathieu Desnoyers
@ 2008-03-20 0:27 ` Mathieu Desnoyers
2008-03-20 0:27 ` [patch 3/4] Markers - Remove ACCESS_ONCE Mathieu Desnoyers
2008-03-20 0:27 ` [patch 4/4] Markers Support for Proprierary Modules Mathieu Desnoyers
3 siblings, 0 replies; 17+ messages in thread
From: Mathieu Desnoyers @ 2008-03-20 0:27 UTC (permalink / raw)
To: akpm, linux-kernel
Cc: Mathieu Desnoyers, Christoph Hellwig, Andrew Morton, Mike Mason,
Dipankar Sarma, David Smith, Paul E. McKenney, Steven Rostedt,
Adrian Bunk
[-- Attachment #1: markers-update-preempt-disable-call-rcu-rcu-barrier-comments.patch --]
[-- Type: text/plain, Size: 3014 bytes --]
Add comments requested by Andrew.
Updated comments about synchronize_sched(). Since we use call_rcu and
rcu_barrier now, these comments were out of sync with the code.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Christoph Hellwig <hch@infradead.org>
CC: Andrew Morton <akpm@osdl.org>
CC: Mike Mason <mmlnx@us.ibm.com>
CC: Dipankar Sarma <dipankar@in.ibm.com>
CC: David Smith <dsmith@redhat.com>
CC: "Paul E. McKenney" <paulmck@us.ibm.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Adrian Bunk <adrian.bunk@movial.fi>
---
kernel/marker.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
Index: linux-2.6-lttng/kernel/marker.c
===================================================================
--- linux-2.6-lttng.orig/kernel/marker.c 2008-03-19 18:26:47.000000000 -0400
+++ linux-2.6-lttng/kernel/marker.c 2008-03-19 18:30:26.000000000 -0400
@@ -104,9 +104,9 @@ void marker_probe_cb(const struct marker
char ptype;
/*
- * disabling preemption to make sure the teardown of the callbacks can
- * be done correctly when they are in modules and they insure RCU read
- * coherency.
+ * preempt_disable does two things : disabling preemption to make sure
+ * the teardown of the callbacks can be done correctly when they are in
+ * modules and they insure RCU read coherency.
*/
preempt_disable();
ptype = ACCESS_ONCE(mdata->ptype);
@@ -551,9 +551,9 @@ static int set_marker(struct marker_entr
/*
* Disable a marker and its probe callback.
- * Note: only after a synchronize_sched() issued after setting elem->call to the
- * empty function insures that the original callback is not used anymore. This
- * insured by preemption disabling around the call site.
+ * Note: only waiting an RCU period after setting elem->call to the empty
+ * function insures that the original callback is not used anymore. This insured
+ * by preempt_disable around the call site.
*/
static void disable_marker(struct marker *elem)
{
@@ -565,8 +565,8 @@ static void disable_marker(struct marker
elem->ptype = 0; /* single probe */
/*
* Leave the private data and id there, because removal is racy and
- * should be done only after a synchronize_sched(). These are never used
- * until the next initialization anyway.
+ * should be done only after an RCU period. These are never used until
+ * the next initialization anyway.
*/
}
@@ -601,9 +601,6 @@ void marker_update_probe_range(struct ma
/*
* Update probes, removing the faulty probes.
- * Issues a synchronize_sched() when no reference to the module passed
- * as parameter is found in the probes so the probe module can be
- * safely unloaded from now on.
*
* Internal callback only changed before the first probe is connected to it.
* Single probe private data can only be changed on 0 -> 1 and 2 -> 1
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
^ permalink raw reply [flat|nested] 17+ messages in thread
* [patch 3/4] Markers - Remove ACCESS_ONCE
2008-03-20 0:27 [patch 0/4] Markers updates for 2.6.25-rc6 Mathieu Desnoyers
2008-03-20 0:27 ` [patch 1/4] Markers - depends on not PREEMPT_RCU Mathieu Desnoyers
2008-03-20 0:27 ` [patch 2/4] Markers - Update preempt_disable. call_rcu, rcu_barrier comments Mathieu Desnoyers
@ 2008-03-20 0:27 ` Mathieu Desnoyers
2008-03-20 0:27 ` [patch 4/4] Markers Support for Proprierary Modules Mathieu Desnoyers
3 siblings, 0 replies; 17+ messages in thread
From: Mathieu Desnoyers @ 2008-03-20 0:27 UTC (permalink / raw)
To: akpm, linux-kernel
Cc: Mathieu Desnoyers, Christoph Hellwig, Andrew Morton, Mike Mason,
Dipankar Sarma, David Smith, Paul E. McKenney, Steven Rostedt,
Adrian Bunk
[-- Attachment #1: markers-remove-access-once.patch --]
[-- Type: text/plain, Size: 2818 bytes --]
As Paul pointed out, the ACCESS_ONCE are not needed because we already have the
explicit surrounding memory barriers.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Christoph Hellwig <hch@infradead.org>
CC: Andrew Morton <akpm@osdl.org>
CC: Mike Mason <mmlnx@us.ibm.com>
CC: Dipankar Sarma <dipankar@in.ibm.com>
CC: David Smith <dsmith@redhat.com>
CC: "Paul E. McKenney" <paulmck@us.ibm.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Adrian Bunk <adrian.bunk@movial.fi>
---
kernel/marker.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
Index: linux-2.6-lttng/kernel/marker.c
===================================================================
--- linux-2.6-lttng.orig/kernel/marker.c 2008-03-19 18:30:26.000000000 -0400
+++ linux-2.6-lttng/kernel/marker.c 2008-03-19 18:32:16.000000000 -0400
@@ -109,13 +109,13 @@ void marker_probe_cb(const struct marker
* modules and they insure RCU read coherency.
*/
preempt_disable();
- ptype = ACCESS_ONCE(mdata->ptype);
+ ptype = mdata->ptype;
if (likely(!ptype)) {
marker_probe_func *func;
/* Must read the ptype before ptr. They are not data dependant,
* so we put an explicit smp_rmb() here. */
smp_rmb();
- func = ACCESS_ONCE(mdata->single.func);
+ func = mdata->single.func;
/* Must read the ptr before private data. They are not data
* dependant, so we put an explicit smp_rmb() here. */
smp_rmb();
@@ -133,7 +133,7 @@ void marker_probe_cb(const struct marker
* in the fast path, so put the explicit barrier here.
*/
smp_read_barrier_depends();
- multi = ACCESS_ONCE(mdata->multi);
+ multi = mdata->multi;
for (i = 0; multi[i].func; i++) {
va_start(args, fmt);
multi[i].func(multi[i].probe_private, call_private, fmt,
@@ -161,13 +161,13 @@ void marker_probe_cb_noarg(const struct
char ptype;
preempt_disable();
- ptype = ACCESS_ONCE(mdata->ptype);
+ ptype = mdata->ptype;
if (likely(!ptype)) {
marker_probe_func *func;
/* Must read the ptype before ptr. They are not data dependant,
* so we put an explicit smp_rmb() here. */
smp_rmb();
- func = ACCESS_ONCE(mdata->single.func);
+ func = mdata->single.func;
/* Must read the ptr before private data. They are not data
* dependant, so we put an explicit smp_rmb() here. */
smp_rmb();
@@ -183,7 +183,7 @@ void marker_probe_cb_noarg(const struct
* in the fast path, so put the explicit barrier here.
*/
smp_read_barrier_depends();
- multi = ACCESS_ONCE(mdata->multi);
+ multi = mdata->multi;
for (i = 0; multi[i].func; i++)
multi[i].func(multi[i].probe_private, call_private, fmt,
&args);
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
^ permalink raw reply [flat|nested] 17+ messages in thread
* [patch 4/4] Markers Support for Proprierary Modules
2008-03-20 0:27 [patch 0/4] Markers updates for 2.6.25-rc6 Mathieu Desnoyers
` (2 preceding siblings ...)
2008-03-20 0:27 ` [patch 3/4] Markers - Remove ACCESS_ONCE Mathieu Desnoyers
@ 2008-03-20 0:27 ` Mathieu Desnoyers
2008-03-20 5:35 ` Rusty Russell
` (2 more replies)
3 siblings, 3 replies; 17+ messages in thread
From: Mathieu Desnoyers @ 2008-03-20 0:27 UTC (permalink / raw)
To: akpm, linux-kernel
Cc: Mathieu Desnoyers, Jon Masters, Frank Ch. Eigler, Jon Masters,
Rusty Russell, Christoph Hellwig, Linus Torvalds
[-- Attachment #1: markers-support-for-proprietary-modules.patch --]
[-- Type: text/plain, Size: 4706 bytes --]
There seems to be good arguments for markers to support proprierary modules. So
I am throwing this one-liner in and let's see how people react. It only makes
sure that a module that has been "forced" to be loaded won't have its markers
used. It is important to leave this check to make sure the kernel does not crash
by expecting the markers part of the struct module by mistake in the case there
is an incorrect checksum.
Discussion so far :
http://lkml.org/lkml/2008/1/22/226
Jon Masters <jcm@redhat.com> writes:
I notice in module.c:
#ifdef CONFIG_MARKERS
if (!mod->taints)
marker_update_probe_range(mod->markers,
mod->markers + mod->num_markers, NULL, NULL);
#endif
Is this an attempt to not set a marker for proprietary modules? [...]
* Frank Ch. Eigler (fche@redhat.com) wrote:
I can't seem to find any discussion about this aspect. If this is the
intent, it seems misguided to me. There may instead be a relationship
to TAINT_FORCED_{RMMOD,MODULE}. Mathieu?
- FChE
On Tue, 2008-01-22 at 22:10 -0500, Mathieu Desnoyers wrote:
On my part, its mostly a matter of not crashing the kernel when someone
tries to force modprobe of a proprietary module (where the checksums
doesn't match) on a kernel that supports the markers. Not doing so
causes the markers to try to find the marker-specific information in
struct module which doesn't exist and OOPSes.
Christoph's point of view is rather more drastic than mine : it's not
interesting for the kernel community to help proprietary modules writers,
so it's a good idea not to give them marker support. (I CC'ed him so he
can clarify his position).
* Frank Ch. Eigler (fche@redhat.com) wrote:
[...]
Another way of looking at this though is that by allowing/encouraging
proprietary module writers to include markers, we and their users get
new diagnostic capabilities. It constitutes a little bit of opening
up, which IMO we should reward rather than punish.
* Vladis Ketnieks (Valdis.Kletnieks@vt.edu) wrote:
On Wed, 23 Jan 2008 09:48:12 EST, Mathieu Desnoyers said:
> This specific one is a kernel policy matter, and I personally don't
> have a strong opinion about it. I agree that you raise a good counter
> argument : it can be useful to proprietary modules users to be able to
> extract tracing information from those modules to argue with their
> vendors that their driver/hardware is broken (a tracer is _very_ useful
> in that kind of situation).
Amen, brother. Been there, done that, got the tshirt (not on Linux, but
other operating systems).
> However, it is also useful to proprieraty
> module writers who can benefit from the merged kernel/modules traces.
> Do we want to give them this ability ?
The proprietary module writer has the *source* for the kernel and their module.
There's no way you can prevent the proprietary module writers from using this
feature as long as you allow other module writers to use it.
> It would surely help writing
> better proprieraty kernel modules.
The biggest complaint against proprietary modules is that they make it
impossible for *us* to debug. And you want to argue *against* a feature that
would allow them to develop better code that causes less crashes, and therefor
less people *asking* for us to debug it?
Remember - when a user tries a Linux box with a proprietary module, and the
experience sucks because the module sucks, they will walk away thinking
"Linux sucks", not "That module sucks".
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Acked-by: Jon Masters <jcm@jonmasters.org>
CC: "Frank Ch. Eigler" <fche@redhat.com>
CC: Jon Masters <jcm@redhat.com>
CC: Rusty Russell <rusty@rustcorp.com.au>
CC: Christoph Hellwig <hch@infradead.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
---
kernel/module.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6-lttng/kernel/module.c
===================================================================
--- linux-2.6-lttng.orig/kernel/module.c 2008-03-14 08:52:12.000000000 -0400
+++ linux-2.6-lttng/kernel/module.c 2008-03-14 08:54:10.000000000 -0400
@@ -2040,7 +2040,7 @@ static struct module *load_module(void _
add_kallsyms(mod, sechdrs, symindex, strindex, secstrings);
#ifdef CONFIG_MARKERS
- if (!mod->taints)
+ if (!(mod->taints & TAINT_FORCED_MODULE))
marker_update_probe_range(mod->markers,
mod->markers + mod->num_markers);
#endif
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [patch 4/4] Markers Support for Proprierary Modules
2008-03-20 0:27 ` [patch 4/4] Markers Support for Proprierary Modules Mathieu Desnoyers
@ 2008-03-20 5:35 ` Rusty Russell
2008-03-20 12:30 ` Mathieu Desnoyers
2008-03-20 8:38 ` Christoph Hellwig
2008-03-20 19:12 ` Ingo Molnar
2 siblings, 1 reply; 17+ messages in thread
From: Rusty Russell @ 2008-03-20 5:35 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: akpm, linux-kernel, Jon Masters, Frank Ch. Eigler, Jon Masters,
Christoph Hellwig, Linus Torvalds
On Thursday 20 March 2008 11:27:41 Mathieu Desnoyers wrote:
> There seems to be good arguments for markers to support proprierary
> modules. So I am throwing this one-liner in and let's see how people react.
> It only makes sure that a module that has been "forced" to be loaded won't
> have its markers used. It is important to leave this check to make sure the
> kernel does not crash by expecting the markers part of the struct module by
> mistake in the case there is an incorrect checksum.
OK, I've applied this to my tree. That means it will be in 2.6.26 unless you
want me to push it earlier.
Thanks,
Rusty.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [patch 4/4] Markers Support for Proprierary Modules
2008-03-20 0:27 ` [patch 4/4] Markers Support for Proprierary Modules Mathieu Desnoyers
2008-03-20 5:35 ` Rusty Russell
@ 2008-03-20 8:38 ` Christoph Hellwig
2008-03-20 19:12 ` Ingo Molnar
2 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2008-03-20 8:38 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: akpm, linux-kernel, Jon Masters, Frank Ch. Eigler, Jon Masters,
Rusty Russell, Christoph Hellwig, Linus Torvalds
On Wed, Mar 19, 2008 at 08:27:41PM -0400, Mathieu Desnoyers wrote:
> There seems to be good arguments for markers to support proprierary modules. So
> I am throwing this one-liner in and let's see how people react. It only makes
> sure that a module that has been "forced" to be loaded won't have its markers
> used. It is important to leave this check to make sure the kernel does not crash
> by expecting the markers part of the struct module by mistake in the case there
> is an incorrect checksum.
I still disagree. Markers are per defintion a very linux specific API
and thus should stay internal.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [patch 4/4] Markers Support for Proprierary Modules
2008-03-20 5:35 ` Rusty Russell
@ 2008-03-20 12:30 ` Mathieu Desnoyers
0 siblings, 0 replies; 17+ messages in thread
From: Mathieu Desnoyers @ 2008-03-20 12:30 UTC (permalink / raw)
To: Rusty Russell
Cc: akpm, linux-kernel, Jon Masters, Frank Ch. Eigler, Jon Masters,
Christoph Hellwig, Linus Torvalds
* Rusty Russell (rusty@rustcorp.com.au) wrote:
> On Thursday 20 March 2008 11:27:41 Mathieu Desnoyers wrote:
> > There seems to be good arguments for markers to support proprierary
> > modules. So I am throwing this one-liner in and let's see how people react.
> > It only makes sure that a module that has been "forced" to be loaded won't
> > have its markers used. It is important to leave this check to make sure the
> > kernel does not crash by expecting the markers part of the struct module by
> > mistake in the case there is an incorrect checksum.
>
> OK, I've applied this to my tree. That means it will be in 2.6.26 unless you
> want me to push it earlier.
>
> Thanks,
> Rusty.
I think it's ok to wait for 2.6.26, since we are already very far in the
2.6.25 release process and it's more a "feature" than a "bugfix".
Thanks,
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [patch 1/4] Markers - depends on not PREEMPT_RCU
2008-03-20 0:27 ` [patch 1/4] Markers - depends on not PREEMPT_RCU Mathieu Desnoyers
@ 2008-03-20 19:03 ` Ingo Molnar
2008-03-20 19:49 ` Paul E. McKenney
0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2008-03-20 19:03 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: akpm, linux-kernel, Christoph Hellwig, Andrew Morton, Mike Mason,
Dipankar Sarma, David Smith, Paul E. McKenney, Steven Rostedt,
Adrian Bunk, Peter Zijlstra
* Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
> +++ linux-2.6-lttng/init/Kconfig 2008-03-19 18:27:43.000000000 -0400
> @@ -742,6 +742,7 @@ config PROFILING
>
> config MARKERS
> bool "Activate markers"
> + depends on !PREEMPT_RCU
> help
Ugh, NACK. Please solve this properly... What kind of kernel
infrastructure is it what disables itself in the presence of another
piece of kernel infrastructure?
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [patch 4/4] Markers Support for Proprierary Modules
2008-03-20 0:27 ` [patch 4/4] Markers Support for Proprierary Modules Mathieu Desnoyers
2008-03-20 5:35 ` Rusty Russell
2008-03-20 8:38 ` Christoph Hellwig
@ 2008-03-20 19:12 ` Ingo Molnar
2008-03-20 19:18 ` Jon Masters
` (2 more replies)
2 siblings, 3 replies; 17+ messages in thread
From: Ingo Molnar @ 2008-03-20 19:12 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: akpm, linux-kernel, Jon Masters, Frank Ch. Eigler, Jon Masters,
Rusty Russell, Christoph Hellwig, Linus Torvalds
* Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
> There seems to be good arguments for markers to support proprierary
> modules. So I am throwing this one-liner in and let's see how people
> react. [...]
ugh, this is unbelievably stupid move technically - so a very strong
NACK. Allowing marker use in unfixable modules (today it's placing
markers into unfixable modules, tomorrow it's marker use by such
modules) has only one clear and predictable effect: it turns marker
calls into essential ABIs because when faced with any breakage in an
unfixable module that makes use of a marker in some kernel subsystem
then all the pressure is on those who _can_ fix their code - meaning the
kernel subsystem maintainers that use markers.
unfixable modules should only be allowed access to easy things they can
access anyway, or to such fundamental things which we wont realistically
change anyway. Markers are neither.
(i also find it puzzling why you go out on a limb helping a piece of
_irrelevant_ technology that has been the unparalleled source of pain
and anguish to both kernel users and kernel developers.)
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [patch 4/4] Markers Support for Proprierary Modules
2008-03-20 19:12 ` Ingo Molnar
@ 2008-03-20 19:18 ` Jon Masters
2008-03-20 22:15 ` Ingo Molnar
2008-03-20 20:17 ` Frank Ch. Eigler
2008-03-20 22:22 ` Mathieu Desnoyers
2 siblings, 1 reply; 17+ messages in thread
From: Jon Masters @ 2008-03-20 19:18 UTC (permalink / raw)
To: Ingo Molnar
Cc: Mathieu Desnoyers, akpm, linux-kernel, Jon Masters,
Frank Ch. Eigler, Rusty Russell, Christoph Hellwig,
Linus Torvalds
On Thu, 2008-03-20 at 20:12 +0100, Ingo Molnar wrote:
> * Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
>
> > There seems to be good arguments for markers to support proprierary
> > modules. So I am throwing this one-liner in and let's see how people
> > react. [...]
>
> ugh, this is unbelievably stupid move technically - so a very strong
> NACK. Allowing marker use in unfixable modules (today it's placing
> markers into unfixable modules, tomorrow it's marker use by such
> modules) has only one clear and predictable effect: it turns marker
> calls into essential ABIs because when faced with any breakage in an
> unfixable module that makes use of a marker in some kernel subsystem
> then all the pressure is on those who _can_ fix their code - meaning the
> kernel subsystem maintainers that use markers.
Mathieu's previous comment was that this was to help improve the quality
of such drivers. Out of interest, why do you dislike markers so much?
Jon.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [patch 1/4] Markers - depends on not PREEMPT_RCU
2008-03-20 19:03 ` Ingo Molnar
@ 2008-03-20 19:49 ` Paul E. McKenney
0 siblings, 0 replies; 17+ messages in thread
From: Paul E. McKenney @ 2008-03-20 19:49 UTC (permalink / raw)
To: Ingo Molnar
Cc: Mathieu Desnoyers, akpm, linux-kernel, Christoph Hellwig,
Andrew Morton, Mike Mason, Dipankar Sarma, David Smith,
Steven Rostedt, Adrian Bunk, Peter Zijlstra
On Thu, Mar 20, 2008 at 08:03:38PM +0100, Ingo Molnar wrote:
>
> * Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
>
> > +++ linux-2.6-lttng/init/Kconfig 2008-03-19 18:27:43.000000000 -0400
> > @@ -742,6 +742,7 @@ config PROFILING
> >
> > config MARKERS
> > bool "Activate markers"
> > + depends on !PREEMPT_RCU
> > help
>
> Ugh, NACK. Please solve this properly... What kind of kernel
> infrastructure is it what disables itself in the presence of another
> piece of kernel infrastructure?
This is a reminder to me to get going on call_rcu_sched(). Until
I get that done and in, they don't have what they need to work
with preemptable RCU. :-/
Thanx, Paul
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [patch 4/4] Markers Support for Proprierary Modules
2008-03-20 19:12 ` Ingo Molnar
2008-03-20 19:18 ` Jon Masters
@ 2008-03-20 20:17 ` Frank Ch. Eigler
2008-03-20 22:05 ` Ingo Molnar
2008-03-20 22:22 ` Mathieu Desnoyers
2 siblings, 1 reply; 17+ messages in thread
From: Frank Ch. Eigler @ 2008-03-20 20:17 UTC (permalink / raw)
To: Ingo Molnar
Cc: Mathieu Desnoyers, akpm, linux-kernel, Jon Masters, Jon Masters,
Rusty Russell, Christoph Hellwig, Linus Torvalds
Ingo Molnar <mingo@elte.hu> writes:
>> There seems to be good arguments for markers to support proprierary
>> modules. So I am throwing this one-liner in and let's see how people
>> react. [...]
>
> ugh, this is unbelievably stupid move technically - so a very strong
> NACK. Allowing marker use in unfixable modules (today it's placing
> markers into unfixable modules,
As the thread suggested, this can benefit us more than it benefits
them, because it may let us see more into the blobs.
> tomorrow it's marker use by such modules) has only one clear and
> predictable effect: it turns marker calls into essential ABIs [...]
The marker_probe_*register calls are already EXPORT_SYMBOL_GPL'd, so
that covers your "tomorrow" case. NACK that all you like when/if
someone proposes changing that.
> [if the proprietary modules attach to kernel markers ...] then all
> the pressure is on those who _can_ fix their code - meaning the
> kernel subsystem maintainers that use [you mean: define] markers.
(In a way, it would be a nice problem to have. At this moment, there
are still no markers actually committed within -mm nor -linus.)
- FChE
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [patch 4/4] Markers Support for Proprierary Modules
2008-03-20 20:17 ` Frank Ch. Eigler
@ 2008-03-20 22:05 ` Ingo Molnar
2008-03-20 22:24 ` Mathieu Desnoyers
0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2008-03-20 22:05 UTC (permalink / raw)
To: Frank Ch. Eigler
Cc: Mathieu Desnoyers, akpm, linux-kernel, Jon Masters, Jon Masters,
Rusty Russell, Christoph Hellwig, Linus Torvalds
* Frank Ch. Eigler <fche@redhat.com> wrote:
> Ingo Molnar <mingo@elte.hu> writes:
>
> >> There seems to be good arguments for markers to support proprierary
> >> modules. So I am throwing this one-liner in and let's see how people
> >> react. [...]
> >
> > ugh, this is unbelievably stupid move technically - so a very strong
> > NACK. Allowing marker use in unfixable modules (today it's placing
> > markers into unfixable modules,
>
> As the thread suggested, this can benefit us more than it benefits
> them, because it may let us see more into the blobs.
>
> > tomorrow it's marker use by such modules) has only one clear and
> > predictable effect: it turns marker calls into essential ABIs [...]
>
> The marker_probe_*register calls are already EXPORT_SYMBOL_GPL'd, so
> that covers your "tomorrow" case. NACK that all you like when/if
> someone proposes changing that.
i very much know that they are exported that way. It's the concept i'm
against - dont we have 9 million lines of proper kernel source code to
worry about? Why are we even arguing about this? Binary modules should
be as isolated as possible - it's a totally untrusted entity and history
has shown it again and again that the less infrastructure coupling we
have to them, the better.
> > [if the proprietary modules attach to kernel markers ...] then all
> > the pressure is on those who _can_ fix their code - meaning the
> > kernel subsystem maintainers that use [you mean: define] markers.
>
> (In a way, it would be a nice problem to have. At this moment, there
> are still no markers actually committed within -mm nor -linus.)
... which makes it doubly problematic to expose them to binary-only
modules in any way, shape or form. Really, once _any_ kernel facility is
used by such a module, it's pain for us to change it from that point on.
Once markers are a 10 year concept that nobody in their right mind would
want to change, sure, we dont _care_ about whether it's export or not,
and basic courtesy might say that it's OK to do it. But to proactively
export any aspect of a half-done piece of infrastructure is crazy.
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [patch 4/4] Markers Support for Proprierary Modules
2008-03-20 19:18 ` Jon Masters
@ 2008-03-20 22:15 ` Ingo Molnar
0 siblings, 0 replies; 17+ messages in thread
From: Ingo Molnar @ 2008-03-20 22:15 UTC (permalink / raw)
To: Jon Masters
Cc: Mathieu Desnoyers, akpm, linux-kernel, Jon Masters,
Frank Ch. Eigler, Rusty Russell, Christoph Hellwig,
Linus Torvalds
* Jon Masters <jcm@redhat.com> wrote:
> > > There seems to be good arguments for markers to support
> > > proprierary modules. So I am throwing this one-liner in and let's
> > > see how people react. [...]
> >
> > ugh, this is unbelievably stupid move technically - so a very strong
> > NACK. Allowing marker use in unfixable modules (today it's placing
> > markers into unfixable modules, tomorrow it's marker use by such
> > modules) has only one clear and predictable effect: it turns marker
> > calls into essential ABIs because when faced with any breakage in an
> > unfixable module that makes use of a marker in some kernel subsystem
> > then all the pressure is on those who _can_ fix their code - meaning
> > the kernel subsystem maintainers that use markers.
>
> Mathieu's previous comment was that this was to help improve the
> quality of such drivers. Out of interest, why do you dislike markers
> so much?
i'm not particularly interested in improving the quality of such
drivers. I'm interested in improving the quality of _open_ code. And not
making too many promises in advance about how our kernel internals will
look like in the future is a fundamental aspect of that.
So i have no problems with export trivial or cast-into-stone aspects of
the kernel - doing that simply has no negative effects on open code. But
the details of markers are far from settled and far from trivial.
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [patch 4/4] Markers Support for Proprierary Modules
2008-03-20 19:12 ` Ingo Molnar
2008-03-20 19:18 ` Jon Masters
2008-03-20 20:17 ` Frank Ch. Eigler
@ 2008-03-20 22:22 ` Mathieu Desnoyers
2 siblings, 0 replies; 17+ messages in thread
From: Mathieu Desnoyers @ 2008-03-20 22:22 UTC (permalink / raw)
To: Ingo Molnar
Cc: akpm, linux-kernel, Jon Masters, Frank Ch. Eigler, Jon Masters,
Rusty Russell, Christoph Hellwig, Linus Torvalds
* Ingo Molnar (mingo@elte.hu) wrote:
>
> * Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
>
> > There seems to be good arguments for markers to support proprierary
> > modules. So I am throwing this one-liner in and let's see how people
> > react. [...]
>
> ugh, this is unbelievably stupid move technically - so a very strong
> NACK. Allowing marker use in unfixable modules (today it's placing
> markers into unfixable modules, tomorrow it's marker use by such
> modules) has only one clear and predictable effect: it turns marker
> calls into essential ABIs because when faced with any breakage in an
> unfixable module that makes use of a marker in some kernel subsystem
> then all the pressure is on those who _can_ fix their code - meaning the
> kernel subsystem maintainers that use markers.
>
> unfixable modules should only be allowed access to easy things they can
> access anyway, or to such fundamental things which we wont realistically
> change anyway. Markers are neither.
>
> (i also find it puzzling why you go out on a limb helping a piece of
> _irrelevant_ technology that has been the unparalleled source of pain
> and anguish to both kernel users and kernel developers.)
>
> Ingo
Please note that this patch has a single purpose : to let proprietary
modules define markers to *export* information. The opposite (connect
callbacks to markers) is not allowed since the rest of the markers API
is EXPORT_SYMBOL_GPL'd.
I would also be strongly against letting proprietary modules access the
information provided by the markers. However, I think it's only useful
for the end user to let proprietary modules open up a bit, considering
that proprierary module writers can use the markers as they want
in-house, but would have to leave them disabled on shipped kernels.
As far as I am concerned, I want to help the end user, not the
technology itself.
Unless I have a proof that markers in proprietary modules (information
*providers* only) would be a pain to maintain, I won't object against
supporting proprietary 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [patch 4/4] Markers Support for Proprierary Modules
2008-03-20 22:05 ` Ingo Molnar
@ 2008-03-20 22:24 ` Mathieu Desnoyers
0 siblings, 0 replies; 17+ messages in thread
From: Mathieu Desnoyers @ 2008-03-20 22:24 UTC (permalink / raw)
To: Ingo Molnar
Cc: Frank Ch. Eigler, akpm, linux-kernel, Jon Masters, Jon Masters,
Rusty Russell, Christoph Hellwig, Linus Torvalds
* Ingo Molnar (mingo@elte.hu) wrote:
>
> * Frank Ch. Eigler <fche@redhat.com> wrote:
>
> > Ingo Molnar <mingo@elte.hu> writes:
> >
[...]
> > > [if the proprietary modules attach to kernel markers ...] then all
> > > the pressure is on those who _can_ fix their code - meaning the
> > > kernel subsystem maintainers that use [you mean: define] markers.
> >
> > (In a way, it would be a nice problem to have. At this moment, there
> > are still no markers actually committed within -mm nor -linus.)
>
> ... which makes it doubly problematic to expose them to binary-only
> modules in any way, shape or form. Really, once _any_ kernel facility is
> used by such a module, it's pain for us to change it from that point on.
> Once markers are a 10 year concept that nobody in their right mind would
> want to change, sure, we dont _care_ about whether it's export or not,
> and basic courtesy might say that it's OK to do it. But to proactively
> export any aspect of a half-done piece of infrastructure is crazy.
>
> Ingo
I agree with you on this : maybe it would be better to wait a bit and
let the core markers in first and learn from that before we open this up
to proprierary 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
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2008-03-20 22:25 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-03-20 0:27 [patch 0/4] Markers updates for 2.6.25-rc6 Mathieu Desnoyers
2008-03-20 0:27 ` [patch 1/4] Markers - depends on not PREEMPT_RCU Mathieu Desnoyers
2008-03-20 19:03 ` Ingo Molnar
2008-03-20 19:49 ` Paul E. McKenney
2008-03-20 0:27 ` [patch 2/4] Markers - Update preempt_disable. call_rcu, rcu_barrier comments Mathieu Desnoyers
2008-03-20 0:27 ` [patch 3/4] Markers - Remove ACCESS_ONCE Mathieu Desnoyers
2008-03-20 0:27 ` [patch 4/4] Markers Support for Proprierary Modules Mathieu Desnoyers
2008-03-20 5:35 ` Rusty Russell
2008-03-20 12:30 ` Mathieu Desnoyers
2008-03-20 8:38 ` Christoph Hellwig
2008-03-20 19:12 ` Ingo Molnar
2008-03-20 19:18 ` Jon Masters
2008-03-20 22:15 ` Ingo Molnar
2008-03-20 20:17 ` Frank Ch. Eigler
2008-03-20 22:05 ` Ingo Molnar
2008-03-20 22:24 ` Mathieu Desnoyers
2008-03-20 22:22 ` Mathieu Desnoyers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).