-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
-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
-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\92t check thread priorities when waking threads that cause
+ scheduling changes
+ -3 Checks for wakeups in \91schedule\92 and not \91timer_tick/thread_tick\92.
+ 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
-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
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\92s priority every 4 ticks
+
Total deductions (capped at -30): XXX
Total score (100 - deductions): YYY
+
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
-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
-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
-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
-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)
-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
-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
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
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
-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
-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
-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
projects / pintos-anon / commitdiff