Re: [Fwd: Re: possible bug x86 2.4.2 SMP in IP receive stack]

[Andrew Morton <andrewm@xxxxxxxxxx>]

Bob Felderman wrote:
> 
> => From andrewm@xxxxxxxxxx Mon Mar  5 15:50:15 2001
> => Your driver doesn't appear to have any SMP locking.  The
> => same skb can be fed to the network stack twice.
> 
> The driver does (and always has had)
> 
>     if (test_and_set_bit(0, (void *) &is->arch.interrupt) != 0) {
>                 /* hmm, interrupt called twice */
>                 [other stuff deleted]
>         }

I see.  I took a closer look and yes, the Rx path looks
OK.  I think your start_xmit could still race with the
tx interrupt though?

Could I suggest that you just lock the heck out
of it?  If the problems go away then it's the
driver and we can stop spamming Alexey.

Something like this:

--- gmip.c.orig Tue Mar  6 11:06:50 2001
+++ gmip.c      Tue Mar  6 11:10:32 2001
@@ -25,6 +25,7 @@
 #include <linux/etherdevice.h>
 #include <asm/byteorder.h>
 #include <asm/io.h>
+#include <linux/spinlock.h>
 
 #include "gm_arch_def.h"
 #include "gm_klog_debug.h"
@@ -43,6 +44,8 @@
 static int gm_net_copy_threshold = 256+16;
 MODULE_PARM(gm_net_copy_threshold, "i");
 
+spinlock_t dumb_lock;
+
 /* should be a multiple of 16 bytes */
 static inline
 gm_size_t
@@ -151,8 +154,19 @@
 #endif
 
 static
+void _gmip_recv_interrupt (void *gmnetp, const unsigned int ulen, gm_u16_t 
csum)
+static
 void gmip_recv_interrupt (void *gmnetp, const unsigned int ulen, gm_u16_t csum)
 {
+       unsigned long flags;
+       spin_lock_irqsave(&dumb_lock, flags);
+       _gmip_recv_interrupt (gmnetp, ulen, csum);
+       spin_unlock_irqrestore(&dumb_lock, flags);
+}
+
+static
+void _gmip_recv_interrupt (void *gmnetp, const unsigned int ulen, gm_u16_t 
csum)
+{
   gm_arch_net_info_t *gmnet = gmnetp;
   struct net_device *dev = gmnet->dev;
   int index = gmnet->rdone & GM_RECV_RING_MAX_INDEX;
@@ -298,8 +312,19 @@
 }
 
 static
+void _gmip_sent_interrupt(void *gmnetp)
+static
 void gmip_sent_interrupt(void *gmnetp)
 {
+       unsigned long flags;
+       spin_lock_irqsave(&dumb_lock, flags);
+       _gmip_sent_interrupt(gmnetp);
+       spin_unlock_irqrestore(&dumb_lock, flags);
+}
+
+static
+void _gmip_sent_interrupt(void *gmnetp)
+{
   int index;
   gm_arch_net_info_t *gmnet = gmnetp;
   struct sk_buff *skb;
@@ -455,7 +480,19 @@
 
 static int gm_max_diff_ring = 0;
 
+static int _gmip_xmit(struct sk_buff *skb, struct net_device *dev);
 static int gmip_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+       int ret;
+       unsigned long flags;
+
+       spinlock_irqsave(&dumb_lock, flags);
+       ret = _gmip_xmit(skb, dev);
+       spin_unlock_irqrestore(&dumb_lock, flags);
+       return ret;
+}
+
+static int _gmip_xmit(struct sk_buff *skb, struct net_device *dev)
 {
   gm_arch_net_info_t *gmnet = dev->priv;
   struct ethhdr *eth = (struct ethhdr*)skb->data;