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

[wait-queue] fix wake all to remove threads from tail #161

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mahavirj
Copy link

@mahavirj mahavirj commented Sep 6, 2016

While waking up all threads from wait-queue, order should
start from tail to maintain correct scheduling sequence.

Signed-off-by: Mahavir Jain mahavirpj@gmail.com

While waking up all threads from wait-queue, order should
start from tail to maintain correct scheduling sequence.

Signed-off-by: Mahavir Jain <mahavirpj@gmail.com>
@travisg
Copy link
Member

travisg commented Sep 6, 2016

No so sure about that, since it takes the items off the queue and then shoves them in the head of the run queue, if you pop from the tail it'll have the effect of putting the thread that's been waiting the longest deeper in the queue, and the thread that's been waiting the least longest at the head. With it popping from the head, it inverts the order and puts the oldest thread at the head, which i think is slightly more fair.

@mahavirj
Copy link
Author

mahavirj commented Sep 7, 2016

Let us consider a scenario, where resource is acquired, and thread1 blocks on it at time t1. And later thread2 blocks at time t2 (t2 > t1). Now if there is call to wake all threads, thread2 will get chance to run earlier than thread1, which seems incorrect (thread2 being at head of run queue). Correct me if I am wrong here.

@zeusk
Copy link
Contributor

zeusk commented Sep 7, 2016

EDIT: I think, you're right.

After removing a thread from head of wait queue on line 1220, it is inserted to run queue's head on line 1227.

So the tail of wait queue will end up as head of run queue instead of our desired behavior (first come, first serve).

@mahavirj
Copy link
Author

mahavirj commented Sep 9, 2016

Ping...

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

Successfully merging this pull request may close these issues.

3 participants