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

[WIP] Add push/pop lockless #363

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Restioson
Copy link

@Restioson Restioson commented Apr 24, 2019

This PR will add non-deadlocking push/pop operations to queues -- they will never deadlock even if a concurrent push/pop stalls for whatever reason indefinitely.

/// assert_eq!(q.push(10), Ok(()));
/// assert_eq!(q.push(20), Err(PushError(20)));
/// ```
pub fn push_lockless(&self, value: T) -> Result<(), PushError<T>> {
Copy link
Contributor

@jeehoonkang jeehoonkang Apr 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is very much similar to push(). Do you think can we somehow merge the code of the two functions?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I did think this and tried to merge them. Unfortunately it made the tests hang... So I will have another go at that when I have time. Before it is merged that should be done though.

@ghost
Copy link

ghost commented Apr 29, 2019

@Restioson I have a high-level question about your use case with interrupts pushing keyboard events into the queue.

So if your interrupt handler is defined like this:

fn handler(event: KeyboardEvent) {
    queue.push(event);
}

What happens if an event handler (call it A) is interrupted before push is called by another interrupt handler (call it B)? Does that mean keyboard event from A will end up in the queue after event from B?

That sounds wrong to me so I wonder how keyboard drivers handle situations like these? Are you sure this is the way to go?

My guess would be that event handlers shouldn't be interruptable and maybe the hardware guarantees so, but I've no idea if what I'm saying is true.

@Restioson
Copy link
Author

Restioson commented Apr 29, 2019

Yes, interrupt handlers cannot be interrupted (interrupts are disabled by the os). This is because if nesting is allowed, excessive nesting depth could cause a stackoverflow. However, the local APIC is able to buffer one IRQ to be dispatched if the cpu has disabled interrupts, which will be dispatched when the cpu re-enables them.

I do believe that I still need the function though, for instance in case the push interrupts a pop from the handler. And nevertheless, it could be useful for others.

@ghost
Copy link

ghost commented Apr 29, 2019

@Restioson Thanks for clarifying!

And just a couple more questions to see if I understood the problem domain correctly:

  1. Is it true that there is only a single consumer?
  2. Is it true that there is only a single producer?
  3. Is it true that even though you're using the queue in the SPSC fashion, you want a MPSC or MPMC interface only to get around Rust's ownership/sharing rules?

@Restioson
Copy link
Author

  1. yes
  2. yes
  3. yes pretty much :) it has to be static unfortunately

@ghost
Copy link

ghost commented Apr 30, 2019

In that case, the way to go would really be #338, which is ~5 times faster than ArrayQueue anyway :)

It's unfortunate that the SPSC queue cannot be used in your use case without unsafe {} blocks, but that is really the nature of the problem of interrupts. So my suggestion would be to use static mut variable then...

@Restioson
Copy link
Author

Aren't those slated to be removed? Perhaps a RefCell. I will consider it though. Do you still think that this has a use case?

@ghost
Copy link

ghost commented May 11, 2019

Aren't those slated to be removed?

Possibly -- I'm not sure. The alternative to static mut would be a static UnsafeCell<T> variable.

Do you still think that this has a use case?

I don't think so. The SPSC queue is really designed to be wait-free and applicable in embedded systems, device drivers, and similar use cases. Others queues are a bit more high-level and can't provide strong progress guarantees without jumping through hoops or having tricky caveats.

@Restioson
Copy link
Author

Sorry, I haven't had much time to work on this because of exams... I will get back to it when they are finished though.

@taiki-e taiki-e added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author label Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author
Development

Successfully merging this pull request may close these issues.

3 participants