Martin Pieuchot wrote:
> Diff below shuffles sys_socket() to look like sys_socketpair().
>
> The goal is to do socket operations first in both functions. Since
> they don't need the KERNEL_LOCK(), we will be able to mark the syscalls
> NOLOCK and only grab it before messing with file descriptors.
this looks ok, but a few comments about future directions.
> + fdplock(fdp);
> + error = falloc(p, cloexec, &fp, &fd);
> + fdpunlock(fdp);
> + fp->f_flag = fflag;
> + fp->f_type = DTYPE_SOCKET;
> + fp->f_ops = &socketops;
> if (type & SOCK_NONBLOCK)
> so->so_state |= SS_NBIO;
> so->so_state |= ss;
fp->f_data = so;
FILE_SET_MATURE(fp, p);
i'm a little concerned about races here. i think we are missing some barriers
to allow the larval flag on files to properly work in a concurrent system.
there's nothing to prevent the compiler from hoisting the flag setting above
the other operations. then another thread in sys_read is going to see bad
stuff.
maybe we want something like this? (maybe i'm getting ahead of myself?)
Index: file.h
===================================================================
RCS file: /cvs/src/sys/sys/file.h,v
retrieving revision 1.39
diff -u -p -r1.39 file.h
--- file.h 2 Jan 2018 06:40:55 -0000 1.39
+++ file.h 7 Feb 2018 21:10:57 -0000
@@ -93,6 +93,7 @@ struct file {
#define FRELE(fp,p) (--(fp)->f_count == 0 ? fdrop(fp, p) : 0)
#define FILE_SET_MATURE(fp,p) do { \
+ membar_producer(); \
(fp)->f_iflags &= ~FIF_LARVAL; \
FRELE(fp, p); \
} while (0)
