[ATM] cli() for net/atm/lec.c

[ATM] cli() for net/atm/lec.c

Post by chas william » Fri, 21 Feb 2003 20:00:19



the bulk of the cli() -> spinlock() conversion was done by Mike Westall
<west...@cs.clemson.edu>.  i fixed a double locking issue and made the
following changes:

. renamed lec_arp_lock/lec_arp_unlock (since it seems confusing to have
LEC_ARP_LOCK and lec_arp_lock) to lec_arp_get/lec_arp_put since these
seem to be doing reference counting

. lec_arp_lock_var was renamed to lec_arp_users

. read lec_arp_lock_var with atomic_read()!

. the original lec_arp_put was renamed to lec_arp_add (so we have the
put/get and add/remove pairs)

this is just a start at cleaning up some of the lec client code.
comments?

Index: linux/net/atm/lec.c
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/lec.c,v
retrieving revision 1.1.1.1
diff -u -d -b -w -r1.1.1.1 lec.c
--- linux/net/atm/lec.c 20 Feb 2003 13:46:30 -0000      1.1.1.1
+++ linux/net/atm/lec.c 20 Feb 2003 15:00:39 -0000
@@ -20,6 +20,7 @@
 #include <net/arp.h>
 #include <net/dst.h>
 #include <linux/proc_fs.h>
+#include <linux/spinlock.h>

 /* TokenRing if needed */
 #ifdef CONFIG_TR
@@ -54,7 +55,11 @@
 extern struct net_bridge_fdb_entry *(*br_fdb_get_hook)(struct net_bridge *br,
        unsigned char *addr);
 extern void (*br_fdb_put_hook)(struct net_bridge_fdb_entry *ent);
+static spinlock_t lec_arp_spinlock = SPIN_LOCK_UNLOCKED;
+static unsigned long lec_arp_flags;

+#define LEC_ARP_LOCK()   spin_lock_irqsave(&lec_arp_spinlock, lec_arp_flags);
+#define LEC_ARP_UNLOCK() spin_unlock_irqrestore(&lec_arp_spinlock, lec_arp_flags);

 #define DUMP_PACKETS 0 /* 0 = None,
                         * 1 = 30 first bytes
@@ -634,6 +639,7 @@
         dev->get_stats = lec_get_stats;
         dev->set_multicast_list = NULL;
         dev->do_ioctl  = NULL;
+       spin_lock_init(&lec_arp_spinlock);
         printk("%s: Initialized!\n",dev->name);
         return;
 }
@@ -1044,15 +1050,15 @@
 #define HASH(ch) (ch & (LEC_ARP_TABLE_SIZE -1))

 static __inline__ void
-lec_arp_lock(struct lec_priv *priv)
+lec_arp_get(struct lec_priv *priv)
 {
-        atomic_inc(&priv->lec_arp_lock_var);
+        atomic_inc(&priv->lec_arp_users);
 }

 static __inline__ void
-lec_arp_unlock(struct lec_priv *priv)
+lec_arp_put(struct lec_priv *priv)
 {
-        atomic_dec(&priv->lec_arp_lock_var);
+        atomic_dec(&priv->lec_arp_users);
 }

 /*
@@ -1103,33 +1109,32 @@
  * LANE2: Add to the end of the list to satisfy 8.1.13
  */
 static __inline__ void
-lec_arp_put(struct lec_arp_table **lec_arp_tables,
-            struct lec_arp_table *to_put)
+lec_arp_add(struct lec_arp_table **lec_arp_tables,
+            struct lec_arp_table *to_add)
 {
         unsigned short place;
-        unsigned long flags;
         struct lec_arp_table *tmp;

-        save_flags(flags);
-        cli();
+        LEC_ARP_LOCK();

-        place = HASH(to_put->mac_addr[ETH_ALEN-1]);
+        place = HASH(to_add->mac_addr[ETH_ALEN-1]);
         tmp = lec_arp_tables[place];
-        to_put->next = NULL;
+        to_add->next = NULL;
         if (tmp == NULL)
-                lec_arp_tables[place] = to_put;
+                lec_arp_tables[place] = to_add;

         else {  /* add to the end */
                 while (tmp->next)
                         tmp = tmp->next;
-                tmp->next = to_put;
+                tmp->next = to_add;
         }

-        restore_flags(flags);
+        LEC_ARP_UNLOCK();
+
         DPRINTK("LEC_ARP: Added entry:%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
-                0xff&to_put->mac_addr[0], 0xff&to_put->mac_addr[1],
-                0xff&to_put->mac_addr[2], 0xff&to_put->mac_addr[3],
-                0xff&to_put->mac_addr[4], 0xff&to_put->mac_addr[5]);
+                0xff&to_add->mac_addr[0], 0xff&to_add->mac_addr[1],
+                0xff&to_add->mac_addr[2], 0xff&to_add->mac_addr[3],
+                0xff&to_add->mac_addr[4], 0xff&to_add->mac_addr[5]);
 }

 /*
@@ -1141,14 +1146,12 @@
 {
         unsigned short place;
         struct lec_arp_table *tmp;
-        unsigned long flags;
         int remove_vcc=1;

-        save_flags(flags);
-        cli();
+        LEC_ARP_LOCK();

         if (!to_remove) {
-                restore_flags(flags);
+                LEC_ARP_UNLOCK();
                 return -1;
         }
         place = HASH(to_remove->mac_addr[ETH_ALEN-1]);
@@ -1160,7 +1163,7 @@
                         tmp = tmp->next;
                 }
                 if (!tmp) {/* Entry was not found */
-                        restore_flags(flags);
+                        LEC_ARP_UNLOCK();
                         return -1;
                 }
         }
@@ -1186,7 +1189,9 @@
                         lec_arp_clear_vccs(to_remove);
         }
         skb_queue_purge(&to_remove->tx_wait); /* FIXME: good place for this? */
-        restore_flags(flags);
+
+        LEC_ARP_UNLOCK();
+
         DPRINTK("LEC_ARP: Removed entry:%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
                 0xff&to_remove->mac_addr[0], 0xff&to_remove->mac_addr[1],
                 0xff&to_remove->mac_addr[2], 0xff&to_remove->mac_addr[3],
@@ -1371,12 +1376,8 @@
 lec_arp_destroy(struct lec_priv *priv)
 {
         struct lec_arp_table *entry, *next;
-        unsigned long flags;
         int i;

-        save_flags(flags);
-        cli();
-
         del_timer(&priv->lec_arp_timer);

         /*
@@ -1419,7 +1420,6 @@
         priv->mcast_vcc = NULL;
         memset(priv->lec_arp_tables, 0,
                sizeof(struct lec_arp_table*)*LEC_ARP_TABLE_SIZE);
-        restore_flags(flags);
 }

@@ -1436,18 +1436,18 @@
         DPRINTK("LEC_ARP: lec_arp_find :%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
                 mac_addr[0]&0xff, mac_addr[1]&0xff, mac_addr[2]&0xff,
                 mac_addr[3]&0xff, mac_addr[4]&0xff, mac_addr[5]&0xff);
-        lec_arp_lock(priv);
+        lec_arp_get(priv);
         place = HASH(mac_addr[ETH_ALEN-1]);

         to_return = priv->lec_arp_tables[place];
         while(to_return) {
                 if (memcmp(mac_addr, to_return->mac_addr, ETH_ALEN) == 0) {
-                        lec_arp_unlock(priv);
+                        lec_arp_put(priv);
                         return to_return;
                 }
                 to_return = to_return->next;
         }
-        lec_arp_unlock(priv);
+        lec_arp_put(priv);
         return NULL;
 }

@@ -1574,11 +1574,11 @@
         del_timer(&priv->lec_arp_timer);

         DPRINTK("lec_arp_check_expire %p,%d\n",priv,
-                priv->lec_arp_lock_var.counter);
+                atomic_read(&priv->lec_arp_users));
         DPRINTK("expire: eo:%p nf:%p\n",priv->lec_arp_empty_ones,
                 priv->lec_no_forward);
-        if (!priv->lec_arp_lock_var.counter) {
-                lec_arp_lock(priv);
+        if (!atomic_read(&priv->lec_arp_users)) {
+                lec_arp_get(priv);
                 now = jiffies;
                 for(i=0;i<LEC_ARP_TABLE_SIZE;i++) {
                         for(entry = lec_arp_tables[i];entry != NULL;) {
@@ -1624,7 +1624,7 @@
                                 }
                         }
                 }
-                lec_arp_unlock(priv);
+                lec_arp_put(priv);
         }
         priv->lec_arp_timer.expires = jiffies + LEC_ARP_REFRESH_INTERVAL;
         add_timer(&priv->lec_arp_timer);
@@ -1686,7 +1686,7 @@
                 if (!entry) {
                         return priv->mcast_vcc;
                 }
-                lec_arp_put(priv->lec_arp_tables, entry);
+                lec_arp_add(priv->lec_arp_tables, entry);
                 /* We want arp-request(s) to be sent */
                 entry->packets_flooded =1;
                 entry->status = ESI_ARP_PENDING;
@@ -1711,7 +1711,7 @@
         struct lec_arp_table *entry, *next;
         int i;

-        lec_arp_lock(priv);
+        lec_arp_get(priv);
         DPRINTK("lec_addr_delete\n");
         for(i=0;i<LEC_ARP_TABLE_SIZE;i++) {
                 for(entry=priv->lec_arp_tables[i];entry != NULL; entry=next) {
@@ -1722,11 +1722,11 @@
                                 lec_arp_remove(priv->lec_arp_tables, entry);
                                 kfree(entry);
                         }
-                        lec_arp_unlock(priv);
+                        lec_arp_put(priv);
                         return 0;
                 }
         }
-        lec_arp_unlock(priv);
+        lec_arp_put(priv);
         return -1;
 }

@@ -1751,7 +1751,7 @@
                 return;   /* LANE2: ignore targetless LE_ARPs for which
                            * we have no entry in the cache. 7.1.30
                            */
-        lec_arp_lock(priv);
+        lec_arp_get(priv);
         if (priv->lec_arp_empty_ones) {
                 entry = priv->lec_arp_empty_ones;
                 if (!memcmp(entry->atm_addr, atm_addr, ATM_ESA_LEN)) {
@@ -1785,13 +1785,13 @@
                                 entry->status = ESI_FORWARD_DIRECT;
                                 memcpy(entry->mac_addr, mac_addr, ETH_ALEN);
                                 entry->last_used = jiffies;
-                                lec_arp_put(priv->lec_arp_tables, entry);
+                                lec_arp_add(priv->lec_arp_tables, entry);
                         }
                         if (remoteflag)
                                 entry->flags|=LEC_REMOTE_FLAG;
                         else
                                 entry->flags&=~LEC_REMOTE_FLAG;
-                        lec_arp_unlock(priv);
+                        lec_arp_put(priv);
                         DPRINTK("After update\n");
                         dump_arp_table(priv);
                         return;
@@ -1801,11 +1801,11 @@
         if (!entry) {
                 entry = make_entry(priv, mac_addr);
                 if (!entry) {
-                        lec_arp_unlock(priv);
+                        lec_arp_put(priv);
                         return;
                 }
                 entry->status
...

read more »

 
 
 

[ATM] cli() for net/atm/lec.c

Post by Christoph Hellwi » Fri, 21 Feb 2003 21:10:09


Quote:>  extern void (*br_fdb_put_hook)(struct net_bridge_fdb_entry *ent);
> +static spinlock_t lec_arp_spinlock = SPIN_LOCK_UNLOCKED;
> +static unsigned long lec_arp_flags;

> +#define LEC_ARP_LOCK()   spin_lock_irqsave(&lec_arp_spinlock, lec_arp_flags);
> +#define LEC_ARP_UNLOCK() spin_unlock_irqrestore(&lec_arp_spinlock, lec_arp_flags);

I don't think this is a good idea - use the spin_lock calls directly and
always use flags on the stack.

Quote:>          dev->get_stats = lec_get_stats;
>          dev->set_multicast_list = NULL;
>          dev->do_ioctl  = NULL;
> +  spin_lock_init(&lec_arp_spinlock);

not needed - you already initialized it at compiletime

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

[ATM] cli() for net/atm/lec.c

Post by Jeff Garzi » Fri, 21 Feb 2003 21:40:04



> >  extern void (*br_fdb_put_hook)(struct net_bridge_fdb_entry *ent);
> > +static spinlock_t lec_arp_spinlock = SPIN_LOCK_UNLOCKED;
> > +static unsigned long lec_arp_flags;

> > +#define LEC_ARP_LOCK()   spin_lock_irqsave(&lec_arp_spinlock, lec_arp_flags);
> > +#define LEC_ARP_UNLOCK() spin_unlock_irqrestore(&lec_arp_spinlock, lec_arp_flags);

> I don't think this is a good idea - use the spin_lock calls directly and
> always use flags on the stack.

Good spotting, though I would be more direct :)

Simon sez, "Don't do that"

1) use 'unsigned long flags' on the stack
2) do _not_ pass this variable between functions

        Jeff

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

[ATM] cli() for net/atm/lec.c

Post by rom.. » Sat, 22 Feb 2003 00:20:15



[...]


>          }
>          if (!entry) {
>                  DPRINTK("LEC_ARP: Arp_check_empties: entry not found!\n");
> -                lec_arp_unlock(priv);
> +                lec_arp_put(priv);
>                  return;
>          }
> -        save_flags(flags);
> -        cli();
> +        LEC_ARP_LOCK();
>          del_timer(&entry->timer);
>          memcpy(entry->mac_addr, src, ETH_ALEN);
>          entry->status = ESI_FORWARD_DIRECT;
>          entry->last_used = jiffies;
>          prev->next = entry->next;
> -        restore_flags(flags);
> +        LEC_ARP_UNLOCK();

It isn't completely trivial that the prev <-> entry relationship
still holds at the point where LEC_ARP_LOCK() is called. Widening
the protected region would spare some brain cycles imho.

--
Ueimor
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

[ATM] cli() for net/atm/lec.c

Post by chas william » Sat, 22 Feb 2003 19:50:10


here is the final version of the net/atm/lec.c patch with the
changes suggested by other on this list.  this should be good
for the 2.4 kernels as well...

Index: linux/net/atm/lec.c
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/lec.c,v
retrieving revision 1.1.1.1
diff -u -d -b -w -r1.1.1.1 lec.c
--- linux/net/atm/lec.c 20 Feb 2003 13:46:30 -0000      1.1.1.1
+++ linux/net/atm/lec.c 21 Feb 2003 00:04:44 -0000
@@ -20,6 +20,7 @@
 #include <net/arp.h>
 #include <net/dst.h>
 #include <linux/proc_fs.h>
+#include <linux/spinlock.h>

 /* TokenRing if needed */
 #ifdef CONFIG_TR
@@ -54,7 +55,7 @@
 extern struct net_bridge_fdb_entry *(*br_fdb_get_hook)(struct net_bridge *br,
        unsigned char *addr);
 extern void (*br_fdb_put_hook)(struct net_bridge_fdb_entry *ent);
-
+static spinlock_t lec_arp_spinlock = SPIN_LOCK_UNLOCKED;

 #define DUMP_PACKETS 0 /* 0 = None,
                         * 1 = 30 first bytes
@@ -634,6 +635,7 @@
         dev->get_stats = lec_get_stats;
         dev->set_multicast_list = NULL;
         dev->do_ioctl  = NULL;
+       spin_lock_init(&lec_arp_spinlock);
         printk("%s: Initialized!\n",dev->name);
         return;
 }
@@ -1044,15 +1046,15 @@
 #define HASH(ch) (ch & (LEC_ARP_TABLE_SIZE -1))

 static __inline__ void
-lec_arp_lock(struct lec_priv *priv)
+lec_arp_get(struct lec_priv *priv)
 {
-        atomic_inc(&priv->lec_arp_lock_var);
+        atomic_inc(&priv->lec_arp_users);
 }

 static __inline__ void
-lec_arp_unlock(struct lec_priv *priv)
+lec_arp_put(struct lec_priv *priv)
 {
-        atomic_dec(&priv->lec_arp_lock_var);
+        atomic_dec(&priv->lec_arp_users);
 }

 /*
@@ -1103,33 +1105,33 @@
  * LANE2: Add to the end of the list to satisfy 8.1.13
  */
 static __inline__ void
-lec_arp_put(struct lec_arp_table **lec_arp_tables,
-            struct lec_arp_table *to_put)
+lec_arp_add(struct lec_arp_table **lec_arp_tables,
+            struct lec_arp_table *to_add)
 {
-        unsigned short place;
         unsigned long flags;
+        unsigned short place;
         struct lec_arp_table *tmp;

-        save_flags(flags);
-        cli();
+        spin_lock_irqsave(&lec_arp_spinlock, flags);

-        place = HASH(to_put->mac_addr[ETH_ALEN-1]);
+        place = HASH(to_add->mac_addr[ETH_ALEN-1]);
         tmp = lec_arp_tables[place];
-        to_put->next = NULL;
+        to_add->next = NULL;
         if (tmp == NULL)
-                lec_arp_tables[place] = to_put;
+                lec_arp_tables[place] = to_add;

         else {  /* add to the end */
                 while (tmp->next)
                         tmp = tmp->next;
-                tmp->next = to_put;
+                tmp->next = to_add;
         }

-        restore_flags(flags);
+        spin_unlock_irqrestore(&lec_arp_spinlock, flags);
+
         DPRINTK("LEC_ARP: Added entry:%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
-                0xff&to_put->mac_addr[0], 0xff&to_put->mac_addr[1],
-                0xff&to_put->mac_addr[2], 0xff&to_put->mac_addr[3],
-                0xff&to_put->mac_addr[4], 0xff&to_put->mac_addr[5]);
+                0xff&to_add->mac_addr[0], 0xff&to_add->mac_addr[1],
+                0xff&to_add->mac_addr[2], 0xff&to_add->mac_addr[3],
+                0xff&to_add->mac_addr[4], 0xff&to_add->mac_addr[5]);
 }

 /*
@@ -1139,16 +1141,15 @@
 lec_arp_remove(struct lec_arp_table **lec_arp_tables,
                struct lec_arp_table *to_remove)
 {
+       unsigned long flags;
         unsigned short place;
         struct lec_arp_table *tmp;
-        unsigned long flags;
         int remove_vcc=1;

-        save_flags(flags);
-        cli();
+        spin_lock_irqsave(&lec_arp_spinlock, flags);

         if (!to_remove) {
-                restore_flags(flags);
+                spin_unlock_irqrestore(&lec_arp_spinlock, flags);
                 return -1;
         }
         place = HASH(to_remove->mac_addr[ETH_ALEN-1]);
@@ -1160,7 +1161,7 @@
                         tmp = tmp->next;
                 }
                 if (!tmp) {/* Entry was not found */
-                        restore_flags(flags);
+                        spin_unlock_irqrestore(&lec_arp_spinlock, flags);
                         return -1;
                 }
         }
@@ -1186,7 +1187,9 @@
                         lec_arp_clear_vccs(to_remove);
         }
         skb_queue_purge(&to_remove->tx_wait); /* FIXME: good place for this? */
-        restore_flags(flags);
+
+        spin_unlock_irqrestore(&lec_arp_spinlock, flags);
+
         DPRINTK("LEC_ARP: Removed entry:%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
                 0xff&to_remove->mac_addr[0], 0xff&to_remove->mac_addr[1],
                 0xff&to_remove->mac_addr[2], 0xff&to_remove->mac_addr[3],
@@ -1371,12 +1374,8 @@
 lec_arp_destroy(struct lec_priv *priv)
 {
         struct lec_arp_table *entry, *next;
-        unsigned long flags;
         int i;

-        save_flags(flags);
-        cli();
-
         del_timer(&priv->lec_arp_timer);

         /*
@@ -1419,7 +1418,6 @@
         priv->mcast_vcc = NULL;
         memset(priv->lec_arp_tables, 0,
                sizeof(struct lec_arp_table*)*LEC_ARP_TABLE_SIZE);
-        restore_flags(flags);
 }

@@ -1436,18 +1434,18 @@
         DPRINTK("LEC_ARP: lec_arp_find :%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
                 mac_addr[0]&0xff, mac_addr[1]&0xff, mac_addr[2]&0xff,
                 mac_addr[3]&0xff, mac_addr[4]&0xff, mac_addr[5]&0xff);
-        lec_arp_lock(priv);
+        lec_arp_get(priv);
         place = HASH(mac_addr[ETH_ALEN-1]);

         to_return = priv->lec_arp_tables[place];
         while(to_return) {
                 if (memcmp(mac_addr, to_return->mac_addr, ETH_ALEN) == 0) {
-                        lec_arp_unlock(priv);
+                        lec_arp_put(priv);
                         return to_return;
                 }
                 to_return = to_return->next;
         }
-        lec_arp_unlock(priv);
+        lec_arp_put(priv);
         return NULL;
 }

@@ -1574,11 +1572,11 @@
         del_timer(&priv->lec_arp_timer);

         DPRINTK("lec_arp_check_expire %p,%d\n",priv,
-                priv->lec_arp_lock_var.counter);
+                atomic_read(&priv->lec_arp_users));
         DPRINTK("expire: eo:%p nf:%p\n",priv->lec_arp_empty_ones,
                 priv->lec_no_forward);
-        if (!priv->lec_arp_lock_var.counter) {
-                lec_arp_lock(priv);
+        if (!atomic_read(&priv->lec_arp_users)) {
+                lec_arp_get(priv);
                 now = jiffies;
                 for(i=0;i<LEC_ARP_TABLE_SIZE;i++) {
                         for(entry = lec_arp_tables[i];entry != NULL;) {
@@ -1624,7 +1622,7 @@
                                 }
                         }
                 }
-                lec_arp_unlock(priv);
+                lec_arp_put(priv);
         }
         priv->lec_arp_timer.expires = jiffies + LEC_ARP_REFRESH_INTERVAL;
         add_timer(&priv->lec_arp_timer);
@@ -1686,7 +1684,7 @@
                 if (!entry) {
                         return priv->mcast_vcc;
                 }
-                lec_arp_put(priv->lec_arp_tables, entry);
+                lec_arp_add(priv->lec_arp_tables, entry);
                 /* We want arp-request(s) to be sent */
                 entry->packets_flooded =1;
                 entry->status = ESI_ARP_PENDING;
@@ -1711,7 +1709,7 @@
         struct lec_arp_table *entry, *next;
         int i;

-        lec_arp_lock(priv);
+        lec_arp_get(priv);
         DPRINTK("lec_addr_delete\n");
         for(i=0;i<LEC_ARP_TABLE_SIZE;i++) {
                 for(entry=priv->lec_arp_tables[i];entry != NULL; entry=next) {
@@ -1722,11 +1720,11 @@
                                 lec_arp_remove(priv->lec_arp_tables, entry);
                                 kfree(entry);
                         }
-                        lec_arp_unlock(priv);
+                        lec_arp_put(priv);
                         return 0;
                 }
         }
-        lec_arp_unlock(priv);
+        lec_arp_put(priv);
         return -1;
 }

@@ -1751,7 +1749,7 @@
                 return;   /* LANE2: ignore targetless LE_ARPs for which
                            * we have no entry in the cache. 7.1.30
                            */
-        lec_arp_lock(priv);
+        lec_arp_get(priv);
         if (priv->lec_arp_empty_ones) {
                 entry = priv->lec_arp_empty_ones;
                 if (!memcmp(entry->atm_addr, atm_addr, ATM_ESA_LEN)) {
@@ -1785,13 +1783,13 @@
                                 entry->status = ESI_FORWARD_DIRECT;
                                 memcpy(entry->mac_addr, mac_addr, ETH_ALEN);
                                 entry->last_used = jiffies;
-                                lec_arp_put(priv->lec_arp_tables, entry);
+                                lec_arp_add(priv->lec_arp_tables, entry);
                         }
                         if (remoteflag)
                                 entry->flags|=LEC_REMOTE_FLAG;
                         else
                                 entry->flags&=~LEC_REMOTE_FLAG;
-                        lec_arp_unlock(priv);
+                        lec_arp_put(priv);
                         DPRINTK("After update\n");
                         dump_arp_table(priv);
                         return;
@@ -1801,11 +1799,11 @@
         if (!entry) {
                 entry = make_entry(priv, mac_addr);
                 if (!entry) {
-                        lec_arp_unlock(priv);
+                        lec_arp_put(priv);
                         return;
                 }
                 entry->status = ESI_UNKNOWN;
-                lec_arp_put(priv->lec_arp_tables, entry);
+                lec_arp_add(priv->lec_arp_tables, entry);
                 /* Temporary, changes before end of function */
         }
         memcpy(entry->atm_addr, atm_addr, ATM_ESA_LEN);
@@ -1840,7 +1838,7 @@
         }
...

read more »

 
 
 

[ATM] cli() for net/atm/lec.c

Post by Christoph Hellwi » Sat, 22 Feb 2003 20:00:21



>          dev->get_stats = lec_get_stats;
>          dev->set_multicast_list = NULL;
>          dev->do_ioctl  = NULL;
> +  spin_lock_init(&lec_arp_spinlock);

This is still superflous..

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

[ATM] cli() for net/atm/lec.c

Post by David S. Mille » Sun, 23 Feb 2003 09:50:06



   Date: Fri, 21 Feb 2003 17:57:02 +0000


   >          dev->get_stats = lec_get_stats;
   >          dev->set_multicast_list = NULL;
   >          dev->do_ioctl  = NULL;
   > +       spin_lock_init(&lec_arp_spinlock);

   This is still superflous..

True, Chas can you cook up a new patch with this deleted?
Thanks.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

[ATM] cli() for net/atm/lec.c

Post by chas william » Tue, 25 Feb 2003 03:20:06


In message <20030221.232212.102082055.da...@redhat.com>,"David S. Miller" write
s:

>True, Chas can you cook up a new patch with this deleted?

sorry about that (i did that -- changed the declaration but forgot
to pull the init

Index: linux/net/atm/lec.c
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/lec.c,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -d -b -w -r1.1 -r1.2
--- linux/net/atm/lec.c 20 Feb 2003 13:46:30 -0000      1.1
+++ linux/net/atm/lec.c 22 Feb 2003 17:35:45 -0000      1.2
@@ -20,6 +20,7 @@
 #include <net/arp.h>
 #include <net/dst.h>
 #include <linux/proc_fs.h>
+#include <linux/spinlock.h>

 /* TokenRing if needed */
 #ifdef CONFIG_TR
@@ -55,6 +56,7 @@
        unsigned char *addr);
 extern void (*br_fdb_put_hook)(struct net_bridge_fdb_entry *ent);

+static spinlock_t lec_arp_spinlock = SPIN_LOCK_UNLOCKED;

 #define DUMP_PACKETS 0 /* 0 = None,
                         * 1 = 30 first bytes
@@ -1044,15 +1046,15 @@
 #define HASH(ch) (ch & (LEC_ARP_TABLE_SIZE -1))

 static __inline__ void
-lec_arp_lock(struct lec_priv *priv)
+lec_arp_get(struct lec_priv *priv)
 {
-        atomic_inc(&priv->lec_arp_lock_var);
+        atomic_inc(&priv->lec_arp_users);
 }

 static __inline__ void
-lec_arp_unlock(struct lec_priv *priv)
+lec_arp_put(struct lec_priv *priv)
 {
-        atomic_dec(&priv->lec_arp_lock_var);
+        atomic_dec(&priv->lec_arp_users);
 }

 /*
@@ -1103,33 +1105,33 @@
  * LANE2: Add to the end of the list to satisfy 8.1.13
  */
 static __inline__ void
-lec_arp_put(struct lec_arp_table **lec_arp_tables,
-            struct lec_arp_table *to_put)
+lec_arp_add(struct lec_arp_table **lec_arp_tables,
+            struct lec_arp_table *to_add)
 {
-        unsigned short place;
         unsigned long flags;
+        unsigned short place;
         struct lec_arp_table *tmp;

-        save_flags(flags);
-        cli();
+        spin_lock_irqsave(&lec_arp_spinlock, flags);

-        place = HASH(to_put->mac_addr[ETH_ALEN-1]);
+        place = HASH(to_add->mac_addr[ETH_ALEN-1]);
         tmp = lec_arp_tables[place];
-        to_put->next = NULL;
+        to_add->next = NULL;
         if (tmp == NULL)
-                lec_arp_tables[place] = to_put;
+                lec_arp_tables[place] = to_add;

         else {  /* add to the end */
                 while (tmp->next)
                         tmp = tmp->next;
-                tmp->next = to_put;
+                tmp->next = to_add;
         }

-        restore_flags(flags);
+        spin_unlock_irqrestore(&lec_arp_spinlock, flags);
+
         DPRINTK("LEC_ARP: Added entry:%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
-                0xff&to_put->mac_addr[0], 0xff&to_put->mac_addr[1],
-                0xff&to_put->mac_addr[2], 0xff&to_put->mac_addr[3],
-                0xff&to_put->mac_addr[4], 0xff&to_put->mac_addr[5]);
+                0xff&to_add->mac_addr[0], 0xff&to_add->mac_addr[1],
+                0xff&to_add->mac_addr[2], 0xff&to_add->mac_addr[3],
+                0xff&to_add->mac_addr[4], 0xff&to_add->mac_addr[5]);
 }

 /*
@@ -1139,16 +1141,15 @@
 lec_arp_remove(struct lec_arp_table **lec_arp_tables,
                struct lec_arp_table *to_remove)
 {
+        unsigned long flags;
         unsigned short place;
         struct lec_arp_table *tmp;
-        unsigned long flags;
         int remove_vcc=1;

-        save_flags(flags);
-        cli();
+        spin_lock_irqsave(&lec_arp_spinlock, flags);

         if (!to_remove) {
-                restore_flags(flags);
+                spin_unlock_irqrestore(&lec_arp_spinlock, flags);
                 return -1;
         }
         place = HASH(to_remove->mac_addr[ETH_ALEN-1]);
@@ -1160,7 +1161,7 @@
                         tmp = tmp->next;
                 }
                 if (!tmp) {/* Entry was not found */
-                        restore_flags(flags);
+                        spin_unlock_irqrestore(&lec_arp_spinlock, flags);
                         return -1;
                 }
         }
@@ -1186,7 +1187,9 @@
                         lec_arp_clear_vccs(to_remove);
         }
         skb_queue_purge(&to_remove->tx_wait); /* FIXME: good place for this? */
-        restore_flags(flags);
+
+        spin_unlock_irqrestore(&lec_arp_spinlock, flags);
+
         DPRINTK("LEC_ARP: Removed entry:%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
                 0xff&to_remove->mac_addr[0], 0xff&to_remove->mac_addr[1],
                 0xff&to_remove->mac_addr[2], 0xff&to_remove->mac_addr[3],
@@ -1371,12 +1374,8 @@
 lec_arp_destroy(struct lec_priv *priv)
 {
         struct lec_arp_table *entry, *next;
-        unsigned long flags;
         int i;

-        save_flags(flags);
-        cli();
-
         del_timer(&priv->lec_arp_timer);

         /*
@@ -1419,7 +1418,6 @@
         priv->mcast_vcc = NULL;
         memset(priv->lec_arp_tables, 0,
                sizeof(struct lec_arp_table*)*LEC_ARP_TABLE_SIZE);
-        restore_flags(flags);
 }

@@ -1436,18 +1434,18 @@
         DPRINTK("LEC_ARP: lec_arp_find :%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
                 mac_addr[0]&0xff, mac_addr[1]&0xff, mac_addr[2]&0xff,
                 mac_addr[3]&0xff, mac_addr[4]&0xff, mac_addr[5]&0xff);
-        lec_arp_lock(priv);
+        lec_arp_get(priv);
         place = HASH(mac_addr[ETH_ALEN-1]);

         to_return = priv->lec_arp_tables[place];
         while(to_return) {
                 if (memcmp(mac_addr, to_return->mac_addr, ETH_ALEN) == 0) {
-                        lec_arp_unlock(priv);
+                        lec_arp_put(priv);
                         return to_return;
                 }
                 to_return = to_return->next;
         }
-        lec_arp_unlock(priv);
+        lec_arp_put(priv);
         return NULL;
 }

@@ -1574,11 +1572,11 @@
         del_timer(&priv->lec_arp_timer);

         DPRINTK("lec_arp_check_expire %p,%d\n",priv,
-                priv->lec_arp_lock_var.counter);
+                atomic_read(&priv->lec_arp_users));
         DPRINTK("expire: eo:%p nf:%p\n",priv->lec_arp_empty_ones,
                 priv->lec_no_forward);
-        if (!priv->lec_arp_lock_var.counter) {
-                lec_arp_lock(priv);
+        if (!atomic_read(&priv->lec_arp_users)) {
+                lec_arp_get(priv);
                 now = jiffies;
                 for(i=0;i<LEC_ARP_TABLE_SIZE;i++) {
                         for(entry = lec_arp_tables[i];entry != NULL;) {
@@ -1624,7 +1622,7 @@
                                 }
                         }
                 }
-                lec_arp_unlock(priv);
+                lec_arp_put(priv);
         }
         priv->lec_arp_timer.expires = jiffies + LEC_ARP_REFRESH_INTERVAL;
         add_timer(&priv->lec_arp_timer);
@@ -1686,7 +1684,7 @@
                 if (!entry) {
                         return priv->mcast_vcc;
                 }
-                lec_arp_put(priv->lec_arp_tables, entry);
+                lec_arp_add(priv->lec_arp_tables, entry);
                 /* We want arp-request(s) to be sent */
                 entry->packets_flooded =1;
                 entry->status = ESI_ARP_PENDING;
@@ -1711,7 +1709,7 @@
         struct lec_arp_table *entry, *next;
         int i;

-        lec_arp_lock(priv);
+        lec_arp_get(priv);
         DPRINTK("lec_addr_delete\n");
         for(i=0;i<LEC_ARP_TABLE_SIZE;i++) {
                 for(entry=priv->lec_arp_tables[i];entry != NULL; entry=next) {
@@ -1722,11 +1720,11 @@
                                 lec_arp_remove(priv->lec_arp_tables, entry);
                                 kfree(entry);
                         }
-                        lec_arp_unlock(priv);
+                        lec_arp_put(priv);
                         return 0;
                 }
         }
-        lec_arp_unlock(priv);
+        lec_arp_put(priv);
         return -1;
 }

@@ -1751,7 +1749,7 @@
                 return;   /* LANE2: ignore targetless LE_ARPs for which
                            * we have no entry in the cache. 7.1.30
                            */
-        lec_arp_lock(priv);
+        lec_arp_get(priv);
         if (priv->lec_arp_empty_ones) {
                 entry = priv->lec_arp_empty_ones;
                 if (!memcmp(entry->atm_addr, atm_addr, ATM_ESA_LEN)) {
@@ -1785,13 +1783,13 @@
                                 entry->status = ESI_FORWARD_DIRECT;
                                 memcpy(entry->mac_addr, mac_addr, ETH_ALEN);
                                 entry->last_used = jiffies;
-                                lec_arp_put(priv->lec_arp_tables, entry);
+                                lec_arp_add(priv->lec_arp_tables, entry);
                         }
                         if (remoteflag)
                                 entry->flags|=LEC_REMOTE_FLAG;
                         else
                                 entry->flags&=~LEC_REMOTE_FLAG;
-                        lec_arp_unlock(priv);
+                        lec_arp_put(priv);
                         DPRINTK("After update\n");
                         dump_arp_table(priv);
                         return;
@@ -1801,11 +1799,11 @@
         if (!entry) {
                 entry = make_entry(priv, mac_addr);
                 if (!entry) {
-                        lec_arp_unlock(priv);
+                        lec_arp_put(priv);
                         return;
                 }
                 entry->status = ESI_UNKNOWN;
-                lec_arp_put(priv->lec_arp_tables, entry);
+                lec_arp_add(priv->lec_arp_tables, entry);
                 /* Temporary, changes before end of function */
         }
         memcpy(entry->atm_addr, atm_addr, ATM_ESA_LEN);
@@ -1840,7 +1838,7 @@
         }
         DPRINTK("After update2\n");
         dump_arp_table(priv);
-        lec_arp_unlock(priv);
+        lec_arp_put(priv);
 }

 /*
@@ -1854,7 +1852,7 @@
         struct lec_arp_table *entry;
         int i, found_entry=0;

-      
...

read more »

 
 
 

[ATM] cli() for net/atm/lec.c

Post by David S. Mille » Tue, 25 Feb 2003 08:40:06



   Date: Sun, 23 Feb 2003 20:13:14 -0500

   sorry about that (i did that -- changed the declaration but forgot
   to pull the init

Applied, thanks.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/