lkml.org 
[lkml]   [2001]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] 2.4.10-pre13: ATM drivers cause panic
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/

\
 
 \ /
  Last update: 2005-03-22 13:03    [W:0.155 / U:2.820 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site