Download raw body.
net lock kernel lock order
On Fri, Jan 05, 2024 at 01:26:15PM +0100, Alexander Bluhm wrote:
> Hi,
>
> We have code that takes kernel lock just before net lock. As net
> lock is a rwlock that releases kernel lock while sleeping, there
> is never deadlock due to lock ordering.
>
> The order kernel lock -> net lock is suboptimal. If net lock is
> held by another thread the following happens:
>
> take kernel lock -> try net lock -> release kernel lock -> sleep
> -> take net lock -> take kernel lock
this is incorrect. the kernel lock is aquired before the rwlock in a sleep
call.
> If we change the order we get a less expensive sequence:
>
> try net lock -> sleep -> take net lock -> take kernel lock
>
> ok?
>
> bluhm
>
> Index: net/if.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/if.c,v
> diff -u -p -r1.714 if.c
> --- net/if.c 29 Dec 2023 11:43:04 -0000 1.714
> +++ net/if.c 5 Jan 2024 12:08:01 -0000
> @@ -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: netinet/in.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in.c,v
> diff -u -p -r1.185 in.c
> --- netinet/in.c 28 Jun 2023 11:49:49 -0000 1.185
> +++ netinet/in.c 5 Jan 2024 12:10:07 -0000
> @@ -260,8 +260,8 @@ in_ioctl(u_long cmd, caddr_t data, struc
> 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, struc
> break;
> }
> err:
> - NET_UNLOCK();
> KERNEL_UNLOCK();
> + NET_UNLOCK();
> return (error);
> }
>
> @@ -353,8 +353,8 @@ in_ioctl_set_ifaddr(u_long cmd, caddr_t
> 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
> 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
> 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
> panic("%s: invalid ioctl %lu", __func__, cmd);
> }
>
> - NET_UNLOCK();
> KERNEL_UNLOCK();
> + NET_UNLOCK();
> return (error);
> }
>
>
--
:wq Claudio
net lock kernel lock order