From dc271293321d3eabe5dcf7cd33a01aa3b790d477 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Thu, 3 Jul 2008 20:37:08 +0000 Subject: [PATCH] stippi + bonefish: * Missing backslash in UPDATE_THREAD_KERNEL_TIME() macro. Caused the thread's kernel time never to be updated when the thread left the kernel. The time would be added to the user time instead. * Changed semantics of thread_at_kernel_exit(). It leaves interrupts disabled when returning, now. All code using it would disable interrupts afterwards anyway, and inbetween the thread could be interrupted causing two more time updates (or actually not causing them erroneously). * The code in x86's interrupt handler entry (int_bottom) also checks thread::in_kernel now, to determine whether the thread was already considered to be in userland. This is necessary since the sysexit instruction doesn't reenable interrupts, so that we have to do that with the preceeding instruction. Hence an interrupt can occur inbetween, which would incorrectly add the interrupt's kernel time to the thread's user time. * For userland threads team_create_thread_start() didn't invoke thread_at_kernel_exit() or any equivalent. We do that in arch_thread_enter_userspace() now. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26240 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/system/kernel/arch/x86/arch_interrupts.S | 16 ++++++++++++---- src/system/kernel/arch/x86/arch_thread.cpp | 3 ++- src/system/kernel/thread.cpp | 11 ++++------- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/system/kernel/arch/x86/arch_interrupts.S b/src/system/kernel/arch/x86/arch_interrupts.S index d63ecbbb52..5842b603c6 100644 --- a/src/system/kernel/arch/x86/arch_interrupts.S +++ b/src/system/kernel/arch/x86/arch_interrupts.S @@ -53,7 +53,7 @@ call system_time; \ \ movl %eax, %ebx; /* save for later */ \ - movl %edx, %ecx; + movl %edx, %ecx; \ \ /* thread->kernel_time += now - thread->last_time; */ \ sub THREAD_last_time(%edi), %eax; \ @@ -248,7 +248,16 @@ int_bottom: testl $0x20000, IFRAME_flags(%ebp) // VM86 mode jnz int_bottom_vm86 - cmp $USER_CODE_SEG, IFRAME_cs(%ebp) + cmp $USER_CODE_SEG, IFRAME_cs(%ebp) // user mode + je int_bottom_user + + // We need to recheck user mode using the thread's in_kernel flag, since + // sysexit introduces a raced condition: It doesn't reenable interrupts, + // so that we have to do it in the instruction before, thus opening a + // window for an interrupt while still being in the kernel, but having set + // up everything for userland already. + movl %dr3, %edi // thread pointer + cmpb $0, THREAD_in_kernel(%edi) je int_bottom_user // disable interrupts -- the handler will enable them, if necessary @@ -474,8 +483,7 @@ handle_syscall: // make sure interrupts are enabled (they are, when coming from a syscall // but otherwise they might be disabled) sti - call thread_at_kernel_exit - cli // disable interrupts + call thread_at_kernel_exit // also disables interrupts jmp kernel_exit_work_done bad_syscall_params: diff --git a/src/system/kernel/arch/x86/arch_thread.cpp b/src/system/kernel/arch/x86/arch_thread.cpp index 7e528acd5a..40ded08709 100644 --- a/src/system/kernel/arch/x86/arch_thread.cpp +++ b/src/system/kernel/arch/x86/arch_thread.cpp @@ -409,7 +409,8 @@ arch_thread_enter_userspace(struct thread *t, addr_t entry, void *args1, if (user_memcpy((void *)stackTop, args, sizeof(args)) < B_OK) return B_BAD_ADDRESS; - disable_interrupts(); + thread_at_kernel_exit(); + // also disables interrupts i386_set_tss_and_kstack(t->kernel_stack_top); diff --git a/src/system/kernel/thread.cpp b/src/system/kernel/thread.cpp index 4e1cb2195e..67e95a350c 100644 --- a/src/system/kernel/thread.cpp +++ b/src/system/kernel/thread.cpp @@ -331,9 +331,6 @@ _create_user_thread_kentry(void) { struct thread *thread = thread_get_current_thread(); - // a signal may have been delivered here - thread_at_kernel_exit(); - // jump to the entry point in user space arch_thread_enter_userspace(thread, (addr_t)thread->entry, thread->args1, thread->args2); @@ -1571,6 +1568,8 @@ thread_at_kernel_entry(bigtime_t now) /*! Called whenever a thread exits kernel space to user space. Tracks time, handles signals, ... + Interrupts must be enabled. When the function returns, interrupts will be + disabled. */ void thread_at_kernel_exit(void) @@ -1584,7 +1583,7 @@ thread_at_kernel_exit(void) scheduler_reschedule(); } - cpu_status state = disable_interrupts(); + disable_interrupts(); thread->in_kernel = false; @@ -1592,14 +1591,12 @@ thread_at_kernel_exit(void) bigtime_t now = system_time(); thread->kernel_time += now - thread->last_time; thread->last_time = now; - - restore_interrupts(state); } /*! The quick version of thread_kernel_exit(), in case no signals are pending and no debugging shall be done. - Interrupts are disabled in this case. + Interrupts must be disabled. */ void thread_at_kernel_exit_no_signals(void)