[BUG] 2.4.10-pre13: ATM drivers cause panic

[BUG] 2.4.10-pre13: ATM drivers cause panic

[Till Immanuel Patzschke]

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: tip@internetwork-ag.de
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

Re: [BUG] 2.4.10-pre13: ATM drivers cause panic

[Alan Cox]

> 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

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

Re: [BUG] 2.4.10-pre13: ATM drivers cause panic

[Till Immanuel Patzschke]

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


Alan Cox wrote:

> > 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
>
> 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

--
Till Immanuel Patzschke                 mailto: tip@internetwork-ag.de
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

[PATCH] 2.4.10-pre13: ATM drivers cause panic

[Till Immanuel Patzschke]

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
--- net/atm/resources.c.bug     Fri Dec 29 23:35:47 2000
+++ net/atm/resources.c Sat Sep 22 15:31:35 2001
@@ -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: tip@internetwork-ag.de
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

Re: [PATCH] 2.4.10-pre13: ATM drivers cause panic

[Alan Cox]

> 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,

That patch cannot possibly be correct. alloc_atm_dev sleeps

Re: [PATCH] 2.4.10-pre13: ATM drivers cause panic

[Alan Cox]

> 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!

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

Re: [PATCH] 2.4.10-pre13: ATM drivers cause panic

[Mitchell Blank Jr]

Alan Cox wrote:
> > 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,
> 
> That patch cannot possibly be correct. alloc_atm_dev sleeps

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

Re: [PATCH] 2.4.10-pre13: ATM drivers cause panic

[Till Immanuel Patzschke]

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)

--- resources.c.bug     Fri Dec 29 23:35:47 2000
+++ resources.c.new     Mon Sep 24 11:39:42 2001
@@ -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;
 }



Alan Cox wrote:

> > 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!
>
> 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

--
Till Immanuel Patzschke                 mailto: tip@internetwork-ag.de
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

Re: [PATCH] 2.4.10-pre13: ATM drivers cause panic

[Mr. James W. Laferriere]

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

	linux-atm-general@lists.sourceforge.net

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

On Mon, 24 Sep 2001, Till Immanuel Patzschke wrote:

> 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)
>
> --- resources.c.bug     Fri Dec 29 23:35:47 2000
> +++ resources.c.new     Mon Sep 24 11:39:42 2001
> @@ -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;
>  }
>
>
>
> Alan Cox wrote:
>
> > > 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!
> >
> > 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
>
> --
> Till Immanuel Patzschke                 mailto: tip@internetwork-ag.de
> 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
>
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

       +------------------------------------------------------------------+
       | James   W.   Laferriere | System    Techniques | Give me VMS     |
       | Network        Engineer |     P.O. Box 854     |  Give me Linux  |
       | babydr@baby-dragons.com | Coudersport PA 16915 |   only  on  AXP |
       +------------------------------------------------------------------+