提交 3029f0e8 创建 作者: Robert Morris's avatar Robert Morris

inode generation #s to avoid race in namecache lookup

fix some races by having ialloc go thru inode cache
上级 cd61c6de
...@@ -222,7 +222,7 @@ ifndef CPUS ...@@ -222,7 +222,7 @@ ifndef CPUS
CPUS := 2 CPUS := 2
endif endif
QEMUOPTS = -hdb fs.img xv6.img -smp $(CPUS) QEMUOPTS = -hdb fs.img xv6.img -smp $(CPUS)
MTRACEOPTS = -mtrace-enable -mtrace-file mtrace.out -mtrace-quantum 10000 MTRACEOPTS = -mtrace-enable -mtrace-file mtrace.out -mtrace-quantum 1000
qemu: fs.img xv6.img qemu: fs.img xv6.img
$(QEMU) -serial mon:stdio $(QEMUOPTS) $(QEMU) -serial mon:stdio $(QEMUOPTS)
......
...@@ -8,6 +8,9 @@ ...@@ -8,6 +8,9 @@
#include "defs.h" #include "defs.h"
#include "x86.h" #include "x86.h"
#include "elf.h" #include "elf.h"
#include "stat.h"
#include "fs.h"
#include "file.h"
int int
exec(char *path, char **argv) exec(char *path, char **argv)
...@@ -27,6 +30,8 @@ exec(char *path, char **argv) ...@@ -27,6 +30,8 @@ exec(char *path, char **argv)
ilock(ip); ilock(ip);
// Check ELF header // Check ELF header
if(ip->type != T_FILE)
goto bad;
if(readi(ip, (char*)&elf, 0, sizeof(elf)) < sizeof(elf)) if(readi(ip, (char*)&elf, 0, sizeof(elf)) < sizeof(elf))
goto bad; goto bad;
if(elf.magic != ELF_MAGIC) if(elf.magic != ELF_MAGIC)
......
...@@ -79,6 +79,8 @@ filestat(struct file *f, struct stat *st) ...@@ -79,6 +79,8 @@ filestat(struct file *f, struct stat *st)
{ {
if(f->type == FD_INODE){ if(f->type == FD_INODE){
ilock(f->ip); ilock(f->ip);
if(f->ip->type == 0)
panic("filestat");
stati(f->ip, st); stati(f->ip, st);
iunlock(f->ip); iunlock(f->ip);
return 0; return 0;
...@@ -98,6 +100,8 @@ fileread(struct file *f, char *addr, int n) ...@@ -98,6 +100,8 @@ fileread(struct file *f, char *addr, int n)
return piperead(f->pipe, addr, n); return piperead(f->pipe, addr, n);
if(f->type == FD_INODE){ if(f->type == FD_INODE){
ilock(f->ip); ilock(f->ip);
if(f->ip->type == 0)
panic("fileread");
if((r = readi(f->ip, addr, f->off, n)) > 0) if((r = readi(f->ip, addr, f->off, n)) > 0)
f->off += r; f->off += r;
iunlock(f->ip); iunlock(f->ip);
......
...@@ -14,6 +14,7 @@ struct file { ...@@ -14,6 +14,7 @@ struct file {
struct inode { struct inode {
uint dev; // Device number uint dev; // Device number
uint inum; // Inode number uint inum; // Inode number
uint gen; // Generation number
int ref; // Reference count int ref; // Reference count
int flags; // I_BUSY, I_VALID int flags; // I_BUSY, I_VALID
struct condvar cv; struct condvar cv;
......
...@@ -148,6 +148,7 @@ iinit(void) ...@@ -148,6 +148,7 @@ iinit(void)
//PAGEBREAK! //PAGEBREAK!
// Allocate a new inode with the given type on device dev. // Allocate a new inode with the given type on device dev.
// Returns a locked inode.
struct inode* struct inode*
ialloc(uint dev, short type) ialloc(uint dev, short type)
{ {
...@@ -160,14 +161,24 @@ ialloc(uint dev, short type) ...@@ -160,14 +161,24 @@ ialloc(uint dev, short type)
for(inum = 1; inum < sb.ninodes; inum++){ // loop over inode blocks for(inum = 1; inum < sb.ninodes; inum++){ // loop over inode blocks
bp = bread(dev, IBLOCK(inum)); bp = bread(dev, IBLOCK(inum));
dip = (struct dinode*)bp->data + inum%IPB; dip = (struct dinode*)bp->data + inum%IPB;
if(dip->type == 0){ // a free inode int seemsfree = (dip->type == 0);
memset(dip, 0, sizeof(*dip));
dip->type = type;
bwrite(bp); // mark it allocated on the disk
brelse(bp); brelse(bp);
return iget(dev, inum); if(seemsfree){
// maybe this inode is free. look at it via the
// inode cache to make sure.
struct inode *ip = iget(dev, inum);
ilock(ip);
if(ip->type == 0){
ip->type = type;
ip->gen += 1;
if(ip->nlink || ip->size || ip->addrs[0])
panic("ialloc not zeroed");
iupdate(ip);
return ip;
}
iunlockput(ip);
cprintf("ialloc oops %d\n", inum); // XXX harmless
} }
brelse(bp);
} }
panic("ialloc: no inodes"); panic("ialloc: no inodes");
} }
...@@ -186,6 +197,7 @@ iupdate(struct inode *ip) ...@@ -186,6 +197,7 @@ iupdate(struct inode *ip)
dip->minor = ip->minor; dip->minor = ip->minor;
dip->nlink = ip->nlink; dip->nlink = ip->nlink;
dip->size = ip->size; dip->size = ip->size;
dip->gen = ip->gen;
memmove(dip->addrs, ip->addrs, sizeof(ip->addrs)); memmove(dip->addrs, ip->addrs, sizeof(ip->addrs));
bwrite(bp); bwrite(bp);
brelse(bp); brelse(bp);
...@@ -238,6 +250,9 @@ idup(struct inode *ip) ...@@ -238,6 +250,9 @@ idup(struct inode *ip)
} }
// Lock the given inode. // Lock the given inode.
// XXX why does ilock() read the inode from disk?
// why doesn't the iget() that allocated the inode cache entry
// read the inode from disk?
void void
ilock(struct inode *ip) ilock(struct inode *ip)
{ {
...@@ -261,11 +276,10 @@ ilock(struct inode *ip) ...@@ -261,11 +276,10 @@ ilock(struct inode *ip)
ip->minor = dip->minor; ip->minor = dip->minor;
ip->nlink = dip->nlink; ip->nlink = dip->nlink;
ip->size = dip->size; ip->size = dip->size;
ip->gen = dip->gen;
memmove(ip->addrs, dip->addrs, sizeof(ip->addrs)); memmove(ip->addrs, dip->addrs, sizeof(ip->addrs));
brelse(bp); brelse(bp);
ip->flags |= I_VALID; ip->flags |= I_VALID;
if(ip->type == 0)
panic("ilock: no type");
} }
} }
...@@ -295,6 +309,9 @@ iput(struct inode *ip) ...@@ -295,6 +309,9 @@ iput(struct inode *ip)
release(&icache.lock); release(&icache.lock);
itrunc(ip); itrunc(ip);
ip->type = 0; ip->type = 0;
ip->major = 0;
ip->minor = 0;
ip->gen += 1;
iupdate(ip); iupdate(ip);
acquire(&icache.lock); acquire(&icache.lock);
ip->flags = 0; ip->flags = 0;
...@@ -590,6 +607,8 @@ namex(char *path, int nameiparent, char *name) ...@@ -590,6 +607,8 @@ namex(char *path, int nameiparent, char *name)
next = nc_lookup(ip, name); next = nc_lookup(ip, name);
if(next == 0){ if(next == 0){
ilock(ip); ilock(ip);
if(ip->type == 0)
panic("namex");
if(ip->type != T_DIR){ if(ip->type != T_DIR){
iunlockput(ip); iunlockput(ip);
return 0; return 0;
......
...@@ -15,17 +15,19 @@ struct superblock { ...@@ -15,17 +15,19 @@ struct superblock {
uint ninodes; // Number of inodes. uint ninodes; // Number of inodes.
}; };
#define NDIRECT 12 #define NDIRECT 11
#define NINDIRECT (BSIZE / sizeof(uint)) #define NINDIRECT (BSIZE / sizeof(uint))
#define MAXFILE (NDIRECT + NINDIRECT) #define MAXFILE (NDIRECT + NINDIRECT)
// On-disk inode structure // On-disk inode structure
// (512 % sizeof(dinode)) == 0
struct dinode { struct dinode {
short type; // File type short type; // File type
short major; // Major device number (T_DEV only) short major; // Major device number (T_DEV only)
short minor; // Minor device number (T_DEV only) short minor; // Minor device number (T_DEV only)
short nlink; // Number of links to inode in file system short nlink; // Number of links to inode in file system
uint size; // Size of file (bytes) uint size; // Size of file (bytes)
uint gen; // Generation # (to check name cache)
uint addrs[NDIRECT+1]; // Data block addresses uint addrs[NDIRECT+1]; // Data block addresses
}; };
......
...@@ -218,6 +218,7 @@ ialloc(ushort type) ...@@ -218,6 +218,7 @@ ialloc(ushort type)
din.type = xshort(type); din.type = xshort(type);
din.nlink = xshort(1); din.nlink = xshort(1);
din.size = xint(0); din.size = xint(0);
din.gen = 1;
winode(inum, &din); winode(inum, &din);
return inum; return inum;
} }
......
...@@ -4,11 +4,11 @@ ...@@ -4,11 +4,11 @@
// to do: // to do:
// use ns.c namespaces // use ns.c namespaces
// does directory inum need to be locked around ns_lookup? // does directory inum need to be locked around ns_lookup?
// need a lock to make table lookup and iget atomic? // maybe generation #s don't need to be in on-disk inode
// unlink/lookup race? // if namecache only referred to cached inodes
// better: inode generation #
// insert when file created, not just looked up // insert when file created, not just looked up
// eviction // eviction
// verify that it hits when it is supposed to
// //
#include "types.h" #include "types.h"
...@@ -33,6 +33,7 @@ struct nce { ...@@ -33,6 +33,7 @@ struct nce {
uint dinum; // directory inumber uint dinum; // directory inumber
char name[DIRSIZ]; char name[DIRSIZ];
uint cinum; // child inumber uint cinum; // child inumber
uint gen; // child ip->gen
}; };
#define NCE 128 #define NCE 128
struct nce nce[NCE]; struct nce nce[NCE];
...@@ -61,28 +62,51 @@ struct inode * ...@@ -61,28 +62,51 @@ struct inode *
nc_lookup(struct inode *dir, char *name) nc_lookup(struct inode *dir, char *name)
{ {
uint inum = 0; uint inum = 0;
uint gen = 0;
acquire(&nc_lock); acquire(&nc_lock);
struct nce *e = nc_lookup1(dir, name); struct nce *e = nc_lookup1(dir, name);
if(e) if(e){
inum = e->cinum; inum = e->cinum;
gen = e->gen;
}
release(&nc_lock); release(&nc_lock);
if(inum) if(inum){
return iget(dir->dev, inum); struct inode *ip = iget(dir->dev, inum);
else // this is a bit ugly
// maybe there should be a real inode cache and
// the namecache should contain direct refs.
ilock(ip);
int ok = (ip->gen == gen);
iunlock(ip);
if(ok){
return ip;
} else {
iput(ip);
return 0;
}
} else
return 0; return 0;
} }
// dir is locked, but ip is not.
void void
nc_insert(struct inode *dir, char *name, struct inode *ip) nc_insert(struct inode *dir, char *name, struct inode *ip)
{ {
ilock(ip);
uint inum = ip->inum;
uint gen = ip->gen;
iunlock(ip);
acquire(&nc_lock); acquire(&nc_lock);
struct nce *e = nc_lookup1(dir, name); struct nce *e = nc_lookup1(dir, name);
if(e){ if(e){
if(e->cinum != ip->inum) if(e->cinum != inum || e->gen != gen){
// someone forgot to call nc_invalidate()
panic("nc_insert change"); panic("nc_insert change");
}
release(&nc_lock); release(&nc_lock);
return; return;
} }
...@@ -95,7 +119,8 @@ nc_insert(struct inode *dir, char *name, struct inode *ip) ...@@ -95,7 +119,8 @@ nc_insert(struct inode *dir, char *name, struct inode *ip)
e->dev = dir->dev; e->dev = dir->dev;
e->dinum = dir->inum; e->dinum = dir->inum;
strncpy(e->name, name, DIRSIZ); strncpy(e->name, name, DIRSIZ);
e->cinum = ip->inum; e->cinum = inum;
e->gen = gen;
break; break;
} }
} }
......
...@@ -177,6 +177,8 @@ sys_unlink(void) ...@@ -177,6 +177,8 @@ sys_unlink(void)
if((dp = nameiparent(path, name)) == 0) if((dp = nameiparent(path, name)) == 0)
return -1; return -1;
ilock(dp); ilock(dp);
if(dp->type != T_DIR)
panic("sys_unlink");
// Cannot unlink "." or "..". // Cannot unlink "." or "..".
if(namecmp(name, ".") == 0 || namecmp(name, "..") == 0){ if(namecmp(name, ".") == 0 || namecmp(name, "..") == 0){
...@@ -205,6 +207,7 @@ sys_unlink(void) ...@@ -205,6 +207,7 @@ sys_unlink(void)
dp->nlink--; dp->nlink--;
iupdate(dp); iupdate(dp);
} }
nc_invalidate(dp, name); nc_invalidate(dp, name);
iunlockput(dp); iunlockput(dp);
...@@ -224,6 +227,8 @@ create(char *path, short type, short major, short minor) ...@@ -224,6 +227,8 @@ create(char *path, short type, short major, short minor)
if((dp = nameiparent(path, name)) == 0) if((dp = nameiparent(path, name)) == 0)
return 0; return 0;
ilock(dp); ilock(dp);
if(dp->type != T_DIR)
panic("create");
if((ip = dirlookup(dp, name, &off)) != 0){ if((ip = dirlookup(dp, name, &off)) != 0){
iunlockput(dp); iunlockput(dp);
...@@ -237,7 +242,6 @@ create(char *path, short type, short major, short minor) ...@@ -237,7 +242,6 @@ create(char *path, short type, short major, short minor)
if((ip = ialloc(dp->dev, type)) == 0) if((ip = ialloc(dp->dev, type)) == 0)
panic("create: ialloc"); panic("create: ialloc");
ilock(ip);
ip->major = major; ip->major = major;
ip->minor = minor; ip->minor = minor;
ip->nlink = 1; ip->nlink = 1;
...@@ -275,6 +279,8 @@ sys_open(void) ...@@ -275,6 +279,8 @@ sys_open(void)
if((ip = namei(path)) == 0) if((ip = namei(path)) == 0)
return -1; return -1;
ilock(ip); ilock(ip);
if(ip->type == 0)
panic("open");
if(ip->type == T_DIR && omode != O_RDONLY){ if(ip->type == T_DIR && omode != O_RDONLY){
iunlockput(ip); iunlockput(ip);
return -1; return -1;
......
...@@ -1567,7 +1567,7 @@ main(int argc, char *argv[]) ...@@ -1567,7 +1567,7 @@ main(int argc, char *argv[])
} }
close(open("usertests.ran", O_CREATE)); close(open("usertests.ran", O_CREATE));
// unopentest(); unopentest();
bigargtest(); bigargtest();
bsstest(); bsstest();
sbrktest(); sbrktest();
......
您添加了 0 到此讨论。请谨慎行事。
请先完成此评论的编辑!
注册 或者 后发表评论