From 902b13f5d6bab19f7a218acc655421cdb63f4313 Mon Sep 17 00:00:00 2001 From: rtm Date: Fri, 24 Aug 2007 19:32:36 +0000 Subject: [PATCH] simplify ide queuing nits in comments --- bio.c | 19 ++++----- buf.h | 5 ++- defs.h | 2 +- fs.c | 20 +++++----- ide.c | 117 ++++++++++++++++++++++++------------------------------ kalloc.c | 2 +- param.h | 1 - syscall.c | 2 +- 8 files changed, 79 insertions(+), 89 deletions(-) diff --git a/bio.c b/bio.c index c819b6b..7f5d158 100644 --- a/bio.c +++ b/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; } diff --git a/buf.h b/buf.h index ccf7f8b..ddc42c7 100644 --- a/buf.h +++ b/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 diff --git a/defs.h b/defs.h index 3b8127d..ba70cbb 100644 --- a/defs.h +++ b/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); diff --git a/fs.c b/fs.c index a6e1787..3b39a42 100644 --- a/fs.c +++ b/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; diff --git a/ide.c b/ide.c index 5bbf0f5..d5fa6dd 100644 --- a/ide.c +++ b/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); + + b->done = 0; + b->qnext = 0; + + // cprintf("enqueue %x %x\n", b, ide_queue); + + // append b to ide_queue + pp = &ide_queue; + while(*pp) + pp = &(*pp)->qnext; + *pp = b; - // Add request to queue. - while((head + 1) % NREQUEST == tail) - sleep(&disk_queue, &ide_lock); - - 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; - - // Start request if necessary. - ide_start_request(); + if(ide_queue == b) + ide_start_request(); - // Wait for request to finish. - while(!r->done) - sleep(r, &ide_lock); + while(!b->done) + sleep(b, &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(); - release(&ide_lock); } diff --git a/kalloc.c b/kalloc.c index 34765c2..4125527 100644 --- a/kalloc.c +++ b/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); diff --git a/param.h b/param.h index 343288c..34edf95 100644 --- a/param.h +++ b/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 diff --git a/syscall.c b/syscall.c index b18b62c..eddf725 100644 --- a/syscall.c +++ b/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) {