[PATCH] RT: fix spin_trylock_irq

[PATCH] RT: fix spin_trylock_irq

[Sébastien Dugué]

  This patch fixes a bug in spin_trylock_irq() where __spin_trylock_irq() is
picked for regular (non-raw) spinlocks instead of _spin_trylock_irq().

  This results in systematic boot hangs and may have been going unnoticed
for quite some time as it only manifests (aside from a compile warning) when
booting with a NUMA config or when using the Chelsio T3 (cxgb3) driver as
these seems to be the sole users.


Signed-off-by: Sébastien Dugué <sebastien.dugue@bull.net>

---
 include/linux/spinlock.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.23-rc9-rt2/include/linux/spinlock.h
===================================================================
--- linux-2.6.23-rc9-rt2.orig/include/linux/spinlock.h
+++ linux-2.6.23-rc9-rt2/include/linux/spinlock.h
@@ -501,7 +501,7 @@ do {									\
 
 #define spin_trylock_irq(lock)	\
 	__cond_lock(lock, PICK_SPIN_OP_RET(__spin_trylock_irq,		\
-		__spin_trylock_irq, lock))
+		_spin_trylock_irq, lock))
 
 #define spin_trylock_irqsave(lock, flags) \
 	__cond_lock(lock, PICK_SPIN_OP_RET(__spin_trylock_irqsave, 	\

Re: [PATCH] RT: fix spin_trylock_irq

[Steven Rostedt]

--
When I'm asked what language is my mother tongue,
I simply answer "C".

On Thu, 11 Oct 2007, [UTF-8] Sébastien Dugué wrote:

> ---
>  include/linux/spinlock.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-2.6.23-rc9-rt2/include/linux/spinlock.h
> ===================================================================
> --- linux-2.6.23-rc9-rt2.orig/include/linux/spinlock.h
> +++ linux-2.6.23-rc9-rt2/include/linux/spinlock.h
> @@ -501,7 +501,7 @@ do {									\
>
>  #define spin_trylock_irq(lock)	\
>  	__cond_lock(lock, PICK_SPIN_OP_RET(__spin_trylock_irq,		\
> -		__spin_trylock_irq, lock))
> +		_spin_trylock_irq, lock))
>
>  #define spin_trylock_irqsave(lock, flags) \
>  	__cond_lock(lock, PICK_SPIN_OP_RET(__spin_trylock_irqsave, 	\

Thanks! Applied.

Daniel, this BUG was introduced by your conversion to PICK_FUNCTION patch.
Please be more careful, and do an audit of your patch. (I don't have time
ATM). This could also be the source of other bugs I've been chasing.

I might be pulling the PICK_FUNCTION patches if there's more bugs like
this.

-- Steve

Re: [PATCH] RT: fix spin_trylock_irq

[Daniel Walker]

On Thu, 2007-10-11 at 10:49 -0400, Steven Rostedt wrote:

> > ---
> >  include/linux/spinlock.h |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Index: linux-2.6.23-rc9-rt2/include/linux/spinlock.h
> > ===================================================================
> > --- linux-2.6.23-rc9-rt2.orig/include/linux/spinlock.h
> > +++ linux-2.6.23-rc9-rt2/include/linux/spinlock.h
> > @@ -501,7 +501,7 @@ do {									\
> >
> >  #define spin_trylock_irq(lock)	\
> >  	__cond_lock(lock, PICK_SPIN_OP_RET(__spin_trylock_irq,		\
> > -		__spin_trylock_irq, lock))
> > +		_spin_trylock_irq, lock))
> >
> >  #define spin_trylock_irqsave(lock, flags) \
> >  	__cond_lock(lock, PICK_SPIN_OP_RET(__spin_trylock_irqsave, 	\
> 
> Thanks! Applied.
> 
> Daniel, this BUG was introduced by your conversion to PICK_FUNCTION patch.
> Please be more careful, and do an audit of your patch. (I don't have time
> ATM). This could also be the source of other bugs I've been chasing.
> 
> I might be pulling the PICK_FUNCTION patches if there's more bugs like
> this.

This is the second fix for this .. The first was in this email (over a
month ago)

http://lkml.org/lkml/2007/8/31/318

The above was emailed to Thomas, but I also sent you that link in IRC as
a link of patches to include .. I'll be happy to audit my code better,
but you should also audit your inclusion process better.. There have
been too many missed patches, and too many double and triples fixes..

Daniel

Re: [PATCH] RT: fix spin_trylock_irq

[Steven Rostedt]

--
On Mon, 15 Oct 2007, Daniel Walker wrote:
> > > +		_spin_trylock_irq, lock))
> > >
> > >  #define spin_trylock_irqsave(lock, flags) \
> > >  	__cond_lock(lock, PICK_SPIN_OP_RET(__spin_trylock_irqsave, 	\
> >
> > Thanks! Applied.
> >
> > Daniel, this BUG was introduced by your conversion to PICK_FUNCTION patch.
> > Please be more careful, and do an audit of your patch. (I don't have time
> > ATM). This could also be the source of other bugs I've been chasing.
> >
> > I might be pulling the PICK_FUNCTION patches if there's more bugs like
> > this.
>
> This is the second fix for this .. The first was in this email (over a
> month ago)
>
> http://lkml.org/lkml/2007/8/31/318

You're right I missed that. (I even read it). But for fixes like this, (or
any patches that are not in the tree), you really need to resend the
series.

When Ingo posts patches to LKML on CFS, if there's a little fix like this,
he'll add that to his next queue and repost.

>
> The above was emailed to Thomas, but I also sent you that link in IRC as
> a link of patches to include .. I'll be happy to audit my code better,
> but you should also audit your inclusion process better.. There have
> been too many missed patches, and too many double and triples fixes..

There wouldn't be if we didn't have to go looking for patches on patches
that are out of the series. If you see that a series is broken, don't
patch against it. Resend the series!

Patches must be against upstream, unless they are more RFC (like what
Gregory is doing). But when you want them to go upstream, they must be
against upstream.  I don't have the time to look for little fixes that
you've done to your own patches that have not been pulled in yet.

-- Steve

Re: [PATCH] RT: fix spin_trylock_irq

[Daniel Walker]

On Mon, 2007-10-15 at 14:14 -0400, Steven Rostedt wrote:

> > This is the second fix for this .. The first was in this email (over a
> > month ago)
> >
> > http://lkml.org/lkml/2007/8/31/318
> 
> You're right I missed that. (I even read it). But for fixes like this, (or
> any patches that are not in the tree), you really need to resend the
> series.

In hindsight I should have resent, but I didn't .. Really it's a matter
of frustration on my part. Where I submit patches that should be
included , but they are ignored for no clear reason .. Then I submit
again, and the patches are ignored again for no clear reason..

Comparing mainline and -rt isn't very interesting since , -rt has few
patches sent in against it .. Andrew is bound to loose a patch here and
there on accident, but -rt has so few patches sent in that we shouldn't
be loosing any.

> When Ingo posts patches to LKML on CFS, if there's a little fix like this,
> he'll add that to his next queue and repost.

That's a git tree tho , so apples and oranges ..

> >
> > The above was emailed to Thomas, but I also sent you that link in IRC as
> > a link of patches to include .. I'll be happy to audit my code better,
> > but you should also audit your inclusion process better.. There have
> > been too many missed patches, and too many double and triples fixes..
> 
> There wouldn't be if we didn't have to go looking for patches on patches
> that are out of the series. If you see that a series is broken, don't
> patch against it. Resend the series!

In all fairness you asked for links to patches, and that's is what I
gave you ..

> Patches must be against upstream, unless they are more RFC (like what
> Gregory is doing). But when you want them to go upstream, they must be
> against upstream.  I don't have the time to look for little fixes that
> you've done to your own patches that have not been pulled in yet.

Not sure what your getting at.. I specifically directed you to a patch.
I don't think that means you had to look for it, all you have to do was
include it ..

Daniel

Re: [PATCH] RT: fix spin_trylock_irq

[Thomas Gleixner]

On Mon, 15 Oct 2007, Daniel Walker wrote:
> On Mon, 2007-10-15 at 14:14 -0400, Steven Rostedt wrote:
> 
> > > This is the second fix for this .. The first was in this email (over a
> > > month ago)
> > >
> > > http://lkml.org/lkml/2007/8/31/318
> > 
> > You're right I missed that. (I even read it). But for fixes like this, (or
> > any patches that are not in the tree), you really need to resend the
> > series.
> 
> In hindsight I should have resent, but I didn't .. Really it's a matter
> of frustration on my part. Where I submit patches that should be
> included , but they are ignored for no clear reason .. Then I submit
> again, and the patches are ignored again for no clear reason..

You were asked to rebase your patches against the latest -rt
kernel. And you bluntly refused to.

I knew exactly why I ignored the patches when I was moving -rt
forward. Simply because I had not enough time to review them in
detail. Ingo acked them in principle, but we had subtle wreckage from
such changes before and I had no intention to put more risk into a non
trivial merge.

Sorry, you have provided sloppy patches more than once and requested
us to pick random bugfixes from a broken ftp server or from the
mailing list. It's you who wrote the track record, not us.

I'm finally fed up with your prima ballerina attitudes. Either you
want to be part of this project or not. If yes, then please accept
that you are not setting the rules how this works. If no, please stop
whining at us and look for a nice sandpit where you can play these
silly games.
 
	tglx

Re: [PATCH] RT: fix spin_trylock_irq

[Daniel Walker]

On Mon, 2007-10-15 at 23:35 +0200, Thomas Gleixner wrote:
> On Mon, 15 Oct 2007, Daniel Walker wrote:
> > On Mon, 2007-10-15 at 14:14 -0400, Steven Rostedt wrote:
> > 
> > > > This is the second fix for this .. The first was in this email (over a
> > > > month ago)
> > > >
> > > > http://lkml.org/lkml/2007/8/31/318
> > > 
> > > You're right I missed that. (I even read it). But for fixes like this, (or
> > > any patches that are not in the tree), you really need to resend the
> > > series.
> > 
> > In hindsight I should have resent, but I didn't .. Really it's a matter
> > of frustration on my part. Where I submit patches that should be
> > included , but they are ignored for no clear reason .. Then I submit
> > again, and the patches are ignored again for no clear reason..
> 
> You were asked to rebase your patches against the latest -rt
> kernel. And you bluntly refused to.

Nope , I did rebase my patches when Steven asked me to ..

> I knew exactly why I ignored the patches when I was moving -rt
> forward. Simply because I had not enough time to review them in
> detail. Ingo acked them in principle, but we had subtle wreckage from
> such changes before and I had no intention to put more risk into a non
> trivial merge.
> 
> Sorry, you have provided sloppy patches more than once and requested
> us to pick random bugfixes from a broken ftp server or from the
> mailing list. It's you who wrote the track record, not us.

Oh boy here we go again .. Let just stop right here..

> I'm finally fed up with your prima ballerina attitudes. Either you
> want to be part of this project or not. If yes, then please accept
> that you are not setting the rules how this works. If no, please stop
> whining at us and look for a nice sandpit where you can play these
> silly games.
>  
> 	tglx