The current AppleTalk stack in 2.2 is not SMP safe. We have been able to
reproduce lockups in the bottom half while doing heavy AppleTalk network
I/O in a few minutes. Michael Zuelsdorf <michael@helios.de> did a
backport of how the locking works in the AppleTalk kernel module in 2.4
to 2.2. After applying this patch we were unable reproduce the lockups.
--- linux-2.2.16/net/appletalk/aarp.c.original Wed Jul 5 10:21:14 2000
+++ linux-2.2.16/net/appletalk/aarp.c Fri Jul 7 15:44:23 2000
@@ -363,6 +363,7 @@
static void aarp_expire_timeout(unsigned long unused)
{
int ct=0;
+ SOCKHASH_LOCK();
for(ct=0;ct<AARP_HASH_SIZE;ct++)
{
aarp_expire_timer(&resolved[ct]);
@@ -370,6 +371,7 @@
aarp_expire_timer(&unresolved[ct]);
aarp_expire_timer(&proxies[ct]);
}
+ SOCKHASH_UNLOCK();
mod_timer(&aarp_timer, jiffies +
(unresolved_count ? sysctl_aarp_tick_time:
@@ -385,12 +387,14 @@
int ct=0;
if(event==NETDEV_DOWN)
{
+ SOCKHASH_LOCK();
for(ct=0;ct<AARP_HASH_SIZE;ct++)
{
aarp_expire_device(&resolved[ct],ptr);
aarp_expire_device(&unresolved[ct],ptr);
aarp_expire_device(&proxies[ct],ptr);
}
+ SOCKHASH_UNLOCK();
}
return NOTIFY_DONE;
}
@@ -414,9 +418,7 @@
*/
static struct aarp_entry *aarp_find_entry(struct aarp_entry *list, struct device *dev, struct at_addr *sat)
{
- unsigned long flags;
- save_flags(flags);
- cli();
+ SOCKHASH_LOCK();
while(list)
{
if(list->target_addr.s_net==sat->s_net &&
@@ -424,7 +426,7 @@
break;
list=list->next;
}
- restore_flags(flags);
+ SOCKHASH_UNLOCK();
return list;
}
@@ -434,12 +436,14 @@
int hash;
hash = sa->s_node % (AARP_HASH_SIZE-1);
+ SOCKHASH_LOCK();
a = aarp_find_entry(proxies[hash], dev, sa);
if (a)
{
a->expires_at = 0;
}
+ SOCKHASH_UNLOCK();
}
struct at_addr* aarp_proxy_find(struct device *dev, struct at_addr *sa)
@@ -547,6 +551,7 @@
entry->target_addr.s_net = sa->s_net;
entry->dev = atif->dev;
+ SOCKHASH_LOCK();
hash = sa->s_node % (AARP_HASH_SIZE-1);
entry->next = proxies[hash];
proxies[hash] = entry;
@@ -559,14 +564,18 @@
* Defer 1/10th
*/
current->state = TASK_INTERRUPTIBLE;
+ SOCKHASH_UNLOCK();
schedule_timeout(HZ/10);
+ SOCKHASH_LOCK();
/*
* our atif may no longer be valid, if the device was brought
* down while we waited!
*/
- if (atalk_find_dev(dev) != atif)
+ if (atalk_find_dev(dev) != atif) {
+ SOCKHASH_UNLOCK();
return -ENODEV;
+ }
if (entry->status & ATIF_PROBE_FAIL)
break;
@@ -583,6 +592,7 @@
entry->expires_at = 0;
/* return network full */
+ SOCKHASH_UNLOCK();
return (-EADDRINUSE);
}
else
@@ -591,6 +601,7 @@
entry->status &= ~ATIF_PROBE;
}
+ SOCKHASH_UNLOCK();
return 1;
}
@@ -603,7 +614,6 @@
static char ddp_eth_multicast[ETH_ALEN]={ 0x09, 0x00, 0x07, 0xFF, 0xFF, 0xFF };
int hash;
struct aarp_entry *a;
- unsigned long flags;
skb->nh.raw=skb->data;
@@ -680,9 +690,7 @@
skb->protocol = htons(ETH_P_ATALK);
hash=sa->s_node%(AARP_HASH_SIZE-1);
- save_flags(flags);
- cli();
-
+
/*
* Do we have a resolved entry ?
*/
@@ -693,9 +701,9 @@
if(skb->sk)
skb->priority = skb->sk->priority;
dev_queue_xmit(skb);
- restore_flags(flags);
return 1;
}
+ SOCKHASH_LOCK();
a=aarp_find_entry(resolved[hash],dev,sa);
if(a!=NULL)
{
@@ -708,7 +716,7 @@
if(skb->sk)
skb->priority = skb->sk->priority;
dev_queue_xmit(skb);
- restore_flags(flags);
+ SOCKHASH_UNLOCK();
return 1;
}
@@ -724,7 +732,7 @@
*/
skb_queue_tail(&a->packet_queue, skb);
- restore_flags(flags);
+ SOCKHASH_UNLOCK();
return 0;
}
@@ -739,7 +747,7 @@
* Whoops slipped... good job it's an unreliable
* protocol 8)
*/
- restore_flags(flags);
+ SOCKHASH_UNLOCK();
return -1;
}
@@ -755,7 +763,6 @@
a->xmit_count=0;
unresolved[hash]=a;
unresolved_count++;
- restore_flags(flags);
/*
* Send an initial request for the address
@@ -777,6 +784,7 @@
* Tell the ddp layer we have taken over for this frame.
*/
+ SOCKHASH_UNLOCK();
return 0;
}
@@ -828,7 +836,6 @@
struct elapaarp *ea=(struct elapaarp *)skb->h.raw;
struct aarp_entry *a;
struct at_addr sa, *ma, da;
- unsigned long flags;
int hash;
struct atalk_iface *ifa;
int func;
@@ -884,16 +891,15 @@
* Process the packet
*/
- save_flags(flags);
-
/*
* Check for replies of me
*/
+ SOCKHASH_LOCK();
ifa=atalk_find_dev(dev);
if(ifa==NULL)
{
- restore_flags(flags);
+ SOCKHASH_UNLOCK();
kfree_skb(skb);
return 1;
}
@@ -906,7 +912,7 @@
*/
ifa->status|=ATIF_PROBE_FAIL;
- restore_flags(flags);
+ SOCKHASH_UNLOCK();
kfree_skb(skb);
return 1;
}
@@ -916,11 +922,6 @@
* Check for replies of proxy AARP entries
*/
- /*
- * FIX ME: do we need a cli() here?
- * aarp_find_entry does one on its own, between saving and restoring flags, so
- * I don't think it is necessary, but I could be wrong -- it's happened before
- */
da.s_node = ea->pa_dst_node;
da.s_net = ea->pa_dst_net;
a = aarp_find_entry(proxies[hash], dev, &da);
@@ -933,7 +934,7 @@
* we do not respond to probe or request packets for
* this address while we are probing this address
*/
- restore_flags(flags);
+ SOCKHASH_UNLOCK();
kfree_skb(skb);
return 1;
}
@@ -947,7 +948,6 @@
* Find the entry
*/
- cli(); /* FIX ME: is this cli() necessary? aarp_find_entry does one on its own... */
if((a=aarp_find_entry(unresolved[hash],dev,&sa))==NULL || dev != a->dev)
break;
/*
@@ -1025,7 +1025,7 @@
aarp_send_reply(dev,ma,&sa,ea->hw_src);
break;
}
- restore_flags(flags);
+ SOCKHASH_UNLOCK();
kfree_skb(skb);
return 1;
}
@@ -1060,6 +1060,7 @@
{
int ct = 0;
+ SOCKHASH_LOCK();
for(ct = 0; ct < AARP_HASH_SIZE; ct++)
{
aarp_expire_device(&resolved[ct], dev);
@@ -1067,6 +1068,7 @@
aarp_expire_device(&proxies[ct], dev);
}
+ SOCKHASH_UNLOCK();
return;
}
@@ -1082,6 +1084,7 @@
len = sprintf(buffer,
"%-10.10s ""%-10.10s""%-18.18s""%12.12s""%12.12s"" xmit_count status\n",
"address","device","hw addr","last_sent", "expires");
+ SOCKHASH_LOCK();
for (ct = 0; ct < AARP_HASH_SIZE; ct++)
{
for (entry = resolved[ct]; entry; entry = entry->next)
@@ -1159,6 +1162,7 @@
}
+ SOCKHASH_UNLOCK();
return len;
}
--- linux-2.2.16/net/appletalk/ddp.c.original Wed Jul 5 13:40:00 2000
+++ linux-2.2.16/net/appletalk/ddp.c Fri Jul 7 15:59:17 2000
@@ -129,18 +129,23 @@
extern inline void atalk_remove_socket(struct sock *sk)
{
+ SOCKHASH_LOCK();
sklist_remove_socket(&atalk_socket_list,sk);
+ SOCKHASH_UNLOCK();
}
extern inline void atalk_insert_socket(struct sock *sk)
{
+ SOCKHASH_LOCK();
sklist_insert_socket(&atalk_socket_list,sk);
+ SOCKHASH_UNLOCK();
}
static struct sock *atalk_search_socket(struct sockaddr_at *to, struct atalk_iface *atif)
{
struct sock *s;
+ SOCKHASH_LOCK();
for(s = atalk_socket_list; s != NULL; s = s->next)
{
if(to->sat_port != s->protinfo.af_at.src_port)
@@ -172,6 +177,7 @@
break;
}
}
+ SOCKHASH_UNLOCK();
return (s);
}
@@ -183,6 +189,8 @@
{
struct sock *s;
+ SOCKHASH_LOCK();
+
for(s = atalk_socket_list; s != NULL; s = s->next)
{
if(s->protinfo.af_at.src_net != sat->sat_addr.s_net)
@@ -203,12 +211,15 @@
break;
}
+ SOCKHASH_UNLOCK();
return (s);
}
extern inline void atalk_destroy_socket(struct sock *sk)
{
+ SOCKHASH_LOCK();
sklist_destroy_socket(&atalk_socket_list, sk);
+ SOCKHASH_UNLOCK();
MOD_DEC_USE_COUNT;
}
@@ -227,6 +238,7 @@
*/
len += sprintf(buffer,"Type local_addr remote_addr tx_queue rx_queue st uid\n");
+ SOCKHASH_LOCK();
for(s = atalk_socket_list; s != NULL; s = s->next)
{
len += sprintf(buffer+len,"%02X ", s->type);
@@ -256,6 +268,8 @@
break;
}
+ SOCKHASH_UNLOCK();
+
/* The data in question runs from begin to begin+len */
*start = buffer + (offset - begin); /* Start of wanted data */
len -= (offset - begin); /* Remove unwanted header data from length */
@@ -288,6 +302,7 @@
struct atalk_iface **iface = &atalk_iface_list;
struct atalk_iface *tmp;
+ SOCKHASH_LOCK();
while((tmp = *iface) != NULL)
{
if(tmp->dev == dev)
@@ -300,14 +315,13 @@
else
iface = &tmp->next;
}
-
+ SOCKHASH_UNLOCK();
}
static struct atalk_iface *atif_add_device(struct device *dev, struct at_addr *sa)
{
struct atalk_iface *iface = (struct atalk_iface *)
kmalloc(sizeof(*iface), GFP_KERNEL);
- unsigned long flags;
if(iface==NULL)
return (NULL);
@@ -316,11 +330,11 @@
dev->atalk_ptr=iface;
iface->address= *sa;
iface->status=0;
- save_flags(flags);
- cli();
+
+ SOCKHASH_LOCK();
iface->next=atalk_iface_list;
atalk_iface_list=iface;
- restore_flags(flags);
+ SOCKHASH_UNLOCK();
MOD_INC_USE_COUNT;
@@ -487,20 +501,29 @@
* Return a point-to-point interface only if
* there is no non-ptp interface available.
*/
+ SOCKHASH_LOCK();
for(iface=atalk_iface_list; iface != NULL; iface=iface->next)
{
if(!fiface && !(iface->dev->flags & IFF_LOOPBACK))
fiface=iface;
- if(!(iface->dev->flags & (IFF_LOOPBACK | IFF_POINTOPOINT)))
+ if(!(iface->dev->flags & (IFF_LOOPBACK | IFF_POINTOPOINT))) {
+ SOCKHASH_UNLOCK();
return (&iface->address);
+ }
}
- if(fiface)
+ if(fiface) {
+ SOCKHASH_UNLOCK();
return (&fiface->address);
- if(atalk_iface_list != NULL)
+ }
+ if(atalk_iface_list != NULL) {
+ SOCKHASH_UNLOCK();
return (&atalk_iface_list->address);
- else
+ }
+ else {
+ SOCKHASH_UNLOCK();
return (NULL);
+ }
}
/*
@@ -527,21 +550,27 @@
{
struct atalk_iface *iface;
+ SOCKHASH_LOCK();
for(iface=atalk_iface_list; iface != NULL; iface=iface->next)
{
if((node==ATADDR_BCAST || node==ATADDR_ANYNODE
|| iface->address.s_node==node)
&& iface->address.s_net==net
- && !(iface->status & ATIF_PROBE))
+ && !(iface->status & ATIF_PROBE)) {
+ SOCKHASH_UNLOCK();
return (iface);
+ }
/* XXXX.0 -- net.0 returns the iface associated with net */
if ((node==ATADDR_ANYNODE) && (net != ATADDR_ANYNET) &&
(ntohs(iface->nets.nr_firstnet) <= ntohs(net)) &&
- (ntohs(net) <= ntohs(iface->nets.nr_lastnet)))
+ (ntohs(net) <= ntohs(iface->nets.nr_lastnet))) {
+ SOCKHASH_UNLOCK();
return (iface);
+ }
}
+ SOCKHASH_UNLOCK();
return (NULL);
}
@@ -561,6 +590,7 @@
struct atalk_route *r;
struct atalk_route *net_route = NULL;
+ SOCKHASH_LOCK();
for(r=atalk_router_list; r != NULL; r=r->next)
{
if(!(r->flags & RTF_UP))
@@ -573,8 +603,10 @@
* if this host route is for the target,
* the we're done
*/
- if (r->target.s_node == target->s_node)
+ if (r->target.s_node == target->s_node) {
+ SOCKHASH_UNLOCK();
return (r);
+ }
}
else
{
@@ -591,12 +623,17 @@
* if we found a network route but not a direct host
* route, then return it
*/
- if (net_route != NULL)
+ if (net_route != NULL) {
+ SOCKHASH_UNLOCK();
return (net_route);
+ }
- if(atrtr_default.dev)
+ if(atrtr_default.dev) {
+ SOCKHASH_UNLOCK();
return (&atrtr_default);
+ }
+ SOCKHASH_UNLOCK();
return (NULL);
}
@@ -637,9 +674,6 @@
struct sockaddr_at *ga=(struct sockaddr_at *)&r->rt_gateway;
struct atalk_route *rt;
struct atalk_iface *iface, *riface;
- unsigned long flags;
-
- save_flags(flags);
/*
* Fixme: Raise/Lower a routing change semaphore for these
@@ -657,6 +691,7 @@
/*
* Now walk the routing table and make our decisions.
*/
+ SOCKHASH_LOCK();
for(rt=atalk_router_list; rt!=NULL; rt=rt->next)
{
if(r->rt_flags != rt->flags)
@@ -685,17 +720,21 @@
riface = iface;
}
- if(riface == NULL)
+ if(riface == NULL) {
+ SOCKHASH_UNLOCK();
return (-ENETUNREACH);
+ }
devhint = riface->dev;
}
if(rt == NULL)
{
rt = (struct atalk_route *)kmalloc(sizeof(struct atalk_route), GFP_KERNEL);
- if(rt == NULL)
+ if(rt == NULL) {
+ SOCKHASH_UNLOCK();
return (-ENOBUFS);
- cli();
+ }
+
rt->next = atalk_router_list;
atalk_router_list = rt;
}
@@ -708,8 +747,7 @@
rt->flags = r->rt_flags;
rt->gateway = ga->sat_addr;
- restore_flags(flags);
-
+ SOCKHASH_UNLOCK();
return (0);
}
@@ -720,7 +758,8 @@
{
struct atalk_route **r = &atalk_router_list;
struct atalk_route *tmp;
-
+
+ SOCKHASH_LOCK();
while((tmp = *r) != NULL)
{
if(tmp->target.s_net == addr->s_net
@@ -729,11 +768,13 @@
{
*r = tmp->next;
kfree_s(tmp, sizeof(struct atalk_route));
+ SOCKHASH_UNLOCK();
return (0);
}
r = &tmp->next;
}
-
+
+ SOCKHASH_UNLOCK();
return (-ENOENT);
}
@@ -746,6 +787,7 @@
struct atalk_route **r = &atalk_router_list;
struct atalk_route *tmp;
+ SOCKHASH_LOCK();
while((tmp = *r) != NULL)
{
if(tmp->dev == dev)
@@ -756,6 +798,7 @@
else
r = &tmp->next;
}
+ SOCKHASH_UNLOCK();
if(atrtr_default.dev == dev)
atrtr_set_default(NULL);
@@ -1061,6 +1104,7 @@
off_t begin=0;
len += sprintf(buffer,"Interface Address Networks Status\n");
+ SOCKHASH_LOCK();
for(iface = atalk_iface_list; iface != NULL; iface = iface->next)
{
len += sprintf(buffer+len,"%-16s %04X:%02X %04X-%04X %d\n",
@@ -1076,6 +1120,8 @@
if(pos > offset + length)
break;
}
+ SOCKHASH_UNLOCK();
+
*start = buffer + (offset - begin);
len -= (offset - begin);
if(len > length)
@@ -1102,6 +1148,7 @@
rt->dev->name);
}
+ SOCKHASH_LOCK();
for(rt = atalk_router_list; rt != NULL; rt = rt->next)
{
len += sprintf(buffer+len,"%04X:%02X %04X:%02X %-4d %s\n",
@@ -1117,6 +1164,7 @@
if(pos > offset + length)
break;
}
+ SOCKHASH_UNLOCK();
*start = buffer + (offset - begin);
len -= (offset - begin);
@@ -1238,12 +1286,15 @@
*/
static int atalk_pick_port(struct sockaddr_at *sat)
{
+ SOCKHASH_LOCK();
for(sat->sat_port = ATPORT_LAST; sat->sat_port >= ATPORT_RESERVED; sat->sat_port--)
{
- if(atalk_find_socket(sat) == NULL)
+ if(atalk_find_socket(sat) == NULL) {
+ SOCKHASH_UNLOCK();
return sat->sat_port;
+ }
}
-
+ SOCKHASH_UNLOCK();
return (-EBUSY);
}
-- Jens-Uwe MagerHELIOS Software GmbH Steinriede 3 30827 Garbsen Germany
Phone: +49 5131 709320 FAX: +49 5131 709325 Internet: jum@helios.de
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Fri Jul 07 2000 - 21:00:21 EST