From 17e3cf15bac0c1ac60780ce7d1d228442ff08ed9 Mon Sep 17 00:00:00 2001 From: rtm Date: Sun, 13 Aug 2006 15:51:58 +0000 Subject: [PATCH] fix iget() bug that allocated in-use inode[] entries --- Notes | 14 +++----------- fd.c | 2 ++ fs.c | 50 +++++++++++++------------------------------------- init.c | 3 --- ioapic.c | 2 -- picirq.c | 7 ------- syscall.c | 3 ++- 7 files changed, 20 insertions(+), 61 deletions(-) diff --git a/Notes b/Notes index 4bb9552..ab28ec3 100644 --- a/Notes +++ b/Notes @@ -357,7 +357,6 @@ OH! recursive interrupts will use up any amount of cpu[].stack! disk scheduling mkdir -more than one directory content block sh arguments sh redirection indirect blocks @@ -366,17 +365,10 @@ two bugs in unlink: don't just return if nlink > 0, is there a create/create race for same file name? resulting in two entries w/ same name in directory? -namei - return just inode - return offset in dir where found, w/ dir locked, for unlink - return dir locked, for mknod - -is the offset alone useful? how do I read/write it? - test: one process unlinks a file while another links to it test: simultaneous create of same file test: one process opens a file while another deletes it -oy, mkfs wants dir size to be last written entry, but i - want it to be nblocks*512 - maybe fix kernel code to handle former +wdir should use writei, to avoid special-case block allocation + also readi + is dir locked? probably diff --git a/fd.c b/fd.c index 25a51fb..e61cab2 100644 --- a/fd.c +++ b/fd.c @@ -8,6 +8,8 @@ #include "fd.h" #include "spinlock.h" #include "dev.h" +#include "fs.h" +#include "fsvar.h" struct spinlock fd_table_lock; struct devsw devsw[NDEV]; diff --git a/fs.c b/fs.c index 37b964d..fbb4a8c 100644 --- a/fs.c +++ b/fs.c @@ -54,7 +54,6 @@ balloc(uint dev) if (b >= size) panic("balloc: out of blocks\n"); - cprintf ("balloc: allocate block %d\n", b); bp->data[bi/8] |= 0x1 << (bi % 8); bwrite (bp, BBLOCK(b, ninodes)); // mark it allocated on disk brelse(bp); @@ -70,7 +69,6 @@ bfree(int dev, uint b) int ninodes; uchar m; - cprintf ("bfree: free block %d\n", b); bp = bread(dev, 1); sb = (struct superblock *) bp->data; ninodes = sb->ninodes; @@ -93,13 +91,14 @@ bfree(int dev, uint b) struct inode * iget(uint dev, uint inum) { - struct inode *ip, *nip = 0; + struct inode *ip, *nip; struct dinode *dip; struct buf *bp; acquire(&inode_table_lock); loop: + nip = 0; for(ip = &inode[0]; ip < &inode[NINODE]; ip++){ if(ip->count > 0 && ip->dev == dev && ip->inum == inum){ if(ip->busy){ @@ -180,11 +179,9 @@ ialloc(uint dev, short type) brelse(bp); } - if (inum >= ninodes) { + if (inum >= ninodes) panic ("ialloc: no inodes left\n"); - } - cprintf ("ialloc: %d\n", inum); dip->type = type; bwrite (bp, IBLOCK(inum)); // mark it allocated on the disk brelse(bp); @@ -195,7 +192,6 @@ ialloc(uint dev, short type) static void ifree(struct inode *ip) { - cprintf("ifree: %d\n", ip->inum); ip->type = 0; iupdate(ip); } @@ -220,7 +216,7 @@ ilock(struct inode *ip) void iunlock(struct inode *ip) { - if(ip->busy != 1) + if(ip->busy != 1 || ip->count < 1) panic("iunlock"); acquire(&inode_table_lock); @@ -271,7 +267,7 @@ iput(struct inode *ip) if(ip->count < 1 || ip->busy != 1) panic("iput"); - if ((ip->count <= 1) && (ip->nlink <= 0)) + if ((ip->count == 1) && (ip->nlink == 0)) iunlink(ip); acquire(&inode_table_lock); @@ -498,8 +494,6 @@ mknod(char *cp, short type, short major, short minor) { struct inode *ip, *dp; - cprintf("mknod: %s %d %d %d\n", cp, type, major, minor); - if ((dp = namei(cp, NAMEI_CREATE, 0)) == 0) return 0; @@ -525,24 +519,17 @@ unlink(char *cp) { struct inode *ip, *dp; struct dirent de; - uint off, inum, cc; + uint off, inum, dev; - cprintf("unlink(%s)\n", cp); - - if ((dp = namei(cp, NAMEI_DELETE, &off)) == 0) { - cprintf("unlink(%s) it doesn't exist\n", cp); + if ((dp = namei(cp, NAMEI_DELETE, &off)) == 0) return -1; - } - if((cc = readi(dp, (char*)&de, off, sizeof(de))) != sizeof(de) || - de.inum == 0){ - cprintf("off %d dp->size %d cc %d de.inum %d", - off, dp->size, cc, de.inum); + dev = dp->dev; + + if(readi(dp, (char*)&de, off, sizeof(de)) != sizeof(de) || de.inum == 0) panic("unlink no entry"); - } + inum = de.inum; - cprintf("dinum %d off %d de %s/%d\n", - dp->inum, off, de.name, de.inum); memset(&de, 0, sizeof(de)); if(writei(dp, (char*)&de, off, sizeof(de)) != sizeof(de)) @@ -551,9 +538,7 @@ unlink(char *cp) iupdate(dp); iput(dp); - ip = iget(dp->dev, inum); - if(ip == 0) - panic("unlink no inode"); + ip = iget(dev, inum); ip->nlink--; @@ -568,14 +553,9 @@ link(char *name1, char *name2) { struct inode *ip, *dp; - cprintf("link(%s, %s)\n", name1, name2); - - if ((ip = namei(name1, NAMEI_LOOKUP, 0)) == 0){ - cprintf(" failed %s does not exist\n", name1); + if ((ip = namei(name1, NAMEI_LOOKUP, 0)) == 0) return -1; - } if(ip->type == T_DIR){ - cprintf(" failed %s is a dir\n", name1); iput(ip); return -1; } @@ -583,12 +563,10 @@ link(char *name1, char *name2) iunlock(ip); if ((dp = namei(name2, NAMEI_CREATE, 0)) == 0) { - cprintf(" failed %s exists\n", name2); idecref(ip); return -1; } if(dp->dev != ip->dev){ - cprintf(" cross-device link\n"); idecref(ip); iput(dp); return -1; @@ -602,7 +580,5 @@ link(char *name1, char *name2) iput(dp); iput(ip); - cprintf(" succeeded\n"); - return 0; } diff --git a/init.c b/init.c index 0a4223c..928371d 100644 --- a/init.c +++ b/init.c @@ -17,10 +17,7 @@ main(void) open("console", 1); open("console", 1); - puts("init...\n"); - while(1){ - puts("running sh...\n"); pid = fork(); if(pid == 0){ exec("sh", sh_args); diff --git a/ioapic.c b/ioapic.c index 0bd672a..5120b23 100644 --- a/ioapic.c +++ b/ioapic.c @@ -43,7 +43,6 @@ ioapic_init(void) id = ioapic_read(io, IOAPIC_ID) >> APIC_ID_SHIFT; if (id != ioapic_id) panic ("ioapic_init: id isn't equal to ioapic_id\n"); - cprintf ("ioapic VER: 0x%x id %d nintr %d\n", l, id, nintr); for (i = 0; i < nintr; i++) { // active-hi and edge-triggered for ISA interrupts // Assume that pin 0 on the first I/O APIC is an ExtINT pin. @@ -78,5 +77,4 @@ ioapic_enable (int irq, int cpunum) h &= ~IOART_DEST; h |= (cpunum << APIC_ID_SHIFT); ioapic_write(io, IOAPIC_REDTBL_HI(irq), h); - cprintf("cpu%d: intr %d: lo 0x%x hi 0x%x\n", cpu(), irq, l, h); } diff --git a/picirq.c b/picirq.c index 9fd7167..28b5e5d 100644 --- a/picirq.c +++ b/picirq.c @@ -22,13 +22,6 @@ irq_setmask_8259A(ushort mask) outb(IO_PIC1+1, (char)mask); outb(IO_PIC2+1, (char)(mask >> 8)); - - cprintf("%d: enabled interrupts:", cpu()); - - for (i = 0; i < 16; i++) - if (~mask & (1<mem + arg0, (short) arg1, (short) arg2, (short) arg3); - iput(nip); + if(nip) + iput(nip); return (nip == 0) ? -1 : 0; }