On Mon, Jul 22, 2002 at 09:48:14PM +0200, Petr Vandrovec wrote:
> Hi Dave, hi Ingo,
> patch below switches 802.2 and SNAP layer to use rwlock instead
> of cli/sti. If you think that read_lock() has too much overhead
> in this code path, I'm sorry, I have no better solution...
>
> It also fixes race between two tasks doing register_*_client
> at once - there was no locking here.
>
> If you agree, tell me and I'll forward it to Linus. Or do so
> yourself.
> Thanks,
> Petr Vandrovec
> vandrove@vc.cvut.cz
Oops. Older version was missing read_unlock in one of psnap.c error
paths. Fixed version follows.
Petr Vandrovec
diff -urdN linux/net/802/p8022.c linux/net/802/p8022.c
--- linux/net/802/p8022.c 2002-07-22 13:32:52.000000000 +0000
+++ linux/net/802/p8022.c 2002-07-22 19:32:20.000000000 +0000
@@ -31,6 +31,8 @@
extern void llc_unregister_sap(unsigned char sap);
static struct datalink_proto *p8022_list;
+static rwlock_t p8022_list_lock = RW_LOCK_UNLOCKED;
+
/*
* We don't handle the loopback SAP stuff, the extended
* 802.2 command set, multicast SAP identifiers and non UI
@@ -53,6 +55,7 @@
struct datalink_proto *proto;
int rc = 0;
+ read_lock(&p8022_list_lock);
proto = find_8022_client(*(skb->h.raw));
if (!proto) {
skb->sk = NULL;
@@ -63,7 +66,8 @@
skb->nh.raw += 3;
skb_pull(skb, 3);
rc = proto->rcvfunc(skb, dev, pt);
-out: return rc;
+out: read_unlock(&p8022_list_lock);
+ return rc;
}
static void p8022_datalink_header(struct datalink_proto *dl,
@@ -84,7 +88,9 @@
struct packet_type *))
{
struct datalink_proto *proto = NULL;
+ unsigned long flags;
+ write_lock_irqsave(&p8022_list_lock, flags);
if (find_8022_client(type))
goto out;
proto = kmalloc(sizeof(*proto), GFP_ATOMIC);
@@ -99,7 +105,8 @@
p8022_list = proto;
llc_register_sap(type, p8022_rcv);
}
-out: return proto;
+out: write_unlock_irqrestore(&p8022_list_lock, flags);
+ return proto;
}
void unregister_8022_client(unsigned char type)
@@ -107,8 +114,7 @@
struct datalink_proto *tmp, **clients = &p8022_list;
unsigned long flags;
- save_flags(flags);
- cli();
+ write_lock_irqsave(&p8022_list_lock, flags);
while (*clients) {
tmp = *clients;
if (tmp->type[0] == type) {
@@ -119,7 +125,7 @@
}
clients = &tmp->next;
}
- restore_flags(flags);
+ write_unlock_irqrestore(&p8022_list_lock, flags);
}
EXPORT_SYMBOL(register_8022_client);
diff -urdN linux/net/802/psnap.c linux/net/802/psnap.c
--- linux/net/802/psnap.c 2002-07-22 13:32:58.000000000 +0000
+++ linux/net/802/psnap.c 2002-07-22 19:52:11.000000000 +0000
@@ -22,6 +22,7 @@
static struct datalink_proto *snap_list = NULL;
static struct datalink_proto *snap_dl = NULL; /* 802.2 DL for SNAP */
+static rwlock_t snap_list_lock = RW_LOCK_UNLOCKED;
/*
* Find a snap client by matching the 5 bytes.
@@ -52,9 +53,12 @@
struct datalink_proto *proto;
+ read_lock(&snap_list_lock);
proto = find_snap_client(skb->h.raw);
if (proto != NULL)
{
+ int err;
+
/*
* Pass the frame on.
*/
@@ -65,8 +69,11 @@
if (psnap_packet_type.type == 0)
psnap_packet_type.type=htons(ETH_P_SNAP);
- return proto->rcvfunc(skb, dev, &psnap_packet_type);
+ err = proto->rcvfunc(skb, dev, &psnap_packet_type);
+ read_unlock(&snap_list_lock);
+ return err;
}
+ read_unlock(&snap_list_lock);
skb->sk = NULL;
kfree_skb(skb);
return 0;
@@ -104,10 +111,12 @@
struct datalink_proto *register_snap_client(unsigned char *desc, int (*rcvfunc)(struct sk_buff *, struct net_device *, struct packet_type *))
{
- struct datalink_proto *proto;
+ struct datalink_proto *proto = NULL;
+ unsigned long flags;
+ write_lock_irqsave(&snap_list_lock, flags);
if (find_snap_client(desc) != NULL)
- return NULL;
+ goto out;
proto = (struct datalink_proto *) kmalloc(sizeof(*proto), GFP_ATOMIC);
if (proto != NULL)
@@ -121,7 +130,7 @@
proto->next = snap_list;
snap_list = proto;
}
-
+out: write_unlock_irqrestore(&snap_list_lock, flags);
return proto;
}
@@ -135,8 +144,7 @@
struct datalink_proto *tmp;
unsigned long flags;
- save_flags(flags);
- cli();
+ write_lock_irqsave(&snap_list_lock, flags);
while ((tmp = *clients) != NULL)
{
@@ -150,5 +158,5 @@
clients = &tmp->next;
}
- restore_flags(flags);
+ write_unlock_irqrestore(&snap_list_lock, flags);
}
-
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/
This archive was generated by hypermail 2b29 : Tue Jul 23 2002 - 22:00:40 EST