Checkpoint switching to per-process locks, in attempt clarify xv6's

locking plan, which is a difficult to understand because ptable lock
protects many invariants.  This implementation has a bug: once in a
while xv6 unlocks a proc lock that is locked by another core.
This commit is contained in:
Frans Kaashoek 2019-07-02 09:14:47 -04:00
parent 535ac52efa
commit 67702cf706
12 changed files with 139 additions and 85 deletions

View file

@ -2,6 +2,7 @@
#include "param.h" #include "param.h"
#include "memlayout.h" #include "memlayout.h"
#include "riscv.h" #include "riscv.h"
#include "spinlock.h"
#include "proc.h" #include "proc.h"
#include "defs.h" #include "defs.h"
#include "elf.h" #include "elf.h"
@ -19,7 +20,6 @@ exec(char *path, char **argv)
struct proghdr ph; struct proghdr ph;
pagetable_t pagetable = 0, oldpagetable; pagetable_t pagetable = 0, oldpagetable;
struct proc *p = myproc(); struct proc *p = myproc();
uint64 oldsz = p->sz;
begin_op(); begin_op();
@ -60,6 +60,9 @@ exec(char *path, char **argv)
end_op(); end_op();
ip = 0; ip = 0;
p = myproc();
uint64 oldsz = p->sz;
// Allocate two pages at the next page boundary. // Allocate two pages at the next page boundary.
// Use the second as the user stack. // Use the second as the user stack.
sz = PGROUNDUP(sz); sz = PGROUNDUP(sz);

View file

@ -14,8 +14,8 @@
#include "defs.h" #include "defs.h"
#include "param.h" #include "param.h"
#include "stat.h" #include "stat.h"
#include "proc.h"
#include "spinlock.h" #include "spinlock.h"
#include "proc.h"
#include "sleeplock.h" #include "sleeplock.h"
#include "fs.h" #include "fs.h"
#include "buf.h" #include "buf.h"

View file

@ -2,9 +2,9 @@
#include "riscv.h" #include "riscv.h"
#include "defs.h" #include "defs.h"
#include "param.h" #include "param.h"
#include "spinlock.h"
#include "proc.h" #include "proc.h"
#include "fs.h" #include "fs.h"
#include "spinlock.h"
#include "sleeplock.h" #include "sleeplock.h"
#include "file.h" #include "file.h"

View file

@ -2,8 +2,8 @@
#include "param.h" #include "param.h"
#include "memlayout.h" #include "memlayout.h"
#include "riscv.h" #include "riscv.h"
#include "proc.h"
#include "spinlock.h" #include "spinlock.h"
#include "proc.h"
#include "defs.h" #include "defs.h"
struct { struct {
@ -21,7 +21,7 @@ extern void forkret(void);
// for returning out of the kernel // for returning out of the kernel
extern void sysexit(void); extern void sysexit(void);
static void wakeup1(void *chan); static void wakeup1(struct proc *chan);
extern char trampout[]; // trampoline.S extern char trampout[]; // trampoline.S
@ -80,11 +80,13 @@ allocproc(void)
return 0; return 0;
found: found:
initlock(&p->lock, "proc");
acquire(&p->lock);
release(&ptable.lock);
p->state = EMBRYO; p->state = EMBRYO;
p->pid = nextpid++; p->pid = nextpid++;
release(&ptable.lock);
// Allocate a page for the kernel stack. // Allocate a page for the kernel stack.
if((p->kstack = kalloc()) == 0){ if((p->kstack = kalloc()) == 0){
p->state = UNUSED; p->state = UNUSED;
@ -177,15 +179,9 @@ userinit(void)
safestrcpy(p->name, "initcode", sizeof(p->name)); safestrcpy(p->name, "initcode", sizeof(p->name));
p->cwd = namei("/"); p->cwd = namei("/");
// this assignment to p->state lets other cores
// run this process. the acquire forces the above
// writes to be visible, and the lock is also needed
// because the assignment might not be atomic.
acquire(&ptable.lock);
p->state = RUNNABLE; p->state = RUNNABLE;
release(&ptable.lock); release(&p->lock);
} }
// Grow current process's memory by n bytes. // Grow current process's memory by n bytes.
@ -196,15 +192,22 @@ growproc(int n)
uint sz; uint sz;
struct proc *p = myproc(); struct proc *p = myproc();
acquire(&p->lock);
sz = p->sz; sz = p->sz;
if(n > 0){ if(n > 0){
if((sz = uvmalloc(p->pagetable, sz, sz + n)) == 0) if((sz = uvmalloc(p->pagetable, sz, sz + n)) == 0) {
release(&p->lock);
return -1; return -1;
}
} else if(n < 0){ } else if(n < 0){
if((sz = uvmdealloc(p->pagetable, sz, sz + n)) == 0) if((sz = uvmdealloc(p->pagetable, sz, sz + n)) == 0) {
release(&p->lock);
return -1; return -1;
}
} }
p->sz = sz; p->sz = sz;
release(&p->lock);
return 0; return 0;
} }
@ -244,11 +247,9 @@ fork(void)
pid = np->pid; pid = np->pid;
acquire(&ptable.lock);
np->state = RUNNABLE; np->state = RUNNABLE;
release(&ptable.lock); release(&np->lock);
return pid; return pid;
} }
@ -260,45 +261,65 @@ void
exit(void) exit(void)
{ {
struct proc *p = myproc(); struct proc *p = myproc();
struct proc *pp;
int fd; int fd;
if(p == initproc) if(p == initproc)
panic("init exiting"); panic("init exiting");
acquire(&p->lock);
// Close all open files. // Close all open files.
for(fd = 0; fd < NOFILE; fd++){ for(fd = 0; fd < NOFILE; fd++){
if(p->ofile[fd]){ if(p->ofile[fd]){
fileclose(p->ofile[fd]); struct file *f = p->ofile[fd];
release(&p->lock);
fileclose(f);
acquire(&p->lock);
p->ofile[fd] = 0; p->ofile[fd] = 0;
} }
} }
struct inode *cwd = p->cwd;
release(&p->lock);
begin_op(); begin_op();
iput(p->cwd); iput(cwd);
end_op(); end_op();
acquire(&p->lock);
p->cwd = 0; p->cwd = 0;
acquire(&ptable.lock);
// Parent might be sleeping in wait().
wakeup1(p->parent);
// Pass abandoned children to init.
for(pp = ptable.proc; pp < &ptable.proc[NPROC]; pp++){
if(pp->parent == p){
pp->parent = initproc;
if(pp->state == ZOMBIE)
wakeup1(initproc);
}
}
// Jump into the scheduler, never to return. // Jump into the scheduler, never to return.
p->state = ZOMBIE; p->state = ZOMBIE;
sched(); sched();
panic("zombie exit"); panic("zombie exit");
} }
void tellparent(struct proc *p, struct proc *parent) {
struct proc *pp;
acquire(&parent->lock);
// Parent might be sleeping in wait().
wakeup1(parent);
// Pass abandoned children to init.
for(pp = ptable.proc; pp < &ptable.proc[NPROC]; pp++){
if(pp->parent == p){
pp->parent = initproc;
if(pp->state == ZOMBIE) {
acquire(&initproc->lock);
wakeup1(initproc);
acquire(&initproc->lock);
}
}
}
release(&parent->lock);
}
// Wait for a child process to exit and return its pid. // Wait for a child process to exit and return its pid.
// Return -1 if this process has no children. // Return -1 if this process has no children.
int int
@ -307,14 +328,17 @@ wait(void)
struct proc *np; struct proc *np;
int havekids, pid; int havekids, pid;
struct proc *p = myproc(); struct proc *p = myproc();
// should p lock!
acquire(&ptable.lock); acquire(&p->lock);
for(;;){ for(;;){
// Scan through table looking for exited children. // Scan through table looking for exited children.
havekids = 0; havekids = 0;
for(np = ptable.proc; np < &ptable.proc[NPROC]; np++){ for(np = ptable.proc; np < &ptable.proc[NPROC]; np++){
if(np->parent != p) if(np->parent != p)
continue; continue;
acquire(&np->lock);
havekids = 1; havekids = 1;
if(np->state == ZOMBIE){ if(np->state == ZOMBIE){
// Found one. // Found one.
@ -330,19 +354,21 @@ wait(void)
np->name[0] = 0; np->name[0] = 0;
np->killed = 0; np->killed = 0;
np->state = UNUSED; np->state = UNUSED;
release(&ptable.lock); release(&np->lock);
release(&p->lock);
return pid; return pid;
} }
release(&np->lock);
} }
// No point waiting if we don't have any children. // No point waiting if we don't have any children.
if(!havekids || p->killed){ if(!havekids || p->killed){
release(&ptable.lock); release(&p->lock);
return -1; return -1;
} }
// Wait for children to exit. (See wakeup1 call in proc_exit.) // Wait for children to exit. (See wakeup1 call in tellparent.)
sleep(p, &ptable.lock); //DOC: wait-sleep sleep(p, &p->lock); //DOC: wait-sleep
} }
} }
@ -366,13 +392,16 @@ scheduler(void)
intr_on(); intr_on();
// Loop over process table looking for process to run. // Loop over process table looking for process to run.
acquire(&ptable.lock); for(p = ptable.proc; p < &ptable.proc[NPROC]; p++) {
for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){ acquire(&p->lock);
if(p->state != RUNNABLE)
if(p->state != RUNNABLE) {
release(&p->lock);
continue; continue;
}
// Switch to chosen process. It is the process's job // Switch to chosen process. It is the process's job
// to release ptable.lock and then reacquire it // to release its lock and then reacquire it
// before jumping back to us. // before jumping back to us.
c->proc = p; c->proc = p;
p->state = RUNNING; p->state = RUNNING;
@ -382,12 +411,17 @@ scheduler(void)
// Process is done running for now. // Process is done running for now.
// It should have changed its p->state before coming back. // It should have changed its p->state before coming back.
c->proc = 0; c->proc = 0;
release(&p->lock);
if(p->state == ZOMBIE) {
tellparent(p, p->parent);
}
} }
release(&ptable.lock);
} }
} }
// Enter scheduler. Must hold only ptable.lock // Enter scheduler. Must hold only p->lock
// and have changed proc->state. Saves and restores // and have changed proc->state. Saves and restores
// intena because intena is a property of this // intena because intena is a property of this
// kernel thread, not this CPU. It should // kernel thread, not this CPU. It should
@ -400,8 +434,8 @@ sched(void)
int intena; int intena;
struct proc *p = myproc(); struct proc *p = myproc();
if(!holding(&ptable.lock)) if(!holding(&p->lock))
panic("sched ptable.lock"); panic("sched p->lock");
if(mycpu()->noff != 1) if(mycpu()->noff != 1)
panic("sched locks"); panic("sched locks");
if(p->state == RUNNING) if(p->state == RUNNING)
@ -418,10 +452,10 @@ sched(void)
void void
yield(void) yield(void)
{ {
acquire(&ptable.lock); //DOC: yieldlock acquire(&myproc()->lock); //DOC: yieldlock
myproc()->state = RUNNABLE; myproc()->state = RUNNABLE;
sched(); sched();
release(&ptable.lock); release(&myproc()->lock);
} }
// A fork child's very first scheduling by scheduler() // A fork child's very first scheduling by scheduler()
@ -431,8 +465,8 @@ forkret(void)
{ {
static int first = 1; static int first = 1;
// Still holding ptable.lock from scheduler. // Still holding p->lock from scheduler.
release(&ptable.lock); release(&myproc()->lock);
if (first) { if (first) {
// Some initialization functions must be run in the context // Some initialization functions must be run in the context
@ -451,60 +485,69 @@ forkret(void)
void void
sleep(void *chan, struct spinlock *lk) sleep(void *chan, struct spinlock *lk)
{ {
struct proc *p = myproc(); if(myproc() == 0)
if(p == 0)
panic("sleep"); panic("sleep");
if(lk == 0) if(lk == 0)
panic("sleep without lk"); panic("sleep without lk");
// Must acquire ptable.lock in order to // Must acquire p->lock in order to
// change p->state and then call sched. // change p->state and then call sched.
// Once we hold ptable.lock, we can be // Once we hold p->lock, we can be
// guaranteed that we won't miss any wakeup // guaranteed that we won't miss any wakeup
// (wakeup runs with ptable.lock locked), // (wakeup runs with p->lock locked),
// so it's okay to release lk. // so it's okay to release lk.
if(lk != &ptable.lock){ //DOC: sleeplock0 if(lk != &myproc()->lock){ //DOC: sleeplock0
acquire(&ptable.lock); //DOC: sleeplock1 acquire(&myproc()->lock); //DOC: sleeplock1
release(lk); release(lk);
} }
// Go to sleep. // Go to sleep.
p->chan = chan; myproc()->chan = chan;
p->state = SLEEPING; myproc()->state = SLEEPING;
sched(); sched();
// Tidy up. // Tidy up.
p->chan = 0; myproc()->chan = 0;
// Reacquire original lock. // Reacquire original lock.
if(lk != &ptable.lock){ //DOC: sleeplock2 if(lk != &myproc()->lock){ //DOC: sleeplock2
release(&ptable.lock); release(&myproc()->lock);
acquire(lk); acquire(lk);
} }
} }
//PAGEBREAK! //PAGEBREAK!
// Wake up all processes sleeping on chan. // Wake up all processes sleeping on chan,
// The ptable lock must be held. // where chan is a proc.
static void static void
wakeup1(void *chan) wakeup1(struct proc *chan)
{ {
struct proc *p; struct proc *p;
for(p = ptable.proc; p < &ptable.proc[NPROC]; p++) for(p = ptable.proc; p < &ptable.proc[NPROC]; p++)
if(p->state == SLEEPING && p->chan == chan) if(p == chan && p->state == SLEEPING && p->chan == chan) {
if(p->state != SLEEPING || p->chan != chan)
panic("wakeup1");
p->state = RUNNABLE; p->state = RUNNABLE;
}
} }
// Wake up all processes sleeping on chan. // Wake up all processes sleeping on chan. Never
// called when holding a p->lock
void void
wakeup(void *chan) wakeup(void *chan)
{ {
acquire(&ptable.lock); struct proc *p;
wakeup1(chan);
release(&ptable.lock); for(p = ptable.proc; p < &ptable.proc[NPROC]; p++)
if(p->state == SLEEPING && p->chan == chan) {
acquire(&p->lock);
if(p->state != SLEEPING || p->chan != chan)
panic("wakeup");
p->state = RUNNABLE;
release(&p->lock);
}
} }
// Kill the process with the given pid. // Kill the process with the given pid.
@ -515,18 +558,19 @@ kill(int pid)
{ {
struct proc *p; struct proc *p;
acquire(&ptable.lock);
for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){ for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){
if(p->pid == pid){ if(p->pid == pid){
acquire(&p->lock);
if(p->pid != pid)
panic("kill");
p->killed = 1; p->killed = 1;
// Wake process from sleep if necessary. // Wake process from sleep if necessary.
if(p->state == SLEEPING) if(p->state == SLEEPING)
p->state = RUNNABLE; p->state = RUNNABLE;
release(&ptable.lock); release(&p->lock);
return 0; return 0;
} }
} }
release(&ptable.lock);
return -1; return -1;
} }

View file

@ -82,6 +82,7 @@ enum procstate { UNUSED, EMBRYO, SLEEPING, RUNNABLE, RUNNING, ZOMBIE };
// Per-process state // Per-process state
struct proc { struct proc {
struct spinlock lock;
char *kstack; // Bottom of kernel stack for this process char *kstack; // Bottom of kernel stack for this process
uint64 sz; // Size of process memory (bytes) uint64 sz; // Size of process memory (bytes)
pagetable_t pagetable; // Page table pagetable_t pagetable; // Page table

View file

@ -5,8 +5,8 @@
#include "defs.h" #include "defs.h"
#include "param.h" #include "param.h"
#include "memlayout.h" #include "memlayout.h"
#include "proc.h"
#include "spinlock.h" #include "spinlock.h"
#include "proc.h"
#include "sleeplock.h" #include "sleeplock.h"
void void

View file

@ -20,7 +20,7 @@ initlock(struct spinlock *lk, char *name)
// Loops (spins) until the lock is acquired. // Loops (spins) until the lock is acquired.
// Holding a lock for a long time may cause // Holding a lock for a long time may cause
// other CPUs to waste time spinning to acquire it. // other CPUs to waste time spinning to acquire it.
void void //__attribute__ ((noinline))
acquire(struct spinlock *lk) acquire(struct spinlock *lk)
{ {
push_off(); // disable interrupts to avoid deadlock. push_off(); // disable interrupts to avoid deadlock.
@ -44,11 +44,13 @@ acquire(struct spinlock *lk)
} }
// Release the lock. // Release the lock.
void void //__attribute__ ((noinline))
release(struct spinlock *lk) release(struct spinlock *lk)
{ {
if(!holding(lk)) if(!holding(lk)) {
printf("%p: !holding %s %p\n", mycpu(), lk->name, lk->cpu);
panic("release"); panic("release");
}
lk->cpu = 0; lk->cpu = 0;

View file

@ -2,6 +2,7 @@
#include "param.h" #include "param.h"
#include "memlayout.h" #include "memlayout.h"
#include "riscv.h" #include "riscv.h"
#include "spinlock.h"
#include "proc.h" #include "proc.h"
#include "syscall.h" #include "syscall.h"
#include "defs.h" #include "defs.h"
@ -170,7 +171,9 @@ dosyscall(void)
num = p->tf->a7; num = p->tf->a7;
if(num > 0 && num < NELEM(syscalls) && syscalls[num]) { if(num > 0 && num < NELEM(syscalls) && syscalls[num]) {
//printf("%d: syscall %d\n", p->pid, num);
p->tf->a0 = syscalls[num](); p->tf->a0 = syscalls[num]();
//printf("%d: syscall %d -> %d\n", p->pid, num, p->tf->a0);
} else { } else {
printf("%d %s: unknown sys call %d\n", printf("%d %s: unknown sys call %d\n",
p->pid, p->name, num); p->pid, p->name, num);

View file

@ -9,9 +9,9 @@
#include "defs.h" #include "defs.h"
#include "param.h" #include "param.h"
#include "stat.h" #include "stat.h"
#include "spinlock.h"
#include "proc.h" #include "proc.h"
#include "fs.h" #include "fs.h"
#include "spinlock.h"
#include "sleeplock.h" #include "sleeplock.h"
#include "file.h" #include "file.h"
#include "fcntl.h" #include "fcntl.h"

View file

@ -4,6 +4,7 @@
#include "date.h" #include "date.h"
#include "param.h" #include "param.h"
#include "memlayout.h" #include "memlayout.h"
#include "spinlock.h"
#include "proc.h" #include "proc.h"
int int

View file

@ -2,8 +2,8 @@
#include "param.h" #include "param.h"
#include "memlayout.h" #include "memlayout.h"
#include "riscv.h" #include "riscv.h"
#include "proc.h"
#include "spinlock.h" #include "spinlock.h"
#include "proc.h"
#include "defs.h" #include "defs.h"
struct spinlock tickslock; struct spinlock tickslock;

View file

@ -2,8 +2,8 @@
#include "param.h" #include "param.h"
#include "memlayout.h" #include "memlayout.h"
#include "riscv.h" #include "riscv.h"
#include "proc.h"
#include "spinlock.h" #include "spinlock.h"
#include "proc.h"
#include "defs.h" #include "defs.h"
// //