Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

cpu: native: don't handle pending signals in isr_thread_yield #6135

Closed
wants to merge 1 commit into from

Conversation

kaspar030
Copy link
Contributor

I've been hitting a bug were a thread wouldn't get scheduled anymore even after putting it on the respective runqueue.

I think the problem is a possibly nested call of isr_thread_yield():

  1. ISR (e.g., incoming network packet) triggers
  2. ISR sets a thread's status, calls thread_yield_higher() to trigger context switch
  3. thread_yield_higher() calls isr_thread_yield()
  4. isr_thread_yield() previously handled pending signals using native_irq_handler()
  5. native_irq_handler() calls cpu_switch_context_exit()

... somehow now the thread woken up by 2.) doesn't get scheduled anymore.

I don't know why (tm), but removing the pending signal handling from isr_thread_yield() solves this.

@kaspar030 kaspar030 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: native Platform: This PR/issue effects the native platform Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Nov 16, 2016
@OlegHahm
Copy link
Member

OlegHahm commented Nov 17, 2016

I think I found something similar when reviewing #2071.

@OlegHahm
Copy link
Member

#2071 (diff)

@OlegHahm
Copy link
Member

(And in my working branch for the 1000-native-nodes experiments, I have the same diff: OlegHahm@9930e17)

@kaspar030
Copy link
Contributor Author

@LudwigKnuepfer could you take a look?

Aren't IRQ's (signals) queued if not handled here?

If not, we need to get rid of the cpu_switch_context_exit() call. One way would be to have a version if native_irq_handler() which does not call it.
And we should probably make sure that isr_thread_yield() doesn't nest.

@LudwigKnuepfer
Copy link
Member

I can't estimate when I have time to investigate this properly. I would advise to try and figure it out on your own unless it can wait for at least two weeks.

@kaspar030
Copy link
Contributor Author

I cannot reproduce anymore. Reading through the description, this should be fixed by #6660.

@kaspar030 kaspar030 closed this Mar 21, 2017
@kaspar030 kaspar030 deleted the native_isr_race branch March 21, 2017 23:20
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Platform: native Platform: This PR/issue effects the native platform Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants