-
Notifications
You must be signed in to change notification settings - Fork 2k
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
cpu: native: fix thread_yield_higher isr_is_in() case #6660
cpu: native: fix thread_yield_higher isr_is_in() case #6660
Conversation
I came to a similar solution. What I don't quite understand: |
The change seems valid to me, but I'd like to run a few more tests before I ACK. |
@@ -225,7 +225,7 @@ void thread_yield_higher(void) | |||
irq_enable(); | |||
} | |||
else { | |||
isr_thread_yield(); | |||
sched_context_switch_request = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think isr_thread_yield
is not called from anywhere else - please remove if this is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's weird. IMHO unrelated, though. |
True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
@OlegHahm Does this patch also solve the stability issues you had when testing yout thesis? |
Trying hard to remember which issues you're talking about... |
I thought they were already fixed with #2071. |
But I found 9930e17 in my working branch that may be related. |
Previously, native created a new "setcontext" context whenever
thread_yield_higher()
was called. That's unnecessary and broken. At the end of interrupt our interrupt handlers,sched_context_switch_request
is checked and triggers an eventually needed context switch.Fixes #6642.