From abf847a083888bbed4260ecacf849ea19f23e810 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Tue, 31 Jan 2017 17:47:16 -0500 Subject: [PATCH 1/7] Start of an experiment to remove the use of gs for cpu local variables. --- console.c | 4 +-- defs.h | 4 ++- exec.c | 15 ++++----- fs.c | 4 +-- ide.c | 3 +- lapic.c | 6 ++-- main.c | 9 +++--- pipe.c | 4 +-- proc.c | 89 +++++++++++++++++++++++++++++------------------------ proc.h | 27 +++++++++++----- sleeplock.c | 2 +- spinlock.c | 14 ++++----- syscall.c | 21 +++++++------ sysfile.c | 14 ++++----- sysproc.c | 6 ++-- trap.c | 24 +++++++-------- vm.c | 22 ++++++------- 17 files changed, 145 insertions(+), 123 deletions(-) diff --git a/console.c b/console.c index 4d678b0b5a..f7e1e73942 100644 --- a/console.c +++ b/console.c @@ -111,7 +111,7 @@ panic(char *s) cli(); cons.locking = 0; - cprintf("cpu with apicid %d: panic: ", cpu->apicid); + cprintf("cpu %d: panic: ", cpuid()); cprintf(s); cprintf("\n"); getcallerpcs(&s, pcs); @@ -242,7 +242,7 @@ consoleread(struct inode *ip, char *dst, int n) acquire(&cons.lock); while(n > 0){ while(input.r == input.w){ - if(proc->killed){ + if(myproc()->killed){ release(&cons.lock); ilock(ip); return -1; diff --git a/defs.h b/defs.h index 300c75c664..67ea9f6cba 100644 --- a/defs.h +++ b/defs.h @@ -74,7 +74,7 @@ void kbdintr(void); // lapic.c void cmostime(struct rtcdate *r); -int cpunum(void); +int lapiccpunum(void); extern volatile uint* lapic; void lapiceoi(void); void lapicinit(void); @@ -103,6 +103,7 @@ int pipewrite(struct pipe*, char*, int); //PAGEBREAK: 16 // proc.c +int cpuid(void); void exit(void); int fork(void); int growproc(int); @@ -111,6 +112,7 @@ void pinit(void); void procdump(void); void scheduler(void) __attribute__((noreturn)); void sched(void); +void setproc(struct proc*); void sleep(void*, struct spinlock*); void userinit(void); int wait(void); diff --git a/exec.c b/exec.c index 4d7d97cc8f..14d673f12e 100644 --- a/exec.c +++ b/exec.c @@ -22,6 +22,7 @@ exec(char *path, char **argv) if((ip = namei(path)) == 0){ end_op(); + cprintf("exec: fail\n"); return -1; } ilock(ip); @@ -89,15 +90,15 @@ exec(char *path, char **argv) for(last=s=path; *s; s++) if(*s == '/') last = s+1; - safestrcpy(proc->name, last, sizeof(proc->name)); + safestrcpy(myproc()->name, last, sizeof(myproc()->name)); // Commit to the user image. - oldpgdir = proc->pgdir; - proc->pgdir = pgdir; - proc->sz = sz; - proc->tf->eip = elf.entry; // main - proc->tf->esp = sp; - switchuvm(proc); + oldpgdir = myproc()->pgdir; + myproc()->pgdir = pgdir; + myproc()->sz = sz; + myproc()->tf->eip = elf.entry; // main + myproc()->tf->esp = sp; + switchuvm(myproc()); freevm(oldpgdir); return 0; diff --git a/fs.c b/fs.c index be8c5c53fa..2f0cf3bda7 100644 --- a/fs.c +++ b/fs.c @@ -169,7 +169,7 @@ iinit(int dev) for(i = 0; i < NINODE; i++) { initsleeplock(&icache.inode[i].lock, "inode"); } - + readsb(dev, &sb); cprintf("sb: size %d nblocks %d ninodes %d nlog %d logstart %d\ inodestart %d bmap start %d\n", sb.size, sb.nblocks, @@ -615,7 +615,7 @@ namex(char *path, int nameiparent, char *name) if(*path == '/') ip = iget(ROOTDEV, ROOTINO); else - ip = idup(proc->cwd); + ip = idup(myproc()->cwd); while((path = skipelem(path, name)) != 0){ ilock(ip); diff --git a/ide.c b/ide.c index b3112b9ad6..881fe0dce3 100644 --- a/ide.c +++ b/ide.c @@ -108,9 +108,9 @@ ideintr(void) // First queued buffer is the active request. acquire(&idelock); + if((b = idequeue) == 0){ release(&idelock); - // cprintf("spurious IDE interrupt\n"); return; } idequeue = b->qnext; @@ -164,5 +164,6 @@ iderw(struct buf *b) sleep(b, &idelock); } + release(&idelock); } diff --git a/lapic.c b/lapic.c index 7507f97439..9039665773 100644 --- a/lapic.c +++ b/lapic.c @@ -99,11 +99,11 @@ lapicinit(void) } int -cpunum(void) +lapiccpunum(void) { int apicid, i; - // Cannot call cpu when interrupts are enabled: + // Cannot call cpunum when interrupts are enabled: // result not guaranteed to last long enough to be used! // Would prefer to panic but even printing is chancy here: // almost everything, including cprintf and panic, calls cpu, @@ -111,7 +111,7 @@ cpunum(void) if(readeflags()&FL_IF){ static int n; if(n++ == 0) - cprintf("cpu called from %x with interrupts enabled\n", + cprintf("cpunum called from %x with interrupts enabled\n", __builtin_return_address(0)); } diff --git a/main.c b/main.c index 731e42907b..d7e59cff0f 100644 --- a/main.c +++ b/main.c @@ -22,7 +22,6 @@ main(void) mpinit(); // detect other processors lapicinit(); // interrupt controller seginit(); // segment descriptors - cprintf("\ncpu%d: starting xv6\n\n", cpunum()); picinit(); // another interrupt controller ioapicinit(); // another interrupt controller consoleinit(); // console hardware @@ -31,7 +30,7 @@ main(void) tvinit(); // trap vectors binit(); // buffer cache fileinit(); // file table - ideinit(); // disk + ideinit(); // disk if(!ismp) timerinit(); // uniprocessor timer startothers(); // start other processors @@ -54,9 +53,9 @@ mpenter(void) static void mpmain(void) { - cprintf("cpu%d: starting\n", cpunum()); + cprintf("cpu%d: starting %d\n", cpuid(), lapiccpunum()); idtinit(); // load idt register - xchg(&cpu->started, 1); // tell startothers() we're up + xchg(&(mycpu()->started), 1); // tell startothers() we're up scheduler(); // start running processes } @@ -78,7 +77,7 @@ startothers(void) memmove(code, _binary_entryother_start, (uint)_binary_entryother_size); for(c = cpus; c < cpus+ncpu; c++){ - if(c == cpus+cpunum()) // We've started already. + if(c == mycpu()) // We've started already. continue; // Tell entryother.S what stack to use, where to enter, and what diff --git a/pipe.c b/pipe.c index a9f471e57f..e9abe7f5b0 100644 --- a/pipe.c +++ b/pipe.c @@ -83,7 +83,7 @@ pipewrite(struct pipe *p, char *addr, int n) acquire(&p->lock); for(i = 0; i < n; i++){ while(p->nwrite == p->nread + PIPESIZE){ //DOC: pipewrite-full - if(p->readopen == 0 || proc->killed){ + if(p->readopen == 0 || myproc()->killed){ release(&p->lock); return -1; } @@ -104,7 +104,7 @@ piperead(struct pipe *p, char *addr, int n) acquire(&p->lock); while(p->nread == p->nwrite && p->writeopen){ //DOC: pipe-empty - if(proc->killed){ + if(myproc()->killed){ release(&p->lock); return -1; } diff --git a/proc.c b/proc.c index 7d03ad7903..bd62e4b97d 100644 --- a/proc.c +++ b/proc.c @@ -26,6 +26,16 @@ pinit(void) initlock(&ptable.lock, "ptable"); } +int +cpuid() { + return mycpu()-cpus; +} + +void +setproc(struct proc* p) { + mycpu()->proc = p; +} + //PAGEBREAK: 32 // Look in the process table for an UNUSED proc. // If found, change state to EMBRYO and initialize @@ -121,16 +131,16 @@ growproc(int n) { uint sz; - sz = proc->sz; + sz = myproc()->sz; if(n > 0){ - if((sz = allocuvm(proc->pgdir, sz, sz + n)) == 0) + if((sz = allocuvm(myproc()->pgdir, sz, sz + n)) == 0) return -1; } else if(n < 0){ - if((sz = deallocuvm(proc->pgdir, sz, sz + n)) == 0) + if((sz = deallocuvm(myproc()->pgdir, sz, sz + n)) == 0) return -1; } - proc->sz = sz; - switchuvm(proc); + myproc()->sz = sz; + switchuvm(myproc()); return 0; } @@ -148,26 +158,26 @@ fork(void) return -1; } - // Copy process state from p. - if((np->pgdir = copyuvm(proc->pgdir, proc->sz)) == 0){ + // Copy process state from proc. + if((np->pgdir = copyuvm(myproc()->pgdir, myproc()->sz)) == 0){ kfree(np->kstack); np->kstack = 0; np->state = UNUSED; return -1; } - np->sz = proc->sz; - np->parent = proc; - *np->tf = *proc->tf; + np->sz = myproc()->sz; + np->parent = myproc(); + *np->tf = *myproc()->tf; // Clear %eax so that fork returns 0 in the child. np->tf->eax = 0; for(i = 0; i < NOFILE; i++) - if(proc->ofile[i]) - np->ofile[i] = filedup(proc->ofile[i]); - np->cwd = idup(proc->cwd); + if(myproc()->ofile[i]) + np->ofile[i] = filedup(myproc()->ofile[i]); + np->cwd = idup(myproc()->cwd); - safestrcpy(np->name, proc->name, sizeof(proc->name)); + safestrcpy(np->name, myproc()->name, sizeof(myproc()->name)); pid = np->pid; @@ -189,30 +199,30 @@ exit(void) struct proc *p; int fd; - if(proc == initproc) + if(myproc() == initproc) panic("init exiting"); // Close all open files. for(fd = 0; fd < NOFILE; fd++){ - if(proc->ofile[fd]){ - fileclose(proc->ofile[fd]); - proc->ofile[fd] = 0; + if(myproc()->ofile[fd]){ + fileclose(myproc()->ofile[fd]); + myproc()->ofile[fd] = 0; } } begin_op(); - iput(proc->cwd); + iput(myproc()->cwd); end_op(); - proc->cwd = 0; + myproc()->cwd = 0; acquire(&ptable.lock); // Parent might be sleeping in wait(). - wakeup1(proc->parent); + wakeup1(myproc()->parent); // Pass abandoned children to init. for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){ - if(p->parent == proc){ + if(p->parent == myproc()){ p->parent = initproc; if(p->state == ZOMBIE) wakeup1(initproc); @@ -220,7 +230,7 @@ exit(void) } // Jump into the scheduler, never to return. - proc->state = ZOMBIE; + myproc()->state = ZOMBIE; sched(); panic("zombie exit"); } @@ -238,7 +248,7 @@ wait(void) // Scan through table looking for exited children. havekids = 0; for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){ - if(p->parent != proc) + if(p->parent != myproc()) continue; havekids = 1; if(p->state == ZOMBIE){ @@ -258,13 +268,13 @@ wait(void) } // No point waiting if we don't have any children. - if(!havekids || proc->killed){ + if(!havekids || myproc()->killed){ release(&ptable.lock); return -1; } // Wait for children to exit. (See wakeup1 call in proc_exit.) - sleep(proc, &ptable.lock); //DOC: wait-sleep + sleep(myproc(), &ptable.lock); //DOC: wait-sleep } } @@ -294,15 +304,15 @@ scheduler(void) // Switch to chosen process. It is the process's job // to release ptable.lock and then reacquire it // before jumping back to us. - proc = p; + setproc(p); switchuvm(p); p->state = RUNNING; - swtch(&cpu->scheduler, p->context); + swtch(&(mycpu()->scheduler), p->context); switchkvm(); // Process is done running for now. // It should have changed its p->state before coming back. - proc = 0; + setproc(0); } release(&ptable.lock); @@ -323,15 +333,15 @@ sched(void) if(!holding(&ptable.lock)) panic("sched ptable.lock"); - if(cpu->ncli != 1) + if(mycpu()->ncli != 1) panic("sched locks"); - if(proc->state == RUNNING) + if(myproc()->state == RUNNING) panic("sched running"); if(readeflags()&FL_IF) panic("sched interruptible"); - intena = cpu->intena; - swtch(&proc->context, cpu->scheduler); - cpu->intena = intena; + intena = mycpu()->intena; + swtch(&myproc()->context, mycpu()->scheduler); + mycpu()->intena = intena; } // Give up the CPU for one scheduling round. @@ -339,7 +349,7 @@ void yield(void) { acquire(&ptable.lock); //DOC: yieldlock - proc->state = RUNNABLE; + myproc()->state = RUNNABLE; sched(); release(&ptable.lock); } @@ -370,7 +380,7 @@ forkret(void) void sleep(void *chan, struct spinlock *lk) { - if(proc == 0) + if(myproc() == 0) panic("sleep"); if(lk == 0) @@ -386,14 +396,13 @@ sleep(void *chan, struct spinlock *lk) acquire(&ptable.lock); //DOC: sleeplock1 release(lk); } - // Go to sleep. - proc->chan = chan; - proc->state = SLEEPING; + myproc()->chan = chan; + myproc()->state = SLEEPING; sched(); // Tidy up. - proc->chan = 0; + myproc()->chan = 0; // Reacquire original lock. if(lk != &ptable.lock){ //DOC: sleeplock2 diff --git a/proc.h b/proc.h index 735280527e..38a2d32c0f 100644 --- a/proc.h +++ b/proc.h @@ -7,25 +7,36 @@ struct cpu { volatile uint started; // Has the CPU started? int ncli; // Depth of pushcli nesting. int intena; // Were interrupts enabled before pushcli? - - // Cpu-local storage variables; see below - struct cpu *cpu; - struct proc *proc; // The currently-running process. + // Per-CPU variables, holding pointers to the current cpu and to the current + // process (see cpu() and proc() in proc.c) + struct cpu *cpu; // On cpu 0, cpu = &cpus[0]; on cpu 1, cpu=&cpus[1], etc. + struct proc *proc; // The currently-running process on this cpu }; extern struct cpu cpus[NCPU]; extern int ncpu; -// Per-CPU variables, holding pointers to the -// current cpu and to the current process. // The asm suffix tells gcc to use "%gs:0" to refer to cpu // and "%gs:4" to refer to proc. seginit sets up the // %gs segment register so that %gs refers to the memory // holding those two variables in the local cpu's struct cpu. // This is similar to how thread-local variables are implemented // in thread libraries such as Linux pthreads. -extern struct cpu *cpu asm("%gs:0"); // &cpus[cpunum()] -extern struct proc *proc asm("%gs:4"); // cpus[cpunum()].proc + +static inline struct cpu* +mycpu(void) { + struct cpu *cpu; + asm("movl %%gs:0, %0" : "=r"(cpu)); + return cpu; +} + +static inline struct proc* +myproc(void) { + struct proc *proc; + asm("movl %%gs:4, %0" : "=r"(proc)); + return proc; +} + //PAGEBREAK: 17 // Saved registers for kernel context switches. diff --git a/sleeplock.c b/sleeplock.c index 2ded78dc85..d0e4d91852 100644 --- a/sleeplock.c +++ b/sleeplock.c @@ -27,7 +27,7 @@ acquiresleep(struct sleeplock *lk) sleep(lk, &lk->lk); } lk->locked = 1; - lk->pid = proc->pid; + lk->pid = myproc()->pid; release(&lk->lk); } diff --git a/spinlock.c b/spinlock.c index 942d93d2c8..9120bf226f 100644 --- a/spinlock.c +++ b/spinlock.c @@ -38,7 +38,7 @@ acquire(struct spinlock *lk) __sync_synchronize(); // Record info about lock acquisition for debugging. - lk->cpu = cpu; + lk->cpu = mycpu(); getcallerpcs(&lk, lk->pcs); } @@ -89,7 +89,7 @@ getcallerpcs(void *v, uint pcs[]) int holding(struct spinlock *lock) { - return lock->locked && lock->cpu == cpu; + return lock->locked && lock->cpu == mycpu(); } @@ -104,9 +104,9 @@ pushcli(void) eflags = readeflags(); cli(); - if(cpu->ncli == 0) - cpu->intena = eflags & FL_IF; - cpu->ncli += 1; + if(mycpu()->ncli == 0) + mycpu()->intena = eflags & FL_IF; + mycpu()->ncli += 1; } void @@ -114,9 +114,9 @@ popcli(void) { if(readeflags()&FL_IF) panic("popcli - interruptible"); - if(--cpu->ncli < 0) + if(--mycpu()->ncli < 0) panic("popcli"); - if(cpu->ncli == 0 && cpu->intena) + if(mycpu()->ncli == 0 && mycpu()->intena) sti(); } diff --git a/syscall.c b/syscall.c index 9ae75367cd..2d6769e204 100644 --- a/syscall.c +++ b/syscall.c @@ -17,7 +17,7 @@ int fetchint(uint addr, int *ip) { - if(addr >= proc->sz || addr+4 > proc->sz) + if(addr >= myproc()->sz || addr+4 > myproc()->sz) return -1; *ip = *(int*)(addr); return 0; @@ -31,13 +31,14 @@ fetchstr(uint addr, char **pp) { char *s, *ep; - if(addr >= proc->sz) + if(addr >= myproc()->sz) return -1; *pp = (char*)addr; - ep = (char*)proc->sz; - for(s = *pp; s < ep; s++) + ep = (char*)myproc()->sz; + for(s = *pp; s < ep; s++){ if(*s == 0) return s - *pp; + } return -1; } @@ -45,7 +46,7 @@ fetchstr(uint addr, char **pp) int argint(int n, int *ip) { - return fetchint(proc->tf->esp + 4 + 4*n, ip); + return fetchint((myproc()->tf->esp) + 4 + 4*n, ip); } // Fetch the nth word-sized system call argument as a pointer @@ -58,7 +59,7 @@ argptr(int n, char **pp, int size) if(argint(n, &i) < 0) return -1; - if(size < 0 || (uint)i >= proc->sz || (uint)i+size > proc->sz) + if(size < 0 || (uint)i >= myproc()->sz || (uint)i+size > myproc()->sz) return -1; *pp = (char*)i; return 0; @@ -128,12 +129,12 @@ syscall(void) { int num; - num = proc->tf->eax; + num = myproc()->tf->eax; if(num > 0 && num < NELEM(syscalls) && syscalls[num]) { - proc->tf->eax = syscalls[num](); + myproc()->tf->eax = syscalls[num](); } else { cprintf("%d %s: unknown sys call %d\n", - proc->pid, proc->name, num); - proc->tf->eax = -1; + myproc()->pid, myproc()->name, num); + myproc()->tf->eax = -1; } } diff --git a/sysfile.c b/sysfile.c index 98e8c433d4..fae69608b2 100644 --- a/sysfile.c +++ b/sysfile.c @@ -26,7 +26,7 @@ argfd(int n, int *pfd, struct file **pf) if(argint(n, &fd) < 0) return -1; - if(fd < 0 || fd >= NOFILE || (f=proc->ofile[fd]) == 0) + if(fd < 0 || fd >= NOFILE || (f=myproc()->ofile[fd]) == 0) return -1; if(pfd) *pfd = fd; @@ -43,8 +43,8 @@ fdalloc(struct file *f) int fd; for(fd = 0; fd < NOFILE; fd++){ - if(proc->ofile[fd] == 0){ - proc->ofile[fd] = f; + if(myproc()->ofile[fd] == 0){ + myproc()->ofile[fd] = f; return fd; } } @@ -97,7 +97,7 @@ sys_close(void) if(argfd(0, &fd, &f) < 0) return -1; - proc->ofile[fd] = 0; + myproc()->ofile[fd] = 0; fileclose(f); return 0; } @@ -386,9 +386,9 @@ sys_chdir(void) return -1; } iunlock(ip); - iput(proc->cwd); + iput(myproc()->cwd); end_op(); - proc->cwd = ip; + myproc()->cwd = ip; return 0; } @@ -432,7 +432,7 @@ sys_pipe(void) fd0 = -1; if((fd0 = fdalloc(rf)) < 0 || (fd1 = fdalloc(wf)) < 0){ if(fd0 >= 0) - proc->ofile[fd0] = 0; + myproc()->ofile[fd0] = 0; fileclose(rf); fileclose(wf); return -1; diff --git a/sysproc.c b/sysproc.c index 6b585e002b..0686d295b6 100644 --- a/sysproc.c +++ b/sysproc.c @@ -39,7 +39,7 @@ sys_kill(void) int sys_getpid(void) { - return proc->pid; + return myproc()->pid; } int @@ -50,7 +50,7 @@ sys_sbrk(void) if(argint(0, &n) < 0) return -1; - addr = proc->sz; + addr = myproc()->sz; if(growproc(n) < 0) return -1; return addr; @@ -67,7 +67,7 @@ sys_sleep(void) acquire(&tickslock); ticks0 = ticks; while(ticks - ticks0 < n){ - if(proc->killed){ + if(myproc()->killed){ release(&tickslock); return -1; } diff --git a/trap.c b/trap.c index e6b37848b8..b5eba82408 100644 --- a/trap.c +++ b/trap.c @@ -37,18 +37,18 @@ void trap(struct trapframe *tf) { if(tf->trapno == T_SYSCALL){ - if(proc->killed) + if(myproc()->killed) exit(); - proc->tf = tf; + myproc()->tf = tf; syscall(); - if(proc->killed) + if(myproc()->killed) exit(); return; } switch(tf->trapno){ case T_IRQ0 + IRQ_TIMER: - if(cpunum() == 0){ + if(cpuid() == 0){ acquire(&tickslock); ticks++; wakeup(&ticks); @@ -74,38 +74,38 @@ trap(struct trapframe *tf) case T_IRQ0 + 7: case T_IRQ0 + IRQ_SPURIOUS: cprintf("cpu%d: spurious interrupt at %x:%x\n", - cpunum(), tf->cs, tf->eip); + cpuid(), tf->cs, tf->eip); lapiceoi(); break; //PAGEBREAK: 13 default: - if(proc == 0 || (tf->cs&3) == 0){ + if(myproc() == 0 || (tf->cs&3) == 0){ // In kernel, it must be our mistake. cprintf("unexpected trap %d from cpu %d eip %x (cr2=0x%x)\n", - tf->trapno, cpunum(), tf->eip, rcr2()); + tf->trapno, cpuid(), tf->eip, rcr2()); panic("trap"); } // In user space, assume process misbehaved. cprintf("pid %d %s: trap %d err %d on cpu %d " "eip 0x%x addr 0x%x--kill proc\n", - proc->pid, proc->name, tf->trapno, tf->err, cpunum(), tf->eip, + myproc()->pid, myproc()->name, tf->trapno, tf->err, cpuid(), tf->eip, rcr2()); - proc->killed = 1; + myproc()->killed = 1; } // Force process exit if it has been killed and is in user space. // (If it is still executing in the kernel, let it keep running // until it gets to the regular system call return.) - if(proc && proc->killed && (tf->cs&3) == DPL_USER) + if(myproc() && myproc()->killed && (tf->cs&3) == DPL_USER) exit(); // Force process to give up CPU on clock tick. // If interrupts were on while locks held, would need to check nlock. - if(proc && proc->state == RUNNING && tf->trapno == T_IRQ0+IRQ_TIMER) + if(myproc() && myproc()->state == RUNNING && tf->trapno == T_IRQ0+IRQ_TIMER) yield(); // Check if the process has been killed since we yielded - if(proc && proc->killed && (tf->cs&3) == DPL_USER) + if(myproc() && myproc()->killed && (tf->cs&3) == DPL_USER) exit(); } diff --git a/vm.c b/vm.c index 9db8b67826..cb159ad841 100644 --- a/vm.c +++ b/vm.c @@ -21,21 +21,19 @@ seginit(void) // Cannot share a CODE descriptor for both kernel and user // because it would have to have DPL_USR, but the CPU forbids // an interrupt from CPL=0 to DPL=3. - c = &cpus[cpunum()]; + c = &cpus[lapiccpunum()]; c->gdt[SEG_KCODE] = SEG(STA_X|STA_R, 0, 0xffffffff, 0); c->gdt[SEG_KDATA] = SEG(STA_W, 0, 0xffffffff, 0); c->gdt[SEG_UCODE] = SEG(STA_X|STA_R, 0, 0xffffffff, DPL_USER); c->gdt[SEG_UDATA] = SEG(STA_W, 0, 0xffffffff, DPL_USER); - + c->cpu = c; // Map cpu and proc -- these are private per cpu. - c->gdt[SEG_KCPU] = SEG(STA_W, &c->cpu, 8, 0); - + c->gdt[SEG_KCPU] = SEG(STA_W, &c->cpu, 4, 0); lgdt(c->gdt, sizeof(c->gdt)); loadgs(SEG_KCPU << 3); - // Initialize cpu-local storage. - cpu = c; - proc = 0; + // setcpu(c); + setproc(0); } // Return the address of the PTE in page table pgdir @@ -171,13 +169,13 @@ switchuvm(struct proc *p) panic("switchuvm: no pgdir"); pushcli(); - cpu->gdt[SEG_TSS] = SEG16(STS_T32A, &cpu->ts, sizeof(cpu->ts)-1, 0); - cpu->gdt[SEG_TSS].s = 0; - cpu->ts.ss0 = SEG_KDATA << 3; - cpu->ts.esp0 = (uint)p->kstack + KSTACKSIZE; + mycpu()->gdt[SEG_TSS] = SEG16(STS_T32A, &mycpu()->ts, sizeof(mycpu()->ts)-1, 0); + mycpu()->gdt[SEG_TSS].s = 0; + mycpu()->ts.ss0 = SEG_KDATA << 3; + mycpu()->ts.esp0 = (uint)p->kstack + KSTACKSIZE; // setting IOPL=0 in eflags *and* iomb beyond the tss segment limit // forbids I/O instructions (e.g., inb and outb) from user space - cpu->ts.iomb = (ushort) 0xFFFF; + mycpu()->ts.iomb = (ushort) 0xFFFF; ltr(SEG_TSS << 3); lcr3(V2P(p->pgdir)); // switch to process's address space popcli(); From fbb4c0944422f860484142010bb9f366f3e87bf8 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Tue, 31 Jan 2017 20:21:14 -0500 Subject: [PATCH 2/7] Read curproc from cpu structure, but be careful because after a schedule event myproc() points to a different thread. myproc(); sched(); myproc(); // this proc maybe different than the one before sched Thus, in a function that operates on one thread better to retrieve the current process once at the start of the function. --- defs.h | 1 + exec.c | 15 ++++---- proc.c | 101 ++++++++++++++++++++++++++++++++++-------------------- proc.h | 3 ++ syscall.c | 23 ++++++++----- sysfile.c | 12 ++++--- vm.c | 4 +-- 7 files changed, 97 insertions(+), 62 deletions(-) diff --git a/defs.h b/defs.h index 67ea9f6cba..5e049aed51 100644 --- a/defs.h +++ b/defs.h @@ -108,6 +108,7 @@ void exit(void); int fork(void); int growproc(int); int kill(int); +struct proc* myproc(); void pinit(void); void procdump(void); void scheduler(void) __attribute__((noreturn)); diff --git a/exec.c b/exec.c index 14d673f12e..b40134f352 100644 --- a/exec.c +++ b/exec.c @@ -17,6 +17,7 @@ exec(char *path, char **argv) struct inode *ip; struct proghdr ph; pde_t *pgdir, *oldpgdir; + struct proc *curproc = myproc(); begin_op(); @@ -90,15 +91,15 @@ exec(char *path, char **argv) for(last=s=path; *s; s++) if(*s == '/') last = s+1; - safestrcpy(myproc()->name, last, sizeof(myproc()->name)); + safestrcpy(curproc->name, last, sizeof(curproc->name)); // Commit to the user image. - oldpgdir = myproc()->pgdir; - myproc()->pgdir = pgdir; - myproc()->sz = sz; - myproc()->tf->eip = elf.entry; // main - myproc()->tf->esp = sp; - switchuvm(myproc()); + oldpgdir = curproc->pgdir; + curproc->pgdir = pgdir; + curproc->sz = sz; + curproc->tf->eip = elf.entry; // main + curproc->tf->esp = sp; + switchuvm(curproc); freevm(oldpgdir); return 0; diff --git a/proc.c b/proc.c index bd62e4b97d..4e8f461a44 100644 --- a/proc.c +++ b/proc.c @@ -26,14 +26,23 @@ pinit(void) initlock(&ptable.lock, "ptable"); } +// XXX get rid off? int cpuid() { return mycpu()-cpus; } -void -setproc(struct proc* p) { - mycpu()->proc = p; +// Disable interrupts so that we are not rescheduled +// while reading proc from the cpu structure +struct proc* +myproc(void) { + struct cpu *c; + struct proc *p; + pushcli(); + c = mycpu(); + p = c->proc; + popcli(); + return p; } //PAGEBREAK: 32 @@ -130,17 +139,18 @@ int growproc(int n) { uint sz; + struct proc *curproc = myproc(); - sz = myproc()->sz; + sz = curproc->sz; if(n > 0){ - if((sz = allocuvm(myproc()->pgdir, sz, sz + n)) == 0) + if((sz = allocuvm(curproc->pgdir, sz, sz + n)) == 0) return -1; } else if(n < 0){ - if((sz = deallocuvm(myproc()->pgdir, sz, sz + n)) == 0) + if((sz = deallocuvm(curproc->pgdir, sz, sz + n)) == 0) return -1; } - myproc()->sz = sz; - switchuvm(myproc()); + curproc->sz = sz; + switchuvm(curproc); return 0; } @@ -152,6 +162,7 @@ fork(void) { int i, pid; struct proc *np; + struct proc *curproc = myproc(); // Allocate process. if((np = allocproc()) == 0){ @@ -159,25 +170,25 @@ fork(void) } // Copy process state from proc. - if((np->pgdir = copyuvm(myproc()->pgdir, myproc()->sz)) == 0){ + if((np->pgdir = copyuvm(curproc->pgdir, curproc->sz)) == 0){ kfree(np->kstack); np->kstack = 0; np->state = UNUSED; return -1; } - np->sz = myproc()->sz; - np->parent = myproc(); - *np->tf = *myproc()->tf; + np->sz = curproc->sz; + np->parent = curproc; + *np->tf = *curproc->tf; // Clear %eax so that fork returns 0 in the child. np->tf->eax = 0; for(i = 0; i < NOFILE; i++) - if(myproc()->ofile[i]) - np->ofile[i] = filedup(myproc()->ofile[i]); - np->cwd = idup(myproc()->cwd); + if(curproc->ofile[i]) + np->ofile[i] = filedup(curproc->ofile[i]); + np->cwd = idup(curproc->cwd); - safestrcpy(np->name, myproc()->name, sizeof(myproc()->name)); + safestrcpy(np->name, curproc->name, sizeof(curproc->name)); pid = np->pid; @@ -196,33 +207,34 @@ fork(void) void exit(void) { + struct proc *curproc = myproc(); struct proc *p; int fd; - if(myproc() == initproc) + if(curproc == initproc) panic("init exiting"); // Close all open files. for(fd = 0; fd < NOFILE; fd++){ - if(myproc()->ofile[fd]){ - fileclose(myproc()->ofile[fd]); - myproc()->ofile[fd] = 0; + if(curproc->ofile[fd]){ + fileclose(curproc->ofile[fd]); + curproc->ofile[fd] = 0; } } begin_op(); - iput(myproc()->cwd); + iput(curproc->cwd); end_op(); - myproc()->cwd = 0; + curproc->cwd = 0; acquire(&ptable.lock); // Parent might be sleeping in wait(). - wakeup1(myproc()->parent); + wakeup1(curproc->parent); // Pass abandoned children to init. for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){ - if(p->parent == myproc()){ + if(p->parent == curproc){ p->parent = initproc; if(p->state == ZOMBIE) wakeup1(initproc); @@ -230,7 +242,7 @@ exit(void) } // Jump into the scheduler, never to return. - myproc()->state = ZOMBIE; + curproc->state = ZOMBIE; sched(); panic("zombie exit"); } @@ -242,13 +254,14 @@ wait(void) { struct proc *p; int havekids, pid; - + struct proc *curproc = myproc(); + acquire(&ptable.lock); for(;;){ // Scan through table looking for exited children. havekids = 0; for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){ - if(p->parent != myproc()) + if(p->parent != curproc) continue; havekids = 1; if(p->state == ZOMBIE){ @@ -268,13 +281,13 @@ wait(void) } // No point waiting if we don't have any children. - if(!havekids || myproc()->killed){ + if(!havekids || curproc->killed){ release(&ptable.lock); return -1; } // Wait for children to exit. (See wakeup1 call in proc_exit.) - sleep(myproc(), &ptable.lock); //DOC: wait-sleep + sleep(curproc, &ptable.lock); //DOC: wait-sleep } } @@ -290,6 +303,7 @@ void scheduler(void) { struct proc *p; + struct cpu *c = mycpu(); for(;;){ // Enable interrupts on this processor. @@ -304,15 +318,18 @@ scheduler(void) // Switch to chosen process. It is the process's job // to release ptable.lock and then reacquire it // before jumping back to us. - setproc(p); + c->proc = p; switchuvm(p); p->state = RUNNING; - swtch(&(mycpu()->scheduler), p->context); + p->cpu = c; + // cprintf("%d: switch to %d\n", c-cpus, p->pid); + swtch(&(p->cpu->scheduler), p->context); switchkvm(); // Process is done running for now. // It should have changed its p->state before coming back. - setproc(0); + c->proc = 0; + p->cpu = 0; } release(&ptable.lock); @@ -330,17 +347,20 @@ void sched(void) { int intena; + struct proc *p = myproc(); if(!holding(&ptable.lock)) panic("sched ptable.lock"); if(mycpu()->ncli != 1) panic("sched locks"); - if(myproc()->state == RUNNING) + if(p->state == RUNNING) panic("sched running"); if(readeflags()&FL_IF) panic("sched interruptible"); intena = mycpu()->intena; - swtch(&myproc()->context, mycpu()->scheduler); + // cprintf("%d: before swtch %d %x\n", p->cpu-cpus, p->pid, * (int *) 0x1d); + swtch(&p->context, p->cpu->scheduler); + // cprintf("%d/%d: after swtch %d %x\n", cpuid(), p->cpu-cpus, p->pid, * (int *) 0x1d); mycpu()->intena = intena; } @@ -380,7 +400,9 @@ forkret(void) void sleep(void *chan, struct spinlock *lk) { - if(myproc() == 0) + struct proc *p = myproc(); + + if(p == 0) panic("sleep"); if(lk == 0) @@ -397,12 +419,15 @@ sleep(void *chan, struct spinlock *lk) release(lk); } // Go to sleep. - myproc()->chan = chan; - myproc()->state = SLEEPING; + p->chan = chan; + p->state = SLEEPING; + + // cprintf("sleep %d\n", p->pid); + sched(); // Tidy up. - myproc()->chan = 0; + p->chan = 0; // Reacquire original lock. if(lk != &ptable.lock){ //DOC: sleeplock2 diff --git a/proc.h b/proc.h index 38a2d32c0f..7047d54939 100644 --- a/proc.h +++ b/proc.h @@ -30,12 +30,14 @@ mycpu(void) { return cpu; } +#if 0 static inline struct proc* myproc(void) { struct proc *proc; asm("movl %%gs:4, %0" : "=r"(proc)); return proc; } +#endif //PAGEBREAK: 17 @@ -74,6 +76,7 @@ struct proc { struct file *ofile[NOFILE]; // Open files struct inode *cwd; // Current directory char name[16]; // Process name (debugging) + struct cpu *cpu; // If running, which cpu. }; // Process memory is laid out contiguously, low addresses first: diff --git a/syscall.c b/syscall.c index 2d6769e204..ee85261602 100644 --- a/syscall.c +++ b/syscall.c @@ -17,7 +17,9 @@ int fetchint(uint addr, int *ip) { - if(addr >= myproc()->sz || addr+4 > myproc()->sz) + struct proc *curproc = myproc(); + + if(addr >= curproc->sz || addr+4 > curproc->sz) return -1; *ip = *(int*)(addr); return 0; @@ -30,11 +32,12 @@ int fetchstr(uint addr, char **pp) { char *s, *ep; + struct proc *curproc = myproc(); - if(addr >= myproc()->sz) + if(addr >= curproc->sz) return -1; *pp = (char*)addr; - ep = (char*)myproc()->sz; + ep = (char*)curproc->sz; for(s = *pp; s < ep; s++){ if(*s == 0) return s - *pp; @@ -56,10 +59,11 @@ int argptr(int n, char **pp, int size) { int i; - + struct proc *curproc = myproc(); + if(argint(n, &i) < 0) return -1; - if(size < 0 || (uint)i >= myproc()->sz || (uint)i+size > myproc()->sz) + if(size < 0 || (uint)i >= curproc->sz || (uint)i+size > curproc->sz) return -1; *pp = (char*)i; return 0; @@ -128,13 +132,14 @@ void syscall(void) { int num; + struct proc *curproc = myproc(); - num = myproc()->tf->eax; + num = curproc->tf->eax; if(num > 0 && num < NELEM(syscalls) && syscalls[num]) { - myproc()->tf->eax = syscalls[num](); + curproc->tf->eax = syscalls[num](); } else { cprintf("%d %s: unknown sys call %d\n", - myproc()->pid, myproc()->name, num); - myproc()->tf->eax = -1; + curproc->pid, curproc->name, num); + curproc->tf->eax = -1; } } diff --git a/sysfile.c b/sysfile.c index fae69608b2..87e508b268 100644 --- a/sysfile.c +++ b/sysfile.c @@ -41,10 +41,11 @@ static int fdalloc(struct file *f) { int fd; + struct proc *curproc = myproc(); for(fd = 0; fd < NOFILE; fd++){ - if(myproc()->ofile[fd] == 0){ - myproc()->ofile[fd] = f; + if(curproc->ofile[fd] == 0){ + curproc->ofile[fd] = f; return fd; } } @@ -373,7 +374,8 @@ sys_chdir(void) { char *path; struct inode *ip; - + struct proc *curproc = myproc(); + begin_op(); if(argstr(0, &path) < 0 || (ip = namei(path)) == 0){ end_op(); @@ -386,9 +388,9 @@ sys_chdir(void) return -1; } iunlock(ip); - iput(myproc()->cwd); + iput(curproc->cwd); end_op(); - myproc()->cwd = ip; + curproc->cwd = ip; return 0; } diff --git a/vm.c b/vm.c index cb159ad841..d1640e8c3c 100644 --- a/vm.c +++ b/vm.c @@ -27,13 +27,11 @@ seginit(void) c->gdt[SEG_UCODE] = SEG(STA_X|STA_R, 0, 0xffffffff, DPL_USER); c->gdt[SEG_UDATA] = SEG(STA_W, 0, 0xffffffff, DPL_USER); c->cpu = c; + c->proc = 0; // Map cpu and proc -- these are private per cpu. c->gdt[SEG_KCPU] = SEG(STA_W, &c->cpu, 4, 0); lgdt(c->gdt, sizeof(c->gdt)); loadgs(SEG_KCPU << 3); - // Initialize cpu-local storage. - // setcpu(c); - setproc(0); } // Return the address of the PTE in page table pgdir From ed396c068b881877330f7d40bfce02db9b1199b3 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Wed, 1 Feb 2017 18:04:13 -0500 Subject: [PATCH 3/7] Eliminate code for gs trick to track per-cpu state. We rely on lapiccpunum() to find a per-cpu id with which we locate a cpu's cpu struct. --- defs.h | 1 + lapic.c | 14 ++------------ main.c | 2 +- mmu.h | 9 ++++----- proc.c | 34 +++++++++++++++++++++++----------- proc.h | 30 +----------------------------- trapasm.S | 3 --- vm.c | 7 +------ 8 files changed, 33 insertions(+), 67 deletions(-) diff --git a/defs.h b/defs.h index 5e049aed51..c4870d0c66 100644 --- a/defs.h +++ b/defs.h @@ -108,6 +108,7 @@ void exit(void); int fork(void); int growproc(int); int kill(int); +struct cpu* mycpu(void); struct proc* myproc(); void pinit(void); void procdump(void); diff --git a/lapic.c b/lapic.c index 9039665773..9a12f17d39 100644 --- a/lapic.c +++ b/lapic.c @@ -98,22 +98,12 @@ lapicinit(void) lapicw(TPR, 0); } +// Should be called with interrupts disabled: the calling thread shouldn't be +// rescheduled between reading lapic[ID] and checking against cpu array. int lapiccpunum(void) { int apicid, i; - - // Cannot call cpunum when interrupts are enabled: - // result not guaranteed to last long enough to be used! - // Would prefer to panic but even printing is chancy here: - // almost everything, including cprintf and panic, calls cpu, - // often indirectly through acquire and release. - if(readeflags()&FL_IF){ - static int n; - if(n++ == 0) - cprintf("cpunum called from %x with interrupts enabled\n", - __builtin_return_address(0)); - } if (!lapic) return 0; diff --git a/main.c b/main.c index d7e59cff0f..e5b7d6453e 100644 --- a/main.c +++ b/main.c @@ -53,7 +53,7 @@ mpenter(void) static void mpmain(void) { - cprintf("cpu%d: starting %d\n", cpuid(), lapiccpunum()); + cprintf("cpu%d: starting %d\n", cpuid(), cpuid()); idtinit(); // load idt register xchg(&(mycpu()->started), 1); // tell startothers() we're up scheduler(); // start running processes diff --git a/mmu.h b/mmu.h index e732ccdcfa..a1afa9f7d0 100644 --- a/mmu.h +++ b/mmu.h @@ -42,13 +42,12 @@ // various segment selectors. #define SEG_KCODE 1 // kernel code #define SEG_KDATA 2 // kernel data+stack -#define SEG_KCPU 3 // kernel per-cpu data -#define SEG_UCODE 4 // user code -#define SEG_UDATA 5 // user data+stack -#define SEG_TSS 6 // this process's task state +#define SEG_UCODE 3 // user code +#define SEG_UDATA 4 // user data+stack +#define SEG_TSS 5 // this process's task state // cpu->gdt[NSEGS] holds the above segments. -#define NSEGS 7 +#define NSEGS 6 //PAGEBREAK! #ifndef __ASSEMBLER__ diff --git a/proc.c b/proc.c index 4e8f461a44..6445725313 100644 --- a/proc.c +++ b/proc.c @@ -26,12 +26,29 @@ pinit(void) initlock(&ptable.lock, "ptable"); } -// XXX get rid off? +// Must be called with interrupts disabled int cpuid() { return mycpu()-cpus; } +// Must be called with interrupts disabled +struct cpu* +mycpu(void) +{ + // Would prefer to panic but even printing is chancy here: almost everything, + // including cprintf and panic, calls mycpu(), often indirectly through + // acquire and release. + if(readeflags()&FL_IF){ + static int n; + if(n++ == 0) + cprintf("mycpu called from %x with interrupts enabled\n", + __builtin_return_address(0)); + } + + return &cpus[lapiccpunum()]; +} + // Disable interrupts so that we are not rescheduled // while reading proc from the cpu structure struct proc* @@ -304,7 +321,8 @@ scheduler(void) { struct proc *p; struct cpu *c = mycpu(); - + c->proc = 0; + for(;;){ // Enable interrupts on this processor. sti(); @@ -321,15 +339,13 @@ scheduler(void) c->proc = p; switchuvm(p); p->state = RUNNING; - p->cpu = c; - // cprintf("%d: switch to %d\n", c-cpus, p->pid); - swtch(&(p->cpu->scheduler), p->context); + + swtch(&(c->scheduler), p->context); switchkvm(); // Process is done running for now. // It should have changed its p->state before coming back. c->proc = 0; - p->cpu = 0; } release(&ptable.lock); @@ -358,9 +374,7 @@ sched(void) if(readeflags()&FL_IF) panic("sched interruptible"); intena = mycpu()->intena; - // cprintf("%d: before swtch %d %x\n", p->cpu-cpus, p->pid, * (int *) 0x1d); - swtch(&p->context, p->cpu->scheduler); - // cprintf("%d/%d: after swtch %d %x\n", cpuid(), p->cpu-cpus, p->pid, * (int *) 0x1d); + swtch(&p->context, mycpu()->scheduler); mycpu()->intena = intena; } @@ -422,8 +436,6 @@ sleep(void *chan, struct spinlock *lk) p->chan = chan; p->state = SLEEPING; - // cprintf("sleep %d\n", p->pid); - sched(); // Tidy up. diff --git a/proc.h b/proc.h index 7047d54939..1647114179 100644 --- a/proc.h +++ b/proc.h @@ -7,39 +7,12 @@ struct cpu { volatile uint started; // Has the CPU started? int ncli; // Depth of pushcli nesting. int intena; // Were interrupts enabled before pushcli? - // Per-CPU variables, holding pointers to the current cpu and to the current - // process (see cpu() and proc() in proc.c) - struct cpu *cpu; // On cpu 0, cpu = &cpus[0]; on cpu 1, cpu=&cpus[1], etc. - struct proc *proc; // The currently-running process on this cpu + struct proc *proc; // The process running on this cpu or null }; extern struct cpu cpus[NCPU]; extern int ncpu; -// The asm suffix tells gcc to use "%gs:0" to refer to cpu -// and "%gs:4" to refer to proc. seginit sets up the -// %gs segment register so that %gs refers to the memory -// holding those two variables in the local cpu's struct cpu. -// This is similar to how thread-local variables are implemented -// in thread libraries such as Linux pthreads. - -static inline struct cpu* -mycpu(void) { - struct cpu *cpu; - asm("movl %%gs:0, %0" : "=r"(cpu)); - return cpu; -} - -#if 0 -static inline struct proc* -myproc(void) { - struct proc *proc; - asm("movl %%gs:4, %0" : "=r"(proc)); - return proc; -} -#endif - - //PAGEBREAK: 17 // Saved registers for kernel context switches. // Don't need to save all the segment registers (%cs, etc), @@ -76,7 +49,6 @@ struct proc { struct file *ofile[NOFILE]; // Open files struct inode *cwd; // Current directory char name[16]; // Process name (debugging) - struct cpu *cpu; // If running, which cpu. }; // Process memory is laid out contiguously, low addresses first: diff --git a/trapasm.S b/trapasm.S index 787727f75b..2271d27ca7 100644 --- a/trapasm.S +++ b/trapasm.S @@ -14,9 +14,6 @@ alltraps: movw $(SEG_KDATA<<3), %ax movw %ax, %ds movw %ax, %es - movw $(SEG_KCPU<<3), %ax - movw %ax, %fs - movw %ax, %gs # Call trap(tf), where tf=%esp pushl %esp diff --git a/vm.c b/vm.c index d1640e8c3c..9ac740190e 100644 --- a/vm.c +++ b/vm.c @@ -21,17 +21,12 @@ seginit(void) // Cannot share a CODE descriptor for both kernel and user // because it would have to have DPL_USR, but the CPU forbids // an interrupt from CPL=0 to DPL=3. - c = &cpus[lapiccpunum()]; + c = &cpus[cpuid()]; c->gdt[SEG_KCODE] = SEG(STA_X|STA_R, 0, 0xffffffff, 0); c->gdt[SEG_KDATA] = SEG(STA_W, 0, 0xffffffff, 0); c->gdt[SEG_UCODE] = SEG(STA_X|STA_R, 0, 0xffffffff, DPL_USER); c->gdt[SEG_UDATA] = SEG(STA_W, 0, 0xffffffff, DPL_USER); - c->cpu = c; - c->proc = 0; - // Map cpu and proc -- these are private per cpu. - c->gdt[SEG_KCPU] = SEG(STA_W, &c->cpu, 4, 0); lgdt(c->gdt, sizeof(c->gdt)); - loadgs(SEG_KCPU << 3); } // Return the address of the PTE in page table pgdir From 7c00ce8110e045a5d0b7b95194561b71d7c0d2b6 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Wed, 1 Feb 2017 19:18:47 -0500 Subject: [PATCH 4/7] shorten comment --- proc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/proc.c b/proc.c index 6445725313..9f500f2695 100644 --- a/proc.c +++ b/proc.c @@ -36,13 +36,9 @@ cpuid() { struct cpu* mycpu(void) { - // Would prefer to panic but even printing is chancy here: almost everything, - // including cprintf and panic, calls mycpu(), often indirectly through - // acquire and release. if(readeflags()&FL_IF){ - static int n; - if(n++ == 0) - cprintf("mycpu called from %x with interrupts enabled\n", + // Would prefer to panic but panic calls mycpu(). + cprintf("mycpu called from %x with interrupts enabled\n", __builtin_return_address(0)); } From 2e2d14c235b570a6beb222fc1bfa53de85a98de3 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Wed, 1 Feb 2017 19:21:43 -0500 Subject: [PATCH 5/7] use panic --- console.c | 3 ++- proc.c | 8 ++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/console.c b/console.c index f7e1e73942..da28f9f5a6 100644 --- a/console.c +++ b/console.c @@ -111,7 +111,8 @@ panic(char *s) cli(); cons.locking = 0; - cprintf("cpu %d: panic: ", cpuid()); + // use lapiccpunum so that we can call panic from mycpu() + cprintf("cpu %d: panic: ", lapiccpunum()); cprintf(s); cprintf("\n"); getcallerpcs(&s, pcs); diff --git a/proc.c b/proc.c index 9f500f2695..ca343cb36d 100644 --- a/proc.c +++ b/proc.c @@ -36,12 +36,8 @@ cpuid() { struct cpu* mycpu(void) { - if(readeflags()&FL_IF){ - // Would prefer to panic but panic calls mycpu(). - cprintf("mycpu called from %x with interrupts enabled\n", - __builtin_return_address(0)); - } - + if(readeflags()&FL_IF) + panic("mycpu called with interrupts enabled\n"); return &cpus[lapiccpunum()]; } From c9fa90f7e514f27fa1ac071cd9795f3830ab6a1b Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Wed, 1 Feb 2017 20:36:41 -0500 Subject: [PATCH 6/7] A tiny bit of clean up (e.g., move code searching cpu array from lapic.c into mycpu() in proc.c. --- console.c | 2 +- defs.h | 2 +- lapic.c | 15 ++------------- proc.c | 15 +++++++++++++-- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/console.c b/console.c index da28f9f5a6..ca00a5fa6a 100644 --- a/console.c +++ b/console.c @@ -112,7 +112,7 @@ panic(char *s) cli(); cons.locking = 0; // use lapiccpunum so that we can call panic from mycpu() - cprintf("cpu %d: panic: ", lapiccpunum()); + cprintf("lapicid %d: panic: ", lapicid()); cprintf(s); cprintf("\n"); getcallerpcs(&s, pcs); diff --git a/defs.h b/defs.h index c4870d0c66..82fb982837 100644 --- a/defs.h +++ b/defs.h @@ -74,7 +74,7 @@ void kbdintr(void); // lapic.c void cmostime(struct rtcdate *r); -int lapiccpunum(void); +int lapicid(void); extern volatile uint* lapic; void lapiceoi(void); void lapicinit(void); diff --git a/lapic.c b/lapic.c index 9a12f17d39..dc69eb6b94 100644 --- a/lapic.c +++ b/lapic.c @@ -9,7 +9,6 @@ #include "traps.h" #include "mmu.h" #include "x86.h" -#include "proc.h" // ncpu // Local APIC registers, divided by 4 for use as uint[] indices. #define ID (0x0020/4) // ID @@ -98,22 +97,12 @@ lapicinit(void) lapicw(TPR, 0); } -// Should be called with interrupts disabled: the calling thread shouldn't be -// rescheduled between reading lapic[ID] and checking against cpu array. int -lapiccpunum(void) +lapicid(void) { - int apicid, i; - if (!lapic) return 0; - - apicid = lapic[ID] >> 24; - for (i = 0; i < ncpu; ++i) { - if (cpus[i].apicid == apicid) - return i; - } - panic("unknown apicid\n"); + return lapic[ID] >> 24; } // Acknowledge interrupt. diff --git a/proc.c b/proc.c index ca343cb36d..aac752309a 100644 --- a/proc.c +++ b/proc.c @@ -32,13 +32,24 @@ cpuid() { return mycpu()-cpus; } -// Must be called with interrupts disabled +// Must be called with interrupts disabled to avoid the caller being rescheduled +// between reading lapicid and running through the loop. struct cpu* mycpu(void) { + int apicid, i; + if(readeflags()&FL_IF) panic("mycpu called with interrupts enabled\n"); - return &cpus[lapiccpunum()]; + + apicid = lapicid(); + // APIC IDs are not guaranteed to be contiguous. Maybe we should have + // a reverse map, or reserve a register to store &cpus[i]. + for (i = 0; i < ncpu; ++i) { + if (cpus[i].apicid == apicid) + return &cpus[i]; + } + panic("unknown apicid\n"); } // Disable interrupts so that we are not rescheduled From 825ce074b10a0e1f63fd3a1fe245220d04054e0a Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Mon, 7 Aug 2017 15:15:18 -0400 Subject: [PATCH 7/7] Remove some debugging statements --- fs.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/fs.c b/fs.c index 36b07d2898..96a0334a08 100644 --- a/fs.c +++ b/fs.c @@ -449,13 +449,6 @@ readi(struct inode *ip, char *dst, uint off, uint n) for(tot=0; totdev, bmap(ip, off/BSIZE)); m = min(n - tot, BSIZE - off%BSIZE); - /* - cprintf("data off %d:\n", off); - for (int j = 0; j < min(m, 10); j++) { - cprintf("%x ", bp->data[off%BSIZE+j]); - } - cprintf("\n"); - */ memmove(dst, bp->data + off%BSIZE, m); brelse(bp); }