[Tux3] Patch: Improve new_btree interface, fix lock init

OGAWA Hirofumi hirofumi at mail.parknet.co.jp
Wed Dec 24 10:34:19 PST 2008


Daniel Phillips <phillips at phunq.net> writes:

> The new_btree function was a piece of prototype code that escaped and
> finally started causing problems now, when we decided to add a new lock
> and realized it had to be initialized in several places.  Its primary
> purpose is to create the persistent form of a btree, not to initialize
> the cache object.  So with this patch new_btree takes an existing btree
> object as a parameter and returns an error code as is proper.
>
> But the patch is not quite ready to apply, we still have destructive
> initialization of the btree cache object in unpack_sb and decode_attrs,
> which needs a clean fix I am not quite prepared to tackle at this time
> of night, so see you all tomorrow.

This is incremental patch of this patch.

This adds init_btree() to initialize btree. And it calls new
->btree_init() method to initialize per btree fields (it is only
->entries_per_leaf currently).

Then, this uses it except btree in main(). So, with this, new_btree()
allocates new btree, and init_btree initialize ->btree with existing
btree.

Thanks.
-- 
OGAWA Hirofumi <hirofumi at mail.parknet.co.jp>



diff -puN user/kernel/tux3.h~init-btree user/kernel/tux3.h
--- tux3/user/kernel/tux3.h~init-btree	2008-12-25 01:40:17.000000000 +0900
+++ tux3-hirofumi/user/kernel/tux3.h	2008-12-25 02:10:05.000000000 +0900
@@ -166,11 +166,11 @@ struct root {
 };
 
 struct btree {
+	struct rw_semaphore lock;
 	struct sb *sb;		/* Convenience to reduce parameter list size */
 	struct btree_ops *ops;	/* Generic btree low level operations */
 	struct root root;	/* Cached description of btree root */
 	u16 entries_per_leaf;	/* Used in btree leaf splitting */
-	struct rw_semaphore lock;
 };
 
 /* Define layout of btree root on disk, endian conversion is elsewhere. */
@@ -445,6 +445,7 @@ static inline void inc_dleaf_groups(stru
 typedef void vleaf;
 
 struct btree_ops {
+	void (*btree_init)(struct btree *btree);
 	int (*leaf_sniff)(struct btree *btree, vleaf *leaf);
 	int (*leaf_init)(struct btree *btree, vleaf *leaf);
 	tuxkey_t (*leaf_split)(struct btree *btree, tuxkey_t key, vleaf *from, vleaf *into);
@@ -607,6 +608,7 @@ void free_cursor(struct cursor *cursor);
 void level_pop_brelse_dirty(struct cursor *cursor);
 void level_push(struct cursor *cursor, struct buffer_head *buffer, struct index_entry *next);
 
+void init_btree(struct btree *btree, struct sb *sb, struct root root, struct btree_ops *ops);
 int new_btree(struct btree *btree, struct sb *sb, struct btree_ops *ops);
 struct buffer_head *new_leaf(struct btree *btree);
 int probe(struct btree *btree, tuxkey_t key, struct cursor *cursor);
diff -puN user/kernel/btree.c~init-btree user/kernel/btree.c
--- tux3/user/kernel/btree.c~init-btree	2008-12-25 01:43:55.000000000 +0900
+++ tux3-hirofumi/user/kernel/btree.c	2008-12-25 02:57:00.000000000 +0900
@@ -586,22 +586,34 @@ void *tree_expand(struct btree *btree, t
 	return NULL;
 }
 
+void init_btree(struct btree *btree, struct sb *sb, struct root root, struct btree_ops *ops)
+{
+	btree->sb = sb;
+	btree->ops = ops;
+	btree->root = root;
+	btree->entries_per_leaf = 0;
+	if (ops)
+		ops->btree_init(btree);
+}
+
 int new_btree(struct btree *btree, struct sb *sb, struct btree_ops *ops)
 {
+	/* Initialize btree with dummy root */
+	init_btree(btree, sb, (struct root){}, ops);
+
 	struct buffer_head *rootbuf = new_node(btree);
 	struct buffer_head *leafbuf = new_leaf(btree);
 	if (!rootbuf || !leafbuf)
 		goto eek;
-	struct bnode *root = bufdata(rootbuf);
-	root->entries[0].block = to_be_u64(bufindex(leafbuf));
-	root->count = to_be_u32(1);
-	btree->root = (struct root){ .block = bufindex(rootbuf), .depth = 1 };
+	struct bnode *rootnode = bufdata(rootbuf);
+	rootnode->entries[0].block = to_be_u64(bufindex(leafbuf));
+	rootnode->count = to_be_u32(1);
 	printf("root at %Lx\n", (L)bufindex(rootbuf));
 	printf("leaf at %Lx\n", (L)bufindex(leafbuf));
 	brelse_dirty(rootbuf);
 	brelse_dirty(leafbuf);
-	btree->ops = ops;
-	btree->sb = sb;
+	struct root root = { .block = bufindex(rootbuf), .depth = 1 };
+	init_btree(btree, sb, root, ops);
 	return 0;
 eek:
 	if (rootbuf)
diff -puN user/btree.c~init-btree user/btree.c
--- tux3/user/btree.c~init-btree	2008-12-25 01:52:29.000000000 +0900
+++ tux3-hirofumi/user/btree.c	2008-12-25 02:28:27.000000000 +0900
@@ -33,6 +33,12 @@ static inline struct uleaf *to_uleaf(vle
 	return leaf;
 }
 
+static void uleaf_btree_init(struct btree *btree)
+{
+	struct sb *sb = btree->sb;
+	btree->entries_per_leaf = (sb->blocksize - offsetof(struct uleaf, entries)) / sizeof(struct entry);
+}
+
 int uleaf_sniff(struct btree *btree, vleaf *leaf)
 {
 	return to_uleaf(leaf)->magic == 0xc0de;
@@ -112,6 +118,7 @@ void uleaf_merge(struct btree *btree, vl
 }
 
 struct btree_ops ops = {
+	.btree_init = uleaf_btree_init,
 	.leaf_sniff = uleaf_sniff,
 	.leaf_init = uleaf_init,
 	.leaf_split = uleaf_split,
@@ -152,7 +159,6 @@ int main(int argc, char *argv[])
 	printf("entries_per_node = %i\n", sb->entries_per_node);
 	struct btree btree = { };
 	assert(!new_btree(&btree, sb, &ops));
-	btree.entries_per_leaf = (sb->blocksize - offsetof(struct uleaf, entries)) / sizeof(struct entry);
 
 	if (0) {
 		struct buffer_head *buffer = new_leaf(&btree);
diff -puN user/dleaf.c~init-btree user/dleaf.c
diff -puN user/iattr.c~init-btree user/iattr.c
--- tux3/user/iattr.c~init-btree	2008-12-25 01:55:56.000000000 +0900
+++ tux3-hirofumi/user/iattr.c	2008-12-25 02:18:10.000000000 +0900
@@ -15,6 +15,13 @@
 
 #ifndef iattr_notmain_from_inode
 static struct btree_ops dtree_ops;
+static void init_btree(struct btree *btree, struct sb *sb, struct root root, struct btree_ops *ops)
+{
+	btree->sb = sb;
+	btree->root = root;
+	btree->ops = ops;
+	btree->entries_per_leaf = 0;
+}
 #endif
 
 #ifndef trace
diff -puN user/ileaf.c~init-btree user/ileaf.c
--- tux3/user/ileaf.c~init-btree	2008-12-25 01:57:12.000000000 +0900
+++ tux3-hirofumi/user/ileaf.c	2008-12-25 02:14:04.000000000 +0900
@@ -65,8 +65,11 @@ int main(int argc, char *argv[])
 {
 	printf("--- test inode table leaf methods ---\n");
 	struct sb *sb = &(struct sb){ .blocksize = 4096 };
-	struct btree *btree = &(struct btree){ .sb = sb, .ops = &itable_ops };
-	btree->entries_per_leaf = 64; // !!! should depend on blocksize
+	struct btree *btree = &(struct btree){
+		.sb = sb,
+		.ops = &itable_ops,
+		.entries_per_leaf = 64, // !!! should depend on blocksize
+	};
 	struct ileaf *leaf = ileaf_create(btree);
 	struct ileaf *dest = ileaf_create(btree);
 	leaf->ibase = to_be_u64(0x10);
diff -puN user/kernel/iattr.c~init-btree user/kernel/iattr.c
--- tux3/user/kernel/iattr.c~init-btree	2008-12-25 01:59:30.000000000 +0900
+++ tux3-hirofumi/user/kernel/iattr.c	2008-12-25 02:08:19.000000000 +0900
@@ -126,16 +126,17 @@ void *encode_attrs(struct inode *inode, 
 void *decode_attrs(struct inode *inode, void *attrs, unsigned size)
 {
 	trace_off("decode %u attr bytes", size);
-	u64 v64;
-	u32 v32;
+	struct sb *sb = tux_sb(inode->i_sb);
 	tuxnode_t *tuxnode = tux_inode(inode);
 	struct xattr *xattr = tuxnode->xcache ? tuxnode->xcache->xattrs : NULL;
 	void *limit = attrs + size;
+	u64 v64;
+	u32 v32;
 	while (attrs < limit - 1) {
 		unsigned head;
 		attrs = decode16(attrs, &head);
 		unsigned version = head & 0xfff, kind = head >> 12;
-		if (version != tux_sb(inode->i_sb)->version) {
+		if (version != sb->version) {
 			attrs += atsize[kind];
 			continue;
 		}
@@ -159,12 +160,7 @@ void *decode_attrs(struct inode *inode, 
 			break;
 		case DATA_BTREE_ATTR:
 			attrs = decode64(attrs, &v64);
-			tuxnode->btree = (struct btree){
-				.sb = tux_sb(inode->i_sb),
-				.entries_per_leaf = 64, // !!! should depend on blocksize
-				.ops = &dtree_ops,
-				.root = unpack_root(v64),
-			};
+			init_btree(&tuxnode->btree, sb, unpack_root(v64), &dtree_ops);
 			break;
 		case LINK_COUNT_ATTR:
 			attrs = decode32(attrs, &inode->i_nlink);
diff -puN user/kernel/super.c~init-btree user/kernel/super.c
--- tux3/user/kernel/super.c~init-btree	2008-12-25 02:00:59.000000000 +0900
+++ tux3-hirofumi/user/kernel/super.c	2008-12-25 03:11:35.000000000 +0900
@@ -17,6 +17,7 @@
 
 static int unpack_sb(struct sb *sb, struct disksuper *super, int silent)
 {
+	u64 iroot = from_be_u64(super->iroot);
 	if (memcmp(super->magic, (char[])SB_MAGIC, sizeof(super->magic))) {
 		if (!silent)
 			printf("invalid superblock [%Lx]\n",
@@ -24,18 +25,10 @@ static int unpack_sb(struct sb *sb, stru
 		return -EINVAL;
 	}
 
-	int blockbits = from_be_u16(super->blockbits);
-
-	sb->itable = (struct btree){
-		.sb	= sb,
-		.ops	= &itable_ops,
-		.root	= unpack_root(from_be_u64(super->iroot)),
-		.entries_per_leaf = 1 << (blockbits - 6),
-	};
 //	sb->rootbuf;
-	sb->blockbits = blockbits;
-	sb->blocksize = 1 << blockbits;
-	sb->blockmask = (1 << blockbits) - 1;
+	sb->blockbits = from_be_u16(super->blockbits);
+	sb->blocksize = 1 << sb->blockbits;
+	sb->blockmask = (1 << sb->blockbits) - 1;
 	/* FIXME: those should be initialized based on blocksize. */
 	sb->entries_per_node = 20;
 	sb->max_inodes_per_block = 64;
@@ -48,6 +41,9 @@ static int unpack_sb(struct sb *sb, stru
 	sb->atomgen = from_be_u32(super->atomgen);
 	sb->freeatom = from_be_u32(super->freeatom);
 	sb->dictsize = from_be_u64(super->dictsize);
+
+	init_btree(&sb->itable, sb, unpack_root(iroot), &itable_ops);
+
 	return 0;
 }
 
@@ -93,7 +89,7 @@ static struct inode *tux3_alloc_inode(st
 	tuxnode_t *tuxi = kmem_cache_alloc(tux_inode_cachep, GFP_KERNEL);
 	if (!tuxi)
 		return NULL;
-	tuxi->btree = (struct btree){};
+	init_btree(&tuxi->btree, NULL, (struct root){}, NULL);
 	tuxi->present = 0;
 	tuxi->xcache = NULL;
 
diff -puN user/tux3.c~init-btree user/tux3.c
--- tux3/user/tux3.c~init-btree	2008-12-25 02:02:42.000000000 +0900
+++ tux3-hirofumi/user/tux3.c	2008-12-25 02:03:40.000000000 +0900
@@ -78,8 +78,8 @@ int main(int argc, const char *argv[])
 		.blockmask = (1 << dev->bits) - 1,
 		.volblocks = volsize >> dev->bits,
 		.freeblocks = volsize >> dev->bits,
-		.itable = (struct btree){ .sb = sb, .ops = &itable_ops,
-			.entries_per_leaf = 1 << (dev->bits - 6) } };
+	};
+	init_btree(&sb->itable, sb, (struct root){}, &itable_ops);
 
 	if (!strcmp(command, "mkfs") || !strcmp(command, "make")) {
 		if (poptPeekArg(popt))
diff -puN user/filemap.c~init-btree user/filemap.c
--- tux3/user/filemap.c~init-btree	2008-12-25 02:19:12.000000000 +0900
+++ tux3-hirofumi/user/filemap.c	2008-12-25 02:19:22.000000000 +0900
@@ -18,15 +18,15 @@
 #include "xattr.c"
 #undef main
 
+#define main notmain4
+#include "btree.c"
+#undef main
+
 #define iattr_notmain_from_inode
 #define main iattr_notmain_from_inode
 #include "ileaf.c"
 #undef main
 
-#define main notmain4
-#include "btree.c"
-#undef main
-
 #include "tux3.h"	/* include user/tux3.h, not user/kernel/tux3.h */
 #include "kernel/filemap.c"
 
diff -puN user/kernel/dleaf.c~init-btree user/kernel/dleaf.c
--- tux3/user/kernel/dleaf.c~init-btree	2008-12-25 02:19:53.000000000 +0900
+++ tux3-hirofumi/user/kernel/dleaf.c	2008-12-25 03:17:05.000000000 +0900
@@ -68,6 +68,11 @@ static inline struct dleaf *to_dleaf(vle
 	return leaf;
 }
 
+static void dleaf_btree_init(struct btree *btree)
+{
+	btree->entries_per_leaf = 64; /* FIXME: should depend on blocksize */
+}
+
 int dleaf_init(struct btree *btree, vleaf *leaf)
 {
 	if (!leaf)
@@ -745,6 +750,7 @@ static int dleaf_chop(struct btree *btre
 }
 
 struct btree_ops dtree_ops = {
+	.btree_init = dleaf_btree_init,
 	.leaf_sniff = dleaf_sniff,
 	.leaf_init = dleaf_init,
 	.leaf_dump = dleaf_dump,
diff -puN user/kernel/ileaf.c~init-btree user/kernel/ileaf.c
--- tux3/user/kernel/ileaf.c~init-btree	2008-12-25 02:33:21.000000000 +0900
+++ tux3-hirofumi/user/kernel/ileaf.c	2008-12-25 02:35:14.000000000 +0900
@@ -47,6 +47,11 @@ static inline tuxkey_t ibase(struct ilea
 	return from_be_u64(leaf->ibase);
 }
 
+static void ileaf_btree_init(struct btree *btree)
+{
+	btree->entries_per_leaf = 1 << (btree->sb->blockbits - 6);
+}
+
 static int ileaf_init(struct btree *btree, vleaf *leaf)
 {
 	printf("initialize inode leaf %p\n", leaf);
@@ -286,6 +291,7 @@ int ileaf_purge(struct btree *btree, inu
 }
 
 struct btree_ops itable_ops = {
+	.btree_init = ileaf_btree_init,
 	.leaf_dump = ileaf_dump,
 	.leaf_sniff = ileaf_sniff,
 	.leaf_init = ileaf_init,
diff -puN user/super.c~init-btree user/super.c
--- tux3/user/super.c~init-btree	2008-12-25 02:37:02.000000000 +0900
+++ tux3-hirofumi/user/super.c	2008-12-25 02:37:45.000000000 +0900
@@ -95,10 +95,9 @@ int make_tux3(struct sb *sb)
 	}
 
 	trace("create inode table");
-	assert(!new_btree(&sb->itable, sb, &itable_ops));
-	if (!sb->itable.ops)
+	err = new_btree(&sb->itable, sb, &itable_ops);
+	if (err)
 		goto eek;
-	sb->itable.entries_per_leaf = 64; // !!! should depend on blocksize
 	sb->bitmap->i_size = (sb->volblocks + 7) >> 3;
 	trace("create bitmap inode");
 	if (make_inode(sb->bitmap, TUX_BITMAP_INO))
diff -puN user/inode.c~init-btree user/inode.c
--- tux3/user/inode.c~init-btree	2008-12-25 03:12:09.000000000 +0900
+++ tux3-hirofumi/user/inode.c	2008-12-25 03:12:50.000000000 +0900
@@ -27,6 +27,7 @@ struct inode *new_inode(struct sb *sb)
 	if (!inode)
 		goto eek;
 	*inode = (struct inode){ .i_sb = sb, .map = map, .i_version = 1, .i_nlink = 1, };
+	init_btree(&tux_inode(inode)->btree, NULL, (struct root){}, NULL);
 	return inode->map->inode = inode;
 eek:
 	if (map)
_

_______________________________________________
Tux3 mailing list
Tux3 at tux3.org
http://mailman.tux3.org/cgi-bin/mailman/listinfo/tux3



More information about the Tux3 mailing list