-
Notifications
You must be signed in to change notification settings - Fork 552
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
Implement Multithreading #1610
base: master
Are you sure you want to change the base?
Implement Multithreading #1610
Conversation
ProcessContext.m_CurrentContext = context; | ||
|
||
IOPort counter0 = new IOPort(0x40); | ||
IOPort cmd = new IOPort(0x43); |
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.
Find a way to use another port or something else than Pit (APIC timer?)
With this using PC Speaker won't be possible and will hang the processor scheduler due to PIT conflicts
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.
maybe use PIT implemented in Cosmos, not IOPort PIT?
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.
@AsertCreator they are one in the same
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.
maybe you can implement the nanosecond time in RTC, and use RTC to approximate? or maybe (pure speculation, as a junior dev who knows nothing about low level) reimplement PIT wait to tally up the number of times IRQ 0 has been triggered since the os was powered on, and use this to wait x amount of time. in the case of the second option, it would be possible to implement a system where multiple things can use PIT wait at the same time. see osdev wiki.
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.
RTC is too slow for scheduling and idk if we could make interrupts with that
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.
In fact RTC may not be a bad idea until we have HPET or local APIC timers, it indeed can generate IRQs. I'll try, I hope this won't mess up date time
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.
01:35:10.769067 Msg: Text from kernel: SwitchTask
01:35:10.787021 Msg: Text from kernel: SwitchTask
01:35:10.802028 Msg: Text from kernel: SwitchTask
01:35:10.818118 Msg: Text from kernel: SwitchTask
01:35:10.834006 Msg: Text from kernel: SwitchTask
01:35:10.849588 Msg: Text from kernel: SwitchTask
01:35:10.865546 Msg: Text from kernel: SwitchTask
I can get no more than 60Hz but it's possible
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.
Does that function as a stopgap until APIC gets finished?
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.
Oh. I just looked at the commit and it appears y'all got the APIC timer working. Ok then.
This would be very beneficial. |
Guys have a huge problem with while loops...
|
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.
Really impressive changes here... but have some serious troubles with while loops!
I'll try to track down the error then publish my results
UPDATE: 🤔 I'll dig more into that EDIT: I think i may know the problem The Console.ReadLine is being split across the threads needing it... Now let's imagine a scenario where one of the two threads doesn't use 'Console.ReadLine()': The only way to fix this is by modifying the behaviour of Console.ReadLine |
Is it any interrupt or just keyboard?
… On Jan 4, 2021, at 12:21 PM, F4lc0131 ***@***.***> wrote:
UPDATE:
Ok so I just found out that the problem isn't the while loop but the keyboard instead...
Here's the thing:
When I load a task as long as I don't press any key it works... but as soon as I click a key the entire system shuts down
🤔
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
From what i remember with this, a lot of the core kernel systems (interrupts, heap, device management) are not thread safe, meaning that you will get a lot of situations like this where threads are tripping each other up. A fix for things like the heap would be to implement something like spinlocks |
as of now just the keyboard... I'll try to bust and fix those errors before releasing this to real cosmos |
Actually guys I had an idea on how we could handle this but I'll have to work on a "lock" system, so that a thread can acquire the lock over something (in this case the Console.ReadLine) tell me what you guys think (especially @valentinbreiz) |
UPDATE: UPDATE: |
UPDATE: UPDATE: EDIT: UPDATE: Sys.Thread(() => { while (true) Console.WriteLine("test"); }); that's the thread, and happens the same exact thing... shell I add a locksystem to WriteLine too? Or just use the same lock in common with ReadLine?? lemme know |
With the trend we're seeing with methods like |
You're right @TFTWPhoenix, some parts of Cosmos are not thread safe and maybe needs some locks too |
Stale pull request message |
Will this ever be finished? Kinda really waiting for this. |
Any news? |
i got this to work (maybe). currently there are two problems: stack corruption detection and |
i would make a PR as the owner of the RP does not have time to work on this |
ok |
|
||
public void Lock() | ||
{ | ||
while (gate != 0) { } |
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.
we should yield here so we dont waste as much cpu time
i found a bug with how multithreading interacts with the AC97 driver here |
i found a bug with graphics. i have some system init code in a function called |
Hello @Neil-Ciciglycerin I think this issue is also in the main branch unfortunally. Could you try and tell me please? |
/// <summary> | ||
/// MMIOBase abstract class. | ||
/// </summary> | ||
public unsafe abstract class MMIOBase |
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.
How does this differ from a MemoryBlock?
} | ||
} | ||
} | ||
|
||
/* |
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.
Do we need this commented out code?
@@ -20,6 +21,8 @@ public enum ObjectGCStatus : ushort | |||
public static unsafe class Heap | |||
{ | |||
private static uint* StackStart; | |||
private static Mutex mMemeoryGate = new Mutex(); |
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.
Typo
@@ -44,6 +47,11 @@ public static unsafe void Init() | |||
/// <returns>New pointer with specified size while maintaining old data.</returns> | |||
public static byte* Realloc(byte* aPtr, uint newSize) | |||
{ | |||
if (mMemeoryGate != null) |
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.
How can the mMemoryGate ever be null? Seems like we are doing useless checks
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.
we could replace it with mMemeoryGate?.Lock();
it would do that same
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 would prefer if we dont do needless checks to reduce the performance cost, as these calls happen quite often
} | ||
|
||
[PlugMethod(PlugRequired = true)] | ||
public static uint GetPointer(Object aVal) { return 0; } |
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.
We already have
Cosmos/source/Cosmos.Core/GCImplementation.cs
Line 108 in f7fd569
public static unsafe uint* GetPointer(object aObj) => throw null; // this is plugged |
If you need a uint we also have
Cosmos/source/Cosmos.Core/GCImplementation.cs
Line 115 in f7fd569
public static unsafe uint GetSafePointer(object aObj) |
|
||
else | ||
{ | ||
new LiteralAssemblerCode("pushad"); |
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.
Why not use the XS equivalent commands?
[Plug(Target = typeof(ObjUtilities))] | ||
public static unsafe class ObjUtilitiesImpl | ||
{ | ||
[PlugMethod(Assembler = typeof(ObjUtilitiesGetPointer))] |
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.
See previous comment if we really need this method new here.
|
||
public static void Ctor(ThreadStart aThis, ThreadStart aEntry) | ||
{ | ||
Console.WriteLine("Thread started"); |
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 this should be removed.
Console.WriteLine("Thread started"); | ||
} | ||
|
||
// public static void SleepInternal(int ms) |
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.
Can this code be removed?
|
||
Console.WriteLine("Starting ACPI"); | ||
debugger.Send("ACPI Init"); | ||
ACPI.Start(); | ||
|
||
Console.WriteLine("Finding PCI Devices"); |
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 don't think we should write as much to the console, since the user cant control this behavior.
The last time I checked it wasn't in the main branch, although I was using userkit. I'll check now |
Yeah, it's in the main branch. I'll go start an issue. |
This pull request is not to merge in master yet, this is more an opening for discussion on the implementation of multithreading in Cosmos.
All the original code of the first commit comes from @Og-Rok's work from Aura's fork of Cosmos. (the original pull request can be found here if needed: https://github.com/aura-systems/Cosmos/pull/40)
Here is a short demonstration video: https://www.youtube.com/watch?v=rea5sAkYyec
To do: