simplify ide queuing

nits in comments

bio.c

@@ -4,15 +4,14 @@
 // 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.
+// modifying 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
+// 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.)
+// 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
@@ -79,7 +78,6 @@ bget(uint dev, uint sector)
         goto loop;
       }
       b->flags |= B_BUSY;
-      // b->flags &= ~B_VALID; // Force reread from disk
       release(&buf_table_lock);
       return b;
     }
@@ -98,7 +96,8 @@ bget(uint dev, uint sector)
   panic("bget: no buffers");
 }
 
-// Read buf's contents from disk.
+// Return a B_BUSY buf with the contents of the indicated
+// disk sector.
 struct buf*
 bread(uint dev, uint sector)
 {
@@ -108,7 +107,8 @@ bread(uint dev, uint sector)
   if(b->flags & B_VALID)
     return b;
 
-  ide_rw(dev & 0xff, sector, b->data, 1, 1);
+  b->flags &= ~B_WRITE;
+  ide_rw(b);
   b->flags |= B_VALID;
 
   return b;
@@ -121,7 +121,8 @@ bwrite(struct buf *b)
 {
   if((b->flags & B_BUSY) == 0)
     panic("bwrite");
-  ide_rw(b->dev & 0xff, b->sector, b->data, 1, 0);
+  b->flags |= B_WRITE;
+  ide_rw(b);
   b->flags |= B_VALID;
 }
 

buf.h

@@ -2,9 +2,12 @@ struct buf {
   int flags;
   uint dev;
   uint sector;
-  struct buf *prev;
+  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

defs.h

@@ -55,7 +55,7 @@ int             writei(struct inode*, char*, uint, uint);
 // ide.c
 void            ide_init(void);
 void            ide_intr(void);
-void            ide_rw(int, uint, void*, uint, int);
+void            ide_rw(struct buf *);
 
 // ioapic.c
 void            ioapic_enable(int irq, int cpu);

fs.c

@@ -6,7 +6,7 @@
 //   + Directories: inode with special contents (list of other inodes!)
 //   + Names: paths like /usr/rtm/xv6/fs.c for convenient naming.
 //
-// Disk layout is: superblock, inodes, disk bitmap, data blocks.
+// Disk layout is: superblock, inodes, block not-free bitmap, data blocks.
 //
 // This file contains the low-level file system manipulation 
 // routines.  The (higher-level) system call implementations
@@ -51,7 +51,7 @@ balloc(uint dev)
     bi = b % BPB;
     m = 0x1 << (bi % 8);
     if((bp->data[bi/8] & m) == 0) {  // is block free?
-      bp->data[bi/8] |= 0x1 << (bi % 8);
+      bp->data[bi/8] |= m;
       bwrite(bp);  // mark it allocated on disk
       brelse(bp);
       return b;
@@ -81,6 +81,8 @@ bfree(int dev, uint b)
   bp = bread(dev, BBLOCK(b, ninodes));
   bi = b % BPB;
   m = 0x1 << (bi % 8);
+  if((bp->data[bi/8] & m) == 0)
+    panic("freeing free block");
   bp->data[bi/8] &= ~m;
   bwrite(bp);  // mark it free on disk
   brelse(bp);
@@ -93,18 +95,17 @@ bfree(int dev, uint b)
 // on-disk structures to provide a place for synchronizing access
 // to inodes shared between multiple processes.
 // 
-// ip->ref counts the number of references to this
+// ip->ref counts the number of pointer references to this cached
 // inode; references are typically kept in struct file and in cp->cwd.
 // When ip->ref falls to zero, the inode is no longer cached.
 // It is an error to use an inode without holding a reference to it.
 //
-// Inodes can be marked busy, just like bufs, meaning
-// that some process has exclusive use of the inode.
+// Inodes can be locked with I_BUSY (like bufs and B_BUSY).
 // Processes are only allowed to read and write inode
 // metadata and contents when holding the inode's lock.
-// Because inodes locks are held during disk accesses, 
-// they are implemented using a flag, as in the buffer cache,
-// not using spin locks.  Callers are responsible for locking
+// Because inode locks are held during disk accesses, 
+// they are implemented using a flag rather than with
+// spin locks.  Callers are responsible for locking
 // inodes before passing them to routines in this file; leaving
 // this responsibility with the caller makes it possible for them
 // to create arbitrarily-sized atomic operations.
@@ -112,7 +113,8 @@ bfree(int dev, uint b)
 // To give maximum control over locking to the callers, 
 // the routines in this file that return inode pointers 
 // return pointers to *unlocked* inodes.  It is the callers'
-// responsibility to lock them before using them.
+// responsibility to lock them before using them. A non-zero
+// ip->ref keeps these unlocked inodes in the cache.
 
 struct {
   struct spinlock lock;

ide.c

@@ -8,6 +8,7 @@
 #include "x86.h"
 #include "traps.h"
 #include "spinlock.h"
+#include "buf.h"
 
 #define IDE_BSY       0x80
 #define IDE_DRDY      0x40
@@ -18,28 +19,17 @@
 #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].
+// ide_queue points to the buf now being read/written to the disk.
+// ide_queue->qnext points to the next buf to be processed.
 // Must hold ide_lock while manipulating queue.
 
-struct ide_request {
-  int diskno;
-  uint secno;
-  void *addr;
-  uint nsecs;
-  uint read;
-  int done;
-};
-
-static struct ide_request request[NREQUEST];
-static int head, tail;
 static struct spinlock ide_lock;
+static struct buf *ide_queue;
 
 static int disk_1_present;
-static int disk_queue;
 
 static int ide_probe_disk1(void);
+static void ide_start_request();
 
 //PAGEBREAK: 10
 // Wait for IDE disk to become ready.
@@ -93,80 +83,75 @@ void
 ide_intr(void)
 {
   acquire(&ide_lock);
-  request[tail].done = 1;
-  wakeup(&request[tail]);
+  if(ide_queue){
+    //cprintf("intr %x\n", ide_queue);
+    if((ide_queue->flags & B_WRITE) == 0)
+      if(ide_wait_ready(1) >= 0)
+        insl(0x1F0, ide_queue->data, 512/4);
+    ide_queue->done = 1;
+    wakeup(ide_queue);
+    ide_queue = ide_queue->qnext;
+    ide_start_request();
+  } else {
+    cprintf("stray ide interrupt\n");
+  }
   release(&ide_lock);
 }
 
 // Start the next request in the queue.
+// Caller must hold ide_lock.
 static void
 ide_start_request (void)
 {
-  struct ide_request *r;
-
-  if(head != tail) {
-    r = &request[tail];
+  if(ide_queue){
     ide_wait_ready(0);
+    //cprintf("start %x\n", ide_queue);
     outb(0x3f6, 0);  // generate interrupt
-    outb(0x1F2, r->nsecs);
-    outb(0x1F3, r->secno & 0xFF);
-    outb(0x1F4, (r->secno >> 8) & 0xFF);
-    outb(0x1F5, (r->secno >> 16) & 0xFF);
-    outb(0x1F6, 0xE0 | ((r->diskno&1)<<4) | ((r->secno>>24)&0x0F));
-    if(r->read)
-      outb(0x1F7, IDE_CMD_READ);
-    else {
+    outb(0x1F2, 1);  // number of sectors
+    outb(0x1F3, ide_queue->sector & 0xFF);
+    outb(0x1F4, (ide_queue->sector >> 8) & 0xFF);
+    outb(0x1F5, (ide_queue->sector >> 16) & 0xFF);
+    outb(0x1F6, 0xE0 |
+         ((ide_queue->dev & 1)<<4) |
+         ((ide_queue->sector>>24)&0x0F));
+    if(ide_queue->flags & B_WRITE){
       outb(0x1F7, IDE_CMD_WRITE);
-      outsl(0x1F0, r->addr, 512/4);
+      outsl(0x1F0, ide_queue->data, 512/4);
+    } else {
+      outb(0x1F7, IDE_CMD_READ);
     }
   }
 }
 
 //PAGEBREAK: 30
-// Run an entire disk operation.
+// Queue up a disk operation and wait for it to finish.
+// b must have B_BUSY set.
 void
-ide_rw(int diskno, uint secno, void *addr, uint nsecs, int read)
+ide_rw(struct buf *b)
 {
-  struct ide_request *r;
+  struct buf **pp;
 
-  if(diskno && !disk_1_present)
+  if((b->dev & 0xff) && !disk_1_present)
     panic("ide disk 1 not present");
 
   acquire(&ide_lock);
 
-  // Add request to queue.
-  while((head + 1) % NREQUEST == tail)
-    sleep(&disk_queue, &ide_lock);
+  b->done = 0;
+  b->qnext = 0;
 
-  r = &request[head];
-  r->secno = secno;
-  r->addr = addr;
-  r->nsecs = nsecs;
-  r->diskno = diskno;
-  r->read = read;
-  r->done = 0;
-  head = (head + 1) % NREQUEST;
+  // cprintf("enqueue %x %x\n", b, ide_queue);
 
-  // Start request if necessary.
+  // append b to ide_queue
+  pp = &ide_queue;
+  while(*pp)
+    pp = &(*pp)->qnext;
+  *pp = b;
+  
+  if(ide_queue == b)
     ide_start_request();
   
-  // Wait for request to finish.
-  while(!r->done)
-    sleep(r, &ide_lock);
-  
-  // Finish request.
-  if(read){
-    if(ide_wait_ready(1) >= 0)
-      insl(0x1F0, addr, 512/4);
-  }
-
-  // Remove request from queue.
-  if((head + 1) % NREQUEST == tail)
-    wakeup(&disk_queue);
-  tail = (tail + 1) % NREQUEST;
-
-  // Start next request in queue, if any.
-  ide_start_request();
+  while(!b->done)
+    sleep(b, &ide_lock);
   
   release(&ide_lock);
 }

kalloc.c

@@ -95,7 +95,7 @@ kalloc(int n)
   char *p;
   struct run *r, **rr;
 
-  if(n % PAGE)
+  if(n % PAGE || n <= 0)
     panic("kalloc");
 
   acquire(&kalloc_lock);

param.h

@@ -5,7 +5,6 @@
 #define NOFILE       16  // open files per process
 #define NFILE       100  // open files per system
 #define NBUF         10  // size of disk block cache
-#define NREQUEST   NBUF  // outstanding disk requests
 #define NINODE       50  // maximum number of active i-nodes
 #define NDEV         10  // maximum major device number
 #define ROOTDEV       1  // device number of file system root disk

syscall.c

@@ -49,7 +49,7 @@ fetchstr(struct proc *p, uint addr, char **pp)
   return -1;
 }
 
-// Fetch the argno'th word-sized system call argument as an integer.
+// Fetch the argno'th 32-bit system call argument.
 int
 argint(int argno, int *ip)
 {