Re: svn commit: r323177 - in head: sys/compat/cloudabi sys/contrib/cloudabi usr.bin/truss

[Ed Schouten]

Hi Jilles,

2017-09-15 12:58 GMT+01:00 Jilles Tjoelker :
> Although this is correct from a functionality point of view (S_IFIFO and
> S_IFSOCK are probably expected to be different in a POSIX context, but
> this is unlikely to break anything that is not already broken), it is a
> partial reversal of previous changes to FreeBSD that created separate
> implementations for pipes (first unnamed pipes in 1996, later fifos in
> 2012 r232055). The main reason for these changes was performance.
>
> Unfortunately, I do not have concrete benchmarks in the CloudABI
> context.

That is important to keep in mind indeed. Thanks for pointing this out!

When I discussed this with some of the other folks working on
CloudABI, we came to the conclusion that this (potential) loss of
performance is acceptable in the nearby future. If it would really
affect us negatively, it should in theory be possible to reimplement
our streaming sockets and pipes with something as simple and fast as
FreeBSD's pipes, but still support file descriptor passing, etc.

-- 
Ed Schouten 
Nuxi, 's-Hertogenbosch, the Netherlands
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r323177 - in head: sys/compat/cloudabi sys/contrib/cloudabi usr.bin/truss

[Jilles Tjoelker]

On Tue, Sep 05, 2017 at 07:46:45AM +, Ed Schouten wrote:
> Author: ed
> Date: Tue Sep  5 07:46:45 2017
> New Revision: 323177
> URL: https://svnweb.freebsd.org/changeset/base/323177

> Log:
>   Merge pipes and socket pairs.

>   Now that CloudABI's sockets API has been changed to be addressless and
>   only connected socket instances are used (e.g., socket pairs), they have
>   become fairly similar to pipes. The only differences on CloudABI is that
>   socket pairs additionally support shutdown(), send() and recv().

>   To simplify the ABI, we've therefore decided to remove pipes as a
>   separate file descriptor type and just let pipe() return a socket pair
>   of type SOCK_STREAM. S_ISFIFO() and S_ISSOCK() are now defined
>   identically.

Although this is correct from a functionality point of view (S_IFIFO and
S_IFSOCK are probably expected to be different in a POSIX context, but
this is unlikely to break anything that is not already broken), it is a
partial reversal of previous changes to FreeBSD that created separate
implementations for pipes (first unnamed pipes in 1996, later fifos in
2012 r232055). The main reason for these changes was performance.

Unfortunately, I do not have concrete benchmarks in the CloudABI
context.

-- 
Jilles Tjoelker
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r323177 - in head: sys/compat/cloudabi sys/contrib/cloudabi usr.bin/truss

[Ed Schouten]

Author: ed
Date: Tue Sep  5 07:46:45 2017
New Revision: 323177
URL: https://svnweb.freebsd.org/changeset/base/323177

Log:
  Merge pipes and socket pairs.
  
  Now that CloudABI's sockets API has been changed to be addressless and
  only connected socket instances are used (e.g., socket pairs), they have
  become fairly similar to pipes. The only differences on CloudABI is that
  socket pairs additionally support shutdown(), send() and recv().
  
  To simplify the ABI, we've therefore decided to remove pipes as a
  separate file descriptor type and just let pipe() return a socket pair
  of type SOCK_STREAM. S_ISFIFO() and S_ISSOCK() are now defined
  identically.

Modified:
  head/sys/compat/cloudabi/cloudabi_fd.c
  head/sys/compat/cloudabi/cloudabi_file.c
  head/sys/contrib/cloudabi/cloudabi_types_common.h
  head/usr.bin/truss/syscalls.c

Modified: head/sys/compat/cloudabi/cloudabi_fd.c
==
--- head/sys/compat/cloudabi/cloudabi_fd.c  Tue Sep  5 06:20:02 2017
(r323176)
+++ head/sys/compat/cloudabi/cloudabi_fd.c  Tue Sep  5 07:46:45 2017
(r323177)
@@ -56,7 +56,6 @@ __FBSDID("$FreeBSD$");
MAPPING(CLOUDABI_RIGHT_FILE_ALLOCATE, CAP_WRITE)\
MAPPING(CLOUDABI_RIGHT_FILE_CREATE_DIRECTORY, CAP_MKDIRAT)  \
MAPPING(CLOUDABI_RIGHT_FILE_CREATE_FILE, CAP_CREATE)\
-   MAPPING(CLOUDABI_RIGHT_FILE_CREATE_FIFO, CAP_MKFIFOAT)  \
MAPPING(CLOUDABI_RIGHT_FILE_LINK_SOURCE, CAP_LINKAT_SOURCE) \
MAPPING(CLOUDABI_RIGHT_FILE_LINK_TARGET, CAP_LINKAT_TARGET) \
MAPPING(CLOUDABI_RIGHT_FILE_OPEN, CAP_LOOKUP)   \
@@ -110,34 +109,21 @@ int
 cloudabi_sys_fd_create2(struct thread *td,
 struct cloudabi_sys_fd_create2_args *uap)
 {
-   struct filecaps fcaps1 = {}, fcaps2 = {};
int fds[2];
-   int error;
+   int error, type;
 
switch (uap->type) {
-   case CLOUDABI_FILETYPE_FIFO:
-   /*
-* CloudABI pipes are unidirectional. Restrict rights on
-* the pipe to simulate this.
-*/
-   cap_rights_init(_rights, CAP_EVENT, CAP_FCNTL,
-   CAP_FSTAT, CAP_READ);
-   fcaps1.fc_fcntls = CAP_FCNTL_SETFL;
-   cap_rights_init(_rights, CAP_EVENT, CAP_FCNTL,
-   CAP_FSTAT, CAP_WRITE);
-   fcaps2.fc_fcntls = CAP_FCNTL_SETFL;
-   error = kern_pipe(td, fds, 0, , );
-   break;
case CLOUDABI_FILETYPE_SOCKET_DGRAM:
-   error = kern_socketpair(td, AF_UNIX, SOCK_DGRAM, 0, fds);
+   type = SOCK_DGRAM;
break;
case CLOUDABI_FILETYPE_SOCKET_STREAM:
-   error = kern_socketpair(td, AF_UNIX, SOCK_STREAM, 0, fds);
+   type = SOCK_STREAM;
break;
default:
return (EINVAL);
}
 
+   error = kern_socketpair(td, AF_UNIX, type, 0, fds);
if (error == 0) {
td->td_retval[0] = fds[0];
td->td_retval[1] = fds[1];
@@ -214,11 +200,11 @@ cloudabi_convert_filetype(const struct file *fp)
 
switch (fp->f_type) {
case DTYPE_FIFO:
-   return (CLOUDABI_FILETYPE_FIFO);
+   return (CLOUDABI_FILETYPE_SOCKET_STREAM);
case DTYPE_KQUEUE:
return (CLOUDABI_FILETYPE_POLL);
case DTYPE_PIPE:
-   return (CLOUDABI_FILETYPE_FIFO);
+   return (CLOUDABI_FILETYPE_SOCKET_STREAM);
case DTYPE_PROCDESC:
return (CLOUDABI_FILETYPE_PROCESS);
case DTYPE_SHM:
@@ -243,7 +229,7 @@ cloudabi_convert_filetype(const struct file *fp)
case VDIR:
return (CLOUDABI_FILETYPE_DIRECTORY);
case VFIFO:
-   return (CLOUDABI_FILETYPE_FIFO);
+   return (CLOUDABI_FILETYPE_SOCKET_STREAM);
case VLNK:
return (CLOUDABI_FILETYPE_SYMBOLIC_LINK);
case VREG:
@@ -286,7 +272,6 @@ cloudabi_remove_conflicting_rights(cloudabi_filetype_t
CLOUDABI_RIGHT_FD_SYNC | CLOUDABI_RIGHT_FILE_ADVISE |
CLOUDABI_RIGHT_FILE_CREATE_DIRECTORY |
CLOUDABI_RIGHT_FILE_CREATE_FILE |
-   CLOUDABI_RIGHT_FILE_CREATE_FIFO |
CLOUDABI_RIGHT_FILE_LINK_SOURCE |
CLOUDABI_RIGHT_FILE_LINK_TARGET |
CLOUDABI_RIGHT_FILE_OPEN |
@@ -312,7 +297,6 @@ cloudabi_remove_conflicting_rights(cloudabi_filetype_t
CLOUDABI_RIGHT_FILE_ALLOCATE |
CLOUDABI_RIGHT_FILE_CREATE_DIRECTORY |
CLOUDABI_RIGHT_FILE_CREATE_FILE |
-   CLOUDABI_RIGHT_FILE_CREATE_FIFO |
CLOUDABI_RIGHT_FILE_LINK_SOURCE |