From 211ff0c67ea6737853cf932313cf4e27cc15f55c Mon Sep 17 00:00:00 2001 From: rtm Date: Sun, 13 Aug 2006 12:22:44 +0000 Subject: [PATCH] namei returns locked parent dir inode for create / unlink don't hold fd table lock across idecref() (latter does block i/o) idecref calls iput() in case last ref -> freeing inode dir size is 512 * # blocks, so readi/writei &c work unlink deletes dirent even if ip->nlink > 0 --- Notes | 15 +++++ defs.h | 2 +- fd.c | 20 ++++--- fs.c | 157 ++++++++++++++++++++++++++++------------------------ fsvar.h | 4 ++ syscall.c | 6 +- usertests.c | 2 +- 7 files changed, 122 insertions(+), 84 deletions(-) diff --git a/Notes b/Notes index 386bc2f..4bb9552 100644 --- a/Notes +++ b/Notes @@ -365,3 +365,18 @@ two bugs in unlink: don't just return if nlink > 0, and search for name, not inum 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 diff --git a/defs.h b/defs.h index c8e1870..da47691 100644 --- a/defs.h +++ b/defs.h @@ -116,7 +116,7 @@ void ilock(struct inode *ip); void iunlock(struct inode *ip); void idecref(struct inode *ip); void iput(struct inode *ip); -struct inode * namei(char *path, uint *); +struct inode * namei(char *path, int, uint *); void stati(struct inode *ip, struct stat *st); int readi(struct inode *ip, char *xdst, uint off, uint n); int writei(struct inode *ip, char *addr, uint off, uint n); diff --git a/fd.c b/fd.c index c5c3e57..25a51fb 100644 --- a/fd.c +++ b/fd.c @@ -108,18 +108,22 @@ fd_close(struct fd *fd) panic("fd_close"); if(--fd->ref == 0){ - if(fd->type == FD_PIPE){ - pipe_close(fd->pipe, fd->writeable); - } else if(fd->type == FD_FILE){ - idecref(fd->ip); + struct fd dummy = *fd; + + fd->ref = 0; + fd->type = FD_CLOSED; + release(&fd_table_lock); + + if(dummy.type == FD_PIPE){ + pipe_close(dummy.pipe, dummy.writeable); + } else if(dummy.type == FD_FILE){ + idecref(dummy.ip); } else { panic("fd_close"); } - fd->ref = 0; - fd->type = FD_CLOSED; + } else { + release(&fd_table_lock); } - - release(&fd_table_lock); } int diff --git a/fs.c b/fs.c index dbecb41..37b964d 100644 --- a/fs.c +++ b/fs.c @@ -286,14 +286,8 @@ iput(struct inode *ip) void idecref(struct inode *ip) { - acquire(&inode_table_lock); - - if(ip->count < 1) - panic("idecref"); - - ip->count -= 1; - - release(&inode_table_lock); + ilock(ip); + iput(ip); } void @@ -376,34 +370,37 @@ writei(struct inode *ip, char *addr, uint off, uint n) } } +// look up a path name, in one of three modes. +// NAMEI_LOOKUP: return locked target inode. +// NAMEI_CREATE: return locked parent inode. +// but return 0 if name does exist. +// NAMEI_DELETE: return locked parent inode, offset of dirent in *ret_off. +// return 0 if name doesn't exist. struct inode * -namei(char *path, uint *ret_pinum) +namei(char *path, int mode, uint *ret_off) { struct inode *dp; - char *cp = path; + char *cp = path, *cp1; uint off, dev; struct buf *bp; struct dirent *ep; - int i; + int i, atend; unsigned ninum; - unsigned pinum; - + dp = iget(rootdev, 1); - pinum = dp->inum; while(*cp == '/') cp++; while(1){ - if(*cp == '\0') { - if (ret_pinum) - *ret_pinum = pinum; - return dp; + if(*cp == '\0'){ + if(mode == NAMEI_LOOKUP) + return dp; + iput(dp); + return 0; } if(dp->type != T_DIR){ - if (ret_pinum) - *ret_pinum = pinum; iput(dp); return 0; } @@ -419,6 +416,7 @@ namei(char *path, uint *ret_pinum) if(cp[i] != ep->name[i]) break; if((cp[i] == '\0' || cp[i] == '/') && (i >= DIRSIZ || ep->name[i] == '\0')){ + off += (uchar*)ep - bp->data; ninum = ep->inum; brelse(bp); cp += i; @@ -427,14 +425,22 @@ namei(char *path, uint *ret_pinum) } brelse(bp); } + atend = 1; + for(cp1 = cp; *cp1; cp1++) + if(*cp1 == '/') + atend = 0; + if(mode == NAMEI_CREATE && atend) + return dp; + iput(dp); - if (ret_pinum) - *ret_pinum = pinum; return 0; found: + if(mode == NAMEI_DELETE && *cp == '\0'){ + *ret_off = off; + return dp; + } dev = dp->dev; - pinum = dp->inum; iput(dp); dp = iget(dev, ninum); if(dp->type == 0 || dp->nlink < 1) @@ -453,6 +459,9 @@ wdir(struct inode *dp, char *name, uint ino) int i; int lb; + if(dp->size % BSIZE) + dp->size += (BSIZE - dp->size % BSIZE); + for(off = 0; off < dp->size; off += BSIZE) { bp = bread(dp->dev, bmap(dp, off / BSIZE)); for(ep = (struct dirent *) bp->data; @@ -469,14 +478,16 @@ wdir(struct inode *dp, char *name, uint ino) } dp->addrs[lb] = balloc(dp->dev); bp = bread(dp->dev, dp->addrs[lb]); + memset(bp->data, 0, BSIZE); ep = (struct dirent *) (bp->data); + dp->size += BSIZE; + found: ep->inum = ino; for(i = 0; i < DIRSIZ && name[i]; i++) ep->name[i] = name[i]; for( ; i < DIRSIZ; i++) ep->name[i] = '\0'; - dp->size += sizeof(struct dirent); bwrite (bp, bmap(dp, off/BSIZE)); // write directory block brelse(bp); iupdate(dp); @@ -486,15 +497,12 @@ struct inode * mknod(char *cp, short type, short major, short minor) { struct inode *ip, *dp; - uint pinum = 0; cprintf("mknod: %s %d %d %d\n", cp, type, major, minor); - if ((ip = namei(cp, &pinum)) != 0) { - iput(ip); + if ((dp = namei(cp, NAMEI_CREATE, 0)) == 0) return 0; - } - dp = iget(rootdev, pinum); + ip = ialloc(dp->dev, type); if (ip == 0) { iput(dp); @@ -515,74 +523,81 @@ mknod(char *cp, short type, short major, short minor) int unlink(char *cp) { - struct inode *ip; - struct inode *dp; - struct dirent *ep = 0; - int off; - struct buf *bp = 0; - uint pinum; - + struct inode *ip, *dp; + struct dirent de; + uint off, inum, cc; - if ((ip = namei(cp, &pinum)) == 0) { + cprintf("unlink(%s)\n", cp); + + if ((dp = namei(cp, NAMEI_DELETE, &off)) == 0) { cprintf("unlink(%s) it doesn't exist\n", cp); return -1; } - ip->nlink--; - if (ip->nlink > 0) { - iput(ip); - return 0; + 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); + panic("unlink no entry"); } + inum = de.inum; + cprintf("dinum %d off %d de %s/%d\n", + dp->inum, off, de.name, de.inum); - dp = iget(rootdev, pinum); - for(off = 0; off < dp->size; off += BSIZE) { - bp = bread(dp->dev, bmap(dp, off / BSIZE)); - for(ep = (struct dirent *) bp->data; - ep < (struct dirent *) (bp->data + BSIZE); - ep++){ - if(ep->inum == ip->inum) { - goto found; - } - } - brelse(bp); - } - panic("unlink: entry doesn't exist\n"); + memset(&de, 0, sizeof(de)); + if(writei(dp, (char*)&de, off, sizeof(de)) != sizeof(de)) + panic("unlink dir write"); - found: - ep->inum = 0; - memset(ep->name, '\0', DIRSIZ); - bwrite (bp, bmap(dp, off/BSIZE)); // write directory block - brelse(bp); - dp->size -= sizeof(struct dirent); - iupdate (dp); + iupdate(dp); iput(dp); + + ip = iget(dp->dev, inum); + if(ip == 0) + panic("unlink no inode"); + + ip->nlink--; + + iupdate(ip); iput(ip); + return 0; } int link(char *name1, char *name2) { - struct inode *ip, *dp, *xip; - uint pinum = 0; + struct inode *ip, *dp; cprintf("link(%s, %s)\n", name1, name2); - if ((xip = namei(name2, &pinum)) != 0) { - cprintf(" failed %s exists\n", name2); - iput(xip); - return -1; - } - - if ((ip = namei(name1, &pinum)) == 0){ + if ((ip = namei(name1, NAMEI_LOOKUP, 0)) == 0){ cprintf(" failed %s does not exist\n", name1); return -1; } + if(ip->type == T_DIR){ + cprintf(" failed %s is a dir\n", name1); + iput(ip); + return -1; + } + + 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; + } + ilock(ip); ip->nlink += 1; iupdate (ip); - dp = iget(rootdev, pinum); wdir(dp, name2, ip->inum); iput(dp); iput(ip); diff --git a/fsvar.h b/fsvar.h index ef678dd..6f4e68a 100644 --- a/fsvar.h +++ b/fsvar.h @@ -14,3 +14,7 @@ struct inode { }; extern uint rootdev; + +#define NAMEI_LOOKUP 1 +#define NAMEI_CREATE 2 +#define NAMEI_DELETE 3 diff --git a/syscall.c b/syscall.c index 4505c89..71a7013 100644 --- a/syscall.c +++ b/syscall.c @@ -228,7 +228,7 @@ sys_open(void) return -1; if((l = checkstring(arg0)) < 0) return -1; - if((ip = namei(cp->mem + arg0, 0)) == 0) { + if((ip = namei(cp->mem + arg0, NAMEI_LOOKUP, 0)) == 0) { if (arg1 & O_CREATE) { if (l >= DIRSIZ) return -1; @@ -356,7 +356,7 @@ sys_exec(void) return -1; if(checkstring(arg0) < 0) return -1; - ip = namei(cp->mem + arg0, 0); + ip = namei(cp->mem + arg0, NAMEI_LOOKUP, 0); if(ip == 0) return -1; @@ -494,7 +494,7 @@ sys_block(void) ip->type, ip->nlink, ip->size, ip->addrs[0]); iput(ip); - ip = namei(".././//./../usertests", 0); + ip = namei(".././//./../usertests", NAMEI_LOOKUP, 0); if(ip){ cprintf("namei(usertests): %d %d %d %d %d %d %d %d\n", ip->dev, ip->inum, ip->count, ip->busy, diff --git a/usertests.c b/usertests.c index 2d6b065..c3e9113 100644 --- a/usertests.c +++ b/usertests.c @@ -231,7 +231,7 @@ void createdelete() { int pid, i, fd; - int n = 10; // for now, fit in one directory block + int n = 20; char name[32]; pid = fork();