From 5053dd6a6d2b481bfcddbd91bacc885b9f0e0ff5 Mon Sep 17 00:00:00 2001
From: Robert Morris <rtm@csail.mit.edu>
Date: Mon, 15 Aug 2011 12:44:20 -0400
Subject: [PATCH] avoid deadlock by calling begin_trans() before locking any
 inodes

---
 file.c      |  6 ++++--
 fs.c        |  7 +++----
 log.c       |  4 ++--
 sysfile.c   | 14 ++++++++++----
 usertests.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++---
 5 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/file.c b/file.c
index 7101a50..f8a14ad 100644
--- a/file.c
+++ b/file.c
@@ -118,7 +118,6 @@ filewrite(struct file *f, char *addr, int n)
   if(f->type == FD_PIPE)
     return pipewrite(f->pipe, addr, n);
   if(f->type == FD_INODE){
-    ilock(f->ip);
     // write a few blocks at a time to avoid exceeding
     // the maximum log transaction size, including
     // i-node, indirect block, allocation blocks,
@@ -131,9 +130,13 @@ filewrite(struct file *f, char *addr, int n)
       int n1 = n - i;
       if(n1 > max)
         n1 = max;
+
       begin_trans();
+      ilock(f->ip);
       r = writei(f->ip, addr + i, f->off, n1);
+      iunlock(f->ip);
       commit_trans();
+
       if(r < 0)
         break;
       if(r != n1)
@@ -141,7 +144,6 @@ filewrite(struct file *f, char *addr, int n)
       f->off += r;
       i += r;
     }
-    iunlock(f->ip);
     return i == n ? n : -1;
   }
   panic("filewrite");
diff --git a/fs.c b/fs.c
index a76788b..fe9ce48 100644
--- a/fs.c
+++ b/fs.c
@@ -43,13 +43,13 @@ bzero(int dev, int bno)
   
   bp = bread(dev, bno);
   memset(bp->data, 0, BSIZE);
-  bwrite(bp);
+  log_write(bp);
   brelse(bp);
 }
 
 // Blocks. 
 
-// Allocate a disk block.
+// Allocate a zeroed disk block.
 static uint
 balloc(uint dev)
 {
@@ -67,6 +67,7 @@ balloc(uint dev)
         bp->data[bi/8] |= m;  // Mark block in use on disk.
         log_write(bp);
         brelse(bp);
+        bzero(dev, b + bi);
         return b + bi;
       }
     }
@@ -83,8 +84,6 @@ bfree(int dev, uint b)
   struct superblock sb;
   int bi, m;
 
-  bzero(dev, b);
-
   readsb(dev, &sb);
   bp = bread(dev, BBLOCK(b, sb.ninodes));
   bi = b % BPB;
diff --git a/log.c b/log.c
index db36ba9..e9a16c5 100644
--- a/log.c
+++ b/log.c
@@ -124,9 +124,9 @@ static void
 recover_from_log(void)
 {
   read_head();      
-  install_trans();  // Install all transactions till head
+  install_trans(); // if committed, copy from log to disk
   log.lh.n = 0;
-  write_head();     //  Reclaim log
+  write_head(); // clear the log
 }
 
 void
diff --git a/sysfile.c b/sysfile.c
index ca54013..c9d3594 100644
--- a/sysfile.c
+++ b/sysfile.c
@@ -116,14 +116,16 @@ sys_link(void)
     return -1;
   if((ip = namei(old)) == 0)
     return -1;
+
+  begin_trans();
+
   ilock(ip);
   if(ip->type == T_DIR){
     iunlockput(ip);
+    commit_trans();
     return -1;
   }
 
-  begin_trans();
-
   ip->nlink++;
   iupdate(ip);
   iunlock(ip);
@@ -180,16 +182,21 @@ sys_unlink(void)
     return -1;
   if((dp = nameiparent(path, name)) == 0)
     return -1;
+
+  begin_trans();
+
   ilock(dp);
 
   // Cannot unlink "." or "..".
   if(namecmp(name, ".") == 0 || namecmp(name, "..") == 0){
     iunlockput(dp);
+    commit_trans();
     return -1;
   }
 
   if((ip = dirlookup(dp, name, &off)) == 0){
     iunlockput(dp);
+    commit_trans();
     return -1;
   }
   ilock(ip);
@@ -199,11 +206,10 @@ sys_unlink(void)
   if(ip->type == T_DIR && !isdirempty(ip)){
     iunlockput(ip);
     iunlockput(dp);
+    commit_trans();
     return -1;
   }
 
-  begin_trans();
-
   memset(&de, 0, sizeof(de));
   if(writei(dp, (char*)&de, off, sizeof(de)) != sizeof(de))
     panic("unlink: writei");
diff --git a/usertests.c b/usertests.c
index ba648a7..8db8385 100644
--- a/usertests.c
+++ b/usertests.c
@@ -364,6 +364,8 @@ sharedfd(void)
   int fd, pid, i, n, nc, np;
   char buf[10];
 
+  printf(1, "sharedfd test\n");
+
   unlink("sharedfd");
   fd = open("sharedfd", O_CREATE|O_RDWR);
   if(fd < 0){
@@ -655,7 +657,7 @@ linktest(void)
   printf(1, "linktest ok\n");
 }
 
-// test concurrent create and unlink of the same file
+// test concurrent create/link/unlink of the same file
 void
 concreate(void)
 {
@@ -728,10 +730,15 @@ concreate(void)
     }
     if(((i % 3) == 0 && pid == 0) ||
        ((i % 3) == 1 && pid != 0)){
-      fd = open(file, 0);
-      close(fd);
+      close(open(file, 0));
+      close(open(file, 0));
+      close(open(file, 0));
+      close(open(file, 0));
     } else {
       unlink(file);
+      unlink(file);
+      unlink(file);
+      unlink(file);
     }
     if(pid == 0)
       exit();
@@ -742,6 +749,42 @@ concreate(void)
   printf(1, "concreate ok\n");
 }
 
+// another concurrent link/unlink/create test,
+// to look for deadlocks.
+void
+linkunlink()
+{
+  int pid, i;
+
+  printf(1, "linkunlink test\n");
+
+  unlink("x");
+  pid = fork();
+  if(pid < 0){
+    printf(1, "fork failed\n");
+    exit();
+  }
+
+  unsigned int x = (pid ? 1 : 97);
+  for(i = 0; i < 100; i++){
+    x = x * 1103515245 + 12345;
+    if((x % 3) == 0){
+      close(open("x", O_RDWR | O_CREATE));
+    } else if((x % 3) == 1){
+      link("cat", "x");
+    } else {
+      unlink("x");
+    }
+  }
+
+  if(pid)
+    wait();
+  else 
+    exit();
+
+  printf(1, "linkunlink ok\n");
+}
+
 // directory that uses indirect blocks
 void
 bigdir(void)
@@ -1518,6 +1561,7 @@ main(int argc, char *argv[])
   bigfile();
   subdir();
   concreate();
+  linkunlink();
   linktest();
   unlinkread();
   createdelete();