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

fix(irq): prevent interrupt reservation and improve error handling #97

Merged
merged 3 commits into from
Oct 16, 2023

Conversation

Diogo21Costa
Copy link
Member

PR Description

This PR enhances the interrupt reservation functions to prevent race conditions during the reservation process. It comprises the following changes:

  1. Added a spinlock to ensure synchronized access to the critical section, preventing race conditions. The original code lacked this synchronization, potentially leading to issues. The spinlock now ensures only one thread can access the critical section at a time, resolving the race condition (196a404);
  2. Introduced a boolean return value to indicate the success of interrupt reservation. This provides a more detailed assessment of interrupt assignment status, aiding in error handling (fc2029b);
  3. Improved the overall error handling for interrupt assignment (5887601)

Your feedback on these changes is greatly appreciated.

src/core/interrupts.c Outdated Show resolved Hide resolved
src/arch/armv8/gic.c Outdated Show resolved Hide resolved
src/arch/riscv/iommu.c Outdated Show resolved Hide resolved
src/arch/riscv/sbi.c Outdated Show resolved Hide resolved
src/core/interrupts.c Outdated Show resolved Hide resolved
@Diogo21Costa
Copy link
Member Author

@josecm consolidated all error message updates into a single commit (878ad2b). As this commit fixes the content introduced in 5887601, may I squash both commits to keep the git history clean?

The same applies to commits 1a898b4 and 196a404 ( The first one introduces the spinlock initialization). Should they be squashed?

@josecm
Copy link
Member

josecm commented Oct 9, 2023

@Diogo21Costa Yeah, I'd say the history should be re-written and include the new fixes directly on the original commits.

@Diogo21Costa
Copy link
Member Author

@Diogo21Costa Yeah, I'd say the history should be re-written and include the new fixes directly on the original commits.

@josecm Done, thank you!

Copy link
Member

@DavidMCerdeira DavidMCerdeira left a comment

Choose a reason for hiding this comment

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

This commit's description df3168c does not match the code changes

Otherwise seems good

@Diogo21Costa Diogo21Costa force-pushed the fix/irq_reserve branch 2 times, most recently from bf3e598 to c27d8fa Compare October 14, 2023 11:36
@Diogo21Costa
Copy link
Member Author

This commit's description df3168c does not match the code changes

Otherwise seems good

@DavidMCerdeira Nice catch, thank you. Solved.

@DavidMCerdeira DavidMCerdeira self-requested a review October 14, 2023 16:10
@josecm josecm merged commit 8459a8e into main Oct 16, 2023
12 checks passed
@josecm josecm deleted the fix/irq_reserve branch October 16, 2023 09:24
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