Take net lock before kernel lock.
authorbluhm <bluhm@openbsd.org>
Sat, 6 Jan 2024 10:58:45 +0000 (10:58 +0000)
committerbluhm <bluhm@openbsd.org>
Sat, 6 Jan 2024 10:58:45 +0000 (10:58 +0000)
Doing KERNEL_LOCK() just before NET_LOCK() does not make sense.
Net lock is a rwlock that releases kernel lock during sleep.  To
avoid an unnecessary release and take kernel lock cycle, move
KERNEL_LOCK() after NET_LOCK().
There is no lock order reversal deadlock issue.  Both locks are
used in any order thoughout the kernel.  As NET_LOCK() releases the
kernel lock when it cannot take the lock immediately and has to
sleep, we always end in the order kernel lock before net lock after
sleeping.

OK sashan@

sys/net/if.c
sys/netinet/in.c

index bb1b891..8b9281d 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if.c,v 1.714 2023/12/29 11:43:04 bluhm Exp $  */
+/*     $OpenBSD: if.c,v 1.715 2024/01/06 10:58:45 bluhm Exp $  */
 /*     $NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $  */
 
 /*
@@ -1774,16 +1774,16 @@ if_linkstate_task(void *xifidx)
        unsigned int ifidx = (unsigned long)xifidx;
        struct ifnet *ifp;
 
-       KERNEL_LOCK();
        NET_LOCK();
+       KERNEL_LOCK();
 
        ifp = if_get(ifidx);
        if (ifp != NULL)
                if_linkstate(ifp);
        if_put(ifp);
 
-       NET_UNLOCK();
        KERNEL_UNLOCK();
+       NET_UNLOCK();
 }
 
 void
index d2207b3..7cf2cc6 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: in.c,v 1.185 2023/06/28 11:49:49 kn Exp $     */
+/*     $OpenBSD: in.c,v 1.186 2024/01/06 10:58:45 bluhm Exp $  */
 /*     $NetBSD: in.c,v 1.26 1996/02/13 23:41:39 christos Exp $ */
 
 /*
@@ -260,8 +260,8 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
                        return (error);
        }
 
-       KERNEL_LOCK();
        NET_LOCK();
+       KERNEL_LOCK();
 
        TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
                if (ifa->ifa_addr->sa_family != AF_INET)
@@ -331,8 +331,8 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
                break;
        }
 err:
-       NET_UNLOCK();
        KERNEL_UNLOCK();
+       NET_UNLOCK();
        return (error);
 }
 
@@ -353,8 +353,8 @@ in_ioctl_set_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp)
        if (error)
                return (error);
 
-       KERNEL_LOCK();
        NET_LOCK();
+       KERNEL_LOCK();
 
        TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
                if (ifa->ifa_addr->sa_family != AF_INET)
@@ -387,8 +387,8 @@ in_ioctl_set_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp)
        if (!error)
                if_addrhooks_run(ifp);
 
-       NET_UNLOCK();
        KERNEL_UNLOCK();
+       NET_UNLOCK();
        return error;
 }
 
@@ -409,8 +409,8 @@ in_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp)
                        return (error);
        }
 
-       KERNEL_LOCK();
        NET_LOCK();
+       KERNEL_LOCK();
 
        TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
                if (ifa->ifa_addr->sa_family != AF_INET)
@@ -527,8 +527,8 @@ in_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp)
                panic("%s: invalid ioctl %lu", __func__, cmd);
        }
 
-       NET_UNLOCK();
        KERNEL_UNLOCK();
+       NET_UNLOCK();
        return (error);
 }