提交 53ed1966 创建 作者: Robert Morris's avatar Robert Morris

remove a deadlock for namei(..) in namecache

read inode from disk in iget, not ilock
上级 3029f0e8
...@@ -205,6 +205,11 @@ iupdate(struct inode *ip) ...@@ -205,6 +205,11 @@ iupdate(struct inode *ip)
// Find the inode with number inum on device dev // Find the inode with number inum on device dev
// and return the in-memory copy. // and return the in-memory copy.
// The inode is not locked, so someone else might
// be modifying it.
// But it has a ref count, so it won't be freed or reused.
// Though unlocked, all fields will be present,
// so looking a ip->inum and ip->gen are OK even w/o lock.
struct inode* struct inode*
iget(uint dev, uint inum) iget(uint dev, uint inum)
{ {
...@@ -217,6 +222,8 @@ iget(uint dev, uint inum) ...@@ -217,6 +222,8 @@ iget(uint dev, uint inum)
for(ip = &icache.inode[0]; ip < &icache.inode[NINODE]; ip++){ for(ip = &icache.inode[0]; ip < &icache.inode[NINODE]; ip++){
if(ip->ref > 0 && ip->dev == dev && ip->inum == inum){ if(ip->ref > 0 && ip->dev == dev && ip->inum == inum){
ip->ref++; ip->ref++;
while((ip->flags & I_VALID) == 0)
cv_sleep(&ip->cv, &icache.lock);
release(&icache.lock); release(&icache.lock);
return ip; return ip;
} }
...@@ -224,7 +231,7 @@ iget(uint dev, uint inum) ...@@ -224,7 +231,7 @@ iget(uint dev, uint inum)
empty = ip; empty = ip;
} }
// Allocate fresh inode. // Allocate fresh inode cache slot.
if(empty == 0) if(empty == 0)
panic("iget: no inodes"); panic("iget: no inodes");
...@@ -232,9 +239,23 @@ iget(uint dev, uint inum) ...@@ -232,9 +239,23 @@ iget(uint dev, uint inum)
ip->dev = dev; ip->dev = dev;
ip->inum = inum; ip->inum = inum;
ip->ref = 1; ip->ref = 1;
ip->flags = 0; ip->flags = I_BUSY;
release(&icache.lock); release(&icache.lock);
struct buf *bp = bread(ip->dev, IBLOCK(ip->inum));
struct dinode *dip = (struct dinode*)bp->data + ip->inum%IPB;
ip->type = dip->type;
ip->major = dip->major;
ip->minor = dip->minor;
ip->nlink = dip->nlink;
ip->size = dip->size;
ip->gen = dip->gen;
memmove(ip->addrs, dip->addrs, sizeof(ip->addrs));
brelse(bp);
ip->flags |= I_VALID;
iunlock(ip);
return ip; return ip;
} }
...@@ -256,9 +277,6 @@ idup(struct inode *ip) ...@@ -256,9 +277,6 @@ idup(struct inode *ip)
void void
ilock(struct inode *ip) ilock(struct inode *ip)
{ {
struct buf *bp;
struct dinode *dip;
if(ip == 0 || ip->ref < 1) if(ip == 0 || ip->ref < 1)
panic("ilock"); panic("ilock");
...@@ -268,6 +286,10 @@ ilock(struct inode *ip) ...@@ -268,6 +286,10 @@ ilock(struct inode *ip)
ip->flags |= I_BUSY; ip->flags |= I_BUSY;
release(&icache.lock); release(&icache.lock);
if((ip->flags & I_VALID) == 0)
panic("ilock");
#if 0
if(!(ip->flags & I_VALID)){ if(!(ip->flags & I_VALID)){
bp = bread(ip->dev, IBLOCK(ip->inum)); bp = bread(ip->dev, IBLOCK(ip->inum));
dip = (struct dinode*)bp->data + ip->inum%IPB; dip = (struct dinode*)bp->data + ip->inum%IPB;
...@@ -281,6 +303,7 @@ ilock(struct inode *ip) ...@@ -281,6 +303,7 @@ ilock(struct inode *ip)
brelse(bp); brelse(bp);
ip->flags |= I_VALID; ip->flags |= I_VALID;
} }
#endif
} }
// Unlock the given inode. // Unlock the given inode.
......
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
// //
// to do: // to do:
// use ns.c namespaces // use ns.c namespaces
// does directory inum need to be locked around ns_lookup?
// maybe generation #s don't need to be in on-disk inode // maybe generation #s don't need to be in on-disk inode
// if namecache only referred to cached inodes // if namecache only referred to cached inodes
// insert when file created, not just looked up // insert when file created, not just looked up
...@@ -77,33 +76,30 @@ nc_lookup(struct inode *dir, char *name) ...@@ -77,33 +76,30 @@ nc_lookup(struct inode *dir, char *name)
// this is a bit ugly // this is a bit ugly
// maybe there should be a real inode cache and // maybe there should be a real inode cache and
// the namecache should contain direct refs. // the namecache should contain direct refs.
ilock(ip);
int ok = (ip->gen == gen); int ok = (ip->gen == gen);
iunlock(ip);
if(ok){ if(ok){
return ip; return ip;
} else { } else {
iput(ip); iput(ip);
return 0; return 0;
} }
} else } else {
return 0; return 0;
}
} }
// dir is locked, but ip is not. // 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); // though ip is not locked, it's still OK to
uint inum = ip->inum; // look at ip->gen.
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 != inum || e->gen != gen){ if(e->cinum != ip->inum || e->gen != ip->gen){
// someone forgot to call nc_invalidate() // someone forgot to call nc_invalidate()
panic("nc_insert change"); panic("nc_insert change");
} }
...@@ -119,8 +115,8 @@ nc_insert(struct inode *dir, char *name, struct inode *ip) ...@@ -119,8 +115,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 = inum; e->cinum = ip->inum;
e->gen = gen; e->gen = ip->gen;
break; break;
} }
} }
......
...@@ -165,7 +165,7 @@ userinit(void) ...@@ -165,7 +165,7 @@ userinit(void)
p->tf->eip = 0; // beginning of initcode.S p->tf->eip = 0; // beginning of initcode.S
safestrcpy(p->name, "initcode", sizeof(p->name)); safestrcpy(p->name, "initcode", sizeof(p->name));
p->cwd = namei("/"); p->cwd = 0; // forkret will fix in the process's context
acquire(&p->lock); acquire(&p->lock);
addrun(p); addrun(p);
release(&p->lock); release(&p->lock);
...@@ -573,12 +573,16 @@ forkret(void) ...@@ -573,12 +573,16 @@ forkret(void)
{ {
// Still holding proc->lock from scheduler. // Still holding proc->lock from scheduler.
release(&proc->lock); release(&proc->lock);
// just for the first process. can't do it earlier
// b/c file system code needs a process context
// in which to call cv_sleep().
if(proc->cwd == 0)
proc->cwd = namei("/");
// Return to "caller", actually trapret (see allocproc). // Return to "caller", actually trapret (see allocproc).
} }
// Kill the process with the given pid. // Kill the process with the given pid.
// Process won't exit until it returns // Process won't exit until it returns
// to user space (see trap in trap.c). // to user space (see trap in trap.c).
......
您添加了 0 到此讨论。请谨慎行事。
请先完成此评论的编辑!
注册 或者 后发表评论