rio icon indicating copy to clipboard operation
rio copied to clipboard

Sqpoll support

Open Licenser opened this issue 5 years ago • 8 comments
trafficstars

heavy WIP, trying to add support for SQPOLL not currently working, will cleanup & squash at the end.

Licenser avatar Feb 04 '20 20:02 Licenser

Hey @Licenser, thanks for taking this on! One thing I was wondering about was maybe allowing various file and buffer-accepting operations to accept generic types that implement traits that trigger the flags for registered files / buffers automatically, and adding types RegisteredBuffer and RegisteredFile that know about their registered offset. I haven't sketched out a prototype of this, but that general approach has been on my mind as a way to minimize API surface area and try to minimize misuse. Something like that wouldn't block this work here, but maybe it's something to wonder about. What do you think about that approach in the long run?

By the way, I would quickly accept a PR that just adds the ability to register files / buffers / both, before any methods that use them exist, so we can just iterate on an API like that quickly.

spacejam avatar Feb 08 '20 17:02 spacejam

I like the idea! I'm still fighting with getting a file to register to start with once I get over that bump I think the rest will fall in place easily. I'm in no hurry to merge, there is nothing immediate I would use it for other than to see the effect in regards to #5 so I'd be happy with both flashing out a nice API or getting it merged and iterating :)

Licenser avatar Feb 08 '20 17:02 Licenser

I rebased and added an example, but I'm somewhat stuck with this error:

thread '<unnamed>' panicked at 'error in cqe reaper: Os { code: 6, kind: Other, message: "No such device or address" }', src/io_uring/cq.rs:96:17

The file registers successfully. The manpage states:

.TP
.B IORING_REGISTER_FILES
Register files for I/O.
.I arg
contains a pointer to an array of
.I nr_args
file descriptors (signed 32 bit integers).

To make use of the registered files, the
.B IOSQE_FIXED_FILE
flag must be set in the
.I flags
member of the
.IR "struct io_uring_sqe" ,
and the
.I fd
member is set to the index of the file in the file descriptor array.

registered_file_prep_rw sets IOSQE_FIXED_FILE before committing the operation, and the FD passed in in the example is 0. I think I'm missing something but I'm not sure what.

Licenser avatar Feb 08 '20 22:02 Licenser

Traced it a bit further, seems the error above was because once a ring is created with sqpoll it can't be used with normal files. I wonder if it should error when it's attempted to be used that way?

Anyway moving things forward it seems the completion for sqpoll requests hangs here https://github.com/spacejam/rio/blob/aae2fb6df0cd6aa2cb6c02562ef018c9a70f62f4/src/completion.rs#L97

I suspect there could be a deadlock somewhere? Is there something obvious that comes to mind?

Licenser avatar Feb 12 '20 19:02 Licenser

With some further digging - it seems that the filler is never executed (Filler::fill) doesn't seem to get called.

Licenser avatar Feb 12 '20 19:02 Licenser

I hit something similar when running this from multiple requesting threads in its usage in sled. Will dive into this in the next few days

spacejam avatar Feb 13 '20 19:02 spacejam

Thank you! I'll keep digging on my side but I think you'll have more luck ;)

Licenser avatar Feb 13 '20 19:02 Licenser

Interestingly I now get the error:

thread '<unnamed>' panicked at 'error in cqe reaper: Os { code: 6, kind: Other, message: "No such device or address" }', src/io_uring/cq.rs:96:17

on creating the ring when sq_poll is true.

Licenser avatar Mar 04 '20 21:03 Licenser