2.4.10-pre13: ATM drivers cause panic

Hi,

the following ATM drivers cause linux to panic during boot (if one of the cards is inserted :-))
iphase
fore200e
he

Machine is a 2 PIII, 3GB, Asus CUR-DLS motherboard - tried NOAPIC, MAXCPUS, same difference.
When installing as a module I'll get the following trace below.
Any help is greatly appreciated!

Thanks

Immanuel

P.S. If you need more info please let me know what kind...

Sep 21 18:03:41 ipat01 kernel: invalid operand: 0000
Sep 21 18:03:41 ipat01 kernel: CPU:    0
Sep 21 18:03:41 ipat01 kernel: EIP:    0010:[atm_dev_register+289/308]
Sep 21 18:03:41 ipat01 kernel: EFLAGS: 00010202
Sep 21 18:03:41 ipat01 kernel: eax: 00000001   ebx: f75303c0   ecx: 0000006f
edx: 0000120c
Sep 21 18:03:41 ipat01 kernel: esi: f898ca09   edi: f753042c   ebp: 00000000
esp: f619beb8
Sep 21 18:03:41 ipat01 kernel: ds: 0018   es: 0018   ss: 0018
Sep 21 18:03:41 ipat01 kernel: Process insmod (pid: 1078, stackpage=f619b000)
Sep 21 18:03:41 ipat01 kernel: Stack: f898d410 00000000 f898d460 c4322800
f89880a6 f898ca09 f898d3c0 ffffffff
Sep 21 18:03:41 ipat01 kernel:        00000000 f898d410 c4322800 f898d460
00000000 c4322800 c024871e c4322800
Sep 21 18:03:41 ipat01 kernel:        f898d410 c4322800 f898d460 00000000
00005580 c0248784 f898d460 c4322800
Sep 21 18:03:41 ipat01 kernel: Call Trace: [<f898d410>] [<f898d460>]
[<f89880a6>] [<f898ca09>] [<f898d3c0>]
Sep 21 18:03:41 ipat01 kernel:    [<f898d410>] [<f898d460>]
[pci_announce_device+54/84] [<f898d410>] [<f898d460>]
[pci_register_driver+72/96]
Sep 21 18:03:41 ipat01 kernel:    [<f898d460>] [<f8988063>] [<f898c8bf>]
[<f898d460>] [sys_init_module+1357/1580] [<f8988060>]
Sep 21 18:03:41 ipat01 kernel:    [system_call+51/56]
Sep 21 18:03:41 ipat01 kernel:
Sep 21 18:03:41 ipat01 kernel: Code: 0f 0b c6 05 14 74 34 c0 01 89 d8 5b 5e 5f

5d c3 8d 76 00 53

--

Till Immanuel Patzschke                 mailto: [email protected]
interNetwork AG                         Phone:  +49-(0)611-1731-121
Bierstadter Str. 7                      Fax:    +49-(0)611-1731-31
D-65189 Wiesbaden                       Web:    http://www.internetwork-ag.de

Thats confusing since I don't immediately see where the BUG() it hits is. Can you rebuild with verbose kernel debugging enabled
Kernel debugging = Y
Verbose BUG() reporting = Y

Alan

Well - here it comes w/ BUG in spinlock.h...

Thanks for your help...

Immanuel

ksymoops 2.4.0 on i686 2.4.10-pre13. Options used
-V (default)
-k /proc/ksyms (default)
-l /proc/modules (default)
-o /lib/modules/2.4.10-pre13/ (default) -m /boot/System.map (specified)

Reading Oops report from the terminal

Sep 21 18:41:15 ipat01 kernel: kernel BUG at
/usr/src/linux-2.4.10-pre13-inw/include/asm/spinlock.h:104!
Sep 21 18:41:15 ipat01 kernel: invalid operand: 0000
Sep 21 18:41:15 iSep 21 18:41:15 ipat01 kernel: kernel BUG at
/usr/src/linux-2.4.10-pre13-inw/include/asm/spinlock.h:104!
pat01 kernel: CPU:    0
Sep 21 18:41:15 ipat01 kernel: EIP:    0010:[atm_dev_register+350/376]
Sep 21 18:41:15 ipat01 kernel: EFLAGS: 00010282
Sep 21 18:41:15 ipat01 kernel: eax: 0000004a   ebx: f6e2e160   ecx: c0374428
edx: 00003da4
Sep 21 18:41:15 ipat01 kernel: esi: f8995fb2   edi: f6e2e1cc   ebp: 00000000
esp: f6b43eb0
Sep 21 18:41:15 ipat01 kernel: ds: 0018   es: 0018   ss: 0018
Sep 21 18:41:15 ipat01 kernel: Process insmod (pid: 520, stackpage=f6b43000)
Sep 21 18:41:15 ipat01 kernel: Stack: c035fd40 00000068 f89969b0 00000000
f8996a00 c4322800 f89910a6 f8995fb2
Sep 21 18:41:15 ipat01 kernel:        f8996960 ffffffff 00000000 f89969b0
c4322800Sep 21 18:41:15 ipat01 kernel: invalid operand: 0000
Sep 21 18:41:15 ipat01 kernel: CPU:    0
Sep 21 18:41:15 ipat01 kernel: EIP:    0010:[atm_dev_register+350/376]
 f8996a00 00000000 c4322800
Sep 21 18:41:15 ipat01 kernel:        c025c90e c4322800 f89969b0 c4322800
f8996a00 00000000 00005b20 c025c974
Sep 21 18:41:15 ipat01 kernel: Call Trace: [<f89969b0>] [<f8996a00>]
[<f89910a6>] [<f8995fb2>] [<f8996960>]
Sep 21 18:41:15 ipat01 kernel:    [<f89969b0>] [<f8996a00>]
[pci_announce_device+54/84] [<f89969b0>] [<f8996a00>]
[pci_register_driver+72/96]
SeSep 21 18:41:15 ipat01 kernel: EFLAGS: 00010282
Sep 21 18:41:15 ipat01 kernel: eax: 0000004a   ebx: f6e2e160   ecx: c0374428
edx: 00003da4
Sep 21 18:41:15 ipat01 kernel: esi: f8995fb2   edi: f6e2e1cc   ebp: 00000000
esp: f6b43eb0
Sep 21 18:41:15 ipat01 kernel: ds: 0018   es: 0018   ss: 0018
p 21 18:41:15 ipat01 kernel:    [<f8996a00>] [<f8991063>] [<f8995c5f>]
[<f8996a00>] [sys_init_module+1373/1648] [<f8991060>]
Sep 21 18:41:15 ipat01 kernel:    [system_call+51/56]
Sep 21 18:41:15 ipat01 kernel:
Sep 21 18:41:15 ipat01 kernel: Code: 0f 0b 83 c4 08 90 8d 74 26 00 c6 05 74 c1
38 c0 01 89 d8 5b
Sep 21 18:41:15 ipat01 kernel: Process insmod (pid: 520, stackpage=f6b43000)
Sep 21 18:41:15 ipat01 kernel: Stack: c035fd40 00000068 f89969b0 00000000
f8996a00 c4322800 f89910a6 f8995fb2
Sep 21 18:41:15 ipat01 kernel:        f8996960 ffffffff 00000000 f89969b0
c4322800 f8996a00 00000000 c4322800
Sep 21 18:41:15 ipat01 kernel:        c025c90e c4322800 f89969b0 c4322800
f8996a00 00000000 00005b20 c025c974
Sep 21 18:41:15 ipat01 kernel: Call Trace: [<f89969b0>] [<f8996a00>]
[<f89910a6>] [<f8995fb2>] [<f8996960>]
Sep 21 18:41:15 ipat01 kernel:    [<f89969b0>] [<f8996a00>]
[pci_announce_device+54/84] [<f89969b0>] [<f8996a00>]
[pci_register_driver+72/96]
Sep 21 18:41:15 ipat01 kernel:    [<f8996a00>] [<f8991063>] [<f8995c5f>]
[<f8996a00>] [sys_init_module+1373/1648] [<f8991060>]
Sep 21 18:41:15 ipat01 kernel: Code: 0f 0b 83 c4 08 90 8d 74 26 00 c6 05 74 c1

38 c0 01 89 d8 5b
Using defaults from ksymoops -t elf32-i386 -a i386

Trace; f89969b0 <[he]he_pci_tbl+0/50>
Trace; f8996a00 <[he]he_driver+0/27>
Trace; f89910a6 <[he]__module_parm_desc_disable64+6/d4> Trace; f8995fb2 <[he].rodata.start+212/abf> Trace; f8996960 <[he]he_ops+0/40>
Trace; f89969b0 <[he]he_pci_tbl+0/50>
Trace; f8996a00 <[he]he_driver+0/27>
Trace; f8996a00 <[he]he_driver+0/27>
Trace; f8991063 <[he]he_init_one+3/2c>
Trace; f8995c5f <[he]he_init+b/38>
Trace; f8996a00 <[he]he_driver+0/27>
Code; 00000000 Before first symbol
00000000 <_EIP>:
Code; 00000000 Before first symbol
0: 0f 0b ud2a
Code; 00000002 Before first symbol
2: 83 c4 08 add $0x8,%esp
Code; 00000005 Before first symbol
5: 90 nop
Code; 00000006 Before first symbol
6: 8d 74 26 00 lea 0x0(%esi,1),%esi
Code; 0000000a Before first symbol
a: c6 05 74 c1 38 c0 01 movb $0x1,0xc038c174
Code; 00000011 Before first symbol
11: 89 d8 mov %ebx,%eax
Code; 00000013 Before first symbol
13: 5b pop %ebx

--

Till Immanuel Patzschke                 mailto: [email protected]
interNetwork AG                         Phone:  +49-(0)611-1731-121
Bierstadter Str. 7                      Fax:    +49-(0)611-1731-31
D-65189 Wiesbaden                       Web:    http://www.internetwork-ag.de

Hi,

seems a couple of spin_lock(s) and a spin_unlock was missing.
Why didn't this problem show up with earlier releases ??? Anyways, please find a (quick) patch below. It would be great if this patch or any other similar could make it into the next release!
Thanks,

Immanuel

diff -Naur net/atm/resources.c.bug net/atm/resources.c
@@ -76,14 +76,17 @@
 {
        struct atm_dev *dev;
 
+       spin_lock (&atm_dev_lock);
        dev = alloc_atm_dev(type);
        if (!dev) {
                printk(KERN_ERR "atm_dev_register: no space for dev %s\n",
                    type);
+               spin_unlock (&atm_dev_lock);
                return NULL;
        }
        if (number != -1) {
                if (atm_find_dev(number)) {
+                       spin_unlock (&atm_dev_lock);
                        free_atm_dev(dev);
                        return NULL;
                }

--

Till Immanuel Patzschke                 mailto: [email protected]
interNetwork AG                         Phone:  +49-(0)611-1731-121
Bierstadter Str. 7                      Fax:    +49-(0)611-1731-31
D-65189 Wiesbaden                       Web:    http://www.internetwork-ag.de

That patch cannot possibly be correct. alloc_atm_dev sleeps

How about

static struct atm_dev *alloc_atm_dev(const char *type) {
struct atm_dev *dev;

	dev = kmalloc(sizeof(*dev),GFP_KERNEL);
	if (!dev) return NULL;
	memset(dev,0,sizeof(*dev));
	dev->type = type;
        dev->signal = ATM_PHY_SIG_UNKNOWN;
        dev->link_rate = ATM_OC3_PCR;
        dev->next = NULL;

        spin_lock(&atm_dev_lock);

        dev->prev = last_dev;

	if (atm_devs) last_dev->next = dev;
	else atm_devs = dev;
	last_dev = dev;
	spin_unlock(&atm_dev_lock);
	return dev;
}

instead. That seems to fix alloc_atm_dev safely. Refcounting wants adding to atm_dev objects too, its impossible currently to make atm_find_dev remotely safe

Alan

Hmm - patch works fine for me - no sleeps! The only spin_lock(&atm_dev_lock) statement in my resource.c (the original from 2.4.10-pre13) is in free_atm_dev BUT the problem is the unmatched spin_unlock(&atm_dev_lock) statements in atm_dev_register...
Why not just protecting the atm_dev_queue in alloc_atm_dev, atm_find_dev, and atm_free_dev individually PLUS removing the two spin_unlock statements in atm_dev_register.

What do you think

(This is diffs from 2.4.10-pre13 ! BTW: Still the same in 2.4.10)

@@ -36,13 +36,16 @@
        if (!dev) return NULL;
        memset(dev,0,sizeof(*dev));
        dev->type = type;
-       dev->prev = last_dev;
        dev->signal = ATM_PHY_SIG_UNKNOWN;
        dev->link_rate = ATM_OC3_PCR;
        dev->next = NULL;
+
+       spin_lock(&atm_dev_lock);
+       dev->prev = last_dev;
        if (atm_devs) last_dev->next = dev;
        else atm_devs = dev;
        last_dev = dev;
+       spin_unlock(&atm_dev_lock);
        return dev;
 }
 

@@ -65,9 +68,13 @@
 {
        struct atm_dev *dev;
 
+       spin_lock(&atm_dev_lock);
        for (dev = atm_devs; dev; dev = dev->next)
-               if (dev->ops && dev->number == number) return dev;
-       return NULL;
+               if (dev->ops && dev->number == number) goto done;
+       dev=(atm_dev *)NULL;
+ done:
+       spin_unlock(&atm_dev_lock);
+       return dev;
 }
 
 

@@ -105,12 +112,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;
 }
 

--

Till Immanuel Patzschke                 mailto: [email protected]
interNetwork AG                         Phone:  +49-(0)611-1731-121
Bierstadter Str. 7                      Fax:    +49-(0)611-1731-31
D-65189 Wiesbaden                       Web:    http://www.internetwork-ag.de

Hello All , Isn't the linux-atm list the more appropriate list for this discussion ? ie:

[email protected]

The people there would definately like to know of these discussions & troubles & fixes that are being shared .
Tia , JimL

       +------------------------------------------------------------------+
       | James   W.   Laferriere | System    Techniques | Give me VMS     |
       | Network        Engineer |     P.O. Box 854     |  Give me Linux  |
       | [email protected] | Coudersport PA 16915 |   only  on  AXP |
       +------------------------------------------------------------------+

Actually there are a LOT of places that atm_dev_lock is held across sleeps - I've been meaning to deal with them for awhile. Some of them are noted by the Stanford checker, others are outside its reach (like calls into the function pointers in atm_dev). I've been meaning to fix it once and for all by turning that spinlock into a semaphore, but have not had a chance to audit the code and make sure that it will be safe in all circumstances. I need to trace all the interrupt paths and see what their locking needs are.

I'm not at home tonight so I can't look at the code much right now, but I'll try to sort out what the best fix is and forward it on to you.

-Mitch