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 SMP support for i386 #292

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

JBouron
Copy link

@JBouron JBouron commented Mar 10, 2019

Smp-support was in a pretty bad shape as the code had not been maintained for a long time.
It even reached a point where the kernel would not even compile with -DCONFIG_SMP=y.

This PR fixes the compilation bugs + other bugs and oddities that were related to the SMP support. Here's a breakdown of the changes:

Compilation failure for -DCONFIG_SMP=y

Just some missing includes and typdefs.

Broken sysenter/syscall

The sysenter and syscall features were not configured properly. The BSP would end up using the wrong kernel stack and APs would use undefined kernel stack, instruction pointer and code segment.

Division by 0 in get_cpu_ticks

Would happen in SMP when opening top for instance. A division by 0 in the kernel is not great for bussiness ...

Wrong LAPIC timer frequency

The code calibrating the LAPIC timer had an overflow bug. The computed frequency would be 5 times lower than it is in reality, leading to time passing 5 times faster in user space.

Scheduler

The scheduler didn't keep track of the load properly. It was also lacking a load balancer.

VFS panic

When closing a socket VFS would panic with "process has two calls (105, 131)". The fix was to make close() blocking. It may be a bit dirty though ...

APs using physical addresses for LAPIC

This one's tricky. When VM execute arch_enable_paging, some APs may still be configuring their lapic and thus use the wrong (ie. non translated) LAPIC register base address. Thus their LAPIC would not be configured properly.

Intermittent failures in lin_lin_copy and verify_grant

lin_lin_copy would sometime panic if srcptr or dstptr had the RTS_VMINHIBIT flag set. Verfiy_grant would also page fault when accessing an entry in the grant table.
For both bug, defering the kernel call (VMSUSPEND) is enough.




That's all folks !
Oh btw, I have a fine-grain implementation of the Minix3 kernel (ie. no BKL) available here. Don't hesitate to take a look if you're curious. I'll probably won't do a PR just yet for this one, too much stuff.

Justinien Bouron added 8 commits March 10, 2019 04:23
Those features were not configured properly and had a race-condition
when enabling back interrupts before a sysexit/sysret.

Configuration: The BSP is executing tss_init on behalf of APs. Thus any
writes in the MSRs in tss_init would end up in the BSP's MSRs leading
to:
	* BSP not using the correct kernel stack.
	* APs using undefined instruction/stack pointer and code segment
upon entering the kernel.
Fix: Let the cores writing in their own MSRs.

Race-condition: When restoring a process' EFLAGS before a sysexit/sysret
interrupts could be enabled while the core was still in the kernel. If
an interrupt is triggered at this point it would lead to a deadlock on
the BKL down the line.
Fix: Disable IF in saved EFLAGS and use sti before sysexit/sysret.
The load tracking in the scheduler had a bug were the BSP would always
have the lowest load on the system leading to large imbalance.
Also add a simple periodic load balancer.
address of LAPIC registers without translating it to virtual ones first.

That would happen if VM had changed the page tables before the APs
configured their LAPIC.
Sometimes, the srcptr or dstptr in lin_lin_copy would have the
RTS_VMINHIBIT flag set leading to an assertion error. In case this
happens defer the kernel call instead of panicking.

Verify_grant would somtimes page-fault when accessing an entry. Defer
the kernel call when that happens.
@boricj
Copy link
Contributor

boricj commented Mar 12, 2019

I can't check this out right now (my main computer is in another town), but this looks impressive, congratulations!

Does it pass/survive the MINIX3 test suite (https://github.com/Stichting-MINIX-Research-Foundation/minix/tree/master/minix/tests)? Have you tried to do some benchmarks (like rebuilding MINIX3 for example) to see if there are scalability issues?

@KSE3
Copy link

KSE3 commented Oct 25, 2020

I see this pull-request takes over the functions of a repository branch and even staff has taken up effort to contribute a lot of work to it. I would wish they could make a nail to it and decide to open a branch "3.4-NT" or so and start collecting the various input that was made to create a usable and modernised version of this operating system. Thanks!

As for multi-coreing, is there any danger arising in synchronisation aspects of the OS through this innovation which hasn't been dealt with multi-processing? I don't think so for now, but are there any objections?

@JBouron
Copy link
Author

JBouron commented Oct 25, 2020

Hi all.

First of all I want to apologize for the radio silence for the past year and a half. This pull request was created while I was working on my Master's thesis. At the time I had to fix the SMP support in Minix in order to get started with my thesis and I thought others could benefit from it too, hence this PR. I have to admit that this PR was not really on top of my priority list while I was completing my thesis and later, for personal reasons, I haven't really got time to get back into it.

Second, around June 25th 2019, I changed my email address in my Github account. To avoid loosing all my contributions of the past 7 years or so on my profile I decided to update the email address of all my commits in my repos (which also requires a force push). Something obviously went wrong with my fork of Minix because now this PR wants to merge more than 2200 commits (some of them are not even mine or related to the fixes) with 5000 files changed !! Originally the PR was much lighter, only a handful of files were changed and the changes were simple (see original description of the fixes). Lucky I kept a backup of the repo before the email change + force push. I'll try to restore it and hopefully this will fix this mess.
EDIT: I restored the backup, the PR looks normal now

A few more comments:
@boricj : At the time I was fixing SMP while adding the changes for my thesis as well. Hence my version of the code was different than that of this PR. Tests were passing and the system seemed to be stable, however I do agree that more testing on this particular PR/set of changes would not hurt. Regarding scalability I remember having some issues with daemons (VFS, PM, ...) running exclusively on CPU 0. I did change this behaviour in this PR however.

@KSE3 : I would love to help to get this merged. Regarding synchronization, I did not have to change anything, the Big Kernel Lock is still there and doing its job properly.

@KSE3
Copy link

KSE3 commented Oct 26, 2020

Done the Gordonic solution? ;) Well, we had no choice.
I have no idea how a scheduling strategy looks like on a multi-core process deployment. Can you talk a little what you have implemented here? I'm also shocked to see the project has about 80,000 objects in the repository.

@emojifreak
Copy link

I have two comments to this PR:

  1. Self compilation on minix requires the following additional patch:
diff -ru minix-netbsdcurrent/minix/kernel/arch/i386/Makefile.inc src/minix/kernel/arch/i386/Makefile.inc
--- minix-netbsdcurrent/minix/kernel/arch/i386/Makefile.inc	Sun Mar 20 18:17:52 2022
+++ src/minix/kernel/arch/i386/Makefile.inc	Fri Mar 11 07:48:41 2022
@@ -84,6 +84,8 @@
 
 .ifdef CONFIG_SMP
 SRCS += arch_smp.c trampoline.S
+trampoline.o: trampoline.S
+	${CC} -E ${AFLAGS} ${AFLAGS.${<:T}} ${CPPFLAGS} ${.IMPSRC} | ${AS} -o ${.TARGET}
 .endif
 
 .if ${USE_ACPI} != "no"
  1. fork() and wait() becomes significantly slower. I started Minix 3 by the following command on QEMU: qemu-system-i386 -machine pc -cpu pentium3-v1 -accel kvm -smp sockets=4 -m 1920 -net user -net nic,model=e1000 -drive discard=unmap,detect-zeroes=unmap,if=ide,file=minix-current-JBouron-smpfix-4cpus.qcow2 -serial vc -serial vc -serial vc -serial vc -boot c. Then I compile the following test program:
/*
clang -Wall -static -O3 -ffunction-sections -fdata-sections -Wl,--gc-sections forktest.c
*/
#include <sys/types.h>
#include <sys/wait.h>
#include <errno.h>
#include <unistd.h>
#include <stdio.h>

int main(void) {
  int counter=200000, exitcode;
  pid_t r;
  errno=0;
  while (counter>0) {
    r=fork();
    if (r<0) { perror("fork():"); errno=0; }
    if (r > 0) {/* parent */
      if (wait(&exitcode)<0) { perror("wait():"); errno=0; }
      else --counter; 
      /*fprintf(stderr, "%d\n", counter);*/
    }
    if (r==0) return 0;  /* child */
  }
  return 0;
}

With the prebuilt kernel in 3.4RC6 CDROM, I got the following result:
スクリーンショット_2022-03-27_09-41-05
But with the SMP Minix CONFIG_MAX_CPUS=4 I had:
スクリーンショット_2022-03-27_09-44-54

By the way, Minix3.4RC6 is faster than Debian Linux Bullseye, which had
スクリーンショット_2022-03-27_05-04-48

But Minix 2.0.4 is even faster than any other...
スクリーンショット_2022-03-27_05-54-20

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.

4 participants