tokio-rs / io-uring

The `io_uring` library for Rust
Apache License 2.0
1.19k stars 131 forks source link

opcode Read misses IOSQE_BUFFER_SELECT flag? #202

Open redbaron opened 1 year ago

redbaron commented 1 year ago

Looks like it is required to set IOSQE_BUFFER_SELECT flag when to make use of provided group at the sqe->buf_group

https://github.com/axboe/liburing/blob/241bbfafe437b8b161d25be81d677f6ba109ea44/test/read-write.c#L148-L149

Kernel code also suggests it is required: https://github.com/torvalds/linux/blob/5e725d112e1a54c2611d5dffd124a79415d0f0de/io_uring/io_uring.c#L2130-L2134

Currently Read opcode neither sets it at build() time nor exposes flags for users to set.

saiintbrisson commented 1 year ago

Do so with the io_uring::squeue::Flags bitflag:

let read = opcode::Read::new(fd, std::ptr::null_mut(), 0)
            .buf_group(bgid)
            .build()
            .flags(squeue::Flags::BUFFER_SELECT); // here
redbaron commented 1 year ago

It looks like a viable workaround, but I'd expect opcode::Read to do the right thing when building SQE

saiintbrisson commented 1 year ago

The opcode structs tend to be more of a "hands-off" experience and usually let users configure things as they wish. But I agree that setting buf_group and still needing to define the buffer select flag can be redundant work.

SUPERCILEX commented 8 months ago

I looked into this and while we can add a check for a null pointer in Read, doing it for Readv and RecvMsg becomes more complicated as you have to actually read the iovec pointers which means the build method would have to become unsafe. So I think this isn't worth it.