Bläddra i källkod

Fix some paging related bugs exposed by the spawn stress test.

- Process::exec() needs to restore the original paging scope when called
  on a non-current process.
- Add missing InterruptDisabler guards around g_processes access.
- Only flush the TLB when modifying the active page tables.
Andreas Kling 6 år sedan
förälder
incheckning
e71cb1c56b
13 ändrade filer med 110 tillägg och 67 borttagningar
  1. 1 1
      AK/Compiler.h
  2. 0 2
      AK/printf.cpp
  3. 13 4
      Kernel/MemoryManager.cpp
  4. 2 0
      Kernel/MemoryManager.h
  5. 35 20
      Kernel/Process.cpp
  6. 4 1
      Kernel/Process.h
  7. 1 1
      Kernel/Scheduler.cpp
  8. 2 2
      Kernel/init.cpp
  9. 4 7
      Kernel/kmalloc.cpp
  10. 12 0
      Kernel/makeall.sh
  11. 10 0
      Kernel/makeuserland.sh
  12. 25 25
      Kernel/sync.sh
  13. 1 4
      Kernel/types.h

+ 1 - 1
AK/Compiler.h

@@ -3,7 +3,7 @@
 #define PACKED __attribute__ ((packed))
 #define NORETURN __attribute__ ((noreturn))
 #undef ALWAYS_INLINE
-#define ALWAYS_INLINE __attribute__ ((always_inline))
+#define ALWAYS_INLINE inline __attribute__ ((always_inline))
 #define NEVER_INLINE __attribute__ ((noinline))
 #define MALLOC_ATTR __attribute__ ((malloc))
 #define PURE __attribute__ ((pure))

+ 0 - 2
AK/printf.cpp

@@ -1,5 +1,3 @@
-#define ALWAYS_INLINE inline __attribute__ ((always_inline))
-
 typedef unsigned char byte;
 typedef unsigned short word;
 typedef unsigned int dword;

+ 13 - 4
Kernel/MemoryManager.cpp

@@ -444,7 +444,8 @@ void MemoryManager::remap_region_page(PageDirectory* page_directory, Region& reg
     else
         pte.setWritable(region.is_writable);
     pte.setUserAllowed(user_allowed);
-    flushTLB(page_laddr);
+    if (page_directory->is_active())
+        flushTLB(page_laddr);
 #ifdef MM_DEBUG
     dbgprintf("MM: >> remap_region_page (PD=%x) '%s' L%x => P%x (@%p)\n", page_directory, region.name.characters(), page_laddr.get(), physical_page->paddr().get(), physical_page.ptr());
 #endif
@@ -478,7 +479,8 @@ void MemoryManager::map_region_at_address(PageDirectory* page_directory, Region&
             pte.setWritable(region.is_writable);
         }
         pte.setUserAllowed(user_allowed);
-        flushTLB(page_laddr);
+        if (page_directory->is_active())
+            flushTLB(page_laddr);
 #ifdef MM_DEBUG
         dbgprintf("MM: >> map_region_at_address (PD=%x) '%s' L%x => P%x (@%p)\n", page_directory, region.name.characters(), page_laddr, physical_page ? physical_page->paddr().get() : 0, physical_page.ptr());
 #endif
@@ -498,7 +500,8 @@ void MemoryManager::unmap_range(PageDirectory* page_directory, LinearAddress lad
         pte.setPresent(false);
         pte.setWritable(false);
         pte.setUserAllowed(false);
-        flushTLB(page_laddr);
+        if (page_directory->is_active())
+            flushTLB(page_laddr);
 #ifdef MM_DEBUG
         dbgprintf("MM: << unmap_range L%x =/> 0\n", page_laddr);
 #endif
@@ -547,7 +550,8 @@ bool MemoryManager::unmapRegion(Process& process, Region& region)
         pte.setPresent(false);
         pte.setWritable(false);
         pte.setUserAllowed(false);
-        flushTLB(laddr);
+        if (process.m_page_directory->is_active())
+            flushTLB(laddr);
 #ifdef MM_DEBUG
         auto& physical_page = region.vmo().physical_pages()[region.first_page_index() + i];
         dbgprintf("MM: >> Unmapped L%x => P%x <<\n", laddr, physical_page ? physical_page->paddr().get() : 0);
@@ -764,3 +768,8 @@ void MemoryManager::unregister_region(Region& region)
     InterruptDisabler disabler;
     m_regions.remove(&region);
 }
+
+inline bool PageDirectory::is_active() const
+{
+    return &current->page_directory() == this;
+}

+ 2 - 0
Kernel/MemoryManager.h

@@ -55,6 +55,8 @@ private:
 struct PageDirectory {
     dword entries[1024];
     RetainPtr<PhysicalPage> physical_pages[1024];
+
+    bool is_active() const;
 };
 
 class VMObject : public Retainable<VMObject> {

+ 35 - 20
Kernel/Process.cpp

@@ -271,8 +271,11 @@ Process* Process::fork(RegisterDump& regs)
 
     ProcFileSystem::the().addProcess(*child);
 
-    g_processes->prepend(child);
-    system.nprocess++;
+    {
+        InterruptDisabler disabler;
+        g_processes->prepend(child);
+        system.nprocess++;
+    }
 #ifdef TASK_DEBUG
     kprintf("Process %u (%s) forked from %u @ %p\n", child->pid(), child->name().characters(), m_pid, child->m_tss.eip);
 #endif
@@ -315,10 +318,12 @@ int Process::exec(const String& path, Vector<String>&& arguments, Vector<String>
     dword entry_eip = 0;
     PageDirectory* old_page_directory = m_page_directory;
     PageDirectory* new_page_directory = reinterpret_cast<PageDirectory*>(kmalloc_page_aligned(sizeof(PageDirectory)));
-    dbgprintf("Process exec: PD=%x created\n", new_page_directory);
+#ifdef MM_DEBUG
+    dbgprintf("Process %u exec: PD=%x created\n", pid(), new_page_directory);
+#endif
     MM.populate_page_directory(*new_page_directory);
     m_page_directory = new_page_directory;
-    MM.enter_process_paging_scope(*this);
+    ProcessPagingScope paging_scope(*this);
 
     // FIXME: Should we consider doing on-demand paging here? Is it actually useful?
     bool success = region->page_in(*new_page_directory);
@@ -400,10 +405,9 @@ int Process::exec(const String& path, Vector<String>&& arguments, Vector<String>
     } // Ready to yield-teleport!
 
     if (current == this) {
-        bool success = Scheduler::yield();
+        Scheduler::yield();
         ASSERT_NOT_REACHED();
     }
-
     return 0;
 }
 
@@ -509,8 +513,11 @@ Process* Process::create_user_process(const String& path, uid_t uid, gid_t gid,
 
     ProcFileSystem::the().addProcess(*process);
 
-    g_processes->prepend(process);
-    system.nprocess++;
+    {
+        InterruptDisabler disabler;
+        g_processes->prepend(process);
+        system.nprocess++;
+    }
 #ifdef TASK_DEBUG
     kprintf("Process %u (%s) spawned @ %p\n", process->pid(), process->name().characters(), process->m_tss.eip);
 #endif
@@ -562,9 +569,11 @@ Process* Process::create_kernel_process(void (*e)(), String&& name)
     process->m_tss.eip = (dword)e;
 
     if (process->pid() != 0) {
-        InterruptDisabler disabler;
-        g_processes->prepend(process);
-        system.nprocess++;
+        {
+            InterruptDisabler disabler;
+            g_processes->prepend(process);
+            system.nprocess++;
+        }
         ProcFileSystem::the().addProcess(*process);
 #ifdef TASK_DEBUG
         kprintf("Kernel process %u (%s) spawned @ %p\n", process->pid(), process->name().characters(), process->m_tss.eip);
@@ -603,7 +612,9 @@ Process::Process(String&& name, uid_t uid, gid_t gid, pid_t ppid, RingLevel ring
     }
 
     m_page_directory = (PageDirectory*)kmalloc_page_aligned(sizeof(PageDirectory));
-    dbgprintf("Process ctor: PD=%x created\n", m_page_directory);
+#ifdef MM_DEBUG
+    dbgprintf("Process %u ctor: PD=%x created\n", pid(), m_page_directory);
+#endif
     MM.populate_page_directory(*m_page_directory);
 
     if (fork_parent) {
@@ -1246,15 +1257,13 @@ mode_t Process::sys$umask(mode_t mask)
     return old_mask;
 }
 
-void Process::reap(pid_t pid)
+void Process::reap(Process& process)
 {
     InterruptDisabler disabler;
-    auto* process = Process::from_pid(pid);
-    ASSERT(process);
-    dbgprintf("reap: %s(%u) {%s}\n", process->name().characters(), process->pid(), toString(process->state()));
-    ASSERT(process->state() == Dead);
-    g_processes->remove(process);
-    delete process;
+    dbgprintf("reap: %s(%u) {%s}\n", process.name().characters(), process.pid(), toString(process.state()));
+    ASSERT(process.state() == Dead);
+    g_processes->remove(&process);
+    delete &process;
 }
 
 pid_t Process::sys$waitpid(pid_t waitee, int* wstatus, int options)
@@ -1273,7 +1282,13 @@ pid_t Process::sys$waitpid(pid_t waitee, int* wstatus, int options)
     sched_yield();
     if (m_was_interrupted_while_blocked)
         return -EINTR;
-    reap(waitee);
+    Process* waitee_process;
+    {
+        InterruptDisabler disabler;
+        waitee_process = Process::from_pid(waitee);
+    }
+    ASSERT(waitee_process);
+    reap(*waitee_process);
     if (wstatus)
         *wstatus = m_waitee_status;
     return m_waitee;

+ 4 - 1
Kernel/Process.h

@@ -67,6 +67,9 @@ public:
         return m_state == BlockedSleep || m_state == BlockedWait || m_state == BlockedRead;
     }
 
+    PageDirectory& page_directory() { return *m_page_directory; }
+    const PageDirectory& page_directory() const { return *m_page_directory; }
+
     bool in_kernel() const { return (m_tss.cs & 0x03) == 0; }
 
     static Process* from_pid(pid_t);
@@ -162,7 +165,7 @@ public:
     static void initialize();
 
     void crash() NORETURN;
-    static void reap(pid_t);
+    static void reap(Process&);
 
     const TTY* tty() const { return m_tty; }
 

+ 1 - 1
Kernel/Scheduler.cpp

@@ -67,7 +67,7 @@ bool Scheduler::pick_next()
 
         if (process.state() == Process::Dead) {
             if (current != &process && !Process::from_pid(process.ppid()))
-                Process::reap(process.pid());
+                Process::reap(process);
             return true;
         }
 

+ 2 - 2
Kernel/init.cpp

@@ -149,8 +149,8 @@ static void spawn_stress()
     for (unsigned i = 0; i < 10000; ++i) {
         int error;
         Process::create_user_process("/bin/id", (uid_t)100, (gid_t)100, (pid_t)0, error, Vector<String>(), Vector<String>(), tty0);
-//        kprintf("malloc stats: alloc:%u free:%u page_aligned:%u eternal:%u\n", sum_alloc, sum_free, kmalloc_page_aligned, kmalloc_sum_eternal);
-//        kprintf("delta:%u\n", sum_alloc - lastAlloc);
+        kprintf("malloc stats: alloc:%u free:%u page_aligned:%u eternal:%u\n", sum_alloc, sum_free, kmalloc_page_aligned, kmalloc_sum_eternal);
+        kprintf("delta:%u\n", sum_alloc - lastAlloc);
         lastAlloc = sum_alloc;
         sleep(60);
     }

+ 4 - 7
Kernel/kmalloc.cpp

@@ -28,7 +28,7 @@ typedef struct
 
 #define RANGE_SIZE 0x100000
 
-PRIVATE BYTE alloc_map[POOL_SIZE / CHUNK_SIZE / 8];
+static byte alloc_map[POOL_SIZE / CHUNK_SIZE / 8];
 
 volatile DWORD sum_alloc = 0;
 volatile DWORD sum_free = POOL_SIZE;
@@ -50,8 +50,7 @@ bool is_kmalloc_address(void* ptr)
     return ptr >= (void*)BASE_PHYS && ptr <= ((void*)BASE_PHYS + POOL_SIZE);
 }
 
-PUBLIC void
-kmalloc_init()
+void kmalloc_init()
 {
     memset( &alloc_map, 0, sizeof(alloc_map) );
     memset( (void *)BASE_PHYS, 0, POOL_SIZE );
@@ -88,8 +87,7 @@ void* kmalloc_page_aligned(size_t size)
 }
 
 
-PUBLIC void *
-kmalloc( DWORD size )
+void* kmalloc(dword size)
 {
     InterruptDisabler disabler;
 
@@ -162,8 +160,7 @@ kmalloc( DWORD size )
     return nullptr;
 }
 
-PUBLIC void
-kfree( void *ptr )
+void kfree(void *ptr)
 {
     if( !ptr )
         return;

+ 12 - 0
Kernel/makeall.sh

@@ -0,0 +1,12 @@
+#!/bin/bash
+
+sudo id
+
+make -C ../LibC clean && \
+make -C ../LibC && \
+make -C ../Userland clean && \
+make -C ../Userland && \
+make clean &&\
+make && \
+sudo ./sync.sh
+

+ 10 - 0
Kernel/makeuserland.sh

@@ -0,0 +1,10 @@
+#!/bin/bash
+
+sudo id
+
+make -C ../LibC clean && \
+make -C ../LibC && \
+make -C ../Userland clean && \
+make -C ../Userland && \
+sudo ./sync.sh
+

+ 25 - 25
Kernel/sync.sh

@@ -1,30 +1,30 @@
-rm -f _fs_contents.lock
-rm -f _fs_contents
-cp -p _fs_contents.stock _fs_contents
-mkdir -p mnt
+rm -vf _fs_contents.lock
+rm -vf _fs_contents
+cp -vp _fs_contents.stock _fs_contents
+mkdir -vp mnt
 mount -o loop _fs_contents mnt/
 cp -R ../Base/* mnt/
-cp ../Userland/sh mnt/bin/sh
-cp ../Userland/id mnt/bin/id
-cp ../Userland/ps mnt/bin/ps
-cp ../Userland/ls mnt/bin/ls
-cp ../Userland/pwd mnt/bin/pwd
-cp ../Userland/sleep mnt/bin/sleep
-cp ../Userland/date mnt/bin/date
-cp ../Userland/true mnt/bin/true
-cp ../Userland/false mnt/bin/false
-cp ../Userland/hostname mnt/bin/hostname
-cp ../Userland/cat mnt/bin/cat
-cp ../Userland/uname mnt/bin/uname
-cp ../Userland/clear mnt/bin/clear
-cp ../Userland/tst mnt/bin/tst
-cp ../Userland/ft mnt/bin/ft
-cp ../Userland/ft2 mnt/bin/ft2
-cp ../Userland/mm mnt/bin/mm
-cp ../Userland/kill mnt/bin/kill
-cp ../Userland/tty mnt/bin/tty
-cp ../Userland/strsignal mnt/bin/strsignal
+cp -v ../Userland/sh mnt/bin/sh
+cp -v ../Userland/id mnt/bin/id
+cp -v ../Userland/ps mnt/bin/ps
+cp -v ../Userland/ls mnt/bin/ls
+cp -v ../Userland/pwd mnt/bin/pwd
+cp -v ../Userland/sleep mnt/bin/sleep
+cp -v ../Userland/date mnt/bin/date
+cp -v ../Userland/true mnt/bin/true
+cp -v ../Userland/false mnt/bin/false
+cp -v ../Userland/hostname mnt/bin/hostname
+cp -v ../Userland/cat mnt/bin/cat
+cp -v ../Userland/uname mnt/bin/uname
+cp -v ../Userland/clear mnt/bin/clear
+cp -v ../Userland/tst mnt/bin/tst
+cp -v ../Userland/ft mnt/bin/ft
+cp -v ../Userland/ft2 mnt/bin/ft2
+cp -v ../Userland/mm mnt/bin/mm
+cp -v ../Userland/kill mnt/bin/kill
+cp -v ../Userland/tty mnt/bin/tty
+cp -v ../Userland/strsignal mnt/bin/strsignal
 sh sync-local.sh
-cp kernel.map mnt/
+cp -v kernel.map mnt/
 umount mnt
 sync

+ 1 - 4
Kernel/types.h

@@ -1,13 +1,10 @@
 #pragma once
 
+#include <AK/Compiler.h>
 #include <AK/Types.h>
 
 #define PACKED __attribute__ ((packed))
-#define NORETURN __attribute__ ((noreturn))
-#define ALWAYS_INLINE __attribute__ ((always_inline))
 #define PURE __attribute__ ((pure))
-#define PUBLIC
-#define PRIVATE static
 
 typedef unsigned char BYTE;
 typedef unsigned short WORD;