Skip to content
This repository has been archived by the owner on Aug 11, 2019. It is now read-only.

Resolve all known deadlock issues with PIT scheduling #6

Merged
merged 5 commits into from
Dec 29, 2017
Merged

Resolve all known deadlock issues with PIT scheduling #6

merged 5 commits into from
Dec 29, 2017

Conversation

robert-w-gries
Copy link
Contributor

I've combined various fixes from robert-w-gries/rxinu#47, robert-w-gries/rxinu#49, and robert-w-gries/rxinu#52 to stabilize scheduling in lambdaOS.

There was significant refactoring effort to ensure that all critical sections of the kernel is safely wrapped in disable_interrupts_and_then(). I've generally kept the naming conventions I used in rxinu. If you have any preferences for naming or code organization, let me know.

I can comfortably say that the scheduling code is stable now because lambdaOS is able to create and run 1500 processes without deadlocking or running out of memory. Keyboard input still works while the null process is running and input also works after the final process is created and ran.

Restoring interrupts instead of enabling all interrupts

Derived from robert-w-gries/rxinu#47

Previously, the disable_interrupts_and_then() function would globally disable interrupts, run some important kernel code, then it would globally enable interrupts.

This procedure worked when the null process called resched() because we could ensure that resched() would only be called at one point in the kernel.

After moving resched() to the timer handler, the kernel could now see a resched() call at any time when interrupts are enabled. This led to various deadlock issues where interrupts would be called when we didn't expect it.

My solution was to add many disable_interrupts_and_then() wrappers to any section of the kernel that held a lock (such as the Heap Allocator). This led to some nested calls to disable_interrupts_and_then(). Since the wrapper globally enables interrupts after running code, interrupts were re-enabled inside of a disable_interrupts_and_then() lower in the call stack. Thus, code that was expected to run interrupted was actually being interrupted.

To solve this problem, I modified disable_interrupts_and_then() to save the interrupt masks of the Chained PICS. Now, we can nest calls to disable_interrupts_and_then() as much as we want because the inner calls will see that the interrupt mask registers should be 0xFF.

Refactoring the Heap Allocator

A consequence of using the PIT to schedule processes is that the Heap Allocator could be interrupted during allocation or de-allocation. Since our allocator is implemented with a Mutex wrapper, we need to ensure that allocation is not interrupted while the lock is held.

Your previous organization of the global allocator did not allow me to wrap the alloc methods in disable_interrupts_and_then(), so I needed to move the heap allocator code to src/memory/heap_allocator.rs.

Notes

  • Printing seems unusually slow. Is this a known issue?
  • PIT_TICKS was not being reset, so I added that fix to this PR

@toor toor self-requested a review December 29, 2017 09:39
toor
toor previously requested changes Dec 29, 2017
Copy link
Owner

@toor toor left a comment

Choose a reason for hiding this comment

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

After these changes have been made I think this will be good for merge.


//Attribute tells Rust to use this as the default heap allocator.
#[global_allocator]
static GLOBAL_ALLOC: HeapAllocator = HeapAllocator::new();
Copy link
Owner

Choose a reason for hiding this comment

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

Can you move this to being under src/memory/mod.rs? I dislike there being too much clutter in src/lib.rs and feel it would be more appropriate to keep it in the memory module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but there's unfortunately a bug in rust where global allocators need to be defined in src/lib.rs

rust-lang/rust#44113

@toor toor dismissed their stale review December 29, 2017 09:51

This is something that can be changed by me later, no need to worry about it in this PR.

@toor
Copy link
Owner

toor commented Dec 29, 2017

Thanks, this looks really good. I don't know how I missed the reset of PIT_TICKS - I feel like an idiot now D: . Regarding the printing to screen, this is something to do with the fact that the buffer methods are not directly accessing the VGA buffer, but instead call into an intermediary that then syncs the VGA buffer itself.

@toor toor merged commit ac0951c into toor:master Dec 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants