[oops -tip] : x86 AMD 64

[oops -tip] : x86 AMD 64

[Jaswinder Singh Rajput]

Good: f4c3c4cdb1de232
Bad : 1e08816af0bc345

Config:
http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318

oops:
http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg

<freeze>

Thanks,

--
JSR

oops in tracepoint_update_probe_range() (was: Re: [oops -tip] : x86 AMD 64)

[Ingo Molnar]

* Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:

> Good: f4c3c4cdb1de232
> Bad : 1e08816af0bc345
> 
> Config:
> http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318
> 
> oops:
> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg
> 
> <freeze>

Steve, Frederic - the crashes above are in:

	tracepoint_update_probe_range()

in a modular kernel apparently.

	Ingo

Re: oops in tracepoint_update_probe_range() (was: Re: [oops -tip] : x86 AMD 64)

[Frederic Weisbecker]

On Wed, Mar 18, 2009 at 05:35:31PM +0100, Ingo Molnar wrote:
> 
> * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> 
> > Good: f4c3c4cdb1de232
> > Bad : 1e08816af0bc345
> > 
> > Config:
> > http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318
> > 
> > oops:
> > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
> > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
> > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
> > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg
> > 
> > <freeze>
> 
> Steve, Frederic - the crashes above are in:
> 
> 	tracepoint_update_probe_range()
> 
> in a modular kernel apparently.
> 
> 	Ingo

Darn, ok I test this one.

Re: oops in tracepoint_update_probe_range() (was: Re: [oops -tip] : x86 AMD 64)

[Jaswinder Singh Rajput]

On Wed, 2009-03-18 at 17:35 +0100, Ingo Molnar wrote:
> * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> 
> > Good: f4c3c4cdb1de232
> > Bad : 1e08816af0bc345
> > 
> > Config:
> > http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318
> > 
> > oops:
> > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
> > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
> > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
> > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg
> > 
> > <freeze>
> 
> Steve, Frederic - the crashes above are in:
> 
> 	tracepoint_update_probe_range()
> 
> in a modular kernel apparently.
> 

This fixed the oops for me, Is this looks OK to you:

Subject: [PATCH] x86: tracepoint.c fix oops

BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
PGD 13d5fb067 PUD 13d688067 PMD 0
Oops: 0000 [#1] SMP

Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
---
 kernel/tracepoint.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 7960274..80d1353 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -280,6 +280,8 @@ void tracepoint_update_probe_range(struct tracepoint *begin,
 
 	mutex_lock(&tracepoints_mutex);
 	for (iter = begin; iter < end; iter++) {
+		if (!iter)
+			goto out;
 		mark_entry = get_tracepoint(iter->name);
 		if (mark_entry) {
 			set_tracepoint(&mark_entry, iter,
@@ -288,6 +290,7 @@ void tracepoint_update_probe_range(struct tracepoint *begin,
 			disable_tracepoint(iter);
 		}
 	}
+out:
 	mutex_unlock(&tracepoints_mutex);
 }
 
-- 
1.6.0.6

Re: oops in tracepoint_update_probe_range() (was: Re: [oops -tip] : x86 AMD 64)

[Ingo Molnar]

* Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:

> On Wed, 2009-03-18 at 17:35 +0100, Ingo Molnar wrote:
> > * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> > 
> > > Good: f4c3c4cdb1de232
> > > Bad : 1e08816af0bc345
> > > 
> > > Config:
> > > http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318
> > > 
> > > oops:
> > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
> > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
> > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
> > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg
> > > 
> > > <freeze>
> > 
> > Steve, Frederic - the crashes above are in:
> > 
> > 	tracepoint_update_probe_range()
> > 
> > in a modular kernel apparently.
> > 
> 
> This fixed the oops for me, Is this looks OK to you:
> 
> Subject: [PATCH] x86: tracepoint.c fix oops
> 
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
> PGD 13d5fb067 PUD 13d688067 PMD 0
> Oops: 0000 [#1] SMP
> 
> Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> ---
>  kernel/tracepoint.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 7960274..80d1353 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -280,6 +280,8 @@ void tracepoint_update_probe_range(struct tracepoint *begin,
>  
>  	mutex_lock(&tracepoints_mutex);
>  	for (iter = begin; iter < end; iter++) {
> +		if (!iter)
> +			goto out;
>  		mark_entry = get_tracepoint(iter->name);
>  		if (mark_entry) {
>  			set_tracepoint(&mark_entry, iter,
> @@ -288,6 +290,7 @@ void tracepoint_update_probe_range(struct tracepoint *begin,
>  			disable_tracepoint(iter);
>  		}
>  	}
> +out:
>  	mutex_unlock(&tracepoints_mutex);

hm, that does not look right - how can 'iter' become zero? It's a 
text-ish symbol.

I think what we might have here is a corruption of the tracepoint 
data structures.

	Ingo

Re: oops in tracepoint_update_probe_range() (was: Re: [oops -tip] : x86 AMD 64)

[Frederic Weisbecker]

On Wed, Mar 18, 2009 at 10:18:56PM +0530, Jaswinder Singh Rajput wrote:
> On Wed, 2009-03-18 at 17:35 +0100, Ingo Molnar wrote:
> > * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> > 
> > > Good: f4c3c4cdb1de232
> > > Bad : 1e08816af0bc345
> > > 
> > > Config:
> > > http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318
> > > 
> > > oops:
> > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
> > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
> > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
> > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg
> > > 
> > > <freeze>
> > 
> > Steve, Frederic - the crashes above are in:
> > 
> > 	tracepoint_update_probe_range()
> > 
> > in a modular kernel apparently.
> > 
> 
> This fixed the oops for me, Is this looks OK to you:
> 
> Subject: [PATCH] x86: tracepoint.c fix oops
> 
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
> PGD 13d5fb067 PUD 13d688067 PMD 0
> Oops: 0000 [#1] SMP
> 
> Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> ---
>  kernel/tracepoint.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 7960274..80d1353 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -280,6 +280,8 @@ void tracepoint_update_probe_range(struct tracepoint *begin,
>  
>  	mutex_lock(&tracepoints_mutex);
>  	for (iter = begin; iter < end; iter++) {
> +		if (!iter)
> +			goto out;
>  		mark_entry = get_tracepoint(iter->name);
>  		if (mark_entry) {
>  			set_tracepoint(&mark_entry, iter,
> @@ -288,6 +290,7 @@ void tracepoint_update_probe_range(struct tracepoint *begin,
>  			disable_tracepoint(iter);
>  		}
>  	}
> +out:
>  	mutex_unlock(&tracepoints_mutex);
>  }


Ok, it should fix the crash.
But I think the real problem remains: iter is not supposed to point to NULL,
this is a section range:

tracepoint_update_probe_range(__start___tracepoints,
		__stop___tracepoints);

It seems to mean that the section is empty.

  
> -- 
> 1.6.0.6
> 
> 
> 

Re: oops in tracepoint_update_probe_range() (was: Re: [oops -tip] : x86 AMD 64)

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Wed, Mar 18, 2009 at 10:18:56PM +0530, Jaswinder Singh Rajput wrote:
> > On Wed, 2009-03-18 at 17:35 +0100, Ingo Molnar wrote:
> > > * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> > > 
> > > > Good: f4c3c4cdb1de232
> > > > Bad : 1e08816af0bc345
> > > > 
> > > > Config:
> > > > http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318
> > > > 
> > > > oops:
> > > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
> > > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
> > > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
> > > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg
> > > > 
> > > > <freeze>
> > > 
> > > Steve, Frederic - the crashes above are in:
> > > 
> > > 	tracepoint_update_probe_range()
> > > 
> > > in a modular kernel apparently.
> > > 
> > 
> > This fixed the oops for me, Is this looks OK to you:
> > 
> > Subject: [PATCH] x86: tracepoint.c fix oops
> > 
> > BUG: unable to handle kernel NULL pointer dereference at (null)
> > IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
> > PGD 13d5fb067 PUD 13d688067 PMD 0
> > Oops: 0000 [#1] SMP
> > 
> > Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> > ---
> >  kernel/tracepoint.c |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> > 
> > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > index 7960274..80d1353 100644
> > --- a/kernel/tracepoint.c
> > +++ b/kernel/tracepoint.c
> > @@ -280,6 +280,8 @@ void tracepoint_update_probe_range(struct tracepoint *begin,
> >  
> >  	mutex_lock(&tracepoints_mutex);
> >  	for (iter = begin; iter < end; iter++) {
> > +		if (!iter)
> > +			goto out;
> >  		mark_entry = get_tracepoint(iter->name);
> >  		if (mark_entry) {
> >  			set_tracepoint(&mark_entry, iter,
> > @@ -288,6 +290,7 @@ void tracepoint_update_probe_range(struct tracepoint *begin,
> >  			disable_tracepoint(iter);
> >  		}
> >  	}
> > +out:
> >  	mutex_unlock(&tracepoints_mutex);
> >  }
> 
> 
> Ok, it should fix the crash.
> But I think the real problem remains: iter is not supposed to point to NULL,
> this is a section range:
> 
> tracepoint_update_probe_range(__start___tracepoints,
> 		__stop___tracepoints);
> 
> It seems to mean that the section is empty.

OK - so checking for !iter on entry and emitting a WARN_ONCE in that 
case ought to change the crash for a warning, right?

	Ingo

Re: oops in tracepoint_update_probe_range() (was: Re: [oops -tip] : x86 AMD 64)

[Frederic Weisbecker]

On Wed, Mar 18, 2009 at 06:27:50PM +0100, Ingo Molnar wrote:
> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > On Wed, Mar 18, 2009 at 10:18:56PM +0530, Jaswinder Singh Rajput wrote:
> > > On Wed, 2009-03-18 at 17:35 +0100, Ingo Molnar wrote:
> > > > * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> > > > 
> > > > > Good: f4c3c4cdb1de232
> > > > > Bad : 1e08816af0bc345
> > > > > 
> > > > > Config:
> > > > > http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318
> > > > > 
> > > > > oops:
> > > > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
> > > > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
> > > > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
> > > > > http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg
> > > > > 
> > > > > <freeze>
> > > > 
> > > > Steve, Frederic - the crashes above are in:
> > > > 
> > > > 	tracepoint_update_probe_range()
> > > > 
> > > > in a modular kernel apparently.
> > > > 
> > > 
> > > This fixed the oops for me, Is this looks OK to you:
> > > 
> > > Subject: [PATCH] x86: tracepoint.c fix oops
> > > 
> > > BUG: unable to handle kernel NULL pointer dereference at (null)
> > > IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
> > > PGD 13d5fb067 PUD 13d688067 PMD 0
> > > Oops: 0000 [#1] SMP
> > > 
> > > Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> > > ---
> > >  kernel/tracepoint.c |    3 +++
> > >  1 files changed, 3 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > > index 7960274..80d1353 100644
> > > --- a/kernel/tracepoint.c
> > > +++ b/kernel/tracepoint.c
> > > @@ -280,6 +280,8 @@ void tracepoint_update_probe_range(struct tracepoint *begin,
> > >  
> > >  	mutex_lock(&tracepoints_mutex);
> > >  	for (iter = begin; iter < end; iter++) {
> > > +		if (!iter)
> > > +			goto out;
> > >  		mark_entry = get_tracepoint(iter->name);
> > >  		if (mark_entry) {
> > >  			set_tracepoint(&mark_entry, iter,
> > > @@ -288,6 +290,7 @@ void tracepoint_update_probe_range(struct tracepoint *begin,
> > >  			disable_tracepoint(iter);
> > >  		}
> > >  	}
> > > +out:
> > >  	mutex_unlock(&tracepoints_mutex);
> > >  }
> > 
> > 
> > Ok, it should fix the crash.
> > But I think the real problem remains: iter is not supposed to point to NULL,
> > this is a section range:
> > 
> > tracepoint_update_probe_range(__start___tracepoints,
> > 		__stop___tracepoints);
> > 
> > It seems to mean that the section is empty.
> 
> OK - so checking for !iter on entry and emitting a WARN_ONCE in that 
> case ought to change the crash for a warning, right?
> 
> 	Ingo


The real bug is elsewhere, Jaswinder has one user of tracepoints
which is blktrace.
So this section is not supposed to be empty.

But, I guess it is possible to raise via a randconfig by enabling
CONFIG_TRACEPOINTS without any user of it because when a module
is loaded, the section is checked without the !NULL safety.

Thus I think Jaswinder's patch could be picked without WARN or anything,
because this situation can happen in a normal randconfig case.

Concerning the real bug (there is blktrace here, so this NULL is weird)
I'm building Jaswinder's config, may be I will find something...

[tip:tracing/ftrace] tracing: fix oops in tracepoint_update_probe_range()

[Jaswinder Singh Rajput]

Commit-ID:  7d1832698e6e422cc2cf0d80b9c50ca567e758a3
Gitweb:     http://git.kernel.org/tip/7d1832698e6e422cc2cf0d80b9c50ca567e758a3
Author:     Jaswinder Singh Rajput <jaswinder@kernel.org>
AuthorDate: Wed, 18 Mar 2009 22:18:56 +0530
Commit:     Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 18 Mar 2009 18:30:43 +0100

tracing: fix oops in tracepoint_update_probe_range()

Change this crash:

 BUG: unable to handle kernel NULL pointer dereference at (null)
 IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
 PGD 13d5fb067 PUD 13d688067 PMD 0
 Oops: 0000 [#1] SMP

To a more debuggable WARN_ONCE().

Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <1237394936.3132.1.camel@localhost.localdomain>
[ moved the check outside the lock and added a WARN_ON(). ]
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 kernel/tracepoint.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 7960274..8bc1a06 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -272,12 +272,17 @@ static void disable_tracepoint(struct tracepoint *elem)
  *
  * Updates the probe callback corresponding to a range of tracepoints.
  */
-void tracepoint_update_probe_range(struct tracepoint *begin,
-	struct tracepoint *end)
+void
+tracepoint_update_probe_range(struct tracepoint *begin, struct tracepoint *end)
 {
 	struct tracepoint *iter;
 	struct tracepoint_entry *mark_entry;
 
+	if (!iter) {
+		WARN_ON_ONCE(1);
+		goto out;
+	}
+
 	mutex_lock(&tracepoints_mutex);
 	for (iter = begin; iter < end; iter++) {
 		mark_entry = get_tracepoint(iter->name);

Re: [tip:tracing/ftrace] tracing: fix oops in tracepoint_update_probe_range()

[Jaswinder Singh Rajput]

On Wed, 2009-03-18 at 17:33 +0000, Jaswinder Singh Rajput wrote:
> Commit-ID:  7d1832698e6e422cc2cf0d80b9c50ca567e758a3
> Gitweb:     http://git.kernel.org/tip/7d1832698e6e422cc2cf0d80b9c50ca567e758a3
> Author:     Jaswinder Singh Rajput <jaswinder@kernel.org>
> AuthorDate: Wed, 18 Mar 2009 22:18:56 +0530
> Commit:     Ingo Molnar <mingo@elte.hu>
> CommitDate: Wed, 18 Mar 2009 18:30:43 +0100
> 
> tracing: fix oops in tracepoint_update_probe_range()
> 
> Change this crash:
> 
>  BUG: unable to handle kernel NULL pointer dereference at (null)
>  IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
>  PGD 13d5fb067 PUD 13d688067 PMD 0
>  Oops: 0000 [#1] SMP
> 
> To a more debuggable WARN_ONCE().
> 
> Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> LKML-Reference: <1237394936.3132.1.camel@localhost.localdomain>
> [ moved the check outside the lock and added a WARN_ON(). ]
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 
> 
> ---
>  kernel/tracepoint.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 7960274..8bc1a06 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -272,12 +272,17 @@ static void disable_tracepoint(struct tracepoint *elem)
>   *
>   * Updates the probe callback corresponding to a range of tracepoints.
>   */
> -void tracepoint_update_probe_range(struct tracepoint *begin,
> -	struct tracepoint *end)
> +void
> +tracepoint_update_probe_range(struct tracepoint *begin, struct tracepoint *end)
>  {
>  	struct tracepoint *iter;
>  	struct tracepoint_entry *mark_entry;
>  
> +	if (!iter) {
> +		WARN_ON_ONCE(1);
> +		goto out;
> +	}
> +

There is no out, it should be :

+	if (!iter) {
+		WARN_ON_ONCE(1);
+		return;
+	}
+

Re: [tip:tracing/ftrace] tracing: fix oops in tracepoint_update_probe_range()

[Jaswinder Singh Rajput]

On Wed, 2009-03-18 at 23:08 +0530, Jaswinder Singh Rajput wrote:
> On Wed, 2009-03-18 at 17:33 +0000, Jaswinder Singh Rajput wrote:
> > Commit-ID:  7d1832698e6e422cc2cf0d80b9c50ca567e758a3
> > Gitweb:     http://git.kernel.org/tip/7d1832698e6e422cc2cf0d80b9c50ca567e758a3
> > Author:     Jaswinder Singh Rajput <jaswinder@kernel.org>
> > AuthorDate: Wed, 18 Mar 2009 22:18:56 +0530
> > Commit:     Ingo Molnar <mingo@elte.hu>
> > CommitDate: Wed, 18 Mar 2009 18:30:43 +0100
> > 
> > tracing: fix oops in tracepoint_update_probe_range()
> > 
> > Change this crash:
> > 
> >  BUG: unable to handle kernel NULL pointer dereference at (null)
> >  IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
> >  PGD 13d5fb067 PUD 13d688067 PMD 0
> >  Oops: 0000 [#1] SMP
> > 
> > To a more debuggable WARN_ONCE().
> > 
> > Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > LKML-Reference: <1237394936.3132.1.camel@localhost.localdomain>
> > [ moved the check outside the lock and added a WARN_ON(). ]
> > Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > 
> > 
> > ---
> >  kernel/tracepoint.c |    9 +++++++--
> >  1 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > index 7960274..8bc1a06 100644
> > --- a/kernel/tracepoint.c
> > +++ b/kernel/tracepoint.c
> > @@ -272,12 +272,17 @@ static void disable_tracepoint(struct tracepoint *elem)
> >   *
> >   * Updates the probe callback corresponding to a range of tracepoints.
> >   */
> > -void tracepoint_update_probe_range(struct tracepoint *begin,
> > -	struct tracepoint *end)
> > +void
> > +tracepoint_update_probe_range(struct tracepoint *begin, struct tracepoint *end)
> >  {
> >  	struct tracepoint *iter;
> >  	struct tracepoint_entry *mark_entry;
> >  
> > +	if (!iter) {
> > +		WARN_ON_ONCE(1);
> > +		goto out;
> > +	}
> > +
> 
> There is no out, it should be :
> 
> +	if (!iter) {
> +		WARN_ON_ONCE(1);
> +		return;
> +	}
> +
> 

hmm, who is setting iter :p

--
JSR

[tip:tracing/ftrace] tracing: fix oops in tracepoint_update_probe_range()

[Jaswinder Singh Rajput]

Commit-ID:  966a6fdf6210e3ac8ce00b61cd1107cdf97ce744
Gitweb:     http://git.kernel.org/tip/966a6fdf6210e3ac8ce00b61cd1107cdf97ce744
Author:     Jaswinder Singh Rajput <jaswinder@kernel.org>
AuthorDate: Wed, 18 Mar 2009 22:18:56 +0530
Commit:     Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 18 Mar 2009 18:48:43 +0100

tracing: fix oops in tracepoint_update_probe_range()

Change this crash:

 BUG: unable to handle kernel NULL pointer dereference at (null)
 IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
 PGD 13d5fb067 PUD 13d688067 PMD 0
 Oops: 0000 [#1] SMP

To a more debuggable WARN_ONCE().

Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <1237394936.3132.1.camel@localhost.localdomain>
[ moved the check outside the lock and added a WARN_ON(). ]
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 kernel/tracepoint.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 7960274..dd15df9 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -272,12 +272,17 @@ static void disable_tracepoint(struct tracepoint *elem)
  *
  * Updates the probe callback corresponding to a range of tracepoints.
  */
-void tracepoint_update_probe_range(struct tracepoint *begin,
-	struct tracepoint *end)
+void
+tracepoint_update_probe_range(struct tracepoint *begin, struct tracepoint *end)
 {
 	struct tracepoint *iter;
 	struct tracepoint_entry *mark_entry;
 
+	if (!iter) {
+		WARN_ON_ONCE(1);
+		return;
+	}
+
 	mutex_lock(&tracepoints_mutex);
 	for (iter = begin; iter < end; iter++) {
 		mark_entry = get_tracepoint(iter->name);

Re: [tip:tracing/ftrace] tracing: fix oops in tracepoint_update_probe_range()

[Jaswinder Singh Rajput]

On Wed, 2009-03-18 at 17:51 +0000, Jaswinder Singh Rajput wrote:
> Commit-ID:  966a6fdf6210e3ac8ce00b61cd1107cdf97ce744
> Gitweb:     http://git.kernel.org/tip/966a6fdf6210e3ac8ce00b61cd1107cdf97ce744
> Author:     Jaswinder Singh Rajput <jaswinder@kernel.org>
> AuthorDate: Wed, 18 Mar 2009 22:18:56 +0530
> Commit:     Ingo Molnar <mingo@elte.hu>
> CommitDate: Wed, 18 Mar 2009 18:48:43 +0100
> 
> tracing: fix oops in tracepoint_update_probe_range()
> 
> Change this crash:
> 
>  BUG: unable to handle kernel NULL pointer dereference at (null)
>  IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
>  PGD 13d5fb067 PUD 13d688067 PMD 0
>  Oops: 0000 [#1] SMP
> 
> To a more debuggable WARN_ONCE().
> 
> Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> LKML-Reference: <1237394936.3132.1.camel@localhost.localdomain>
> [ moved the check outside the lock and added a WARN_ON(). ]
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 
> 
> ---
>  kernel/tracepoint.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 7960274..dd15df9 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -272,12 +272,17 @@ static void disable_tracepoint(struct tracepoint *elem)
>   *
>   * Updates the probe callback corresponding to a range of tracepoints.
>   */
> -void tracepoint_update_probe_range(struct tracepoint *begin,
> -	struct tracepoint *end)
> +void
> +tracepoint_update_probe_range(struct tracepoint *begin, struct tracepoint *end)
>  {
>  	struct tracepoint *iter;
>  	struct tracepoint_entry *mark_entry;
>  
> +	if (!iter) {
> +		WARN_ON_ONCE(1);
> +		return;
> +	}
> +
>  	mutex_lock(&tracepoints_mutex);
>  	for (iter = begin; iter < end; iter++) {
>  		mark_entry = get_tracepoint(iter->name);

my original patch was correct.

--
JSR

Re: [tip:tracing/ftrace] tracing: fix oops in tracepoint_update_probe_range()

[Frederic Weisbecker]

On Wed, Mar 18, 2009 at 11:26:37PM +0530, Jaswinder Singh Rajput wrote:
> On Wed, 2009-03-18 at 17:51 +0000, Jaswinder Singh Rajput wrote:
> > Commit-ID:  966a6fdf6210e3ac8ce00b61cd1107cdf97ce744
> > Gitweb:     http://git.kernel.org/tip/966a6fdf6210e3ac8ce00b61cd1107cdf97ce744
> > Author:     Jaswinder Singh Rajput <jaswinder@kernel.org>
> > AuthorDate: Wed, 18 Mar 2009 22:18:56 +0530
> > Commit:     Ingo Molnar <mingo@elte.hu>
> > CommitDate: Wed, 18 Mar 2009 18:48:43 +0100
> > 
> > tracing: fix oops in tracepoint_update_probe_range()
> > 
> > Change this crash:
> > 
> >  BUG: unable to handle kernel NULL pointer dereference at (null)
> >  IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
> >  PGD 13d5fb067 PUD 13d688067 PMD 0
> >  Oops: 0000 [#1] SMP
> > 
> > To a more debuggable WARN_ONCE().
> > 
> > Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > LKML-Reference: <1237394936.3132.1.camel@localhost.localdomain>
> > [ moved the check outside the lock and added a WARN_ON(). ]
> > Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > 
> > 
> > ---
> >  kernel/tracepoint.c |    9 +++++++--
> >  1 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > index 7960274..dd15df9 100644
> > --- a/kernel/tracepoint.c
> > +++ b/kernel/tracepoint.c
> > @@ -272,12 +272,17 @@ static void disable_tracepoint(struct tracepoint *elem)
> >   *
> >   * Updates the probe callback corresponding to a range of tracepoints.
> >   */
> > -void tracepoint_update_probe_range(struct tracepoint *begin,
> > -	struct tracepoint *end)
> > +void
> > +tracepoint_update_probe_range(struct tracepoint *begin, struct tracepoint *end)
> >  {
> >  	struct tracepoint *iter;
> >  	struct tracepoint_entry *mark_entry;
> >  
> > +	if (!iter) {
> > +		WARN_ON_ONCE(1);
> > +		return;
> > +	}
> > +
> >  	mutex_lock(&tracepoints_mutex);
> >  	for (iter = begin; iter < end; iter++) {
> >  		mark_entry = get_tracepoint(iter->name);
> 
> my original patch was correct.
> 
> --
> JSR
> 
> 

Jaswinder, It's hard for me to reproduce it via your config.
May be it's because I had to update it to match latest -tip tree and
then I inserted some noise inside.

Could you please send me your bad vmlinux, so that I can have a first look at
your elf sections and see if there is something helpful inside.

Thanks.

Re: [tip:tracing/ftrace] tracing: fix oops in tracepoint_update_probe_range()

[Jaswinder Singh Rajput]

On Wed, 2009-03-18 at 19:00 +0100, Frederic Weisbecker wrote:

> Jaswinder, It's hard for me to reproduce it via your config.
> May be it's because I had to update it to match latest -tip tree and
> then I inserted some noise inside.
> 
> Could you please send me your bad vmlinux, so that I can have a first look at
> your elf sections and see if there is something helpful inside.
> 

OK I am uploading vmlinux (file size 14729799) and dmesg on :

http://userweb.kernel.org/~jaswinder/oops_20090318/

Are you testing on 32 bit or 64 bit machine.

I am getting this error on 64 bit AMD box.

Thanks
--
JSR

Re: [tip:tracing/ftrace] tracing: fix oops in tracepoint_update_probe_range()

[Frederic Weisbecker]

On Wed, Mar 18, 2009 at 11:42:10PM +0530, Jaswinder Singh Rajput wrote:
> On Wed, 2009-03-18 at 19:00 +0100, Frederic Weisbecker wrote:
> 
> > Jaswinder, It's hard for me to reproduce it via your config.
> > May be it's because I had to update it to match latest -tip tree and
> > then I inserted some noise inside.
> > 
> > Could you please send me your bad vmlinux, so that I can have a first look at
> > your elf sections and see if there is something helpful inside.
> > 
> 
> OK I am uploading vmlinux (file size 14729799) and dmesg on :
> 
> http://userweb.kernel.org/~jaswinder/oops_20090318/
> 
> Are you testing on 32 bit or 64 bit machine.
> 
> I am getting this error on 64 bit AMD box.
> 
> Thanks
> --
> JSR
> 

Actually I haven't tested but only looked at my elf sections and haven't
seen anything weird. (I run a 64 too).

Your sections are normal too:

$ objdump -t ./vmlinux | grep __tracepoint | sort -d

ffffffff81623b00 l     O __ksymtab_gpl	0000000000000010 __ksymtab___tracepoint_block_remap
ffffffff8162eeb3 l     O __ksymtab_strings	0000000000000019 __kstrtab___tracepoint_block_remap
ffffffff816fb800 g       .data	0000000000000000 __start___tracepoints
ffffffff816fb800 g     O .data	0000000000000020 __tracepoint_power_start
ffffffff816fb820 g     O .data	0000000000000020 __tracepoint_power_end
ffffffff816fb840 g     O .data	0000000000000020 __tracepoint_power_mark
ffffffff816fb860 g     O .data	0000000000000020 __tracepoint_sched_wait_task
ffffffff816fb880 g     O .data	0000000000000020 __tracepoint_sched_wakeup
ffffffff816fb8a0 g     O .data	0000000000000020 __tracepoint_sched_wakeup_new
ffffffff816fb8c0 g     O .data	0000000000000020 __tracepoint_sched_switch
ffffffff816fb8e0 g     O .data	0000000000000020 __tracepoint_sched_migrate_task
ffffffff816fb900 g     O .data	0000000000000020 __tracepoint_sched_process_fork
ffffffff816fb920 g     O .data	0000000000000020 __tracepoint_sched_process_free
ffffffff816fb940 g     O .data	0000000000000020 __tracepoint_sched_process_exit
ffffffff816fb960 g     O .data	0000000000000020 __tracepoint_sched_process_wait
ffffffff816fb980 g     O .data	0000000000000020 __tracepoint_softirq_entry
ffffffff816fb9a0 g     O .data	0000000000000020 __tracepoint_softirq_exit
ffffffff816fb9c0 g     O .data	0000000000000020 __tracepoint_sched_signal_send
ffffffff816fb9e0 g     O .data	0000000000000020 __tracepoint_workqueue_insertion
ffffffff816fba00 g     O .data	0000000000000020 __tracepoint_workqueue_execution
ffffffff816fba20 g     O .data	0000000000000020 __tracepoint_workqueue_creation
ffffffff816fba40 g     O .data	0000000000000020 __tracepoint_workqueue_destruction
ffffffff816fba60 g     O .data	0000000000000020 __tracepoint_sched_kthread_stop
ffffffff816fba80 g     O .data	0000000000000020 __tracepoint_sched_kthread_stop_ret
ffffffff816fbaa0 g     O .data	0000000000000020 __tracepoint_irq_handler_entry
ffffffff816fbac0 g     O .data	0000000000000020 __tracepoint_irq_handler_exit
ffffffff816fbae0 g     O .data	0000000000000020 __tracepoint_block_bio_bounce
ffffffff816fbb00 g     O .data	0000000000000020 __tracepoint_block_split
ffffffff816fbb20 g     O .data	0000000000000020 __tracepoint_block_rq_abort
ffffffff816fbb40 g     O .data	0000000000000020 __tracepoint_block_rq_insert
ffffffff816fbb60 g     O .data	0000000000000020 __tracepoint_block_rq_issue
ffffffff816fbb80 g     O .data	0000000000000020 __tracepoint_block_plug
ffffffff816fbba0 g     O .data	0000000000000020 __tracepoint_block_unplug_io
ffffffff816fbbc0 g     O .data	0000000000000020 __tracepoint_block_unplug_timer
ffffffff816fbbe0 g     O .data	0000000000000020 __tracepoint_block_getrq
ffffffff816fbc00 g     O .data	0000000000000020 __tracepoint_block_sleeprq
ffffffff816fbc20 g     O .data	0000000000000020 __tracepoint_block_rq_requeue
ffffffff816fbc40 g     O .data	0000000000000020 __tracepoint_block_bio_backmerge
ffffffff816fbc60 g     O .data	0000000000000020 __tracepoint_block_bio_frontmerge
ffffffff816fbc80 g     O .data	0000000000000020 __tracepoint_block_bio_queue
ffffffff816fbca0 g     O .data	0000000000000020 __tracepoint_block_rq_complete
ffffffff816fbcc0 g     O .data	0000000000000020 __tracepoint_block_remap
ffffffff816fbce0 g     O .data	0000000000000020 __tracepoint_block_bio_complete
ffffffff816fbd00 g       .data	0000000000000000 __stop___tracepoints

You see? __start___tracepoints and __stop___tracepoints embrace very well
a good number of tracepoints, so it's not empty.

You even have tracepoints for options that you haven't selected.
It seems that if CONFIG_TRACEPOINTS is enabled, all tracepoints that are found
in a built file will be configured.

And there are things such as workqueues that are always built in a kernel, so
the tracepoint section is _never_ supposed to be empty if CONFIG_TRACEPOINTS=y

Something screwed up somewhere...

Re: [tip:tracing/ftrace] tracing: fix oops in tracepoint_update_probe_range()

[Ingo Molnar]

* Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:

> On Wed, 2009-03-18 at 17:51 +0000, Jaswinder Singh Rajput wrote:
> > Commit-ID:  966a6fdf6210e3ac8ce00b61cd1107cdf97ce744
> > Gitweb:     http://git.kernel.org/tip/966a6fdf6210e3ac8ce00b61cd1107cdf97ce744
> > Author:     Jaswinder Singh Rajput <jaswinder@kernel.org>
> > AuthorDate: Wed, 18 Mar 2009 22:18:56 +0530
> > Commit:     Ingo Molnar <mingo@elte.hu>
> > CommitDate: Wed, 18 Mar 2009 18:48:43 +0100
> > 
> > tracing: fix oops in tracepoint_update_probe_range()
> > 
> > Change this crash:
> > 
> >  BUG: unable to handle kernel NULL pointer dereference at (null)
> >  IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
> >  PGD 13d5fb067 PUD 13d688067 PMD 0
> >  Oops: 0000 [#1] SMP
> > 
> > To a more debuggable WARN_ONCE().
> > 
> > Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > LKML-Reference: <1237394936.3132.1.camel@localhost.localdomain>
> > [ moved the check outside the lock and added a WARN_ON(). ]
> > Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > 
> > 
> > ---
> >  kernel/tracepoint.c |    9 +++++++--
> >  1 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > index 7960274..dd15df9 100644
> > --- a/kernel/tracepoint.c
> > +++ b/kernel/tracepoint.c
> > @@ -272,12 +272,17 @@ static void disable_tracepoint(struct tracepoint *elem)
> >   *
> >   * Updates the probe callback corresponding to a range of tracepoints.
> >   */
> > -void tracepoint_update_probe_range(struct tracepoint *begin,
> > -	struct tracepoint *end)
> > +void
> > +tracepoint_update_probe_range(struct tracepoint *begin, struct tracepoint *end)
> >  {
> >  	struct tracepoint *iter;
> >  	struct tracepoint_entry *mark_entry;
> >  
> > +	if (!iter) {
> > +		WARN_ON_ONCE(1);
> > +		return;
> > +	}
> > +
> >  	mutex_lock(&tracepoints_mutex);
> >  	for (iter = begin; iter < end; iter++) {
> >  		mark_entry = get_tracepoint(iter->name);
> 
> my original patch was correct.

It might have worked but it was rather ugly: it took the 
tracepoints_mutex for no reason.

The clean fix to skip zero-sized sections early in the function, 
without taking any lock, and without emitting a warning.

	Ingo

Re: [tip:tracing/ftrace] tracing: fix oops in tracepoint_update_probe_range()

[Jaswinder Singh Rajput]

On Wed, 2009-03-18 at 19:58 +0100, Ingo Molnar wrote:
> It might have worked but it was rather ugly: it took the 
> tracepoints_mutex for no reason.
> 
> The clean fix to skip zero-sized sections early in the function, 
> without taking any lock, and without emitting a warning.
> 

Yes, now it looks better, thanks :-)

--
JSR

[tip:tracing/ftrace] tracing: fix oops in tracepoint_update_probe_range()

[Jaswinder Singh Rajput]

Commit-ID:  09933a108e6730a464a1ab676c9decc11aee0edc
Gitweb:     http://git.kernel.org/tip/09933a108e6730a464a1ab676c9decc11aee0edc
Author:     Jaswinder Singh Rajput <jaswinder@kernel.org>
AuthorDate: Wed, 18 Mar 2009 22:18:56 +0530
Commit:     Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 18 Mar 2009 18:54:39 +0100

tracing: fix oops in tracepoint_update_probe_range()

Change this crash:

 BUG: unable to handle kernel NULL pointer dereference at (null)
 IP: [<ffffffff8107d4de>] tracepoint_update_probe_range+0x1f/0x9b
 PGD 13d5fb067 PUD 13d688067 PMD 0
 Oops: 0000 [#1] SMP

To a more debuggable WARN_ONCE().

Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <1237394936.3132.1.camel@localhost.localdomain>
[ moved the check outside the lock and added a WARN_ON(). ]
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 kernel/tracepoint.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 7960274..adf2873 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -272,12 +272,17 @@ static void disable_tracepoint(struct tracepoint *elem)
  *
  * Updates the probe callback corresponding to a range of tracepoints.
  */
-void tracepoint_update_probe_range(struct tracepoint *begin,
-	struct tracepoint *end)
+void
+tracepoint_update_probe_range(struct tracepoint *begin, struct tracepoint *end)
 {
 	struct tracepoint *iter;
 	struct tracepoint_entry *mark_entry;
 
+	if (!begin) {
+		WARN_ON_ONCE(1);
+		return;
+	}
+
 	mutex_lock(&tracepoints_mutex);
 	for (iter = begin; iter < end; iter++) {
 		mark_entry = get_tracepoint(iter->name);

[tip:tracing/ftrace] tracepoints: dont update zero-sized tracepoint sections

[Ingo Molnar]

Commit-ID:  ec625cb29e66824f7ce41082617aeb93fa4e42e2
Gitweb:     http://git.kernel.org/tip/ec625cb29e66824f7ce41082617aeb93fa4e42e2
Author:     Ingo Molnar <mingo@elte.hu>
AuthorDate: Wed, 18 Mar 2009 19:54:04 +0100
Commit:     Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 18 Mar 2009 19:55:00 +0100

tracepoints: dont update zero-sized tracepoint sections

Zero-sized tracepoint sections can occur if tracing is enabled but
no tracepoint is defined. Do not emit a warning in that case.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
LKML-Reference: <1237394936.3132.1.camel@localhost.localdomain>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 kernel/tracepoint.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index adf2873..1ef5d3a 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -278,10 +278,8 @@ tracepoint_update_probe_range(struct tracepoint *begin, struct tracepoint *end)
 	struct tracepoint *iter;
 	struct tracepoint_entry *mark_entry;
 
-	if (!begin) {
-		WARN_ON_ONCE(1);
+	if (!begin)
 		return;
-	}
 
 	mutex_lock(&tracepoints_mutex);
 	for (iter = begin; iter < end; iter++) {

Re: oops in tracepoint_update_probe_range()

[Lai Jiangshan]

Ingo Molnar wrote:
> * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> 
>> Good: f4c3c4cdb1de232
>> Bad : 1e08816af0bc345
>>
>> Config:
>> http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318
>>
>> oops:
>> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
>> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
>> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
>> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg
>>
>> <freeze>
> 
> Steve, Frederic - the crashes above are in:
> 
> 	tracepoint_update_probe_range()
> 
> in a modular kernel apparently.
> 
>

I look up the jpg files, this oops is occurred when a new module is
being loaded.

tracepoint_module_notify() is added by Mathieu Desnoyers on the
suggestion of me.

tracepoint_update_probe_range() and tracepoint_module_notify()
can not trigger this oops if the arguments are correct.

If @begin is NULL, @end is NULL too, it's ensued by kernel/module.c.

load_module(...):
	mod->tracepoints = section_objs(hdr, sechdrs, secstrings,
					"__tracepoints",
					sizeof(*mod->tracepoints),
					&mod->num_tracepoints);
static void *section_objs(...)
{
	unsigned int sec = find_sec(hdr, sechdrs, secstrings, name);

	/* Section 0 has sh_addr 0 and sh_size 0. */
	*num = sechdrs[sec].sh_size / object_size;
	return (void *)sechdrs[sec].sh_addr;
}

If the module has not "__tracepoints" section, find_sec() returns 0.
So I think, sechdrs[0].sh_size is corrupted.

Is the following fix fixed the oops for you?
---
diff --git a/kernel/module.c b/kernel/module.c
index 7fa134e..2ee47ff 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1950,6 +1950,7 @@ static noinline struct module *load_module(void __user *umod,
 	sechdrs = (void *)hdr + hdr->e_shoff;
 	secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
 	sechdrs[0].sh_addr = 0;
+	sechdrs[0].sh_size = 0;
 
 	for (i = 1; i < hdr->e_shnum; i++) {
 		if (sechdrs[i].sh_type != SHT_NOBITS

Re: oops in tracepoint_update_probe_range()

[Ingo Molnar]

* Lai Jiangshan <laijs@cn.fujitsu.com> wrote:

> Ingo Molnar wrote:
> > * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> > 
> >> Good: f4c3c4cdb1de232
> >> Bad : 1e08816af0bc345
> >>
> >> Config:
> >> http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318
> >>
> >> oops:
> >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
> >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
> >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
> >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg
> >>
> >> <freeze>
> > 
> > Steve, Frederic - the crashes above are in:
> > 
> > 	tracepoint_update_probe_range()
> > 
> > in a modular kernel apparently.
> > 
> >
> 
> I look up the jpg files, this oops is occurred when a new module is
> being loaded.
> 
> tracepoint_module_notify() is added by Mathieu Desnoyers on the
> suggestion of me.
> 
> tracepoint_update_probe_range() and tracepoint_module_notify()
> can not trigger this oops if the arguments are correct.
> 
> If @begin is NULL, @end is NULL too, it's ensued by kernel/module.c.
> 
> load_module(...):
> 	mod->tracepoints = section_objs(hdr, sechdrs, secstrings,
> 					"__tracepoints",
> 					sizeof(*mod->tracepoints),
> 					&mod->num_tracepoints);
> static void *section_objs(...)
> {
> 	unsigned int sec = find_sec(hdr, sechdrs, secstrings, name);
> 
> 	/* Section 0 has sh_addr 0 and sh_size 0. */
> 	*num = sechdrs[sec].sh_size / object_size;
> 	return (void *)sechdrs[sec].sh_addr;
> }
> 
> If the module has not "__tracepoints" section, find_sec() returns 0.
> So I think, sechdrs[0].sh_size is corrupted.
> 
> Is the following fix fixed the oops for you?
> ---
> diff --git a/kernel/module.c b/kernel/module.c
> index 7fa134e..2ee47ff 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1950,6 +1950,7 @@ static noinline struct module *load_module(void __user *umod,
>  	sechdrs = (void *)hdr + hdr->e_shoff;
>  	secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
>  	sechdrs[0].sh_addr = 0;
> +	sechdrs[0].sh_size = 0;
>  
>  	for (i = 1; i < hdr->e_shnum; i++) {
>  		if (sechdrs[i].sh_type != SHT_NOBITS

Jaswinder, could you please try the fix from Lai, but first do:

 git revert ec625cb  # tracepoints: dont update zero-sized tracepoint sections
 git revert 09933a1  # tracing: fix oops in tracepoint_update_probe_range()

?

	Ingo

Re: oops in tracepoint_update_probe_range()

[Jaswinder Singh Rajput]

On Thu, 2009-03-19 at 08:46 +0100, Ingo Molnar wrote:
> * Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
> 
> > Ingo Molnar wrote:
> > > * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> > > 
> > >> Good: f4c3c4cdb1de232
> > >> Bad : 1e08816af0bc345
> > >>
> > >> Config:
> > >> http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318
> > >>
> > >> oops:
> > >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
> > >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
> > >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
> > >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg
> > >>
> > >> <freeze>
> > > 
> > > Steve, Frederic - the crashes above are in:
> > > 
> > > 	tracepoint_update_probe_range()
> > > 
> > > in a modular kernel apparently.
> > > 
> > >
> > 
> > I look up the jpg files, this oops is occurred when a new module is
> > being loaded.
> > 
> > tracepoint_module_notify() is added by Mathieu Desnoyers on the
> > suggestion of me.
> > 
> > tracepoint_update_probe_range() and tracepoint_module_notify()
> > can not trigger this oops if the arguments are correct.
> > 
> > If @begin is NULL, @end is NULL too, it's ensued by kernel/module.c.
> > 
> > load_module(...):
> > 	mod->tracepoints = section_objs(hdr, sechdrs, secstrings,
> > 					"__tracepoints",
> > 					sizeof(*mod->tracepoints),
> > 					&mod->num_tracepoints);
> > static void *section_objs(...)
> > {
> > 	unsigned int sec = find_sec(hdr, sechdrs, secstrings, name);
> > 
> > 	/* Section 0 has sh_addr 0 and sh_size 0. */
> > 	*num = sechdrs[sec].sh_size / object_size;
> > 	return (void *)sechdrs[sec].sh_addr;
> > }
> > 
> > If the module has not "__tracepoints" section, find_sec() returns 0.
> > So I think, sechdrs[0].sh_size is corrupted.
> > 
> > Is the following fix fixed the oops for you?
> > ---
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 7fa134e..2ee47ff 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -1950,6 +1950,7 @@ static noinline struct module *load_module(void __user *umod,
> >  	sechdrs = (void *)hdr + hdr->e_shoff;
> >  	secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
> >  	sechdrs[0].sh_addr = 0;
> > +	sechdrs[0].sh_size = 0;
> >  
> >  	for (i = 1; i < hdr->e_shnum; i++) {
> >  		if (sechdrs[i].sh_type != SHT_NOBITS
> 
> Jaswinder, could you please try the fix from Lai, but first do:
> 
>  git revert ec625cb  # tracepoints: dont update zero-sized tracepoint sections
>  git revert 09933a1  # tracing: fix oops in tracepoint_update_probe_range()
> 

After reverting above two commits and applying Lai's patch still gives
me oops:
[    5.027136] hub 6-0:1.0: state 7 ports 3 chg 0000 evt 0000
[    5.444563] BUG: unable to handle kernel NULL pointer dereference at (null)
[    5.444906] IP: [<ffffffff8107d50a>] tracepoint_update_probe_range+0x1f/0x9b
[    5.445155] PGD 13d5a8067 PUD 13d5ea067 PMD 0 
[    5.445376] Oops: 0000 [#1] SMP 
[    5.445437] last sysfs file: /sys/class/firmware/timeout
[    5.445437] CPU 0 
[    5.445437] Modules linked in: scsi_wait_scan(+)
[    5.445437] Pid: 877, comm: modprobe Not tainted 2.6.29-rc8-tip #383 HP Pavilion dv5 Notebook PC
[    5.445437] RIP: 0010:[<ffffffff8107d50a>]  [<ffffffff8107d50a>] tracepoint_update_probe_range+0x1f/0x9b
[    5.445437] RSP: 0018:ffff88013d5ede78  EFLAGS: 00010287
[    5.445437] RAX: ffff88013d5ec000 RBX: 0000000000000000 RCX: ffffffff81650940
[    5.445437] RDX: ffffffffa0000300 RSI: 0000001400000000 RDI: ffffffff81650960
[    5.445437] RBP: ffff88013d5ede98 R08: ffffc200006799c8 R09: ffff88013d5eddb8
[    5.445437] R10: dead000000200200 R11: 6db6db6db6db6db7 R12: 00000000fffffffc
[    5.445437] R13: 0000000000000000 R14: 0000001400000000 R15: 0000000000000001
[    5.445437] FS:  00007f9d94e4f6f0(0000) GS:ffff880028022000(0000) knlGS:0000000000000000
[    5.445437] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[    5.445437] CR2: 0000000000000000 CR3: 000000013d5cf000 CR4: 00000000000006a0
[    5.445437] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    5.445437] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[    5.445437] Process modprobe (pid: 877, threadinfo ffff88013d5ec000, task ffff88013e3d2710)
[    5.445437] Stack:
[    5.445437]  0000000000000000 00000000fffffffc 0000000000000000 ffffffffa0000300
[    5.445437]  ffff88013d5edea8 ffffffff8107d5b0 ffff88013d5edee8 ffffffff8143e834
[    5.445437]  ffffffff8164e4d0 ffffffff8164e4d0 0000000000000000 0000000000000000
[    5.445437] Call Trace:
[    5.445437]  [<ffffffff8107d5b0>] tracepoint_module_notify+0x2a/0x2e
[    5.445437]  [<ffffffff8143e834>] notifier_call_chain+0x33/0x5b
[    5.445437]  [<ffffffff8105377c>] __blocking_notifier_call_chain+0x4d/0x6a
[    5.445437]  [<ffffffff810537a8>] blocking_notifier_call_chain+0xf/0x11
[    5.445437]  [<ffffffff81061c90>] sys_init_module+0x94/0x1c8
[    5.445437]  [<ffffffff8100bb2b>] system_call_fastpath+0x16/0x1b
[    5.445437] Code: e8 05 df 3b 00 31 c0 5b 41 5c c9 c3 55 48 89 e5 41 56 49 89 f6 41 55 41 54 53 48 89 fb 48 c7 c7 60 09 65 81 e8 95 e1 3b 00 eb 62 <48> 8b 3b e8 9d fa ff ff 48 85 c0 49 89 c4 74 3f 48 8b 33 48 8d 
[    5.445437] RIP  [<ffffffff8107d50a>] tracepoint_update_probe_range+0x1f/0x9b
[    5.445437]  RSP <ffff88013d5ede78>
[    5.445437] CR2: 0000000000000000
[    5.450260] ---[ end trace 20c410fa785114f0 ]---

Thanks,
--
JSR

Re: oops in tracepoint_update_probe_range()

[Mathieu Desnoyers]

* Ingo Molnar (mingo@elte.hu) wrote:
> 
> * Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
> 
> > Ingo Molnar wrote:
> > > * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> > > 
> > >> Good: f4c3c4cdb1de232
> > >> Bad : 1e08816af0bc345
> > >>
> > >> Config:
> > >> http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318
> > >>
> > >> oops:
> > >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
> > >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
> > >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
> > >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg
> > >>
> > >> <freeze>
> > > 
> > > Steve, Frederic - the crashes above are in:
> > > 
> > > 	tracepoint_update_probe_range()
> > > 
> > > in a modular kernel apparently.
> > > 
> > >
> > 

Jaswinder : maybe you have old modules in your /lib/modules/`uname -r`
directory which have the correct version, but the wrong module.h header ?

It does not matter if the core kernel __tracepoint section is there or
not, what really matters here is if the problematic module has a correct
module header.

Also, I wonder which module is being loaded when your crash occurs. Does
this specific module contain tracepoints or not ? Maybe we could add a
little printk() to show the current module being loaded before the crash ?

As Lai said, the loop should be skipped because begin and end will both
be the same value when there are no tracepoints in a given module. So
adding extra length check will not help anything here.

The "return if NULL" test probably works just because the specific
module header mismatch we have here happen to put this field to NULL for
some reason.

Mathieu

> > I look up the jpg files, this oops is occurred when a new module is
> > being loaded.
> > 
> > tracepoint_module_notify() is added by Mathieu Desnoyers on the
> > suggestion of me.
> > 
> > tracepoint_update_probe_range() and tracepoint_module_notify()
> > can not trigger this oops if the arguments are correct.
> > 
> > If @begin is NULL, @end is NULL too, it's ensued by kernel/module.c.
> > 
> > load_module(...):
> > 	mod->tracepoints = section_objs(hdr, sechdrs, secstrings,
> > 					"__tracepoints",
> > 					sizeof(*mod->tracepoints),
> > 					&mod->num_tracepoints);
> > static void *section_objs(...)
> > {
> > 	unsigned int sec = find_sec(hdr, sechdrs, secstrings, name);
> > 
> > 	/* Section 0 has sh_addr 0 and sh_size 0. */
> > 	*num = sechdrs[sec].sh_size / object_size;
> > 	return (void *)sechdrs[sec].sh_addr;
> > }
> > 
> > If the module has not "__tracepoints" section, find_sec() returns 0.
> > So I think, sechdrs[0].sh_size is corrupted.
> > 
> > Is the following fix fixed the oops for you?
> > ---
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 7fa134e..2ee47ff 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -1950,6 +1950,7 @@ static noinline struct module *load_module(void __user *umod,
> >  	sechdrs = (void *)hdr + hdr->e_shoff;
> >  	secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
> >  	sechdrs[0].sh_addr = 0;
> > +	sechdrs[0].sh_size = 0;
> >  
> >  	for (i = 1; i < hdr->e_shnum; i++) {
> >  		if (sechdrs[i].sh_type != SHT_NOBITS
> 
> Jaswinder, could you please try the fix from Lai, but first do:
> 
>  git revert ec625cb  # tracepoints: dont update zero-sized tracepoint sections
>  git revert 09933a1  # tracing: fix oops in tracepoint_update_probe_range()
> 
> ?
> 
> 	Ingo

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: oops in tracepoint_update_probe_range()

[Steven Rostedt]

On Thu, 19 Mar 2009, Mathieu Desnoyers wrote:
> 
> Jaswinder : maybe you have old modules in your /lib/modules/`uname -r`
> directory which have the correct version, but the wrong module.h header ?

Isn't there a module versioning that prevents such things?

-- Steve

Re: oops in tracepoint_update_probe_range()

[Mathieu Desnoyers]

* Steven Rostedt (rostedt@goodmis.org) wrote:
> 
> On Thu, 19 Mar 2009, Mathieu Desnoyers wrote:
> > 
> > Jaswinder : maybe you have old modules in your /lib/modules/`uname -r`
> > directory which have the correct version, but the wrong module.h header ?
> 
> Isn't there a module versioning that prevents such things?
> 
> -- Steve

I think it's only enabled when CONFIG_MODVERSIONS is on, which is not
the case for his config :

# CONFIG_MODVERSIONS is not set

So I might be wrong, but I suspect the compatibility check in this case
would only be based on kernel version.

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: oops in tracepoint_update_probe_range()

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 19 Mar 2009, Mathieu Desnoyers wrote:
> > 
> > Jaswinder : maybe you have old modules in your /lib/modules/`uname -r`
> > directory which have the correct version, but the wrong module.h header ?
> 
> Isn't there a module versioning that prevents such things?

It can be overriden. In any case, the NULL check we have there now 
makes sense.

	Ingo

Re: oops in tracepoint_update_probe_range()

[Mathieu Desnoyers]

* Ingo Molnar (mingo@elte.hu) wrote:
> 
> * Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Thu, 19 Mar 2009, Mathieu Desnoyers wrote:
> > > 
> > > Jaswinder : maybe you have old modules in your /lib/modules/`uname -r`
> > > directory which have the correct version, but the wrong module.h header ?
> > 
> > Isn't there a module versioning that prevents such things?
> 
> It can be overriden. In any case, the NULL check we have there now 
> makes sense.
> 
> 	Ingo

Well, it duplicates the check for begin == end. Actually, if begin !=
end _and_ being is NULL, this should be a WARN_ON or BUG_ON, because
the kernel would be trying to load a module with incompatible struct
module.

Are we supposed to assume that module.c allows loading modules with
incompatible struct module at all ? That sounds like we would be trying
to fix up things broken by the module loader in the first place.

Mathieu


-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: oops in tracepoint_update_probe_range()

[Steven Rostedt]

On Thu, 19 Mar 2009, Mathieu Desnoyers wrote:

> * Ingo Molnar (mingo@elte.hu) wrote:
> > 
> > * Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > On Thu, 19 Mar 2009, Mathieu Desnoyers wrote:
> > > > 
> > > > Jaswinder : maybe you have old modules in your /lib/modules/`uname -r`
> > > > directory which have the correct version, but the wrong module.h header ?
> > > 
> > > Isn't there a module versioning that prevents such things?
> > 
> > It can be overriden. In any case, the NULL check we have there now 
> > makes sense.
> > 
> > 	Ingo
> 
> Well, it duplicates the check for begin == end. Actually, if begin !=
> end _and_ being is NULL, this should be a WARN_ON or BUG_ON, because
> the kernel would be trying to load a module with incompatible struct
> module.
> 
> Are we supposed to assume that module.c allows loading modules with
> incompatible struct module at all ? That sounds like we would be trying
> to fix up things broken by the module loader in the first place.

Then it should be WARN_ON, no need to lock up a box hard, and give the 
user in X with no serial, no idea why the box just locked up.

-- Steve

Re: oops in tracepoint_update_probe_range()

[Rusty Russell]

On Friday 20 March 2009 02:50:30 Steven Rostedt wrote:
> > Are we supposed to assume that module.c allows loading modules with
> > incompatible struct module at all ? That sounds like we would be trying
> > to fix up things broken by the module loader in the first place.
> 
> Then it should be WARN_ON, no need to lock up a box hard, and give the 
> user in X with no serial, no idea why the box just locked up.

In case anyone wonders, I don't care in general (though I won't protest if
anyone else wants to put checks in their code).  You can crash in all kinds of exotic ways using the wrong modules.  If you are a kernel dev, modversions will often save you (eg. struct module changing).  If not, the kernel version  string should change.  Otherwise, someone's just messing with you.

Hope that helps,
Rusty.

Re: oops in tracepoint_update_probe_range()

[Jaswinder Singh Rajput]

On Thu, 2009-03-19 at 09:22 -0400, Mathieu Desnoyers wrote:
> * Ingo Molnar (mingo@elte.hu) wrote:
> > 
> > * Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
> > 
> > > Ingo Molnar wrote:
> > > > * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> > > > 
> > > >> Good: f4c3c4cdb1de232
> > > >> Bad : 1e08816af0bc345
> > > >>
> > > >> Config:
> > > >> http://userweb.kernel.org/~jaswinder/oops_20090318/config-hpdv5-tip-bad-20090318
> > > >>
> > > >> oops:
> > > >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page1.jpg
> > > >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page2.jpg
> > > >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page3.jpg
> > > >> http://userweb.kernel.org/~jaswinder/oops_20090318/oops_page4.jpg
> > > >>
> > > >> <freeze>
> > > > 
> > > > Steve, Frederic - the crashes above are in:
> > > > 
> > > > 	tracepoint_update_probe_range()
> > > > 
> > > > in a modular kernel apparently.
> > > > 
> > > >
> > > 
> 
> Jaswinder : maybe you have old modules in your /lib/modules/`uname -r`
> directory which have the correct version, but the wrong module.h header ?
> 

yes, it was having old modules, after deleting old modules.

[root@hpdv5 linux-2.6-tip]# make modules_install
  INSTALL arch/x86/kernel/test_nx.ko
  INSTALL drivers/hid/hid-dummy.ko
  INSTALL drivers/scsi/scsi_wait_scan.ko
  DEPMOD  2.6.29-rc8-tip
[root@hpdv5 jaswinder-git]# /sbin/mkinitrd /boot/initrd-2.6-tip.img 2.6.29-rc8-tip
[root@hpdv5 linux-2.6-tip]#


By following above steps, I get rid of oops, and here is my lsmod:
[jaswinder@hpdv5]$ /sbin/lsmod 
Module                  Size  Used by
[jaswinder@hpdv5]$ 

--
JSR