Hi,
I would like to have feedback and testing on this diff. The initial work
was done by ian@.
It unlocks read(2) and write(2) families, and push the KERNEL_LOCK
deeper in the code path. With it, read(2) and write(2) on socket will be
KERNEL_LOCK-free.
read(2) and write(2) are file type agnostics by using `struct fileops'
(see sys/file.h) which define effective implementation per file type.
Currently, we have the following fileops defined:
- dmabuf dev/pci/drm/drm_linux.c
- kqueue kern/kern_event.c
- pipe kern/sys_pipe.c
- socket kern/sys_socket.c
- vnode kern/vfs_vnops.c
read(2) family uses dofilereadv(), which calls:
- fileops::fo_read()
- FRELE() -> fdrop() -> fileops::fo_close()
write(2) family uses dofilewritev(), which calls:
- fileops::fo_write()
- FRELE() -> fdrop() -> fileops::fo_close()
The diff pushs the KERNEL_LOCK inside each function, where it is
effectively needed. But as some file types doesn't need the lock (socket
for example), it makes read(2) and write(2) lock-free for them, while
still grabbing the lock for others (vnode for example).
fileops::fo_read
- dmabuf_read fine lock-free (just return ENXIO)
- kqueue_read fine lock-free (just return ENXIO)
- pipe_read KERNEL_LOCK/UNLOCK added (tsleep, wakeup, selwakeup)
- soo_read calls soreceive() which is already called without KERNEL_LOCK
by recvit() (via recvmsg(2))
- vn_read KERNEL_LOCK/UNLOCK added
fileops::fo_write
- dmabuf_write fine lock-free (just return ENXIO)
- kqueue_write fine lock-free (just return ENXIO)
- pipe_write KERNEL_LOCK/UNLOCK added (tsleep, wakeup, selwakeup)
- soo_write calls sosend() which is already called without KERNEL_LOCK by
sendit() (via sendmsg(2))
- vn_write KERNEL_LOCK/UNLOCK added
fileops::fo_close
- dmabuf_close already take care of it
- kqueue_close already take care of it
- pipe_close already take care of it
- soo_close call soclose() which is already called without KERNEL_LOCK by
socketpair(2)
- vn_closefile already take care of it
In dofilewritev(), I take care of calling ptsignal() with the
KERNEL_LOCK() too, as it requires the lock (it is asserted).
Others functions should be fine to be called without the KERNEL_LOCK, as
they are already used in such context.
Thanks.
--
Sebastien Marie
Index: kern/sys_generic.c
===================================================================
RCS file: /cvs/src/sys/kern/sys_generic.c,v
retrieving revision 1.123
diff -u -p -r1.123 sys_generic.c
--- kern/sys_generic.c 21 Jan 2019 23:41:26 -0000 1.123
+++ kern/sys_generic.c 4 Jun 2019 06:19:23 -0000
@@ -366,8 +366,11 @@ dofilewritev(struct proc *p, int fd, str
if (uio->uio_resid != cnt && (error == ERESTART ||
error == EINTR || error == EWOULDBLOCK))
error = 0;
- if (error == EPIPE)
+ if (error == EPIPE) {
+ KERNEL_LOCK();
ptsignal(p, SIGPIPE, STHREAD);
+ KERNEL_UNLOCK();
+ }
}
cnt -= uio->uio_resid;
Index: kern/sys_pipe.c
===================================================================
RCS file: /cvs/src/sys/kern/sys_pipe.c,v
retrieving revision 1.87
diff -u -p -r1.87 sys_pipe.c
--- kern/sys_pipe.c 13 Nov 2018 13:02:20 -0000 1.87
+++ kern/sys_pipe.c 5 Jun 2019 08:06:19 -0000
@@ -314,9 +314,11 @@ pipe_read(struct file *fp, struct uio *u
int error;
size_t size, nread = 0;
+ KERNEL_LOCK();
+
error = pipelock(rpipe);
if (error)
- return (error);
+ goto done;
++rpipe->pipe_busy;
@@ -420,6 +422,8 @@ unlocked_error:
if ((rpipe->pipe_buffer.size - rpipe->pipe_buffer.cnt) >= PIPE_BUF)
pipeselwakeup(rpipe);
+done:
+ KERNEL_UNLOCK();
return (error);
}
@@ -430,6 +434,8 @@ pipe_write(struct file *fp, struct uio *
size_t orig_resid;
struct pipe *wpipe, *rpipe;
+ KERNEL_LOCK();
+
rpipe = fp->f_data;
wpipe = rpipe->pipe_peer;
@@ -437,7 +443,8 @@ pipe_write(struct file *fp, struct uio *
* detect loss of pipe read side, issue SIGPIPE if lost.
*/
if ((wpipe == NULL) || (wpipe->pipe_state & PIPE_EOF)) {
- return (EPIPE);
+ error = EPIPE;
+ goto done;
}
++wpipe->pipe_busy;
@@ -471,7 +478,7 @@ pipe_write(struct file *fp, struct uio *
wpipe->pipe_state &= ~(PIPE_WANT | PIPE_WANTR);
wakeup(wpipe);
}
- return (error);
+ goto done;
}
orig_resid = uio->uio_resid;
@@ -642,6 +649,8 @@ retrywrite:
if (wpipe->pipe_buffer.cnt)
pipeselwakeup(wpipe);
+done:
+ KERNEL_UNLOCK();
return (error);
}
Index: kern/syscalls.master
===================================================================
RCS file: /cvs/src/sys/kern/syscalls.master,v
retrieving revision 1.191
diff -u -p -r1.191 syscalls.master
--- kern/syscalls.master 13 May 2019 22:55:27 -0000 1.191
+++ kern/syscalls.master 5 Jun 2019 08:06:19 -0000
@@ -51,8 +51,8 @@
0 INDIR { int sys_syscall(int number, ...); }
1 STD { void sys_exit(int rval); }
2 STD { int sys_fork(void); }
-3 STD { ssize_t sys_read(int fd, void *buf, size_t nbyte); }
-4 STD { ssize_t sys_write(int fd, const void *buf, \
+3 STD NOLOCK { ssize_t sys_read(int fd, void *buf, size_t nbyte); }
+4 STD NOLOCK { ssize_t sys_write(int fd, const void *buf, \
size_t nbyte); }
5 STD { int sys_open(const char *path, \
int flags, ... mode_t mode); }
@@ -247,9 +247,9 @@
118 STD { 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, \
+120 STD NOLOCK { ssize_t sys_readv(int fd, \
const struct iovec *iovp, int iovcnt); }
-121 STD { ssize_t sys_writev(int fd, \
+121 STD NOLOCK { ssize_t sys_writev(int fd, \
const struct iovec *iovp, int iovcnt); }
122 STD { int sys_kill(int pid, int signum); }
123 STD { int sys_fchown(int fd, uid_t uid, gid_t gid); }
@@ -316,9 +316,9 @@
170 OBSOL msgsys10
171 OBSOL shmsys10
172 UNIMPL
-173 STD { ssize_t sys_pread(int fd, void *buf, \
+173 STD NOLOCK { ssize_t sys_pread(int fd, void *buf, \
size_t nbyte, int pad, off_t offset); }
-174 STD { ssize_t sys_pwrite(int fd, const void *buf, \
+174 STD NOLOCK { ssize_t sys_pwrite(int fd, const void *buf, \
size_t nbyte, int pad, off_t offset); }
175 UNIMPL ntp_gettime
176 UNIMPL ntp_adjtime
@@ -452,10 +452,10 @@
264 STD { int sys_fhopen(const fhandle_t *fhp, int flags); }
265 UNIMPL
266 UNIMPL
-267 STD { ssize_t sys_preadv(int fd, \
+267 STD NOLOCK { ssize_t sys_preadv(int fd, \
const struct iovec *iovp, int iovcnt, \
int pad, off_t offset); }
-268 STD { ssize_t sys_pwritev(int fd, \
+268 STD NOLOCK { ssize_t sys_pwritev(int fd, \
const struct iovec *iovp, int iovcnt, \
int pad, off_t offset); }
269 STD { int sys_kqueue(void); }
Index: kern/vfs_vnops.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_vnops.c,v
retrieving revision 1.97
diff -u -p -r1.97 vfs_vnops.c
--- kern/vfs_vnops.c 20 Aug 2018 16:00:22 -0000 1.97
+++ kern/vfs_vnops.c 5 Jun 2019 08:06:19 -0000
@@ -343,6 +343,8 @@ vn_read(struct file *fp, struct uio *uio
off_t offset;
int error;
+ KERNEL_LOCK();
+
/*
* Check below can race. We can block on the vnode lock
* and resume with a different `fp->f_offset' value.
@@ -353,11 +355,15 @@ vn_read(struct file *fp, struct uio *uio
offset = uio->uio_offset;
/* no wrap around of offsets except on character devices */
- if (vp->v_type != VCHR && count > LLONG_MAX - offset)
- return (EINVAL);
+ if (vp->v_type != VCHR && count > LLONG_MAX - offset) {
+ error = EINVAL;
+ goto done;
+ }
- if (vp->v_type == VDIR)
- return (EISDIR);
+ if (vp->v_type == VDIR) {
+ error = EISDIR;
+ goto done;
+ }
vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
if ((fflags & FO_POSITION) == 0)
@@ -367,6 +373,8 @@ vn_read(struct file *fp, struct uio *uio
if ((fflags & FO_POSITION) == 0)
fp->f_offset += count - uio->uio_resid;
VOP_UNLOCK(vp);
+done:
+ KERNEL_UNLOCK();
return (error);
}
@@ -381,6 +389,8 @@ vn_write(struct file *fp, struct uio *ui
int error, ioflag = IO_UNIT;
size_t count;
+ KERNEL_LOCK();
+
/* note: pwrite/pwritev are unaffected by O_APPEND */
if (vp->v_type == VREG && (fp->f_flag & O_APPEND) &&
(fflags & FO_POSITION) == 0)
@@ -402,6 +412,8 @@ vn_write(struct file *fp, struct uio *ui
fp->f_offset += count - uio->uio_resid;
}
VOP_UNLOCK(vp);
+
+ KERNEL_UNLOCK();
return (error);
}
Index: sys/file.h
===================================================================
RCS file: /cvs/src/sys/sys/file.h,v
retrieving revision 1.53
diff -u -p -r1.53 file.h
--- sys/file.h 20 Aug 2018 16:00:22 -0000 1.53
+++ sys/file.h 5 Jun 2019 08:06:19 -0000
@@ -46,6 +46,13 @@ struct stat;
struct file;
struct ucred;
+/**
+ * File operations.
+ * The following entries could be called without KERNEL_LOCK hold:
+ * - fo_read
+ * - fo_write
+ * - fo_close
+ */
struct fileops {
int (*fo_read)(struct file *, struct uio *, int);
int (*fo_write)(struct file *, struct uio *, int);
