Another attempt at the bio.c comment.

Rename B_WRITE to B_DIRTY and then let ide.c
maintain the B_VALID and B_DIRTY flags.

3 files changed, 36 insertions, 39 deletions

diff --git a/bio.c b/bio.c
index 6efc43e..5d09acf 100644
--- a/bio.c
+++ b/bio.c
@@ -1,27 +1,25 @@
 // 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
-// using buf, so other code is not allowed to use it.
-// To wait for a buffer that is B_BUSY, sleep on buf.
-// (See bget below.)
+// The buffer cache is a linked list of buf structures holding
+// cached copies of disk block contents.  Caching disk blocks
+// in memory reduces the number of disk reads and also provides
+// a synchronization point for disk blocks used by multiple processes.
 // 
-// If B_VALID is set, it means that the sector in b->data is
-// the same as on the disk. If B_VALID is not set, the contents
-// of buf must be initialized, often by calling bread,
-// before being used.
+// Interface:
+// * To get a buffer for a particular disk block, call bread.
+// * After changing buffer data, call bwrite to flush it to disk.
+// * When done with the buffer, call brelse.
+// * Do not use the buffer after calling brelse.
+// * Only one process at a time can use a buffer,
+//     so do not keep them longer than necessary.
 // 
-// 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.
+// The implementation uses three state flags internally:
+// * B_BUSY: the block has been returned from bread
+//     and has not been passed back to brelse.  
+// * B_VALID: the buffer data has been initialized
+//     with the associated disk block contents.
+// * B_DIRTY: the buffer data has been modified
+//     and needs to be written to disk.
 
 #include "types.h"
 #include "param.h"
@@ -103,13 +101,8 @@ bread(uint dev, uint sector)
   struct buf *b;
 
   b = bget(dev, sector);
-  if(b->flags & B_VALID)
-    return b;
-
-  b->flags &= ~B_WRITE;
-  ide_rw(b);
-  b->flags |= B_VALID;
-
+  if(!(b->flags & B_VALID))
+    ide_rw(b);
   return b;
 }
 
@@ -119,9 +112,8 @@ bwrite(struct buf *b)
 {
   if((b->flags & B_BUSY) == 0)
     panic("bwrite");
-  b->flags |= B_WRITE;
+  b->flags |= B_DIRTY;
   ide_rw(b);
-  b->flags |= B_VALID;
 }
 
 // Release the buffer buf.
diff --git a/buf.h b/buf.h
index ddc42c7..9c586f2 100644
--- a/buf.h
+++ b/buf.h
@@ -5,9 +5,9 @@ struct buf {
   struct buf *prev; // LRU cache list
   struct buf *next;
   struct buf *qnext; // disk queue
-  int done;
   uchar data[512];
 };
-#define B_BUSY 0x1  // buffer is locked by some process
-#define B_VALID 0x2 // buffer contains the data of the sector
-#define B_WRITE 0x4 // asking device driver to write, else read
+#define B_BUSY  0x1  // buffer is locked by some process
+#define B_VALID 0x2  // buffer has been read from disk
+#define B_DIRTY 0x4  // buffer needs to be written to disk
+
diff --git a/ide.c b/ide.c
index 83bffcf..092b8ce 100644
--- a/ide.c
+++ b/ide.c
@@ -78,7 +78,7 @@ ide_start_request(struct buf *b)
   outb(0x1f4, (b->sector >> 8) & 0xff);
   outb(0x1f5, (b->sector >> 16) & 0xff);
   outb(0x1f6, 0xE0 | ((b->dev&1)<<4) | ((b->sector>>24)&0x0f));
-  if(b->flags & B_WRITE){
+  if(b->flags & B_DIRTY){
     outb(0x1f7, IDE_CMD_WRITE);
     outsl(0x1f0, b->data, 512/4);
   }else{
@@ -100,11 +100,12 @@ ide_intr(void)
   }
 
   // Read data if needed.
-  if((b->flags & B_WRITE) == 0 && ide_wait_ready(1) >= 0)
+  if(!(b->flags & B_DIRTY) && ide_wait_ready(1) >= 0)
     insl(0x1f0, b->data, 512/4);
   
   // Wake process waiting for this buf.
-  b->done = 1;
+  b->flags |= B_VALID;
+  b->flags &= ~B_DIRTY;
   wakeup(b);
   
   // Start disk on next buf in queue.
@@ -115,7 +116,9 @@ ide_intr(void)
 }
 
 //PAGEBREAK!
-// Queue a disk operation and wait for it to finish.
+// Sync buf with disk. 
+// If B_DIRTY is set, write buf to disk, clear B_DIRTY, set B_VALID.
+// Else if B_VALID is not set, read buf from disk, set B_VALID.
 void
 ide_rw(struct buf *b)
 {
@@ -123,13 +126,14 @@ ide_rw(struct buf *b)
 
   if(!(b->flags & B_BUSY))
     panic("ide_rw: buf not busy");
+  if((b->flags & (B_VALID|B_DIRTY)) == B_VALID)
+    panic("ide_rw: nothing to do");
   if(b->dev != 0 && !disk_1_present)
     panic("ide disk 1 not present");
 
   acquire(&ide_lock);
 
   // Append b to ide_queue.
-  b->done = 0;
   b->qnext = 0;
   for(pp=&ide_queue; *pp; pp=&(*pp)->qnext)
     ;
@@ -140,7 +144,8 @@ ide_rw(struct buf *b)
     ide_start_request(b);
   
   // Wait for request to finish.
-  while(!b->done)
+  while((b->flags & (B_VALID|B_DIRTY)) != B_VALID)
     sleep(b, &ide_lock);
+
   release(&ide_lock);
 }