diff options
| -rw-r--r-- | TRICKS | 18 | ||||
| -rw-r--r-- | proc.c | 10 | ||||
| -rw-r--r-- | proc.h | 2 |
3 files changed, 19 insertions, 11 deletions
@@ -116,21 +116,25 @@ processors will need it. --- The code in fork needs to read np->pid before -setting np->state to RUNNABLE. +setting np->state to RUNNABLE. The following +is not a correct way to do this: int fork(void) { ... - pid = np->pid; np->state = RUNNABLE; - return pid; + return np->pid; // oops } After setting np->state to RUNNABLE, some other CPU might run the process, it might exit, and then it might get reused for a different process (with a new pid), all -before the return statement. So it's not safe to just do -"return np->pid;". - -This works because proc.h marks the pid as volatile. +before the return statement. So it's not safe to just +"return np->pid". Even saving a copy of np->pid before +setting np->state isn't safe, since the compiler is +allowed to re-order statements. + +The real code saves a copy of np->pid, then acquires a lock +around the write to np->state. The acquire() prevents the +compiler from re-ordering. @@ -153,10 +153,16 @@ fork(void) if(proc->ofile[i]) np->ofile[i] = filedup(proc->ofile[i]); np->cwd = idup(proc->cwd); + + safestrcpy(np->name, proc->name, sizeof(proc->name)); pid = np->pid; + + // lock to force the compiler to emit the np->state write last. + acquire(&ptable.lock); np->state = RUNNABLE; - safestrcpy(np->name, proc->name, sizeof(proc->name)); + release(&ptable.lock); + return pid; } @@ -455,5 +461,3 @@ procdump(void) cprintf("\n"); } } - - @@ -57,7 +57,7 @@ struct proc { pde_t* pgdir; // Page table char *kstack; // Bottom of kernel stack for this process enum procstate state; // Process state - volatile int pid; // Process ID + int pid; // Process ID struct proc *parent; // Parent process struct trapframe *tf; // Trap frame for current syscall struct context *context; // swtch() here to run process |