On 20.06.2018 13:13, Martin Pieuchot wrote:
Diff below unlocks the following syscalls:

  recvmsg(2), recvfrom(2), accept(2), getpeername(2), getsockname(2),
  accept4(2), connect(2), bind(2), setsockopt(2), listen(2),
  getsockopt(2), shutdown(2)

It doesn't mean that they won't run without the KERNEL_LOCK().  Instead
a lock will be picked based on the socket type.  For Unix/pfkey/routing
sockets it is still the KERNEL_LOCK().  That means the KERNEL_LOCK()
will be taken a bit later in the syscall.  But for TCP/UDP sockets it
will grab the NET_LOCK() instead, just like in sendto(2) and sendmsg(2).

Tests & oks welcome!

Hi Martin,

I run your diff on my HP ProBook 450 G3 for some days now without problems. The only thing to mention is that your diff seems to be incompatible with the DRI3 diff from Mark Kettenis[1]. When I apply both diffs to the -current kernel the kernel panics during boot:

root on sda2a (f0b2020d2a12099d.a) swap on sd2b dump on sd2b
iwm0: hw rev 0x210, fw ver 16.242414.0, address 01:23:45:67:89:ab
panic: rw_enter: fdlock locking against myself
Stopped at      db_enter+0x12:  popq    %r11
   TID    PID    UID     PRFLAGS     PFLAGS  CPU  COMMAND
*451237  29199      0         0x1          0    0K init
db_enter() at db_enter+0x12
panic() at panic+0x138
_rw_enter(5cb404b46de93a98, ffffff041ffb0898,0,3) at _rw_enter+0x338
fdrelease(dd362798c66e5c5b,3) at fdrelease+0xfe
sys_close(359bdf7160fe01fb) at syscall+0x32a
Xsyscall_untramp(6,0,0,0,0,6) at Xsyscall_untramp+0xe4
end of kernel
end trace frame 0x7f7ffffdb9a0, count: 8

[1] https://marc.info/?l=openbsd-tech&m=152954666910012&w=2

Cheers,
Bruno


Index: kern/syscalls.master
===================================================================
RCS file: /cvs/src/sys/kern/syscalls.master,v
retrieving revision 1.182
diff -u -p -r1.182 syscalls.master
--- kern/syscalls.master        20 Jun 2018 10:52:49 -0000      1.182
+++ kern/syscalls.master        20 Jun 2018 11:04:05 -0000
@@ -88,18 +88,18 @@
 #else
 26     UNIMPL          ptrace
 #endif
-27     STD             { ssize_t sys_recvmsg(int s, struct msghdr *msg, \
+27     STD NOLOCK      { ssize_t sys_recvmsg(int s, struct msghdr *msg, \
                            int flags); }
 28     STD NOLOCK      { ssize_t sys_sendmsg(int s, \
                            const struct msghdr *msg, int flags); }
-29     STD             { ssize_t sys_recvfrom(int s, void *buf, size_t len, \
+29     STD NOLOCK      { ssize_t sys_recvfrom(int s, void *buf, size_t len, \
                            int flags, struct sockaddr *from, \
                            socklen_t *fromlenaddr); }
-30     STD             { int sys_accept(int s, struct sockaddr *name, \
+30     STD NOLOCK      { int sys_accept(int s, struct sockaddr *name, \
                            socklen_t *anamelen); }
-31     STD             { int sys_getpeername(int fdes, struct sockaddr *asa, \
+31     STD NOLOCK      { int sys_getpeername(int fdes, struct sockaddr *asa, \
                            socklen_t *alen); }
-32     STD             { int sys_getsockname(int fdes, struct sockaddr *asa, \
+32     STD NOLOCK      { int sys_getsockname(int fdes, struct sockaddr *asa, \
                            socklen_t *alen); }
 33     STD             { int sys_access(const char *path, int amode); }
 34     STD             { int sys_chflags(const char *path, u_int flags); }
@@ -205,7 +205,7 @@
 91     STD             { int sys_nanosleep(const struct timespec *rqtp, \
                            struct timespec *rmtp); }
 92     STD             { int sys_fcntl(int fd, int cmd, ... void *arg); }
-93     STD             { int sys_accept4(int s, struct sockaddr *name, \
+93     STD NOLOCK      { int sys_accept4(int s, struct sockaddr *name, \
                            socklen_t *anamelen, int flags); }
 94     STD             { int sys___thrsleep(const volatile void *ident, \
                            clockid_t clock_id, const struct timespec *tp, \
@@ -213,18 +213,18 @@
 95     STD             { int sys_fsync(int fd); }
 96     STD             { int sys_setpriority(int which, id_t who, int prio); }
 97     STD NOLOCK      { int sys_socket(int domain, int type, int protocol); }
-98     STD             { int sys_connect(int s, const struct sockaddr *name, \
+98     STD NOLOCK      { int sys_connect(int s, const struct sockaddr *name, \
                            socklen_t namelen); }
 99     STD             { int sys_getdents(int fd, void *buf, size_t buflen); }
 100    STD             { int sys_getpriority(int which, id_t who); }
 101    STD             { int sys_pipe2(int *fdp, int flags); }
 102    STD             { int sys_dup3(int from, int to, int flags); }
 103    STD             { int sys_sigreturn(struct sigcontext *sigcntxp); }
-104    STD             { int sys_bind(int s, const struct sockaddr *name, \
+104    STD NOLOCK      { int sys_bind(int s, const struct sockaddr *name, \
                            socklen_t namelen); }
-105    STD             { int sys_setsockopt(int s, int level, int name, \
+105    STD NOLOCK      { int sys_setsockopt(int s, int level, int name, \
                            const void *val, socklen_t valsize); }
-106    STD             { int sys_listen(int s, int backlog); }
+106    STD NOLOCK      { int sys_listen(int s, int backlog); }
 107    STD             { int sys_chflagsat(int fd, const char *path, \
                            u_int flags, int atflags); }
 108    STD             { int sys_pledge(const char *promises, \
@@ -243,7 +243,7 @@
 115    OBSOL           vtrace
 116    OBSOL           t32_gettimeofday
 117    OBSOL           t32_getrusage
-118    STD             { int sys_getsockopt(int s, int level, int name, \
+118    STD NOLOCK      { int sys_getsockopt(int s, int level, int name, \
                            void *val, socklen_t *avalsize); }
 119    STD             { int sys_thrkill(pid_t tid, int signum, void *tcb); }
 120    STD             { ssize_t sys_readv(int fd, \
@@ -264,7 +264,7 @@
 133    STD NOLOCK      { ssize_t sys_sendto(int s, const void *buf, \
                            size_t len, int flags, const struct sockaddr *to, \
                            socklen_t tolen); }
-134    STD             { int sys_shutdown(int s, int how); }
+134    STD NOLOCK      { int sys_shutdown(int s, int how); }
 135    STD NOLOCK      { int sys_socketpair(int domain, int type, \
                            int protocol, int *rsv); }
 136    STD             { int sys_mkdir(const char *path, mode_t mode); }
Index: net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.555
diff -u -p -r1.555 if.c
--- net/if.c    18 Jun 2018 12:13:10 -0000      1.555
+++ net/if.c    20 Jun 2018 11:04:05 -0000
@@ -1379,7 +1379,7 @@ ifa_ifwithaddr(struct sockaddr *addr, u_
        struct ifaddr *ifa;
        u_int rdomain;

-       KERNEL_ASSERT_LOCKED();
+       KERNEL_LOCK();
        rdomain = rtable_l2(rtableid);
        TAILQ_FOREACH(ifp, &ifnet, if_list) {
                if (ifp->if_rdomain != rdomain)
@@ -1389,10 +1389,13 @@ ifa_ifwithaddr(struct sockaddr *addr, u_
                        if (ifa->ifa_addr->sa_family != addr->sa_family)
                                continue;

-                       if (equal(addr, ifa->ifa_addr))
+                       if (equal(addr, ifa->ifa_addr)) {
+                               KERNEL_UNLOCK();
                                return (ifa);
+                       }
                }
        }
+       KERNEL_UNLOCK();
        return (NULL);
 }

@@ -1405,8 +1408,8 @@ ifa_ifwithdstaddr(struct sockaddr *addr,
        struct ifnet *ifp;
        struct ifaddr *ifa;

-       KERNEL_ASSERT_LOCKED();
        rdomain = rtable_l2(rdomain);
+       KERNEL_LOCK();
        TAILQ_FOREACH(ifp, &ifnet, if_list) {
                if (ifp->if_rdomain != rdomain)
                        continue;
@@ -1415,11 +1418,14 @@ ifa_ifwithdstaddr(struct sockaddr *addr,
                                if (ifa->ifa_addr->sa_family !=
                                    addr->sa_family || ifa->ifa_dstaddr == NULL)
                                        continue;
-                               if (equal(addr, ifa->ifa_dstaddr))
+                               if (equal(addr, ifa->ifa_dstaddr)) {
+                                       KERNEL_UNLOCK();
                                        return (ifa);
+                               }
                        }
                }
        }
+       KERNEL_UNLOCK();
        return (NULL);
 }

Reply via email to