From e57528f46f949e98570e7298e96e59c256221560 Mon Sep 17 00:00:00 2001 From: John Ousterhout Date: Thu, 17 Dec 2015 09:59:33 -0800 Subject: [PATCH] Rubric updates from Winter 2013 CS 140 at Stanford --- ta-advice/hw1.txt | 28 ++++++++++++++++++++++++---- ta-advice/hw2.txt | 17 +++++++++++++++-- ta-advice/hw3.txt | 11 +++++++++++ 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/ta-advice/hw1.txt b/ta-advice/hw1.txt index 28c30bf..ff44b71 100644 --- a/ta-advice/hw1.txt +++ b/ta-advice/hw1.txt @@ -9,8 +9,11 @@ OVERALL -10 Numerous capitalization, punctuation, spelling, or grammar errors DESIGN + -5 Gratuitous use of malloc() -10 Failure to check return value of malloc() -10 Use of ASSERT to check something that can actually fail, e.g. malloc() + -2: ASSERT() on user provided input (thread niceness) + -2: Re-enabling interrupts instead of returning them to their old value CODING STYLE -10 Inconsistent or bad coding style: no indentation, cramming @@ -36,8 +39,11 @@ PROBLEM 1: ALARM CLOCK -2 A1: Forgot to include some declarations [which] -5 A2: Missing/non-responsive/too long/too short -5 A3: Missing/non-responsive/too long/too short + -5 A3: "non-responsive" if only mentioned micro optimizations, e.g. not + double checking conditions -5 A4: Missing/non-responsive/too long/too short -5 A5: Missing/non-responsive/too long/too short + -5 A5: correct mechanism but incorrect reasoning -5 A6: Missing/non-responsive/too long/too short -3 A6: "Straw man"--comparing correct design to an incorrect one -10 Claim or implication that list operations are atomic @@ -47,15 +53,23 @@ PROBLEM 1: ALARM CLOCK -10 Interrupt handler always examines or modifies every waiting thread -5 Race between list modification and interrupt handler -10 A timer tick that occurs during list modification delays waking - threads until the next timer tick + threads until the next timer tick -10 Race between two threads modifying list -10 Wakes only one thread even if multiple are finished sleeping -15 Malfunctions (e.g. by busy waiting or not waiting the full time), - even in corner case (e.g. when malloc() returns NULL) + even in corner case (e.g. when malloc() returns NULL) -15 Fixed limit on number of threads that may sleep - -5 Uses thread_block() instead of higher-level synchronization primitives -5 Disables interrupts unnecessarily -5 Unnecessary or redundant synchronization in timer_sleep() + -5 Implementation split across timer.c and thread.[ch] + -2 Doesn’t check thread priorities when waking threads that cause + scheduling changes + -3 Checks for wakeups in ‘schedule’ and not ‘timer_tick/thread_tick’. + Hence there are a lot of spurious checks each time schedule is called. + This is probably why the tests passed too since the checks were too + frequent anyway. + -2 Interrupts turned off and on instead of storing the current level + and restoring the old level. Total deductions (capped at -30): XXX @@ -89,11 +103,14 @@ PROBLEM 2: PRIORITY SCHEDULING -15 Grossly simplified design egregiously limits generality, e.g. small, fixed limit on total number of donations, donees, donor locks, etc. -5 Global list of donations is unnecessary and inefficient + -5 Donated priority is more than priority of donor -3 sema_up() yields regardless of whether a higher-priority thread was unblocked -5 sema_up() yields even when it does not unblock a thread -8 Race in lock_acquire() between priority donation and "down"ing sema -8 Race in lock_release() between release of donated pri and "up"ing sema + -10 Races in priority management code in general, i.e. no protection + when walking/updating priority graph Total deductions (capped at -30): XXX @@ -123,10 +140,13 @@ PROBLEM 3: ADVANCED SCHEDULER once per second is unreadable -5 Code to update load average, recent_cpu, and thread priorities once per second is difficult to read - -5 Wastefully recalculates every thread's priority every 4th timer tick + -0 Wastefully recalculates every thread's priority every 4th timer tick -5 Race against timer interrupt in thread_get_recent_cpu() or thread_get_load_avg() + -5 Failing to recompute current thread’s priority every 4 ticks + Total deductions (capped at -30): XXX Total score (100 - deductions): YYY + diff --git a/ta-advice/hw2.txt b/ta-advice/hw2.txt index b0ed143..bd45909 100644 --- a/ta-advice/hw2.txt +++ b/ta-advice/hw2.txt @@ -11,6 +11,8 @@ OVERALL DESIGN -10 Failure to check return value of malloc() -10 Use of ASSERT to check something that can actually fail, e.g. malloc() + -10 Fails to free all resources during normal execution [specify where] + -5 Fails to free all resources when handling errors [specify where] CODING STYLE -10 Inconsistent or bad coding style: no indentation, cramming @@ -36,7 +38,7 @@ PROBLEM 1: ARGUMENT PASSING -10 A2: Missing/non-responsive/too long/too short -10 A3: Missing/non-responsive/too long/too short -10 A3: Claims that strtok_r() does not modify the string it parses - -10 A3: Claims that strtok() uses a static buffer for parsing + -0 A3: Claims that strtok() uses a static buffer for parsing -10 A4: Missing/non-responsive/too long/too short -5 A4: Claims time or space advantage for user-space parsing -5 A4: Claims that the shell implements relative paths @@ -47,11 +49,16 @@ PROBLEM 1: ARGUMENT PASSING -30 Not implemented -10 Uses global variables, locks, etc. -15 Doesn't check for stack overflow + -5 Stack overflow checking incomplete/buggy -10 Assumes any command line under N bytes long will fit in a page [change N to the value assumed, where N >= 1,366] -10 Assumes that the command line is capped at 128 bytes -1 Assumes that individual strings must be word-aligned for performance -10 Argument passing code is difficult to read or poorly abstracted + -10 Uses buffer on stack to copy command, possibly overflowing the + kernel stack + -10 Uses buffer on stack to hold pointers to each argument, possibly + overflowing the kernel stack Total deductions (capped at -30): XXX @@ -68,7 +75,7 @@ PROBLEM 2: SYSTEM CALLS -3 B1: Omitted data for "wait" system call -3 B1: Omitted data for "exec" to wait for process loading to complete -6 B2: Missing/non-responsive/too long/too short - -2 B2: Claims that Pintos has stderr file descriptor + -0 B2: Claims that Pintos has stderr file descriptor -6 B3: Missing/non-responsive/too long/too short -3 B3: Claims that struct intr_frame's eax member is on user stack -6 B4: Missing/non-responsive/too long/too short @@ -84,6 +91,9 @@ PROBLEM 2: SYSTEM CALLS -3 B5+B8: "wait" always returns -1 if child has already exited -3 B5+B8: "wait" or "exit" searches a global list of all processes -6 B5+B8: "wait" or "exit" has race condition [which] + -3 B5+B8: each additional, independent race condition for exec/wait/exit. + (e.g. unsynchronized global list access vs. race in setting return + values vs. race for cleanup of shared state) -6 B6: Missing/non-responsive/too long/too short -6 B7: Missing/non-responsive/too long/too short -3 B7: Calls thread_block() directly (use semaphores instead) @@ -97,6 +107,8 @@ PROBLEM 2: SYSTEM CALLS -60 Not implemented -5 Global counter for file descriptors lacks synchronization -10 Global table of file descriptors lacks synchronization + -5 Global table of file descriptors has synchronization bugs + -10 No external synchronization in handling filesystem calls. -5 Global table allows multiple processes to access same file descriptor -5 Added big array (>= 512 bytes) to struct thread without justifying -10 Added big array (>= 1024 bytes) to struct thread @@ -104,6 +116,7 @@ PROBLEM 2: SYSTEM CALLS -10 System call handler is poorly abstracted or unreadable -5 Failed to update comment on process_wait() function after implementing -5 "open" system call fails to release all resources in error cases + -10 strlen before (in the call to) validate a null terminated string Total deductions (capped at -60): XXX diff --git a/ta-advice/hw3.txt b/ta-advice/hw3.txt index 59f195e..0426bca 100644 --- a/ta-advice/hw3.txt +++ b/ta-advice/hw3.txt @@ -11,6 +11,8 @@ OVERALL DESIGN -10 Failure to check return value of malloc() -10 Use of ASSERT to check something that can actually fail, e.g. malloc() + -6 First race condition [specify where] + -3 For each additional race conditiona [specify where] CODING STYLE -10 Inconsistent or bad coding style: no indentation, cramming @@ -45,6 +47,8 @@ PROBLEM 1: PAGE TABLE MANAGEMENT DESIGN -25 Not implemented -10 Page dirty status must be determined accurately, not approximately + -10 SPT implemented with a list (per page) + -10 Per-process supplementary page table not protected Total deductions (capped at -25): XXX @@ -61,6 +65,7 @@ PROBLEM 2: PAGING TO AND FROM DISK -5 B4: Missing/non-responsive/too long/too short -5 B4: Heuristic does not make sense -2 B4: Claim that "struct intr_frame" is on user stack + -2 B4: Forgot to mention stack limit heuristic -5 B5: Missing/non-responsive/too long/too short -5 B5: Claim that deadlock is "rare" or has a "short" window -5 B6: Missing/non-responsive/too long/too short @@ -79,8 +84,13 @@ PROBLEM 2: PAGING TO AND FROM DISK -45 Not implemented -5 Clock algorithm implementation literally uses a timer -10 Clock algorithm implementation resets dirty bits + -5 Eagerly writes dirty pages to disk when running clock algorithm -5 Stack fault heuristic allows more than 32 bytes below stack pointer -3 Stack limit is smaller than 1 MB + -20 Globally locked and all page faults serialized + -10 Holds per process single SPT lock during the entire length of I/O + -5 Does not clear the swap slot after bringing the page back in memory. + (Could potentially run out of swap slots) Total deductions (capped at -45): XXX @@ -106,5 +116,6 @@ PROBLEM 3: MEMORY-MAPPED FILES -5 Global table of file mappings lacks synchronization -5 Global table allows multiple processes to access same file mapping -5 Superfluous locking on per-thread data structure [which] + -5 Fails to release resources in normal / failure case [specify where] Total deductions (capped at -20): XXX -- 2.30.2