From 294e12b360e309450aacc8a534fea46b87d4442a Mon Sep 17 00:00:00 2001 From: Matheus Date: Tue, 25 Jul 2023 19:35:48 -0300 Subject: [PATCH] ext2: ASSERT_UNCLEAN macro and do some TODOs --- kernel-src/fs/ext2.c | 111 ++++++++++++++++++++++++++----------------- 1 file changed, 68 insertions(+), 43 deletions(-) diff --git a/kernel-src/fs/ext2.c b/kernel-src/fs/ext2.c index 86cf2533..0260346e 100644 --- a/kernel-src/fs/ext2.c +++ b/kernel-src/fs/ext2.c @@ -13,7 +13,7 @@ #define SUPERBLOCK_OFFSET 1024 #define SB_SIGNATURE 0xef53 #define SB_STATE_CLEAN 1 -#define SB_STATE_ERROR 2 +#define SB_STATE_UNCLEAN 2 #define SB_ERRORACTION_IGNORE 1 #define SB_ERRORACTION_READONLY 2 #define SB_ERRORACTION_PANIC 3 @@ -166,6 +166,14 @@ typedef uint32_t blockptr_t; #define DESC_GETDISKOFFSET(fs, x) (BLOCK_GETDISKOFFSET(fs, (fs)->superblock.superblockstart + 1) + sizeof(blockgroupdesc_t) * (x)) #define BLOCKS_IN_INDIRECT(fs) ((fs)->blocksize / sizeof(blockptr_t)) +#define ASSERT_UNCLEAN(fs, x) \ + if (!(x)) { \ + printf("ext2: assert %s on line %d on function %s failed. setting filesystem as unclean\n", #x, __LINE__, __func__); \ + MUTEX_ACQUIRE(&fs->superblocklock, false); \ + fs->superblock.state = SB_STATE_UNCLEAN; \ + syncsuperblock(fs); \ + MUTEX_RELEASE(&fs->superblocklock); \ + } static const int ext2tovfstypetable[] = { -1, V_TYPE_FIFO, V_TYPE_CHDEV, -1, V_TYPE_DIR, -1, V_TYPE_BLKDEV, -1, V_TYPE_REGULAR, -1, V_TYPE_LINK, -1, V_TYPE_SOCKET @@ -256,7 +264,6 @@ static int allocatestructure(ext2fs_t *fs, uintmax_t *retid, bool inode) { goto cleanup; } - // XXX maybe it shouldn't be assumed this will always be a power of 2? size_t bmsize = (inode ? fs->superblock.inodespergroup : fs->superblock.blockspergroup) / 8; uint8_t *bm = alloc(bmsize); if (bm == NULL) { @@ -295,13 +302,13 @@ static int allocatestructure(ext2fs_t *fs, uintmax_t *retid, bool inode) { goto cleanup; // update block group desc free structure count - // TODO if an error happens here, the filesystem should probably be marked to be in an unclean state. if (inode) desc.freeinodes -= 1; else desc.freeblocks -= 1; e = vfs_write(fs->backing, &desc, sizeof(blockgroupdesc_t), DESC_GETDISKOFFSET(fs, bg), &writec, 0); + ASSERT_UNCLEAN(fs, e == 0); if (e) goto cleanup; @@ -323,6 +330,7 @@ static int allocatestructure(ext2fs_t *fs, uintmax_t *retid, bool inode) { fs->superblock.unallocatedblocks -= 1; e = syncsuperblock(fs); MUTEX_RELEASE(&fs->superblocklock); + ASSERT_UNCLEAN(fs, e == 0); cleanup: MUTEX_RELEASE(&fs->descriptorlock); @@ -341,7 +349,6 @@ static int freestructure(ext2fs_t *fs, uintmax_t id, bool inode) { if (e) goto cleanup; - // XXX maybe it shouldn't be assumed this will always be a power of 2? size_t bmsize = (inode ? fs->superblock.inodespergroup : fs->superblock.blockspergroup) / 8; uint8_t *bm = alloc(bmsize); if (bm == NULL) { @@ -369,7 +376,7 @@ static int freestructure(ext2fs_t *fs, uintmax_t id, bool inode) { goto cleanup; // update block group desc free structure count - // TODO if an error happens here, the filesystem should probably be marked to be in an unclean state. + ASSERT_UNCLEAN(fs, e == 0); if (inode) desc.freeinodes += 1; else @@ -510,7 +517,7 @@ static int allocandset(ext2fs_t *fs, ext2node_t *node, uintmax_t setoffset, bloc size_t writec; e = vfs_write(fs->backing, &blockptr, sizeof(blockptr_t), setoffset, &writec, 0); if (e) { - freestructure(fs, block, false); // TODO in case of error set filesystem as unclean? + ASSERT_UNCLEAN(fs, freestructure(fs, block, false) == 0); return e; } @@ -530,7 +537,7 @@ static int inodeallocateindirect(ext2fs_t *fs, ext2node_t *node, int indirect) { size_t writec; e = vfs_write(fs->backing, util_zerobuffer, fs->blocksize, BLOCK_GETDISKOFFSET(fs, v), &writec, 0); if (e) { - freestructure(fs, v, false); // TODO if error mark as unclean + ASSERT_UNCLEAN(fs, freestructure(fs, v, false) == 0); return e; } @@ -671,13 +678,15 @@ static int setinodeblock(ext2fs_t *fs, ext2node_t *node, uintmax_t index, blockp if (oldblock) { node->inode.sectcount -= INODE_SECTSPERBLOCK(fs); - freestructure(fs, oldblock, false); // TODO set as unclean in case of error? + ASSERT_UNCLEAN(fs, freestructure(fs, oldblock, false) == 0); } if (block) node->inode.sectcount += INODE_SECTSPERBLOCK(fs); - return writeinode(fs, &node->inode, node->id); // TODO set fs as unclean if an error happened here? + int e = writeinode(fs, &node->inode, node->id); + ASSERT_UNCLEAN(fs, e == 0); + return e; } static int resizeinode(ext2fs_t *fs, ext2node_t *node, size_t newsize) { @@ -692,8 +701,9 @@ static int resizeinode(ext2fs_t *fs, ext2node_t *node, size_t newsize) { if (e) return e; e = setinodeblock(fs, node, i, newblock); + ASSERT_UNCLEAN(fs, e == 0); if (e) - return e; // TODO set as unclean? + return e; } } else if (newblockcount < currentblockcount) { // shrink @@ -706,7 +716,7 @@ static int resizeinode(ext2fs_t *fs, ext2node_t *node, size_t newsize) { INODE_SETSIZE(&node->inode, newsize); - writeinode(fs, &node->inode, node->id); // TODO if error mark fs as unclean + ASSERT_UNCLEAN(fs, writeinode(fs, &node->inode, node->id) == 0); return 0; } @@ -839,7 +849,7 @@ static int insertdent(ext2fs_t *fs, ext2node_t *node, char *name, int inode, int memcpy(dentbuffer->name, name, namelen); size_t inodesize = INODE_SIZE(&node->inode); - // XXX mapping the whole dir into memory isn't the best idea + // XXX loading the whole dir into memory isn't the best idea void *dirbuffer = vmm_map(NULL, inodesize + 1, VMM_FLAGS_ALLOCATE, BUFFER_MAP_FLAGS, NULL); // + 1 to make sure there is at least one page mapped int err = dirbuffer ? 0 : ENOMEM; if (err) @@ -853,8 +863,8 @@ static int insertdent(ext2fs_t *fs, ext2node_t *node, char *name, int inode, int uintmax_t offset = 0; size_t truesize, freesize; while (offset < inodesize) { - splitdent = (ext2dent_t *)((uintptr_t)dirbuffer + offset); // TODO unused dents - truesize = ROUND_UP(sizeof(ext2dent_t) + splitdent->namelen, 4); + splitdent = (ext2dent_t *)((uintptr_t)dirbuffer + offset); + truesize = splitdent->inode == 0 ? 0 : ROUND_UP(sizeof(ext2dent_t) + splitdent->namelen, 4); freesize = splitdent->size - truesize; if (entlen <= freesize) @@ -871,8 +881,8 @@ static int insertdent(ext2fs_t *fs, ext2node_t *node, char *name, int inode, int goto cleanup; dentbuffer->size = fs->blocksize; - // TODO if this fails set fs as unclean err = rwbytes(fs, node, dentbuffer, dentbuffer->size, inodesize, true); + ASSERT_UNCLEAN(fs, err == 0); } else { size_t writesize = splitdent->size; splitdent->size = truesize; @@ -976,7 +986,6 @@ static int freeindirect(ext2fs_t *fs, uintmax_t block, int depth) { } static void freeinode(ext2fs_t *fs, inode_t *inode, int id) { - // TODO if any error happens here, set fs as unclean int err = 0; int type = INODE_TYPEPERM_TYPE(inode->typeperm); @@ -1003,10 +1012,14 @@ static void freeinode(ext2fs_t *fs, inode_t *inode, int id) { err |= freestructure(fs, id, true); if (type == INODE_TYPE_DIR) err |= changedircount(fs, INODE_GETGROUP(fs, id), -1); + ASSERT_UNCLEAN(fs, err == 0); } -static int ext2_open(vnode_t **vnode, int flags, cred_t *cred) { - // TODO device files +static int ext2_open(vnode_t **vnodep, int flags, cred_t *cred) { + vnode_t *vnode = *vnodep; + int type = vnode->type; + // TODO these types of files + __assert(type == V_TYPE_REGULAR || type == V_TYPE_DIR || type == V_TYPE_LINK); return 0; } @@ -1257,7 +1270,8 @@ int ext2_write(vnode_t *vnode, void *buffer, size_t size, uintmax_t offset, int if (err) goto cleanup; - // TODO mtime + node->inode.mtime = timekeeper_time().s; + ASSERT_UNCLEAN(fs, writeinode(fs, &node->inode, node->id) == 0); cleanup: VOP_UNLOCK(vnode); @@ -1295,7 +1309,7 @@ static int ext2_link(vnode_t *vnode, vnode_t *dirvnode, char *name, cred_t *cred node->inode.links += 1; - writeinode(fs, &node->inode, node->id); // TODO mark as unclean if error + ASSERT_UNCLEAN(fs, writeinode(fs, &node->inode, node->id) == 0); cleanup: VOP_UNLOCK(vnode); @@ -1325,7 +1339,7 @@ static int ext2_create(vnode_t *parent, char *name, vattr_t *attr, int type, vno newnode->id = id; memset(&newnode->inode, 0, sizeof(inode_t)); newnode->inode = (inode_t){ - .typeperm = 0, // TODO + .typeperm = 0, .uid = attr->uid, .atime = timekeeper_time().s, .ctime = timekeeper_time().s, @@ -1348,24 +1362,25 @@ static int ext2_create(vnode_t *parent, char *name, vattr_t *attr, int type, vno MUTEX_ACQUIRE(&fs->inodetablelock, false); err = hashtable_set(&fs->inodetable, newnode, &newnode->id, sizeof(newnode->id), true); MUTEX_RELEASE(&fs->inodetablelock); - if (err) // TODO if this fails set fs as unclean (directory entry is still there) + ASSERT_UNCLEAN(fs, err == 0); + if (err) goto cleanup; VOP_HOLD(&newnode->vnode); *result = &newnode->vnode; - // create . and .. entries and increase block group dir count if dir TODO if any error set as unclean + // create . and .. entries and increase block group dir count if dir if (type == V_TYPE_DIR) { - insertdent(fs, newnode, ".", newnode->id, vfstoext2denttypetable[V_TYPE_DIR]); - insertdent(fs, newnode, "..", node->id, vfstoext2denttypetable[V_TYPE_DIR]); + ASSERT_UNCLEAN(fs, insertdent(fs, newnode, ".", newnode->id, vfstoext2denttypetable[V_TYPE_DIR]) == 0); + ASSERT_UNCLEAN(fs, insertdent(fs, newnode, "..", node->id, vfstoext2denttypetable[V_TYPE_DIR]) == 0); node->inode.links += 1; - writeinode(fs, &node->inode, node->id); - changedircount(fs, INODE_GETGROUP(fs, newnode->id), 1); + ASSERT_UNCLEAN(fs, writeinode(fs, &node->inode, node->id) == 0); + ASSERT_UNCLEAN(fs, changedircount(fs, INODE_GETGROUP(fs, newnode->id), 1) == 0); } cleanup: if (err && id) - freestructure(fs, id, true); // TODO if error set as unclean + ASSERT_UNCLEAN(fs, freestructure(fs, id, true) == 0); if (err) slab_free(nodecache, newnode); @@ -1400,16 +1415,16 @@ static int ext2_symlink(vnode_t *vnode, char *name, vattr_t *attr, char *path, c err = rwbytes(fs, newnode, path, linklen, 0, true); if (err) goto cleanup; - // TODO in case of error, unlink } INODE_SETSIZE(&newnode->inode, linklen); - // TODO in case of error, unlink err = writeinode(fs, &newnode->inode, newnode->id); VOP_RELEASE(newvnode); cleanup: + if (err) + ASSERT_UNCLEAN(fs, VOP_UNLINK(vnode, name, cred) == 0); VOP_UNLOCK(newvnode); VOP_UNLOCK(vnode); return err; @@ -1472,21 +1487,25 @@ static int ext2_unlink(vnode_t *vnode, char *name, cred_t *cred) { err = hashtable_get(&fs->inodetable, &v, &inode, sizeof(inode)); if (err == ENOENT) { inode_t buff; - err = readinode(fs, &buff, inode); // TODO if error mark as unclean + err = readinode(fs, &buff, inode); + ASSERT_UNCLEAN(fs, err == 0); + if (err == 0) { + if (INODE_TYPEPERM_TYPE(buff.typeperm) == INODE_TYPE_DIR) { // account for . entry + isdir = true; + buff.links -= 1; + } - if (INODE_TYPEPERM_TYPE(buff.typeperm) == INODE_TYPE_DIR) { // account for . entry - isdir = true; - buff.links -= 1; + if (--buff.links == 0) + freeinode(fs, &buff, inode); + else + err = writeinode(fs, &buff, inode); + + ASSERT_UNCLEAN(fs, err == 0); } - - if (--buff.links == 0) - freeinode(fs, &buff, inode); - else - err = writeinode(fs, &buff, inode); // TODO if error mark as unclean - MUTEX_RELEASE(&fs->inodetablelock); } else if (err) { - MUTEX_RELEASE(&fs->inodetablelock); // TODO mark as unclean + ASSERT_UNCLEAN(fs, !"Failed to get from inode table"); + MUTEX_RELEASE(&fs->inodetablelock); goto cleanup; } else { ext2node_t *unlinknode = v; @@ -1499,7 +1518,8 @@ static int ext2_unlink(vnode_t *vnode, char *name, cred_t *cred) { } int nlinks = --unlinknode->inode.links; - err = writeinode(fs, &unlinknode->inode, unlinknode->id); // TODO mark as unclean + err = writeinode(fs, &unlinknode->inode, unlinknode->id); + ASSERT_UNCLEAN(fs, err == 0); VOP_UNLOCK(&unlinknode->vnode); if (nlinks == 0) { vnode_t *unlinkvnode = v; @@ -1511,7 +1531,7 @@ static int ext2_unlink(vnode_t *vnode, char *name, cred_t *cred) { if (isdir) { // account for .. entry in unlinked dir --node->inode.links; - writeinode(fs, &node->inode, node->id); // TODO unclean if error + ASSERT_UNCLEAN(fs, writeinode(fs, &node->inode, node->id) == 0); } cleanup: @@ -1577,6 +1597,9 @@ static int ext2_root(vfs_t *vfs, vnode_t **vnodep) { fs->root = node; leave: + if (err && node) + slab_free(nodecache, node); + MUTEX_RELEASE(&fs->rootlock); return err; } @@ -1656,6 +1679,8 @@ static int ext2_mount(vfs_t **vfs, vnode_t *mountpoint, vnode_t *backing, void * *vfs = &fs->vfs; err = 0; + ASSERT_UNCLEAN(fs, fs->blocksize == 123546667); + cleanup: if (err) free(fs);