Skip to content
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

Question about "prepare_sqes" correctness. #61

Open
twissel opened this issue Dec 11, 2020 · 2 comments
Open

Question about "prepare_sqes" correctness. #61

twissel opened this issue Dec 11, 2020 · 2 comments

Comments

@twissel
Copy link

twissel commented Dec 11, 2020

Here

let head: u32 = *sq.khead;
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
Copy link
Contributor

mxxo commented Dec 11, 2020

Isn't that covered here by the acquire fence beforehand? Is it possible the actual variable access should use an atomic load?

iou/src/submission_queue.rs

Lines 158 to 160 in 045f8d4

atomic::fence(Ordering::Acquire);
let head: u32 = *sq.khead;

@twissel
Copy link
Author

twissel commented Dec 11, 2020

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants