New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OS I/O: Raw Devices #272

Merged
merged 29 commits into from Jun 12, 2017

Conversation

Projects
None yet
3 participants
@vyzo
Copy link
Contributor

vyzo commented Jun 8, 2017

Cherry picked from #271.

Adds raw devices, currently implemented for POSIX only, so that special devices can be used to perform userland read/write operations while integrating with the Gambit scheduler.

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 10, 2017

@feeley any progress with the generic port hierarchy?
I can do them if you don't have the time!

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 10, 2017

I discovered a bug in the ___raw_device_select_virt -- it fails to handle for_writing correctly; fixing.

@feeley

This comment has been minimized.

Copy link
Member

feeley commented Jun 10, 2017

Halfway done...

@feeley

This comment has been minimized.

Copy link
Member

feeley commented Jun 11, 2017

Waitable ports are now implemented by commit 05cec0e.

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 11, 2017

Excellent! I will merge and update the raw-device-ports to be descendants of waitable-ports.

By the way, how do you generate the type-ids for the structs? Is it a random uuid or something structured?
I used a hash to generate it for raw-device-ports.

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 11, 2017

btw, ___PROCESS_DEVICE_KIND disagrees with (macro-process-kind).
I assume the two should be the same?

os_io.c:
#define ___PROCESS_DEVICE_KIND    (___PIPE_DEVICE_KIND+131072)

_io#.scm:
(##define-macro (macro-process-kind)     (+ 31 64))
@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 11, 2017

unless the process is just a pipe in userland, in which case it's ok:

os_io.c:
#define ___PIPE_DEVICE_KIND       (___DEVICE_KIND+64)
@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 11, 2017

Ok, merged! I'll do some casual testing and update #273 as well.

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 11, 2017

seems like my implementation of wait is not correct - ##wait-device expects a device-port, so i'll have to call ##wait-for-io! directly.

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 11, 2017

ok, verified that all port operations work as intended, this should be ready for merge!

_io: raw-device-port updates
- raw-device-port has a device. not an fd
- ##make-raw-device is more intelligently written per comments
@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 11, 2017

Updates for review comments in #273.

The only sticking point left is the utility of ##open-raw-device and its name.
For now it takes a device (not an fd :), opens a raw device and calls ##make-raw-device-port.

We can drop the function altogether if you think the caller should be doing the actual opening of the device (ie if you think it's too POSIX specific)

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 12, 2017

Updated per review comments.

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 12, 2017

small brainfart in usage of id field, fixing.

lib/os_io.c Outdated
return ___FIX(___NO_ERR);
}

___HIDDEN ___SCMOBJ ___device_raw_select_virt

This comment has been minimized.

@feeley

feeley Jun 12, 2017

Member

Please make a copy of ___device_file_select_raw_virt including the WIN32 part, because then it will probably work as-is on Windows.

lib/os_io.c Outdated

fd = ___INT(index);

if ((e = ___device_raw_setup_from_fd

This comment has been minimized.

@feeley

feeley Jun 12, 2017

Member

Indent properly... open paren for parameters should be 2 spaces indented from the ___device_....

This comment has been minimized.

@vyzo

vyzo Jun 12, 2017

Author Contributor

do you have an emacs mode for your indentation style?

This comment has been minimized.

@feeley

feeley Jun 12, 2017

Member

Nope.

lib/os_io.c Outdated
!= ___FIX(___NO_ERR))
return e;

if ((e = ___NONNULLPOINTER_to_SCMOBJ

This comment has been minimized.

@feeley

feeley Jun 12, 2017

Member

Same indent style should be applied here.

lib/os_io.c Outdated
}
#endif

___SCMOBJ ___os_device_raw_open

This comment has been minimized.

@feeley

feeley Jun 12, 2017

Member

I keep thinking that it is wrong to expose this function in Scheme since is is POSIX specific. It is OK however to make it available to C code, so that a device can be constructed from a fd using the FFI. The "index" parameter should be "fd" because that's what it is. The function should be called ___os_device_raw_open_from_fd.

@@ -4279,6 +4279,12 @@ end-of-code
scheme-object ;; device
"___os_device_stream_open_process"))

(define-prim ##os-device-open-raw

This comment has been minimized.

@feeley

feeley Jun 12, 2017

Member

See comment below. This functionality should not be exposed to the Scheme level.

@feeley

This comment has been minimized.

Copy link
Member

feeley commented Jun 12, 2017

Seems like ___device_raw_select_virt should be called ___device_raw_select_raw_virt to be consistent with the other device kinds.

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 12, 2017

ok, will rename.

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 12, 2017

ok, updated! Sorry for the indentation mismatch, i tend to just follow emacs in GNU style.

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 12, 2017

actually there is an accident from the copy paste and it's a wrong cast in ___device_raw_select_raw_virt -- fixing.

@vyzo vyzo force-pushed the vyzo:os-io-raw-devices branch from ee7da5b to 49aa0bf Jun 12, 2017

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 12, 2017

ok, now we should be good -- waiting for the build to finish to run some tests, but it looks good to me.

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jun 12, 2017

testing successful.

@feeley feeley merged commit babd59d into gambit:master Jun 12, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@vyzo vyzo deleted the vyzo:os-io-raw-devices branch Jun 12, 2017

@adam-7

This comment has been minimized.

Copy link

adam-7 commented Jul 25, 2017

@vyzo , what's a raw device, what are their usecases? Can you provide a short example script that illustrates their utility here?

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Jul 25, 2017

Things like sockets, epoll, inotify and kquque file descriptors.
With raw devices we can open these OS-specific devices and integrate them with the Gambit scheduler.

@adam-7

This comment has been minimized.

Copy link

adam-7 commented Jul 25, 2017

Ah!

By "raw device", do you just mean that you have created a facility whereby the user can allocate his own OS file descriptor of any type that the OS can produce,

(I have not studied epoll/inotify/kqueue so it's not in my awareness that they produce their own FD:s, but I understand that's what you're saying, so they fit the picture too;)

and, what you have done here is to provide a facility where the user can create a Gambit port object that functions only to represent such a file descriptor, so that the user may do |##device-port-wait-for-input!| and |##device-port-wait-for-output!| on that port object?

I guess such a raw port object is actually totally passive, as in, Gambit will not do any operations on it such as OS write, read, close operations, but they function only to make Gambit include that FD in Gambit's select/poll() call, during the ##device-port-wait-for-input/output! call?

Are any other operations than ##device-port-wait-for-input/output! supported on those raw ports?

Are the raw ports documented in the manual now or do you keep these out of the manual due to being too low level?

Probably the raw ports will be used for pretty low level stuff, right?

Except for this and making Gambit's epoll() support unlimited number of ports (whereas previously there was an ~8000 FD:s limit and exceeding that led to SIGSEGV), have you done any other major addition now?

Looks great! :) Thanks

@feeley

This comment has been minimized.

Copy link
Member

feeley commented Nov 17, 2017

This is just a heads up that the API for ##make-raw-device-port will be changing to accommodate other types of raw-devices. In particular what used to be called the "id" is now called the "type", and the "name" of the port is an explicit parameter (so it is easy to choose a custom name). The last field is a "specific" field that can be used to store anything.

Vyzo, here is how the old API can be emulated:

(define (##vyzos-make-raw-device-port raw-device device id direction)
  (##make-raw-device-port direction raw-device id (##list id device) device))

To access the "device" from the port, use (macro-raw-device-port-specific port).

@adam-7

This comment has been minimized.

Copy link

adam-7 commented Nov 18, 2017

Like what types?

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Nov 18, 2017

thanks marc!

@feeley

This comment has been minimized.

Copy link
Member

feeley commented Nov 18, 2017

I'm implementing UDP ports and it is convenient to build them on top of raw-device ports. By the way, the API has changed again to: (##make-raw-device-port direction device kind id specific). The third parameter is a numeric "kind", for example (macro-raw-device-kind). This simplifies type testing.

@vyzo

This comment has been minimized.

Copy link
Contributor Author

vyzo commented Nov 18, 2017

Can the kind be an arbitrary object (eg a symbol) and not just a numeric kind in the new API?
That would be much better for identifying user port types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment