From df95126ccd09fc121b9737bb81201ed9f5155016 Mon Sep 17 00:00:00 2001 From: apio Date: Fri, 14 Apr 2023 21:10:38 +0200 Subject: [PATCH] kernel: Remove unneeded debug logs & random cleanups --- kernel/debug.cmake | 3 +++ kernel/src/ELF.cpp | 22 ++++++++++++---------- kernel/src/arch/x86_64/MMU.cpp | 16 ++++++++++------ kernel/src/fs/VFS.cpp | 4 ---- kernel/src/fs/devices/DeviceRegistry.cpp | 2 +- kernel/src/main.cpp | 1 - kernel/src/memory/UserVM.cpp | 4 ---- kernel/src/sys/exec.cpp | 10 ++-------- kernel/src/sys/mkdir.cpp | 2 -- kernel/src/sys/mknod.cpp | 2 -- kernel/src/sys/mmap.cpp | 4 ++++ kernel/src/sys/open.cpp | 6 +----- kernel/src/thread/Scheduler.cpp | 4 ---- 13 files changed, 33 insertions(+), 47 deletions(-) diff --git a/kernel/debug.cmake b/kernel/debug.cmake index 85d5d129..b15446a3 100644 --- a/kernel/debug.cmake +++ b/kernel/debug.cmake @@ -1,3 +1,6 @@ target_compile_definitions(moon PRIVATE LOCKED_VALUE_DEBUG) target_compile_definitions(moon PRIVATE DEBUG_MODE) +target_compile_definitions(moon PRIVATE ELF_DEBUG) +target_compile_definitions(moon PRIVATE MMU_DEBUG) +target_compile_definitions(moon PRIVATE MMAP_DEBUG) target_compile_options(moon PRIVATE -fsanitize=undefined) diff --git a/kernel/src/ELF.cpp b/kernel/src/ELF.cpp index 6e872bae..226ca00b 100644 --- a/kernel/src/ELF.cpp +++ b/kernel/src/ELF.cpp @@ -32,48 +32,48 @@ namespace ELFLoader if (nread < sizeof elf_header) { - kdbgln("Error while loading ELF: ELF header does not fit in file"); + kerrorln("Error while loading ELF: ELF header does not fit in file"); return err(ENOEXEC); } if (memcmp(elf_header.e_ident, ELFMAG, SELFMAG) != 0) { - kdbgln("Error while loading ELF: ELF header has no valid magic"); + kerrorln("Error while loading ELF: ELF header has no valid magic"); return err(ENOEXEC); } if (elf_header.e_ident[EI_CLASS] != ELFCLASS64) { - kdbgln("Error while loading ELF: ELF object is not 64-bit"); + kerrorln("Error while loading ELF: ELF object is not 64-bit"); return err(ENOEXEC); } if (elf_header.e_ident[EI_DATA] != ELFDATA2LSB) { - kdbgln("Error while loading ELF: ELF object is not 2's complement little-endian"); + kerrorln("Error while loading ELF: ELF object is not 2's complement little-endian"); return err(ENOEXEC); } if (elf_header.e_type != ET_EXEC) { - kdbgln("Error while loading ELF: ELF object is not an executable"); + kerrorln("Error while loading ELF: ELF object is not an executable"); return err(ENOEXEC); } if (elf_header.e_machine != EM_MACH) { - kdbgln("Error while loading ELF: ELF object's target architecture does not match the current one (%s)", - CPU::platform_string()); + kerrorln("Error while loading ELF: ELF object's target architecture does not match the current one (%s)", + CPU::platform_string()); return err(ENOEXEC); } if (elf_header.e_phnum == 0) { - kdbgln("Error while loading ELF: ELF object has no program headers"); + kerrorln("Error while loading ELF: ELF object has no program headers"); return err(ENOEXEC); } - kdbgln("ELF: Loading ELF with entry=%#.16lx", elf_header.e_entry); + kinfoln("ELF: Loading ELF with entry=%#.16lx", elf_header.e_entry); usize i; Elf64_Phdr program_header; @@ -85,9 +85,11 @@ namespace ELFLoader { if (program_header.p_type == PT_LOAD) { +#ifdef ELF_DEBUG kdbgln("ELF: Loading segment (offset=%zu, base=%#.16lx, filesize=%zu, memsize=%zu)", program_header.p_offset, program_header.p_vaddr, program_header.p_filesz, program_header.p_memsz); +#endif u64 base_vaddr = align_down(program_header.p_vaddr); u64 vaddr_diff = program_header.p_vaddr - base_vaddr; @@ -109,7 +111,7 @@ namespace ELFLoader memset((void*)(program_header.p_vaddr + program_header.p_filesz), 0, program_header.p_memsz - program_header.p_filesz); } - else { kdbgln("ELF: Encountered non-loadable program header, skipping"); } + else { kwarnln("ELF: Encountered non-loadable program header, skipping"); } } return ELFData { elf_header.e_entry }; diff --git a/kernel/src/arch/x86_64/MMU.cpp b/kernel/src/arch/x86_64/MMU.cpp index a5e776c8..a56156f8 100644 --- a/kernel/src/arch/x86_64/MMU.cpp +++ b/kernel/src/arch/x86_64/MMU.cpp @@ -8,9 +8,6 @@ #include #include -#pragma GCC push_options -#pragma GCC diagnostic ignored "-Wconversion" - PageDirectory* g_kernel_directory; u64 g_kernel_directory_virt; @@ -18,6 +15,9 @@ u64 g_kernel_directory_virt; // physical memory at 0xFFFF800000000000. u64 g_physical_mapping_base = 0; +#pragma GCC push_options +#pragma GCC diagnostic ignored "-Wconversion" + void PageTableEntry::set_address(u64 addr) { this->address = (addr >> 12); @@ -207,7 +207,7 @@ namespace MMU if (flags & Flags::ReadWrite) l3.read_write = true; if (flags & Flags::User) l3.user = true; - if (l3.larger_pages) return err(EEXIST); + if (l3.larger_pages) return err(ENOMEM); auto& l2 = l2_entry(l3, virt); if (!l2.present) @@ -225,7 +225,7 @@ namespace MMU if (flags & Flags::ReadWrite) l2.read_write = true; if (flags & Flags::User) l2.user = true; - if (l2.larger_pages) return err(EEXIST); + if (l2.larger_pages) return err(ENOMEM); else if (use_huge_pages == UseHugePages::Yes) { l2.larger_pages = true; @@ -234,7 +234,7 @@ namespace MMU } auto& l1 = l1_entry(l2, virt); - if (l1.present) return err(EEXIST); // Please explicitly unmap the page before mapping it again. + if (l1.present) return err(ENOMEM); set_page_table_entry_properties(l1, phys, flags); return {}; } @@ -296,7 +296,9 @@ namespace MMU g_kernel_directory_virt = translate_physical((u64)g_kernel_directory); +#ifdef MMU_DEBUG kdbgln("MMU init page directory (ring0): virt %#.16lx, phys %p", g_kernel_directory_virt, g_kernel_directory); +#endif } Result create_page_directory_for_userspace() @@ -312,7 +314,9 @@ namespace MMU memcpy(offset_ptr(directory, HALF_PAGE), offset_ptr((PageDirectory*)g_kernel_directory_virt, HALF_PAGE), HALF_PAGE); +#ifdef MMU_DEBUG kdbgln("MMU init page directory (ring3): virt %p, phys %#.16lx", directory, directory_phys); +#endif return (PageDirectory*)directory_phys; } diff --git a/kernel/src/fs/VFS.cpp b/kernel/src/fs/VFS.cpp index b8aa6ca8..05837a3b 100644 --- a/kernel/src/fs/VFS.cpp +++ b/kernel/src/fs/VFS.cpp @@ -44,8 +44,6 @@ namespace VFS auto child_name = TRY(parser.basename()); - // kdbgln("vfs: creating directory '%s' in parent '%s'", child_name.chars(), parent_path.chars()); - return parent_inode->create_subdirectory(child_name.chars()); } @@ -60,8 +58,6 @@ namespace VFS auto child_name = TRY(parser.basename()); - // kdbgln("vfs: creating file '%s' in parent '%s'", child_name.chars(), parent_path.chars()); - return parent_inode->create_file(child_name.chars()); } diff --git a/kernel/src/fs/devices/DeviceRegistry.cpp b/kernel/src/fs/devices/DeviceRegistry.cpp index 059376b5..47ecc182 100644 --- a/kernel/src/fs/devices/DeviceRegistry.cpp +++ b/kernel/src/fs/devices/DeviceRegistry.cpp @@ -33,7 +33,7 @@ namespace DeviceRegistry if (descriptor.major == major && descriptor.minor == minor) return err(EEXIST); } - kdbgln("device manager: registered new device type %u:%u", major, minor); + kdbgln("DeviceRegistry: registered new device type %u:%u", major, minor); return g_available_devices.try_append( DeviceDescriptor { .initializer = initializer, .major = major, .minor = minor }); diff --git a/kernel/src/main.cpp b/kernel/src/main.cpp index 9454e537..a8357ca4 100644 --- a/kernel/src/main.cpp +++ b/kernel/src/main.cpp @@ -92,7 +92,6 @@ static u64 allocate_initial_kernel_stack() // First page is a guard page, the rest is stack. MMU::unmap(address); // Unmap (without deallocating VM) one guard page so that attempts to access it fail with a // non-present page fault. - kdbgln("stack guard page: %p", (void*)address); // The actual stack. Stack stack { address + ARCH_PAGE_SIZE, BOOTSTRAP_STACK_PAGES * ARCH_PAGE_SIZE }; diff --git a/kernel/src/memory/UserVM.cpp b/kernel/src/memory/UserVM.cpp index 49c45acf..a28c0772 100644 --- a/kernel/src/memory/UserVM.cpp +++ b/kernel/src/memory/UserVM.cpp @@ -40,7 +40,6 @@ Result> UserVM::clone() UserVM::UserVM(void* base, usize size) { - kdbgln("user vm created with base=%p, size=%zu", base, size); m_bitmap.initialize(base, size); m_bitmap.clear(false); } @@ -57,8 +56,6 @@ Result UserVM::try_expand(usize size) m_bitmap.resize(new_size); m_bitmap.clear_region(old_size * 8, (new_size - old_size) * 8, false); - kdbgln("user vm expanded to base=%p, size=%zu", m_bitmap.location(), new_size); - return true; } @@ -121,6 +118,5 @@ Result UserVM::free_several_pages(u64 address, usize count) UserVM::~UserVM() { - kdbgln("user vm destroyed: base=%p, size=%zu", m_bitmap.location(), m_bitmap.size_in_bytes()); m_bitmap.deallocate(); } diff --git a/kernel/src/sys/exec.cpp b/kernel/src/sys/exec.cpp index de302d1a..cfa9e248 100644 --- a/kernel/src/sys/exec.cpp +++ b/kernel/src/sys/exec.cpp @@ -51,13 +51,9 @@ Result sys_execve(Registers* regs, SyscallArgs args) auto image = TRY(ThreadImage::try_load_from_elf(inode)); - kdbgln("exec: copying argv to image memory (argc = %zu)", argv.size()); - u64 user_argv = TRY(image->push_string_vector_on_stack(argv)); usize user_argc = argv.size(); - kdbgln("exec: copying envp to image memory (envc = %zu)", envp.size()); - u64 user_envp = TRY(image->push_string_vector_on_stack(envp)); usize user_envc = envp.size(); @@ -89,8 +85,6 @@ Result sys_execve(Registers* regs, SyscallArgs args) memcpy(regs, ¤t->regs, sizeof(*regs)); - kinfoln("exec: done"); - return 0; } @@ -100,8 +94,6 @@ Result sys_fork(Registers* regs, SyscallArgs) auto guard = make_scope_guard([current] { MMU::switch_page_directory(current->directory); }); - kinfoln("fork: trying to duplicate process %lu", current->id); - memcpy(¤t->regs, regs, sizeof(*regs)); auto current_directory_path = TRY(current->current_directory_path.clone()); @@ -129,5 +121,7 @@ Result sys_fork(Registers* regs, SyscallArgs) Scheduler::add_thread(thread); + kinfoln("fork: thread %lu forked into child %lu", current->id, thread->id); + return thread->id; } diff --git a/kernel/src/sys/mkdir.cpp b/kernel/src/sys/mkdir.cpp index dccdd3f6..19b5fbb1 100644 --- a/kernel/src/sys/mkdir.cpp +++ b/kernel/src/sys/mkdir.cpp @@ -11,8 +11,6 @@ Result sys_mkdir(Registers*, SyscallArgs args) Thread* current = Scheduler::current(); - kinfoln("mkdir: attempting to create %s", path.chars()); - auto inode = TRY(VFS::create_directory(path.chars(), current->auth, current->current_directory)); inode->chmod(mode); inode->chown(current->auth.euid, current->auth.egid); diff --git a/kernel/src/sys/mknod.cpp b/kernel/src/sys/mknod.cpp index a94e84e3..9cdf8d64 100644 --- a/kernel/src/sys/mknod.cpp +++ b/kernel/src/sys/mknod.cpp @@ -17,8 +17,6 @@ Result sys_mknod(Registers*, SyscallArgs args) u32 maj = luna_dev_major(dev); u32 min = luna_dev_minor(dev); - kdbgln("mknod: attempting to create device %u:%u in path %s", maj, min, path.chars()); - auto parser = TRY(PathParser::create(path.chars())); auto dirname = TRY(parser.dirname()); diff --git a/kernel/src/sys/mmap.cpp b/kernel/src/sys/mmap.cpp index 7eb9b6a7..3992ab66 100644 --- a/kernel/src/sys/mmap.cpp +++ b/kernel/src/sys/mmap.cpp @@ -49,7 +49,9 @@ Result sys_mmap(Registers*, SyscallArgs args) if (prot & PROT_EXEC) mmu_flags &= ~MMU::NoExecute; if (prot == PROT_NONE) mmu_flags = MMU::NoExecute; +#ifdef MMAP_DEBUG kdbgln("mmap: mapping memory at %#lx, size=%zu", address, len); +#endif return MemoryManager::alloc_at(address, get_blocks_from_size(len, ARCH_PAGE_SIZE), mmu_flags); } @@ -69,7 +71,9 @@ Result sys_munmap(Registers*, SyscallArgs args) // POSIX says munmap should silently do nothing if the memory was not already mapped. if (!ok) return 0; +#ifdef MMAP_DEBUG kdbgln("munmap: unmapping memory at %#lx, size=%zu", address, size); +#endif TRY(MemoryManager::unmap_owned(address, get_blocks_from_size(size, ARCH_PAGE_SIZE))); diff --git a/kernel/src/sys/open.cpp b/kernel/src/sys/open.cpp index 54f545eb..b21f19d6 100644 --- a/kernel/src/sys/open.cpp +++ b/kernel/src/sys/open.cpp @@ -17,8 +17,6 @@ Result sys_open(Registers*, SyscallArgs args) Thread* current = Scheduler::current(); - kinfoln("open: trying to open file %s, flags %d", path.chars(), flags); - SharedPtr inode; // Caller did not pass either O_RDONLY, O_WRONLY or O_RDWR @@ -54,7 +52,7 @@ Result sys_open(Registers*, SyscallArgs args) current->fd_table[fd] = FileDescriptor { inode, 0, flags & FLAGS_TO_KEEP }; - kinfoln("open: allocated file descriptor %d for inode %zu", fd, inode->inode_number()); + kinfoln("open: opening file %s, flags %d, mode %#o = fd %d", path.chars(), flags, mode, fd); return (u64)fd; } @@ -70,8 +68,6 @@ Result sys_close(Registers*, SyscallArgs args) if (!descriptor.has_value()) return err(EBADF); - kinfoln("close: closing file descriptor %d (was referencing inode %zu)", fd, descriptor->inode->inode_number()); - descriptor = {}; return 0; diff --git a/kernel/src/thread/Scheduler.cpp b/kernel/src/thread/Scheduler.cpp index 7f6089b4..50a5d70a 100644 --- a/kernel/src/thread/Scheduler.cpp +++ b/kernel/src/thread/Scheduler.cpp @@ -157,15 +157,11 @@ namespace Scheduler if (thread->is_kernel) { auto stack = thread->stack; - // FIXME: Propagate errors I guess? - kinfoln("deleting stack @ %#lx", stack.bottom()); MemoryManager::unmap_owned_and_free_vm(stack.bottom(), stack.bytes() / ARCH_PAGE_SIZE).release_value(); } else { auto stack = thread->kernel_stack; - kinfoln("deleting kstack @ %#lx", stack.bottom()); - // FIXME: Propagate errors I guess? MemoryManager::unmap_owned_and_free_vm(stack.bottom(), stack.bytes() / ARCH_PAGE_SIZE).release_value(); }