search

ringbahn / iou

Rust interface to io_uring
Apache License 2.0
328 stars 22 forks source link

Question about "prepare_sqes" correctness. #61

Open twissel opened 3 years ago

twissel commented 3 years ago

Here https://github.com/ringbahn/iou/blob/045f8d44c235e35b3731625356a11e6ee05966f9/src/submission_queue.rs#L160 plain(non atomic) load from sq.khead is used which is fine if sqpoll is not enabled, but may lead to troubles if it is enabled. Should't this load be something like atomic_load(acquire) https://github.com/axboe/liburing/blob/3bdd9839005900440c7f74aafa476db48e6e9985/src/queue.c#L386 ? If I'm wrong. I'm sorry.

mxxo commented 3 years ago

Isn't that covered here by the acquire fence beforehand? Is it possible the actual variable access should use an atomic load? https://github.com/ringbahn/iou/blob/045f8d44c235e35b3731625356a11e6ee05966f9/src/submission_queue.rs#L158-L160

twissel commented 3 years ago

@mxxo you can use fence, but then this should be written like:

let head: u32 = atomic_load_relaxed(sq.khead);
fence(Acquire);

оr just use atomic_load_acquire(sq.head) and remove fence.