[PATCH] spinlock not locked when unlocking in atm_dev_register

[PATCH] spinlock not locked when unlocking in atm_dev_register

[Frode Isaksen]

If you compile the kernel with SMP and spinlock debugging, BUG() will be 
called when registering your atm driver, since the "atm_dev_lock" spinlock is 
not locked when unlocking it.

kernel 2.4.18

Regards,
Frode

--- resources.c.orig	Fri Mar  1 18:34:02 2002
+++ resources.c	Fri Mar  1 18:34:17 2002
@@ -110,12 +110,10 @@
 		if (atm_proc_dev_register(dev) < 0) {
 			printk(KERN_ERR "atm_dev_register: "
 			    "atm_proc_dev_register failed for dev %s\n",type);
-			spin_unlock (&atm_dev_lock);	
 			free_atm_dev(dev);
 			return NULL;
 		}
 #endif
-	spin_unlock (&atm_dev_lock);		
 	return dev;
 }

Re: [PATCH] spinlock not locked when unlocking in atm_dev_register

[Robert Love]

On Fri, 2002-03-01 at 12:46, Frode Isaksen wrote:
> If you compile the kernel with SMP and spinlock debugging, BUG() will be 
> called when registering your atm driver, since the "atm_dev_lock" spinlock is 
> not locked when unlocking it.

I don't have any knowledge of the source in question, but wouldn't a
possibility (perhaps even more likely) be that you should _add_ the
spin_lock instead of remove the spin_unlocks ?

	Robert Love

Re: [PATCH] spinlock not locked when unlocking in atm_dev_register

[Maksim Krasnyanskiy]

> > If you compile the kernel with SMP and spinlock debugging, BUG() will be
> > called when registering your atm driver, since the "atm_dev_lock" 
> spinlock is
> > not locked when unlocking it.
>
>I don't have any knowledge of the source in question, but wouldn't a
>possibility (perhaps even more likely) be that you should _add_ the
>spin_lock instead of remove the spin_unlocks ?
Absolutely correct :)
I've got a patch for that, tested on SMP. I'll send it today or tomorrow.

btw ATM locking seems to be messed up. Is anybody working on that ?

Max

Re: [PATCH] spinlock not locked when unlocking in atm_dev_register

[Robert Love]

On Fri, 2002-03-01 at 17:32, Maksim Krasnyanskiy wrote:

> > I don't have any knowledge of the source in question, but wouldn't a
> > possibility (perhaps even more likely) be that you should _add_ the
> > spin_lock instead of remove the spin_unlocks ?
>
> Absolutely correct :)
> I've got a patch for that, tested on SMP. I'll send it today or tomorrow.

Yah, I just looked at the code - again, I don't profess to know the atm
driver - but it seems to need the lock.  Thanks.
 
> btw ATM locking seems to be messed up. Is anybody working on that ?

Not that I know of.  Volunteer? :)

	Robert Love

Re: [PATCH] spinlock not locked when unlocking in atm_dev_register

[David S. Miller]

   From: Robert Love <rml@tech9.net>
   Date: 01 Mar 2002 17:35:08 -0500
    
   > btw ATM locking seems to be messed up. Is anybody working on that ?
   
   Not that I know of.  Volunteer? :)

I consider it pretty much unmaintained.  Feel free to take it
up :)

How to get kernel data using /proc system?

[Lei Wang]

Hello, everyone, could anybody help me out of this? Any help would be
highly appreciated. Thanks!

I want to read out some kernel data. But I searched the web a lot and
all the methods I found in using /proc filesystem seem to be out of
date. Does anybody know how to do this in Linux 2.4 or 2.2? 

Also, are other ways to get access to kernel data than using
/proc?

Thanks a lot!

Re: [PATCH] spinlock not locked when unlocking in atm_dev_register

[Mitchell Blank Jr]

Maksim Krasnyanskiy wrote:
> btw ATM locking seems to be messed up. Is anybody working on that ?

Yes, pretty badly.  What we (the atm folks) really need to do for 2.5 is
rewrite a lot of code.  The current driver model predates SMP in the kernel
and it's pretty much impossible to make a driver 100% SMP correct (although,
in practice, you can get pretty close and keep the races pretty tiny)
Really a lot of the driver model needs to be rethought.

As for maintainership I readily admit I haven't spent nearly as much time
as I would have liked on the ATM stuff in the last year or so.  Too much
other work to do, etc.  I'm really hoping to get some free time to play
with stuff in the next couple months...  There are others actively working
on stuff.  Particularly Paul Schroeder at IBM did a lot of good work on
the (long-suffering) userland tools.

Anyway I have been thinking that we should start kicking around ideas for
the next-generation ATM code.  I'd recommend interested parties subscribe
to the linux-atm-general mailing list:
  http://lists.sourceforge.net/lists/listinfo/linux-atm-general

-Mitch

Re: [PATCH] spinlock not locked when unlocking in atm_dev_register

[Maksim Krasnyanskiy]

>    > btw ATM locking seems to be messed up. Is anybody working on that ?
>
>    Not that I know of.  Volunteer? :)
>
>I consider it pretty much unmaintained.  Feel free to take it up :)
I'd rather work on my Bluetooth stuff for now :)

The reason I looked at the ATM stuff is that I got this DSL thing from 
PacBell that
came with Alcatel ADSL USB modem which talks to the ATM layer. The guy who
wrote that USB driver had a comment that it doesn't work on SMP for 
_unknown reason_ :)
Quick glance at the ATM code, revealed that one of reasons is messed up 
looking.
So I fixed device registration and now it works. But ATM locking in general 
has to be fixed.

Max

Re: [PATCH] spinlock not locked when unlocking in atm_dev_register

[Frode Isaksen]

> On Fri, 2002-03-01 at 12:46, Frode Isaksen wrote:
>> If you compile the kernel with SMP and spinlock debugging, BUG() will be
>> called when registering your atm driver, since the "atm_dev_lock" spinlock is
>> not locked when unlocking it.
> 
> I don't have any knowledge of the source in question, but wouldn't a
> possibility (perhaps even more likely) be that you should _add_ the
> spin_lock instead of remove the spin_unlocks ?
The atm_dev_register function is calling functions that are using the same
spinlock, so you cannot just lock the spinlock when entering the function..

Hilsen,
Frode

Re: [PATCH] spinlock not locked when unlocking in atm_dev_register

[Robert Love]

On Mon, 2002-03-04 at 03:29, Frode Isaksen wrote:

> The atm_dev_register function is calling functions that are using the same
> spinlock, so you cannot just lock the spinlock when entering the function..

Ah, OK -- my apologies.  Then, a few things need to be checked:

	- atm_dev_register (ignoring its callees) does not need a lock
	- every callee of atm_dev_register that does need the lock
	  acquires and releases it itself (this is good design, too)

I suspect the second point may be missing the releases in cases, since
those spin_unlocks were in the code.

	Robert Love