namei returns locked parent dir inode for create / unlink

don't hold fd table lock across idecref() (latter does block i/o) idecref calls iput() in case last ref -> freeing inode dir size is 512 * # blocks, so readi/writei &c work unlink deletes dirent even if ip->nlink > 0

namei returns locked parent dir inode for create / unlink
211ff0c6 · rtm · c372e8dc · 211ff0c6 · 211ff0c6 · 211ff0c6
--- a/Notes
+++ b/Notes
@@ -365,3 +365,18 @@ two bugs in unlink: don't just return if nlink > 0,
  and search for name, not inum
 is there a create/create race for same file name?
  resulting in two entries w/ same name in directory?
+namei
+  return just inode
+  return offset in dir where found, w/ dir locked, for unlink
+  return dir locked, for mknod
+is the offset alone useful? how do I read/write it?
+test: one process unlinks a file while another links to it
+test: simultaneous create of same file
+test: one process opens a file while another deletes it
+oy, mkfs wants dir size to be last written entry, but i
+  want it to be nblocks*512
+  maybe fix kernel code to handle former
--- a/defs.h
+++ b/defs.h
@@ -116,7 +116,7 @@ void ilock(struct inode *ip);
 void iunlock(struct inode *ip);
 void idecref(struct inode *ip);
 void iput(struct inode *ip);
-struct inode * namei(char *path, uint *);
+struct inode * namei(char *path, int, uint *);
 void stati(struct inode *ip, struct stat *st);
 int readi(struct inode *ip, char *xdst, uint off, uint n);
 int writei(struct inode *ip, char *addr, uint off, uint n);

--- a/fd.c
+++ b/fd.c
@@ -108,18 +108,22 @@ fd_close(struct fd *fd)
    panic("fd_close");
  if(--fd->ref == 0){
-    if(fd->type == FD_PIPE){
+    struct fd dummy = *fd;
-      pipe_close(fd->pipe, fd->writeable);
-    } else if(fd->type == FD_FILE){
+    fd->ref = 0;
-      idecref(fd->ip);
+    fd->type = FD_CLOSED;
+    release(&fd_table_lock);
+    if(dummy.type == FD_PIPE){
+      pipe_close(dummy.pipe, dummy.writeable);
+    } else if(dummy.type == FD_FILE){
+      idecref(dummy.ip);
    } else {
      panic("fd_close");
    }
-    fd->ref = 0;
+  } else {
-    fd->type = FD_CLOSED;
+    release(&fd_table_lock);
  }
-  release(&fd_table_lock);
 }
 int

--- a/fs.c
+++ b/fs.c
@@ -286,14 +286,8 @@ iput(struct inode *ip)
 void
 idecref(struct inode *ip)
 {
-  acquire(&inode_table_lock);
+  ilock(ip);
+  iput(ip);
-  if(ip->count < 1)
-    panic("idecref");
-  ip->count -= 1;
-  release(&inode_table_lock);
 }
 void
@@ -376,34 +370,37 @@ writei(struct inode *ip, char *addr, uint off, uint n)
  }
 }
+// look up a path name, in one of three modes.
+// NAMEI_LOOKUP: return locked target inode.
+// NAMEI_CREATE: return locked parent inode.
+//   but return 0 if name does exist.
+// NAMEI_DELETE: return locked parent inode, offset of dirent in *ret_off.
+//   return 0 if name doesn't exist.
 struct inode *
-namei(char *path, uint *ret_pinum)
+namei(char *path, int mode, uint *ret_off)
 {
  struct inode *dp;
-  char *cp = path;
+  char *cp = path, *cp1;
  uint off, dev;
  struct buf *bp;
  struct dirent *ep;
-  int i;
+  int i, atend;
  unsigned ninum;
-  unsigned pinum;
  dp = iget(rootdev, 1);
-  pinum = dp->inum;
  while(*cp == '/')
    cp++;
  while(1){
-    if(*cp == '\0') {
+    if(*cp == '\0'){
-      if (ret_pinum)
+      if(mode == NAMEI_LOOKUP)
-	*ret_pinum = pinum;
+        return dp;
-      return dp;
+      iput(dp);
+      return 0;
    }
    if(dp->type != T_DIR){
-      if (ret_pinum)
-	*ret_pinum = pinum;
      iput(dp);
      return 0;
    }
@@ -419,6 +416,7 @@ namei(char *path, uint *ret_pinum)
          if(cp[i] != ep->name[i])
            break;
        if((cp[i] == '\0' || cp[i] == '/') && (i >= DIRSIZ || ep->name[i] == '\0')){
+          off += (uchar*)ep - bp->data;
          ninum = ep->inum;
          brelse(bp);
          cp += i;
@@ -427,14 +425,22 @@ namei(char *path, uint *ret_pinum)
      }
      brelse(bp);
    }
+    atend = 1;
+    for(cp1 = cp; *cp1; cp1++)
+      if(*cp1 == '/')
+        atend = 0;
+    if(mode == NAMEI_CREATE && atend)
+      return dp;
    iput(dp);
-    if (ret_pinum)
-      *ret_pinum = pinum;
    return 0;
  found:
+    if(mode == NAMEI_DELETE && *cp == '\0'){
+      *ret_off = off;
+      return dp;
+    }
    dev = dp->dev;
-    pinum = dp->inum;
    iput(dp);
    dp = iget(dev, ninum);
    if(dp->type == 0 || dp->nlink < 1)
@@ -453,6 +459,9 @@ wdir(struct inode *dp, char *name, uint ino)
  int i;
  int lb;
+  if(dp->size % BSIZE)
+    dp->size += (BSIZE - dp->size % BSIZE);
  for(off = 0; off < dp->size; off += BSIZE) {
    bp = bread(dp->dev, bmap(dp, off / BSIZE));
    for(ep = (struct dirent *) bp->data;
@@ -469,14 +478,16 @@ wdir(struct inode *dp, char *name, uint ino)
  }
  dp->addrs[lb] = balloc(dp->dev);
  bp = bread(dp->dev, dp->addrs[lb]);
+  memset(bp->data, 0, BSIZE);
  ep = (struct dirent *) (bp->data);
+  dp->size += BSIZE;
 found:
  ep->inum = ino;
  for(i = 0; i < DIRSIZ && name[i]; i++)
    ep->name[i] = name[i];
  for( ; i < DIRSIZ; i++)
    ep->name[i] = '\0';
-  dp->size += sizeof(struct dirent);
  bwrite (bp, bmap(dp, off/BSIZE));   // write directory block
  brelse(bp);
  iupdate(dp);
@@ -486,15 +497,12 @@ struct inode *
 mknod(char *cp, short type, short major, short minor)
 {
  struct inode *ip, *dp;
-  uint pinum = 0;
  cprintf("mknod: %s %d %d %d\n", cp, type, major, minor);
-  if ((ip = namei(cp, &pinum)) != 0) {
+  if ((dp = namei(cp, NAMEI_CREATE, 0)) == 0)
-    iput(ip);
    return 0;
-  }
-  dp = iget(rootdev, pinum);
  ip = ialloc(dp->dev, type);
  if (ip == 0) {
    iput(dp);
@@ -515,74 +523,81 @@ mknod(char *cp, short type, short major, short minor)
 int
 unlink(char *cp)
 {
-  struct inode *ip;
+  struct inode *ip, *dp;
-  struct inode *dp;
+  struct dirent de;
-  struct dirent *ep = 0;
+  uint off, inum, cc;
-  int off;
-  struct buf *bp = 0;
-  uint pinum;
-  if ((ip = namei(cp, &pinum)) == 0) {
+  cprintf("unlink(%s)\n", cp);
+  if ((dp = namei(cp, NAMEI_DELETE, &off)) == 0) {
    cprintf("unlink(%s) it doesn't exist\n", cp);
    return -1;
  }
-  ip->nlink--;
+  if((cc = readi(dp, (char*)&de, off, sizeof(de))) != sizeof(de) ||
-  if (ip->nlink > 0) {
+     de.inum == 0){
-    iput(ip);
+    cprintf("off %d dp->size %d cc %d de.inum %d",
-    return 0;
+            off, dp->size, cc, de.inum);
+    panic("unlink no entry");
  }
+  inum = de.inum;
+  cprintf("dinum %d off %d de %s/%d\n", 
+          dp->inum, off, de.name, de.inum);
-  dp = iget(rootdev, pinum);
+  memset(&de, 0, sizeof(de));
-  for(off = 0; off < dp->size; off += BSIZE) {
+  if(writei(dp, (char*)&de, off, sizeof(de)) != sizeof(de))
-    bp = bread(dp->dev, bmap(dp, off / BSIZE));
+    panic("unlink dir write");
-    for(ep = (struct dirent *) bp->data;
-	ep < (struct dirent *) (bp->data + BSIZE);
-	ep++){
-      if(ep->inum == ip->inum) {
-	goto found;
-      }
-    }
-    brelse(bp);
-  }
-  panic("unlink: entry doesn't exist\n");
- found:
+  iupdate(dp);
-  ep->inum = 0;
-  memset(ep->name, '\0', DIRSIZ);
-  bwrite (bp, bmap(dp, off/BSIZE));   // write directory block
-  brelse(bp);
-  dp->size -= sizeof(struct dirent);
-  iupdate (dp);
  iput(dp);
+  ip = iget(dp->dev, inum);
+  if(ip == 0)
+    panic("unlink no inode");
+  ip->nlink--;
+  iupdate(ip);
  iput(ip);
  return 0;
 }
 int
 link(char *name1, char *name2)
 {
-  struct inode *ip, *dp, *xip;
+  struct inode *ip, *dp;
-  uint pinum = 0;
  cprintf("link(%s, %s)\n", name1, name2);
-  if ((xip = namei(name2, &pinum)) != 0) {
+  if ((ip = namei(name1, NAMEI_LOOKUP, 0)) == 0){
-    cprintf("  failed %s exists\n", name2);
+    cprintf("  failed %s does not exist\n", name1);
-    iput(xip);
+    return -1;
+  }
+  if(ip->type == T_DIR){
+    cprintf("  failed %s is a dir\n", name1);
+    iput(ip);
    return -1;
  }
-  if ((ip = namei(name1, &pinum)) == 0){
+  iunlock(ip);
-    cprintf("  failed %s does not exist\n", name1);
+  if ((dp = namei(name2, NAMEI_CREATE, 0)) == 0) {
+    cprintf("  failed %s exists\n", name2);
+    idecref(ip);
+    return -1;
+  }
+  if(dp->dev != ip->dev){
+    cprintf("  cross-device link\n");
+    idecref(ip);
+    iput(dp);
    return -1;
  }
+  ilock(ip);
  ip->nlink += 1;
  iupdate (ip);
-  dp = iget(rootdev, pinum);
  wdir(dp, name2, ip->inum);
  iput(dp);
  iput(ip);

--- a/fsvar.h
+++ b/fsvar.h
@@ -14,3 +14,7 @@ struct inode {
 };
 extern uint rootdev;
+#define NAMEI_LOOKUP 1
+#define NAMEI_CREATE 2
+#define NAMEI_DELETE 3
--- a/syscall.c
+++ b/syscall.c
@@ -228,7 +228,7 @@ sys_open(void)
    return -1;
  if((l = checkstring(arg0)) < 0)
    return -1;
-  if((ip = namei(cp->mem + arg0, 0)) == 0) {
+  if((ip = namei(cp->mem + arg0, NAMEI_LOOKUP, 0)) == 0) {
    if (arg1 & O_CREATE) {
      if (l >= DIRSIZ)
 	return -1;
@@ -356,7 +356,7 @@ sys_exec(void)
    return -1;
  if(checkstring(arg0) < 0)
    return -1;
-  ip = namei(cp->mem + arg0, 0);
+  ip = namei(cp->mem + arg0, NAMEI_LOOKUP, 0);
  if(ip == 0)
    return -1;
@@ -494,7 +494,7 @@ sys_block(void)
          ip->type, ip->nlink, ip->size, ip->addrs[0]);
  iput(ip);
-  ip = namei(".././//./../usertests", 0);
+  ip = namei(".././//./../usertests", NAMEI_LOOKUP, 0);
  if(ip){
    cprintf("namei(usertests): %d %d %d %d %d %d %d %d\n",
            ip->dev, ip->inum, ip->count, ip->busy,

--- a/usertests.c
+++ b/usertests.c
@@ -231,7 +231,7 @@ void
 createdelete()
 {
  int pid, i, fd;
-  int n = 10; // for now, fit in one directory block
+  int n = 20;
  char name[32];
  pid = fork();