X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=blobdiff_plain;f=TODO;h=5bf19c1d3c94176bbc9e523b0538e12b893fa4ce;hb=24116bb51c88e33ca0274bdd01abb7f03ec90fb7;hp=85bd51a0a73484bc5c9940c88fb705bfbbec40c6;hpb=3389cbb515f92b56550c41b1ca0bda9d4a6fbf9e;p=pintos-anon diff --git a/TODO b/TODO index 85bd51a..5bf19c1 100644 --- a/TODO +++ b/TODO @@ -1,5 +1,175 @@ -*- text -*- +* Bochs is not fully reproducible. + +Godmar says: + +- In Project 2, we're missing tests that pass arguments to system calls +that span multiple pages, where some are mapped and some are not. +An implementation that only checks the first page, rather than all pages +that can be touched during a call to read()/write() passes all tests. + +- In Project 2, we're missing a test that would fail if they assumed +that contiguous user-virtual addresses are laid out contiguously +in memory. The loading code should ensure that non-contiguous +physical pages are allocated for the data segment (at least.) + +- Need some tests that test that illegal accesses lead to process +termination. I have written some, will add them. In P2, obviously, +this would require that the students break this functionality since +the page directory is initialized for them, still it would be good +to have. + +- There does not appear to be a test that checks that they close all +fd's on exit. Idea: add statistics & self-diagnostics code to palloc.c +and malloc.c. Self-diagnostics code could be used for debugging. +The statistics code would report how much kernel memory is free. +Add a system call "get_kernel_memory_information". User programs +could engage in a variety of activities and notice leaks by checking +the kernel memory statistics. + +--- + +From: "Godmar Back" +Subject: priority donation tests +To: "Ben Pfaff" +Date: Fri, 3 Mar 2006 11:02:08 -0500 + +Ben, + +it seems the priority donation tests are somewhat incomplete and allow +incorrect implementations to pass with a perfect score. + +We are seeing the following wrong implementations pass all tests: + +- Implementations that assume locks are released in the opposite order +in which they're acquired. The students implement this by +popping/pushing on the donation list. + +- Implementations that assume that the priority of a thread waiting on +a semaphore or condition variable cannot change between when the +thread was blocked and when it is unblocked. The students implement +this by doing an insert into an ordered list on block, rather than +picking the maximum thread on unblock. + +Neither of these two cases is detected; do you currently check for +these mistakes manually? + +I wrote a test that checks for the first case; it is here: +http://people.cs.vt.edu/~gback/pintos/priority-donate-multiple-2.patch + +[...] + +I also wrote a test case for the second scenario: +http://people.cs.vt.edu/~gback/pintos/priority-donate-sema.c +http://people.cs.vt.edu/~gback/pintos/priority-donate-sema.ck + +I put the other tests up here: +http://people.cs.vt.edu/~gback/pintos/priority-donate-multiple2.c +http://people.cs.vt.edu/~gback/pintos/priority-donate-multiple2.ck + +From: "Godmar Back" +Subject: multiple threads waking up at same clock tick +To: "Ben Pfaff" +Date: Wed, 1 Mar 2006 08:14:47 -0500 + +Greg Benson points out another potential TODO item for P1. + +---- +One thing I recall: + +The alarm tests do not test to see if multiple threads are woken up if +their timers have expired. That is, students can write a solution +that just wakes up the first thread on the sleep queue rather than +check for additional threads. Of course, the next thread will be +woken up on the next tick. Also, this might be hard to test. + +--- +Way to test this: (from Godmar Back) + +Thread A with high priority spins until 'ticks' changes, then calls to +timer_sleep(X), Thread B with lower priority is then resumed, calls +set_priority to make its priority equal to that of thread A, then +calls timer_sleep(X), all of that before the next clock interrupt +arrives. + +On wakeup, each thread records wake-up time and calls yield +immediately, forcing the scheduler to switch to the other +equal-priority thread. Both wake-up times must be the same (and match +the planned wake-up time.) + +PS: +I actually tested it and it's hard to pass with the current ips setting. +The bounds on how quickly a thread would need to be able to return after +sleep appear too tight. Need another idea. + +--- +From: "Waqar Mohsin" +Subject: 3 questions about switch_threads() in switch.S +To: blp@cs.stanford.edu, joshwise@stanford.edu +Date: Fri, 3 Mar 2006 17:09:21 -0800 + +QUESTION 1 + +In the section + + # Save current stack pointer to old thread's stack, if any. + movl SWITCH_CUR(%esp), %eax + test %eax, %eax + jz 1f + movl %esp, (%eax,%edx,1) +1: + + # Restore stack pointer from new thread's stack. + movl SWITCH_NEXT(%esp), %ecx + movl (%ecx,%edx,1), %esp + +why are we saving the current stack pointer only if the "cur" thread pointer +is non-NULL ? Isn't it gauranteed to be non-NULL because switch_threads() is +only called form schedule(), where we have + + struct thread *cur = running_thread (); + +which should always be non-NULL (given the way kernel pool is laid out). + +QUESTION 2 + + # This stack frame must match the one set up by thread_create(). + pushl %ebx + pushl %ebp + pushl %esi + pushl %edi + +I find the comment confusing. thread_create() is a special case: the set of +registers popped from switch_threads stack frame for a newly created thread +are all zero, so their order shouldn't dictate the order above. + +I think all that matters is that the order of pops at the end of +switch_threads() is the opposite of the pushes at the beginning (as shown +above). + +QUESTION 3 + +Is it true that struct switch_threads_frame does NOT strictly require + + struct thread *cur; /* 20: switch_threads()'s CUR argument. */ + struct thread *next; /* 24: switch_threads()'s NEXT argument. */ +at the end ? + +When a newly created thread's stack pointer is installed in switch_threads(), +all we do is pop the saved registers and return to switch_entry() which pops +off and discards the above two simulated (and not used) arguments to +switch_threads(). + +If we remove these two from struct switch_threads_frame and don't do a + + # Discard switch_threads() arguments. + addl $8, %esp +in switch_entry(), things should still work. Am I right ? + +Thanks +Waqar + From: "Godmar Back" Subject: thread_yield in irq handler To: "Ben Pfaff" @@ -152,6 +322,11 @@ via Godmar Back: * lock_acquire(), lock_release() don't need additional intr_dis/enable calls, because the semaphore protects lock->holder. + [ Think this over: is this really true when priority donation is + implemented? intr_dis/enable prevents the race with thread_set_priority. + Leaving it there could help the students getting the correct synchronization + right. + ]