pintos-os.org Git - pintos-anon/blob - ta-advice/hw1.txt

   1 HW1 DESIGN SUMMARY
   2 ==================
   3
   4 OVERALL
   5
   6   DOCUMENTATION
   7     -20 Egregious violation of style versus design document example
   8     -10 Numerous excessively long lines (> 79 characters)
   9     -10 Numerous capitalization, punctuation, spelling, or grammar errors
  10
  11   DESIGN
  12      -5 Gratuitous use of malloc()
  13     -10 Failure to check return value of malloc()
  14     -10 Use of ASSERT to check something that can actually fail, e.g. malloc()
  15     -2: ASSERT() on user provided input (thread niceness)
  16     -2: Re-enabling interrupts instead of returning them to their old value
  17
  18   CODING STYLE
  19     -10 Inconsistent or bad coding style: no indentation, cramming
  20         many statements into one line, other issues at TA's discretion
  21     -10 Numerous very long source code lines (> 100 characters)
  22     -10 Commented-out or #if'd out code makes real code hard to read
  23     -10 Many missing or uninformative comments on structures, structure
  24         members, global or static variables, or function definitions
  25     -10 Function(s) should be decomposed for clarity [indicate function]
  26     -10 Cut-and-pasted code should be made into function [indicate where]
  27     -10 Uninformative or deceptive identifiers
  28
  29 Total deductions (capped at -40): XXX
  30
  31 PROBLEM 1: ALARM CLOCK
  32
  33   DOCUMENTATION
  34     -20 Grossly inaccurate: documentation has no resemblance to code
  35     -10 Important inaccuracies: documentation and code differ significantly
  36         [indicate how]
  37      -5 Minor inaccuracies: documentation and code differ [indicate how]
  38      -5 A1: Missing entirely/missing comments or purpose/too long
  39      -2 A1: Forgot to include some declarations [which]
  40      -5 A2: Missing/non-responsive/too long/too short
  41      -5 A3: Missing/non-responsive/too long/too short
  42      -5 A3: "non-responsive" if only mentioned micro optimizations, e.g. not
  43         double checking conditions
  44      -5 A4: Missing/non-responsive/too long/too short
  45      -5 A5: Missing/non-responsive/too long/too short
  46      -5 A5: correct mechanism but incorrect reasoning
  47      -5 A6: Missing/non-responsive/too long/too short
  48      -3 A6: "Straw man"--comparing correct design to an incorrect one
  49     -10 Claim or implication that list operations are atomic
  50
  51   DESIGN
  52     -30 Not implemented
  53     -10 Interrupt handler always examines or modifies every waiting thread
  54      -5 Race between list modification and interrupt handler
  55     -10 A timer tick that occurs during list modification delays waking
  56         threads until the next timer tick
  57     -10 Race between two threads modifying list
  58     -10 Wakes only one thread even if multiple are finished sleeping
  59     -15 Malfunctions (e.g. by busy waiting or not waiting the full time),
  60         even in corner case (e.g. when malloc() returns NULL)
  61     -15 Fixed limit on number of threads that may sleep
  62      -5 Disables interrupts unnecessarily
  63      -5 Unnecessary or redundant synchronization in timer_sleep()
  64      -5 Implementation split across timer.c and thread.[ch]
  65      -2 Doesn\92t check thread priorities when waking threads that cause
  66         scheduling changes
  67      -3 Checks for wakeups in \91schedule\92 and not \91timer_tick/thread_tick\92.
  68         Hence there are a lot of spurious checks each time schedule is called.
  69         This is probably why the tests passed too since the checks were too
  70         frequent anyway.
  71      -2 Interrupts turned off and on instead of storing the current level
  72         and restoring the old level.
  73
  74 Total deductions (capped at -30): XXX
  75
  76 PROBLEM 2: PRIORITY SCHEDULING
  77
  78   DOCUMENTATION
  79     -20 Grossly inaccurate: documentation has no resemblance to code
  80     -10 Important inaccuracies: documentation and code differ significantly
  81         [indicate how]
  82      -5 Minor inaccuracies: documentation and code differ [indicate how]
  83      -5 B1: Missing entirely/missing comments or purpose/too long
  84      -2 B1: Forgot to include some declarations [which]
  85      -5 B2: Missing/non-responsive/too long/too short
  86      -3 B2: Diagram is difficult to follow
  87      -3 B2: Diagram is not specific to the chosen design
  88      -3 B2: Didn't include explanatory text, just a diagram
  89      -3 B2: Didn't include diagram, just explanatory text
  90      -5 B3: Missing/non-responsive/too long/too short
  91      -3 B3: Didn't explain semaphores
  92      -3 B3: Didn't explain locks
  93      -3 B3: Didn't explain condition variables
  94      -5 B4: Missing/non-responsive/too long/too short
  95      -5 B5: Missing/non-responsive/too long/too short
  96      -5 B6: Missing/non-responsive/too long/too short
  97      -5 B7: Missing/non-responsive/too long/too short
  98      -3 B7: "Straw man"--comparing correct design to an incorrect one
  99
 100   DESIGN
 101     -30 Not implemented
 102     -15 Malfunctions in corner case (e.g. when malloc() returns NULL)
 103     -15 Grossly simplified design egregiously limits generality, e.g. small,
 104         fixed limit on total number of donations, donees, donor locks, etc.
 105      -5 Global list of donations is unnecessary and inefficient
 106      -5 Donated priority is more than priority of donor
 107      -3 sema_up() yields regardless of whether a higher-priority
 108         thread was unblocked
 109      -5 sema_up() yields even when it does not unblock a thread
 110      -8 Race in lock_acquire() between priority donation and "down"ing sema
 111      -8 Race in lock_release() between release of donated pri and "up"ing sema
 112     -10 Races in priority management code in general, i.e. no protection
 113         when walking/updating priority graph
 114
 115 Total deductions (capped at -30): XXX
 116
 117 PROBLEM 3: ADVANCED SCHEDULER
 118
 119   DOCUMENTATION
 120     -20 Grossly inaccurate: documentation has no resemblance to code
 121     -10 Important inaccuracies: documentation and code differ significantly
 122         [indicate how]
 123      -5 Minor inaccuracies: documentation and code differ [indicate how]
 124      -5 C1: Missing entirely/missing comments or purpose/too long
 125      -2 C1: Forgot to include some declarations [which]
 126      -5 C2: Missing/non-responsive/too long/too short
 127      -2 C2: Minor mistakes in table
 128      -5 C2: Major mistakes in table
 129      -5 C3: Missing/non-responsive/too long/too short
 130      -5 C4: Missing/non-responsive/too long/too short
 131      -5 C5: Missing/non-responsive/too long/too short
 132      -3 C5: Did not mention advantage of submitted design
 133      -3 C5: Did not mention disadvantage of submitted design
 134      -5 C6: Missing/non-responsive/too long/too short
 135      -5 C6: Did not properly justify failure to abstract fixed-point math
 136
 137   DESIGN
 138     -30 Not implemented
 139     -10 Code to update load average, recent_cpu, and thread priorities
 140         once per second is unreadable
 141      -5 Code to update load average, recent_cpu, and thread priorities
 142         once per second is difficult to read
 143      -0 Wastefully recalculates every thread's priority every 4th timer tick
 144      -5 Race against timer interrupt in thread_get_recent_cpu()
 145         or thread_get_load_avg()
 146      -5 Failing to recompute current thread\92s priority every 4 ticks
 147
 148
 149 Total deductions (capped at -30): XXX
 150
 151 Total score (100 - deductions): YYY
 152