From bb207a1d42c9a75a64af35de4f12912176ebfc8d Mon Sep 17 00:00:00 2001
From: rsc <rsc>
Date: Thu, 7 Sep 2006 14:28:12 +0000
Subject: [PATCH] comments

---
 bio.c  | 41 ++++++++++++++++++++++++++++++++----
 file.c | 11 +++++-----
 fs.c   | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
 ide.c  |  7 +++++++
 4 files changed, 109 insertions(+), 16 deletions(-)

diff --git a/bio.c b/bio.c
index ab5f41f..dadd1f7 100644
--- a/bio.c
+++ b/bio.c
@@ -1,3 +1,29 @@
+// Buffer cache.
+//
+// The buffer cache is a linked list of buf structures
+// holding cached copies of disk block contents.
+// Each buf has two state bits B_BUSY and B_VALID.
+// If B_BUSY is set, it means that some code is currently
+// editing buf, so other code is not allowed to look at it.
+// To wait for a buffer that is B_BUSY, sleep on buf.
+// (See bget below.)
+// 
+// If B_VALID is set, it means that the memory contents
+// have been initialized by reading them off the disk.
+// (Conversely, if B_VALID is not set, the memory contents
+// of buf must be initialized, often by calling bread,
+// before being used.)
+// 
+// After making changes to a buf's memory, call bwrite to flush
+// the changes out to disk, to keep the disk and memory copies
+// in sync.
+//
+// When finished with a buffer, call brelse to release the buffer
+// (i.e., clear B_BUSY), so that others can access it.
+//
+// Bufs that are not B_BUSY are fair game for reuse for other
+// disk blocks.  It is not allowed to use a buf after calling brelse.
+
 #include "types.h"
 #include "param.h"
 #include "x86.h"
@@ -10,7 +36,7 @@
 struct buf buf[NBUF];
 struct spinlock buf_table_lock;
 
-// linked list of all buffers, through prev/next.
+// Linked list of all buffers, through prev/next.
 // bufhead->next is most recently used.
 // bufhead->tail is least recently used.
 struct buf bufhead;
@@ -22,6 +48,7 @@ binit(void)
 
   initlock(&buf_table_lock, "buf_table");
 
+  // Create linked list of buffers
   bufhead.prev = &bufhead;
   bufhead.next = &bufhead;
   for(b = buf; b < buf+NBUF; b++){
@@ -32,7 +59,10 @@ binit(void)
   }
 }
 
-struct buf*
+// Look through buffer cache for block n on device dev.
+// If not found, allocate fresh block.
+// In either case, return locked buffer.
+static struct buf*
 getblk(uint dev, uint sector)
 {
   struct buf *b;
@@ -63,11 +93,12 @@ getblk(uint dev, uint sector)
           return b;
         }
       }
-      panic("getblk: no buffers");
+      panic("bget: no buffers");
     }
   }
 }
 
+// Read buf's contents from disk.
 struct buf*
 bread(uint dev, uint sector)
 {
@@ -75,7 +106,7 @@ bread(uint dev, uint sector)
   struct buf *b;
   extern struct spinlock ide_lock;
 
-  b = getblk(dev, sector);
+  b = bget(dev, sector);
   if(b->flags & B_VALID)
     return b;
 
@@ -89,6 +120,7 @@ bread(uint dev, uint sector)
   return b;
 }
 
+// Write buf's contents to disk.
 void
 bwrite(struct buf *b, uint sector)
 {
@@ -103,6 +135,7 @@ bwrite(struct buf *b, uint sector)
   release(&ide_lock);
 }
 
+// Release the buffer buf.
 void
 brelse(struct buf *b)
 {
diff --git a/file.c b/file.c
index 4c314ec..95a249b 100644
--- a/file.c
+++ b/file.c
@@ -41,8 +41,7 @@ filealloc(void)
   return 0;
 }
 
-// Write to file descriptor;
-// addr is a kernel address, pointing into some process's p->mem.
+// Write to file f.  Addr is kernel address.
 int
 filewrite(struct file *fd, char *addr, int n)
 {
@@ -64,7 +63,7 @@ filewrite(struct file *fd, char *addr, int n)
   }
 }
 
-// Read from file descriptor.
+// Read from file f.  Addr is kernel address.
 int
 fileread(struct file *fd, char *addr, int n)
 {
@@ -85,7 +84,7 @@ fileread(struct file *fd, char *addr, int n)
   }
 }
 
-// Close file descriptor.
+// Close file f.  (Decrement ref count, close when reaches 0.)
 void
 fileclose(struct file *fd)
 {
@@ -113,7 +112,7 @@ fileclose(struct file *fd)
   }
 }
 
-// Get metadata about file descriptor.
+// Get metadata about file f.
 int
 filestat(struct file *fd, struct stat *st)
 {
@@ -126,7 +125,7 @@ filestat(struct file *fd, struct stat *st)
     return -1;
 }
 
-// Increment file descriptor reference count.
+// Increment ref count for file f.
 void
 fileincref(struct file *fd)
 {
diff --git a/fs.c b/fs.c
index 317a9e2..341a586 100644
--- a/fs.c
+++ b/fs.c
@@ -11,8 +11,24 @@
 #include "fsvar.h"
 #include "dev.h"
 
-// these are inodes currently in use
-// an entry is free if count == 0
+// Inode table.  The inode table is an in-memory cache of the 
+// on-disk inode structures.  If an inode in the table has a non-zero
+// reference count, then some open files refer to it and it must stay
+// in memory.  If an inode has a zero reference count, it is only in
+// memory as a cache in hopes of being used again (avoiding a disk read).
+// Any inode with reference count zero can be evicted from the table.
+// 
+// In addition to having a reference count, inodes can be marked busy
+// (just like bufs), meaning that some code has logically locked the 
+// inode, and others are not allowed to look at it. 
+// This locking can last for a long
+// time (for example, if the inode is busy during a disk access),
+// so we don't use spin locks.  Instead, if a process wants to use
+// a particular inode, it must sleep(ip) to wait for it to be not busy.
+// See iget below.
+//
+// XXX Inodes with dev == 0 exist only in memory.  They have no on-disk
+// representation.  This functionality is used to implement pipes.
 struct inode inode[NINODE];
 struct spinlock inode_table_lock;
 
@@ -61,6 +77,7 @@ balloc(uint dev)
   return b;
 }
 
+// Free a disk block.
 static void
 bfree(int dev, uint b)
 {
@@ -108,6 +125,10 @@ iget(uint dev, uint inum)
     if(ip->count > 0 && ip->dev == dev && ip->inum == inum){
       if(ip->busy){
         sleep(ip, &inode_table_lock);
+        // Since we droped inode_table_lock, ip might have been reused
+        // for some other inode entirely.  Must start the scan over,
+        // and hopefully this time we will find the inode we want 
+        // and it will not be busy.
         goto loop;
       }
       ip->count++;
@@ -142,6 +163,8 @@ iget(uint dev, uint inum)
   return nip;
 }
 
+// Copy ip->d, which has changed, to disk.
+// Caller must have locked ip.
 void
 iupdate(struct inode *ip)
 {
@@ -160,6 +183,8 @@ iupdate(struct inode *ip)
   brelse(bp);
 }
 
+// Allocate a new inode with the given type
+// from the file system on device dev.
 struct inode*
 ialloc(uint dev, short type)
 {
@@ -195,6 +220,7 @@ ialloc(uint dev, short type)
   return ip;
 }
 
+// Free the given inode from its file system.
 static void
 ifree(struct inode *ip)
 {
@@ -202,6 +228,11 @@ ifree(struct inode *ip)
   iupdate(ip);
 }
 
+// Lock the given inode (wait for it to be not busy,
+// and then ip->busy).  
+// Caller must already hold a reference to ip.
+// Otherwise, if all the references to ip go away,
+// it might be reused underfoot.
 void
 ilock(struct inode *ip)
 {
@@ -217,8 +248,9 @@ ilock(struct inode *ip)
   release(&inode_table_lock);
 }
 
-// caller is holding onto a reference to this inode, but no
-// longer needs to examine or change it, so clear ip->busy.
+// Caller holds reference to ip and has locked it.
+// Caller no longer needs to examine / change it.
+// Unlock it, but keep the reference.
 void
 iunlock(struct inode *ip)
 {
@@ -233,6 +265,7 @@ iunlock(struct inode *ip)
   release(&inode_table_lock);
 }
 
+// Return the disk block address of the nth block in inode ip.
 uint
 bmap(struct inode *ip, uint bn)
 {
@@ -259,6 +292,7 @@ bmap(struct inode *ip, uint bn)
   return x;
 }
 
+// Truncate the inode ip, discarding all its data blocks.
 void
 itrunc(struct inode *ip)
 {
@@ -286,8 +320,9 @@ itrunc(struct inode *ip)
   iupdate(ip);
 }
 
-// caller is releasing a reference to this inode.
-// you must have the inode lock.
+// Caller holds reference to ip and has locked it,
+// possibly editing it.
+// Release lock and drop the reference.
 void
 iput(struct inode *ip)
 {
@@ -308,6 +343,8 @@ iput(struct inode *ip)
   release(&inode_table_lock);
 }
 
+// Caller holds reference to ip but not lock.
+// Drop reference.
 void
 idecref(struct inode *ip)
 {
@@ -315,6 +352,7 @@ idecref(struct inode *ip)
   iput(ip);
 }
 
+// Increment reference count for ip.
 void
 iincref(struct inode *ip)
 {
@@ -323,6 +361,8 @@ iincref(struct inode *ip)
   iunlock(ip);
 }
 
+// Copy stat information from inode.
+// XXX Assumes inode is from disk file system.
 void
 stati(struct inode *ip, struct stat *st)
 {
@@ -335,6 +375,8 @@ stati(struct inode *ip, struct stat *st)
 
 #define min(a, b) ((a) < (b) ? (a) : (b))
 
+// Read data from inode.
+// XXX Assumes inode is from disk file system.
 int
 readi(struct inode *ip, char *dst, uint off, uint n)
 {
@@ -361,6 +403,7 @@ readi(struct inode *ip, char *dst, uint off, uint n)
   return target - n;
 }
 
+// Allocate the nth block in inode ip if necessary.
 static int
 newblock(struct inode *ip, uint lbn)
 {
@@ -396,6 +439,8 @@ newblock(struct inode *ip, uint lbn)
   return 0;
 }
 
+// Write data to inode.
+// XXX Assumes inode is from disk file system.
 int
 writei(struct inode *ip, char *addr, uint off, uint n)
 {
@@ -551,6 +596,8 @@ namei(char *path, int mode, uint *ret_off,
   }
 }
 
+// Write a new directory entry (name, ino) into the directory dp.
+// Caller must have locked dp.
 void
 wdir(struct inode *dp, char *name, uint ino)
 {
@@ -575,6 +622,8 @@ wdir(struct inode *dp, char *name, uint ino)
     panic("wdir write");
 }
 
+// Create the path cp and return its locked inode structure.
+// If cp already exists, return 0.
 struct inode*
 mknod(char *cp, short type, short major, short minor)
 {
@@ -591,6 +640,9 @@ mknod(char *cp, short type, short major, short minor)
   return ip;
 }
 
+// Create a new inode named name inside dp
+// and return its locked inode structure.
+// If name already exists, return 0.
 struct inode*
 mknod1(struct inode *dp, char *name, short type, short major, short minor)
 {
@@ -611,6 +663,7 @@ mknod1(struct inode *dp, char *name, short type, short major, short minor)
   return ip;
 }
 
+// Unlink the inode named cp.
 int
 unlink(char *cp)
 {
@@ -649,6 +702,7 @@ unlink(char *cp)
   return 0;
 }
 
+// Create the path new as a link to the same inode as old.
 int
 link(char *name1, char *name2)
 {
diff --git a/ide.c b/ide.c
index 4e2e1a6..3afd19b 100644
--- a/ide.c
+++ b/ide.c
@@ -17,6 +17,12 @@
 #define IDE_CMD_READ  0x20
 #define IDE_CMD_WRITE 0x30
 
+// IDE request queue.
+// The next request will be stored in request[head],
+// and the request currently being served by the disk
+// is request[tail].
+// Must hold ide_lock while manipulating queue.
+
 struct ide_request {
   int diskno;
   uint secno;
@@ -28,6 +34,7 @@ struct ide_request {
 struct ide_request request[NREQUEST];
 int head, tail;
 struct spinlock ide_lock;
+
 int disk_1_present;
 int disk_channel;