From 530431045237d7ccbbc0bb65ed83309845c19893 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Mon, 29 Jul 2019 17:33:16 -0400 Subject: [PATCH] Remove B_DIRTY Use refcnt to pin blocks into the cache Replace flags/B_VALID with a boolean field valid Use info[id].status to signal completion of disk interrupt Pass a read/write flag to virtio_disk_rw --- kernel/bio.c | 20 +++++++------------- kernel/buf.h | 4 +--- kernel/defs.h | 2 +- kernel/log.c | 8 +++++--- kernel/virtio_disk.c | 12 +++++------- 5 files changed, 19 insertions(+), 27 deletions(-) diff --git a/kernel/bio.c b/kernel/bio.c index a08884b..7455c06 100644 --- a/kernel/bio.c +++ b/kernel/bio.c @@ -12,11 +12,7 @@ // * 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. -// -// The implementation uses two state flags internally: -// * B_VALID: the buffer data has been read from the disk. -// * B_DIRTY: the buffer data has been modified -// and needs to be written to disk. + #include "types.h" #include "param.h" @@ -76,13 +72,11 @@ bget(uint dev, uint blockno) } // Not cached; recycle an unused buffer. - // Even if refcnt==0, B_DIRTY indicates a buffer is in use - // because log.c has modified it but not yet committed it. for(b = bcache.head.prev; b != &bcache.head; b = b->prev){ - if(b->refcnt == 0 && (b->flags & B_DIRTY) == 0) { + if(b->refcnt == 0) { b->dev = dev; b->blockno = blockno; - b->flags = 0; + b->valid = 0; b->refcnt = 1; release(&bcache.lock); acquiresleep(&b->lock); @@ -99,8 +93,9 @@ bread(uint dev, uint blockno) struct buf *b; b = bget(dev, blockno); - if((b->flags & B_VALID) == 0) { - virtio_disk_rw(b); + if(!b->valid) { + virtio_disk_rw(b, 0); + b->valid = 1; } return b; } @@ -111,8 +106,7 @@ bwrite(struct buf *b) { if(!holdingsleep(&b->lock)) panic("bwrite"); - b->flags |= B_DIRTY; - virtio_disk_rw(b); + virtio_disk_rw(b, 1); } // Release a locked buffer. diff --git a/kernel/buf.h b/kernel/buf.h index 3266495..ae9d264 100644 --- a/kernel/buf.h +++ b/kernel/buf.h @@ -1,5 +1,5 @@ struct buf { - int flags; + int valid; // has data been read from disk? uint dev; uint blockno; struct sleeplock lock; @@ -9,6 +9,4 @@ struct buf { struct buf *qnext; // disk queue uchar data[BSIZE]; }; -#define B_VALID 0x2 // buffer has been read from disk -#define B_DIRTY 0x4 // buffer needs to be written to disk diff --git a/kernel/defs.h b/kernel/defs.h index de4acfd..8421082 100644 --- a/kernel/defs.h +++ b/kernel/defs.h @@ -177,7 +177,7 @@ void plic_complete(int); // virtio_disk.c void virtio_disk_init(void); -void virtio_disk_rw(struct buf *); +void virtio_disk_rw(struct buf *, int); void virtio_disk_intr(); // number of elements in fixed-size array diff --git a/kernel/log.c b/kernel/log.c index c8f7e62..5aea267 100644 --- a/kernel/log.c +++ b/kernel/log.c @@ -77,6 +77,7 @@ install_trans(void) struct buf *dbuf = bread(log.dev, log.lh.block[tail]); // read dst memmove(dbuf->data, lbuf->data, BSIZE); // copy block to dst bwrite(dbuf); // write dst to disk + dbuf->refcnt--; // unpin buffer from cache brelse(lbuf); brelse(dbuf); } @@ -203,7 +204,7 @@ commit() } // Caller has modified b->data and is done with the buffer. -// Record the block number and pin in the cache with B_DIRTY. +// Record the block number and pin in the cache by increasing refcnt. // commit()/write_log() will do the disk write. // // log_write() replaces bwrite(); a typical use is: @@ -227,9 +228,10 @@ log_write(struct buf *b) break; } log.lh.block[i] = b->blockno; - if (i == log.lh.n) + if (i == log.lh.n) { // Add new block to log? + b->refcnt++; // Pin block in cache log.lh.n++; - b->flags |= B_DIRTY; // prevent eviction + } release(&log.lock); } diff --git a/kernel/virtio_disk.c b/kernel/virtio_disk.c index 3ace5ef..df4473d 100644 --- a/kernel/virtio_disk.c +++ b/kernel/virtio_disk.c @@ -164,7 +164,7 @@ alloc3_desc(int *idx) } void -virtio_disk_rw(struct buf *b) +virtio_disk_rw(struct buf *b, int write) { uint64 sector = b->blockno * (BSIZE / 512); @@ -192,7 +192,7 @@ virtio_disk_rw(struct buf *b) uint64 sector; } buf0; - if(b->flags & B_DIRTY) + if(write) buf0.type = VIRTIO_BLK_T_OUT; // write the disk else buf0.type = VIRTIO_BLK_T_IN; // read the disk @@ -208,7 +208,7 @@ virtio_disk_rw(struct buf *b) desc[idx[1]].addr = (uint64) b->data; desc[idx[1]].len = BSIZE; - if(b->flags & B_DIRTY) + if(write) desc[idx[1]].flags = 0; // device reads b->data else desc[idx[1]].flags = VRING_DESC_F_WRITE; // device writes b->data @@ -235,7 +235,7 @@ virtio_disk_rw(struct buf *b) *R(VIRTIO_MMIO_QUEUE_NOTIFY) = 0; // value is queue number // Wait for virtio_disk_intr() to say request has finished. - while((b->flags & (B_VALID|B_DIRTY)) != B_VALID){ + while(info[idx[0]].status == 0) { sleep(b, &vdisk_lock); } @@ -253,9 +253,7 @@ virtio_disk_intr() if(info[id].status != 0) panic("virtio_disk_intr status"); - info[id].b->flags |= B_VALID; - info[id].b->flags &= ~B_DIRTY; - + info[id].status = 1; wakeup(info[id].b); info[id].b = 0;