Re: [PATCH] Move inetdev/ifa over to RCU

["David S. Miller" <davem@xxxxxxxxxx>]

On Mon, 16 Aug 2004 13:08:26 +1000
Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:

> So we should reverse the two statements and add an smp_wmb().

I totally agree, and let's kill the lock too since it is
in fact useless now.  I've done this as follows:

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/08/15 23:08:21-07:00 davem@xxxxxxxxxxxxxxxxxx 
#   [IPV4]: Kill inetdev_lock, no longer needed.
#   
#   It no longer protects anything, all users held RTNL
#   semaphore to boot.  Also, fix a potential race in the
#   new RCU inetdev code, grab the reference on the idev
#   before attaching it via dev->ip_ptr.
#   
#   Based upon discussions with Herbert Xu.
#   
#   Signed-off-by: David S. Miller <davem@xxxxxxxxxx>
# 
# net/ipv4/devinet.c
#   2004/08/15 23:07:17-07:00 davem@xxxxxxxxxxxxxxxxxx +6 -14
#   [IPV4]: Kill inetdev_lock, no longer needed.
# 
diff -Nru a/net/ipv4/devinet.c b/net/ipv4/devinet.c
--- a/net/ipv4/devinet.c        2004-08-15 23:08:59 -07:00
+++ b/net/ipv4/devinet.c        2004-08-15 23:08:59 -07:00
@@ -90,8 +90,6 @@
 
 /* Locks all the inet devices. */
 
-static spinlock_t inetdev_lock = SPIN_LOCK_UNLOCKED;
-
 static struct in_ifaddr *inet_alloc_ifa(void)
 {
        struct in_ifaddr *ifa = kmalloc(sizeof(*ifa), GFP_KERNEL);
@@ -158,11 +156,12 @@
        neigh_sysctl_register(dev, in_dev->arp_parms, NET_IPV4,
                              NET_IPV4_NEIGH, "ipv4", NULL);
 #endif
-       spin_lock_bh(&inetdev_lock);
-       dev->ip_ptr = in_dev;
+
        /* Account for reference dev->ip_ptr */
        in_dev_hold(in_dev);
-       spin_unlock_bh(&inetdev_lock);
+       smp_wmb();
+       dev->ip_ptr = in_dev;
+
 #ifdef CONFIG_SYSCTL
        devinet_sysctl_register(in_dev, &in_dev->cnf);
 #endif
@@ -201,10 +200,8 @@
 #ifdef CONFIG_SYSCTL
        devinet_sysctl_unregister(&in_dev->cnf);
 #endif
-       spin_lock_bh(&inetdev_lock);
+
        in_dev->dev->ip_ptr = NULL;
-       /* in_dev_put following below will kill the in_device */
-       spin_unlock_bh(&inetdev_lock);
 
 #ifdef CONFIG_SYSCTL
        neigh_sysctl_unregister(in_dev->arp_parms);
@@ -248,9 +245,8 @@
                                ifap1 = &ifa->ifa_next;
                                continue;
                        }
-                       spin_lock_bh(&inetdev_lock);
+
                        *ifap1 = ifa->ifa_next;
-                       spin_unlock_bh(&inetdev_lock);
 
                        rtmsg_ifa(RTM_DELADDR, ifa);
                        notifier_call_chain(&inetaddr_chain, NETDEV_DOWN, ifa);
@@ -260,9 +256,7 @@
 
        /* 2. Unlink it */
 
-       spin_lock_bh(&inetdev_lock);
        *ifap = ifa1->ifa_next;
-       spin_unlock_bh(&inetdev_lock);
 
        /* 3. Announce address deletion */
 
@@ -324,9 +318,7 @@
        }
 
        ifa->ifa_next = *ifap;
-       spin_lock_bh(&inetdev_lock);
        *ifap = ifa;
-       spin_unlock_bh(&inetdev_lock);
 
        /* Send message first, then call notifier.
           Notifier will trigger FIB update, so that