1This file lists subtle things that might not be commented 2as well as they should be in the source code and that 3might be worth pointing out in a longer explanation or in class. 4 5--- 6 7[2009/07/12: No longer relevant; forkret1 changed 8and this is now cleaner.] 9 10forkret1 in trapasm.S is called with a tf argument. 11In order to use it, forkret1 copies the tf pointer into 12%esp and then jumps to trapret, which pops the 13register state out of the trap frame. If an interrupt 14came in between the mov tf, %esp and the iret that 15goes back out to user space, the interrupt stack frame 16would end up scribbling over the tf and whatever memory 17lay under it. 18 19Why is this safe? Because forkret1 is only called 20the first time a process returns to user space, and 21at that point, cp->tf is set to point to a trap frame 22constructed at the top of cp's kernel stack. So tf 23*is* a valid %esp that can hold interrupt state. 24 25If other tf's were used in forkret1, we could add 26a cli before the mov tf, %esp. 27 28--- 29 30In pushcli, must cli() no matter what. It is not safe to do 31 32 if(cpus[cpu()].ncli == 0) 33 cli(); 34 cpus[cpu()].ncli++; 35 36because if interrupts are off then we might call cpu(), get 37rescheduled to a different cpu, look at cpus[oldcpu].ncli, 38and wrongly decide not to disable interrupts on the new cpu. 39 40Instead do 41 42 cli(); 43 cpus[cpu()].ncli++; 44 45always. 46 47--- 48 49There is a (harmless) race in pushcli, which does 50 51 eflags = readeflags(); 52 cli(); 53 if(c->ncli++ == 0) 54 c->intena = eflags & FL_IF; 55 56Consider a bottom-level pushcli. 57If interrupts are disabled already, then the right thing 58happens: read_eflags finds that FL_IF is not set, 59and intena = 0. If interrupts are enabled, then 60it is less clear that the right thing happens: 61the readeflags can execute, then the process 62can get preempted and rescheduled on another cpu, 63and then once it starts running, perhaps with 64interrupts disabled (can happen since the scheduler 65only enables interrupts once per scheduling loop, 66not every time it schedules a process), it will 67incorrectly record that interrupts *were* enabled. 68This doesn't matter, because if it was safe to be 69running with interrupts enabled before the context 70switch, it is still safe (and arguably more correct) 71to run with them enabled after the context switch too. 72 73In fact it would be safe if scheduler always set 74 c->intena = 1; 75before calling swtch, and perhaps it should. 76 77--- 78 79The x86's processor-ordering memory model 80matches spin locks well, so no explicit memory 81synchronization instructions are required in 82acquire and release. 83 84Consider two sequences of code on different CPUs: 85 86CPU0 87A; 88release(lk); 89 90and 91 92CPU1 93acquire(lk); 94B; 95 96We want to make sure that: 97 - all reads in B see the effects of writes in A. 98 - all reads in A do *not* see the effects of writes in B. 99 100The x86 guarantees that writes in A will go out 101to memory before the write of lk->locked = 0 in 102release(lk). It further guarantees that CPU1 103will observe CPU0's write of lk->locked = 0 only 104after observing the earlier writes by CPU0. 105So any reads in B are guaranteed to observe the 106effects of writes in A. 107 108According to the Intel manual behavior spec, the 109second condition requires a serialization instruction 110in release, to avoid reads in A happening after giving 111up lk. No Intel SMP processor in existence actually 112moves reads down after writes, but the language in 113the spec allows it. There is no telling whether future 114processors will need it. 115 116--- 117 118The code in fork needs to read np->pid before 119setting np->state to RUNNABLE. The following 120is not a correct way to do this: 121 122 int 123 fork(void) 124 { 125 ... 126 np->state = RUNNABLE; 127 return np->pid; // oops 128 } 129 130After setting np->state to RUNNABLE, some other CPU 131might run the process, it might exit, and then it might 132get reused for a different process (with a new pid), all 133before the return statement. So it's not safe to just 134"return np->pid". Even saving a copy of np->pid before 135setting np->state isn't safe, since the compiler is 136allowed to re-order statements. 137 138The real code saves a copy of np->pid, then acquires a lock 139around the write to np->state. The acquire() prevents the 140compiler from re-ordering. 141