thr3ads.net - Btrfs devel - [PATCH 1/2] Btrfs: kill location key of in-memory inode [Jun 2011]

If this information is useful, please help other people find it:
Share via:

Liu Bo

2011-Jun-16 09:22 UTC

[PATCH 1/2] Btrfs: kill location key of in-memory inode

In btrfs''s in-memory inode, there is a btrfs_key which has the
structure:
- key.objectid : inode id
- key.type: BTRFS_INODE_ITEM_KEY
- key.offset: 0 or -1ULL

however, we only use key.objectid to search, to check or something else,
and to reduce in-memory inode size I just keep what is valuable.

Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
---
 fs/btrfs/btrfs_inode.h |   10 ++++------
 fs/btrfs/disk-io.c     |    3 +--
 fs/btrfs/export.c      |    2 +-
 fs/btrfs/extent-tree.c |    2 +-
 fs/btrfs/inode.c       |   48 +++++++++++++++++++++++++++++-------------------
 5 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 52d7eca..31337df 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -29,11 +29,6 @@ struct btrfs_inode {
 	/* which subvolume this inode belongs to */
 	struct btrfs_root *root;
 
-	/* key used to find this inode on disk.  This is used by the code
-	 * to read in roots of subvolumes
-	 */
-	struct btrfs_key location;
-
 	/* the extent_tree has caches of all the extent mappings to disk */
 	struct extent_map_tree extent_tree;
 
@@ -72,6 +67,9 @@ struct btrfs_inode {
 	/* the space_info for where this inode''s data allocations are done */
 	struct btrfs_space_info *space_info;
 
+	/* full 64 bit inode number */
+	u64 inode_id;
+
 	/* full 64 bit generation number, struct vfs_inode doesn''t have a big
 	 * enough field for this.
 	 */
@@ -171,7 +169,7 @@ static inline struct btrfs_inode *BTRFS_I(struct inode
*inode)
 
 static inline u64 btrfs_ino(struct inode *inode)
 {
-	u64 ino = BTRFS_I(inode)->location.objectid;
+	u64 ino = BTRFS_I(inode)->inode_id;
 
 	if (ino <= BTRFS_FIRST_FREE_OBJECTID)
 		ino = inode->i_ino;
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a203d36..5d6bbb9 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1693,9 +1693,8 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 
 	BTRFS_I(fs_info->btree_inode)->io_tree.ops = &btree_extent_io_ops;
 
+	BTRFS_I(fs_info->btree_inode)->inode_id = BTRFS_BTREE_INODE_OBJECTID;
 	BTRFS_I(fs_info->btree_inode)->root = tree_root;
-	memset(&BTRFS_I(fs_info->btree_inode)->location, 0,
-	       sizeof(struct btrfs_key));
 	BTRFS_I(fs_info->btree_inode)->dummy_inode = 1;
 	insert_inode_hash(fs_info->btree_inode);
 
diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c
index 1b8dc33..cdd3a84 100644
--- a/fs/btrfs/export.c
+++ b/fs/btrfs/export.c
@@ -43,7 +43,7 @@ static int btrfs_encode_fh(struct dentry *dentry, u32 *fh, int
*max_len,
 		spin_lock(&dentry->d_lock);
 
 		parent = dentry->d_parent->d_inode;
-		fid->parent_objectid = BTRFS_I(parent)->location.objectid;
+		fid->parent_objectid = btrfs_ino(parent);
 		fid->parent_gen = parent->i_generation;
 		parent_root_id = BTRFS_I(parent)->root->objectid;
 
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 5b9b6b6..3a1f8ee 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3037,7 +3037,7 @@ int btrfs_check_data_free_space(struct inode *inode, u64
bytes)
 	bytes = (bytes + root->sectorsize - 1) & ~((u64)root->sectorsize -
1);
 
 	if (root == root->fs_info->tree_root ||
-	    BTRFS_I(inode)->location.objectid == BTRFS_FREE_INO_OBJECTID) {
+	    BTRFS_I(inode)->inode_id == BTRFS_FREE_INO_OBJECTID) {
 		alloc_chunk = 0;
 		committed = 1;
 	}
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 02ff4a1..e01a084 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -754,7 +754,7 @@ static inline bool is_free_space_inode(struct btrfs_root
*root,
 				       struct inode *inode)
 {
 	if (root == root->fs_info->tree_root ||
-	    BTRFS_I(inode)->location.objectid == BTRFS_FREE_INO_OBJECTID)
+	    BTRFS_I(inode)->inode_id == BTRFS_FREE_INO_OBJECTID)
 		return true;
 	return false;
 }
@@ -2513,7 +2513,10 @@ static void btrfs_read_locked_inode(struct inode *inode)
 	path = btrfs_alloc_path();
 	BUG_ON(!path);
 	path->leave_spinning = 1;
-	memcpy(&location, &BTRFS_I(inode)->location, sizeof(location));
+
+	location.objectid = btrfs_ino(inode);
+	location.offset = 0;
+	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
 
 	ret = btrfs_lookup_inode(NULL, root, path, &location, 0);
 	if (ret)
@@ -2667,6 +2670,7 @@ noinline int btrfs_update_inode(struct btrfs_trans_handle
*trans,
 	struct btrfs_inode_item *inode_item;
 	struct btrfs_path *path;
 	struct extent_buffer *leaf;
+	struct btrfs_key location;
 	int ret;
 
 	/*
@@ -2687,8 +2691,12 @@ noinline int btrfs_update_inode(struct btrfs_trans_handle
*trans,
 		return -ENOMEM;
 
 	path->leave_spinning = 1;
-	ret = btrfs_lookup_inode(trans, root, path, &BTRFS_I(inode)->location,
-				 1);
+
+	location.objectid = btrfs_ino(inode);
+	location.offset = 0;
+	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
+
+	ret = btrfs_lookup_inode(trans, root, path, &location, 1);
 	if (ret) {
 		if (ret > 0)
 			ret = -ENOENT;
@@ -2839,6 +2847,7 @@ static struct btrfs_trans_handle
*__unlink_start_trans(struct inode *dir,
 	struct btrfs_path *path;
 	struct btrfs_inode_ref *ref;
 	struct btrfs_dir_item *di;
+	struct btrfs_key location;
 	struct inode *inode = dentry->d_inode;
 	u64 index;
 	int check_link = 1;
@@ -2880,8 +2889,11 @@ static struct btrfs_trans_handle
*__unlink_start_trans(struct inode *dir,
 	path->skip_locking = 1;
 	path->search_commit_root = 1;
 
-	ret = btrfs_lookup_inode(trans, root, path,
-				&BTRFS_I(dir)->location, 0);
+	location.objectid = btrfs_ino(dir);
+	location.offset = 0;
+	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
+
+	ret = btrfs_lookup_inode(trans, root, path, &location, 0);
 	if (ret < 0) {
 		err = ret;
 		goto out;
@@ -2894,8 +2906,11 @@ static struct btrfs_trans_handle
*__unlink_start_trans(struct inode *dir,
 	}
 	btrfs_release_path(path);
 
-	ret = btrfs_lookup_inode(trans, root, path,
-				&BTRFS_I(inode)->location, 0);
+	location.objectid = btrfs_ino(inode);
+	location.offset = 0;
+	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
+
+	ret = btrfs_lookup_inode(trans, root, path, &location, 0);
 	if (ret < 0) {
 		err = ret;
 		goto out;
@@ -3097,7 +3112,7 @@ static int btrfs_rmdir(struct inode *dir, struct dentry
*dentry)
 
 	if (unlikely(btrfs_ino(inode) == BTRFS_EMPTY_SUBVOL_DIR_OBJECTID)) {
 		err = btrfs_unlink_subvol(trans, root, dir,
-					  BTRFS_I(inode)->location.objectid,
+					  BTRFS_I(inode)->inode_id,
 					  dentry->d_name.name,
 					  dentry->d_name.len);
 		goto out;
@@ -3331,7 +3346,7 @@ delete:
 		if (path->slots[0] == 0 ||
 		    path->slots[0] != pending_del_slot) {
 			if (root->ref_cows &&
-			    BTRFS_I(inode)->location.objectid !+			   
BTRFS_I(inode)->inode_id ! 						BTRFS_FREE_INO_OBJECTID) {
 				err = -EAGAIN;
 				goto out;
@@ -3965,7 +3980,7 @@ struct inode *btrfs_iget(struct super_block *s, struct
btrfs_key *location,
 
 	if (inode->i_state & I_NEW) {
 		BTRFS_I(inode)->root = root;
-		memcpy(&BTRFS_I(inode)->location, location, sizeof(*location));
+		BTRFS_I(inode)->inode_id = location->objectid;
 		btrfs_read_locked_inode(inode);
 		inode_tree_add(inode);
 		unlock_new_inode(inode);
@@ -3986,7 +4001,7 @@ static struct inode *new_simple_dir(struct super_block *s,
 		return ERR_PTR(-ENOMEM);
 
 	BTRFS_I(inode)->root = root;
-	memcpy(&BTRFS_I(inode)->location, key, sizeof(*key));
+	BTRFS_I(inode)->inode_id = key->objectid;
 	BTRFS_I(inode)->dummy_inode = 1;
 
 	inode->i_ino = BTRFS_EMPTY_SUBVOL_DIR_OBJECTID;
@@ -4417,7 +4432,6 @@ static struct inode *btrfs_new_inode(struct
btrfs_trans_handle *trans,
 {
 	struct inode *inode;
 	struct btrfs_inode_item *inode_item;
-	struct btrfs_key *location;
 	struct btrfs_path *path;
 	struct btrfs_inode_ref *ref;
 	struct btrfs_key key[2];
@@ -4461,6 +4475,7 @@ static struct inode *btrfs_new_inode(struct
btrfs_trans_handle *trans,
 	BTRFS_I(inode)->generation = trans->transid;
 	inode->i_generation = BTRFS_I(inode)->generation;
 	btrfs_set_inode_space_info(root, inode);
+	BTRFS_I(inode)->inode_id = objectid;
 
 	if (mode & S_IFDIR)
 		owner = 0;
@@ -4500,11 +4515,6 @@ static struct inode *btrfs_new_inode(struct
btrfs_trans_handle *trans,
 	btrfs_mark_buffer_dirty(path->nodes[0]);
 	btrfs_free_path(path);
 
-	location = &BTRFS_I(inode)->location;
-	location->objectid = objectid;
-	location->offset = 0;
-	btrfs_set_key_type(location, BTRFS_INODE_ITEM_KEY);
-
 	btrfs_inherit_iflags(inode, dir);
 
 	if ((mode & S_IFREG)) {
@@ -7029,7 +7039,7 @@ static int btrfs_rename(struct inode *old_dir, struct
dentry *old_dentry,
 		new_inode->i_ctime = CURRENT_TIME;
 		if (unlikely(btrfs_ino(new_inode) = 			     BTRFS_EMPTY_SUBVOL_DIR_OBJECTID))
{
-			root_objectid = BTRFS_I(new_inode)->location.objectid;
+			root_objectid = BTRFS_I(new_inode)->inode_id;
 			ret = btrfs_unlink_subvol(trans, dest, new_dir,
 						root_objectid,
 						new_dentry->d_name.name,
-- 
1.6.5.2

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Liu Bo

2011-Jun-16 09:23 UTC

head link

[PATCH 2/2] Btrfs: fix ''ls -lis'' mismatch

As btrfs uses delay allocation mechanism and data=order mode, there can be
a period window, during which we sub delalloc_bytes and add_inode_bytes,
and we may get a value of ''0'' referred to inode''s
blocks via ''ls -lis''.

ino:291 blocks:198656 i_blocks:0 i_bytes:0 delalloc_bytes:101711872
ino:291 blocks:198656 i_blocks:0 i_bytes:0 delalloc_bytes:101711872
                                                       <---------
ino:291 blocks:0 i_blocks:0 i_bytes:0 delalloc_bytes:0 |  THE
ino:291 blocks:0 i_blocks:0 i_bytes:0 delalloc_bytes:0 |  WINDOW
                                                       <---------
ino:291 blocks:819200 i_blocks:819200 i_bytes:0 delalloc_bytes:0

This may make btrfs''s users confused.

Hence, we use anther counter for the number of delalloc bytes in flight
that are accounted for in coordination with inode_add_bytes to ensure
correct output results.

Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
---
 fs/btrfs/btrfs_inode.h |    8 ++++++--
 fs/btrfs/inode.c       |   15 ++++++++++++---
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 31337df..1e0dc82 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -93,9 +93,13 @@ struct btrfs_inode {
 	 */
 	u64 logged_trans;
 
-	/* total number of bytes pending delalloc, used by stat to calc the
-	 * real block usage of the file
+	/*
+	 * total number of bytes pending delalloc, used by stat to
+	 * calc the real block usage of the file.
 	 */
+	u64 pending_bytes;
+
+	/* total number of bytes pending delalloc */
 	u64 delalloc_bytes;
 
 	/* total number of bytes that may be used for this inode for
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index e01a084..70bd01d 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -106,6 +106,13 @@ static int btrfs_init_inode_security(struct
btrfs_trans_handle *trans,
 	return err;
 }
 
+static void btrfs_inode_add_bytes(struct inode *inode, loff_t bytes)
+{
+	if (BTRFS_I(inode)->pending_bytes >= bytes)
+		BTRFS_I(inode)->pending_bytes -= bytes;
+	inode_add_bytes(inode, bytes);
+}
+
 /*
  * this does all the hard work for inserting an inline extent into
  * the btree.  The caller should have done a btrfs_drop_extents so that
@@ -144,7 +151,7 @@ static noinline int insert_inline_extent(struct
btrfs_trans_handle *trans,
 	btrfs_set_key_type(&key, BTRFS_EXTENT_DATA_KEY);
 	datasize = btrfs_file_extent_calc_inline_size(cur_size);
 
-	inode_add_bytes(inode, size);
+	btrfs_inode_add_bytes(inode, size);
 	ret = btrfs_insert_empty_item(trans, root, path, &key,
 				      datasize);
 	BUG_ON(ret);
@@ -1346,6 +1353,7 @@ static int btrfs_set_bit_hook(struct inode *inode,
 
 		spin_lock(&root->fs_info->delalloc_lock);
 		BTRFS_I(inode)->delalloc_bytes += len;
+		BTRFS_I(inode)->pending_bytes += len;
 		root->fs_info->delalloc_bytes += len;
 		if (do_list && list_empty(&BTRFS_I(inode)->delalloc_inodes)) {
 			list_add_tail(&BTRFS_I(inode)->delalloc_inodes,
@@ -1685,7 +1693,7 @@ static int insert_reserved_file_extent(struct
btrfs_trans_handle *trans,
 
 	btrfs_mark_buffer_dirty(leaf);
 
-	inode_add_bytes(inode, num_bytes);
+	btrfs_inode_add_bytes(inode, num_bytes);
 
 	ins.objectid = disk_bytenr;
 	ins.offset = disk_num_bytes;
@@ -6726,6 +6734,7 @@ struct inode *btrfs_alloc_inode(struct super_block *sb)
 	ei->last_trans = 0;
 	ei->last_sub_trans = 0;
 	ei->logged_trans = 0;
+	ei->pending_bytes = 0;
 	ei->delalloc_bytes = 0;
 	ei->reserved_bytes = 0;
 	ei->disk_i_size = 0;
@@ -6901,7 +6910,7 @@ static int btrfs_getattr(struct vfsmount *mnt,
 	stat->dev = BTRFS_I(inode)->root->anon_super.s_dev;
 	stat->blksize = PAGE_CACHE_SIZE;
 	stat->blocks = (inode_get_bytes(inode) +
-			BTRFS_I(inode)->delalloc_bytes) >> 9;
+			BTRFS_I(inode)->pending_bytes) >> 9;
 	return 0;
 }
 
-- 
1.6.5.2

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Liu Bo

2011-Jun-16 09:41 UTC

head link

[PATCH 1/2] Btrfs: kill location key of in-memory inode

In btrfs''s in-memory inode, there is a btrfs_key which has the
structure:
- key.objectid : inode id
- key.type: BTRFS_INODE_ITEM_KEY
- key.offset: 0 or -1ULL

however, we only use key.objectid to search, to check or something else,
and to reduce in-memory inode size I just keep what is valuable.

Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
---
 fs/btrfs/btrfs_inode.h |   10 ++++------
 fs/btrfs/disk-io.c     |    3 +--
 fs/btrfs/export.c      |    2 +-
 fs/btrfs/extent-tree.c |    2 +-
 fs/btrfs/inode.c       |   48 +++++++++++++++++++++++++++++-------------------
 5 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 52d7eca..31337df 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -29,11 +29,6 @@ struct btrfs_inode {
 	/* which subvolume this inode belongs to */
 	struct btrfs_root *root;
 
-	/* key used to find this inode on disk.  This is used by the code
-	 * to read in roots of subvolumes
-	 */
-	struct btrfs_key location;
-
 	/* the extent_tree has caches of all the extent mappings to disk */
 	struct extent_map_tree extent_tree;
 
@@ -72,6 +67,9 @@ struct btrfs_inode {
 	/* the space_info for where this inode''s data allocations are done */
 	struct btrfs_space_info *space_info;
 
+	/* full 64 bit inode number */
+	u64 inode_id;
+
 	/* full 64 bit generation number, struct vfs_inode doesn''t have a big
 	 * enough field for this.
 	 */
@@ -171,7 +169,7 @@ static inline struct btrfs_inode *BTRFS_I(struct inode
*inode)
 
 static inline u64 btrfs_ino(struct inode *inode)
 {
-	u64 ino = BTRFS_I(inode)->location.objectid;
+	u64 ino = BTRFS_I(inode)->inode_id;
 
 	if (ino <= BTRFS_FIRST_FREE_OBJECTID)
 		ino = inode->i_ino;
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a203d36..5d6bbb9 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1693,9 +1693,8 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 
 	BTRFS_I(fs_info->btree_inode)->io_tree.ops = &btree_extent_io_ops;
 
+	BTRFS_I(fs_info->btree_inode)->inode_id = BTRFS_BTREE_INODE_OBJECTID;
 	BTRFS_I(fs_info->btree_inode)->root = tree_root;
-	memset(&BTRFS_I(fs_info->btree_inode)->location, 0,
-	       sizeof(struct btrfs_key));
 	BTRFS_I(fs_info->btree_inode)->dummy_inode = 1;
 	insert_inode_hash(fs_info->btree_inode);
 
diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c
index 1b8dc33..cdd3a84 100644
--- a/fs/btrfs/export.c
+++ b/fs/btrfs/export.c
@@ -43,7 +43,7 @@ static int btrfs_encode_fh(struct dentry *dentry, u32 *fh, int
*max_len,
 		spin_lock(&dentry->d_lock);
 
 		parent = dentry->d_parent->d_inode;
-		fid->parent_objectid = BTRFS_I(parent)->location.objectid;
+		fid->parent_objectid = btrfs_ino(parent);
 		fid->parent_gen = parent->i_generation;
 		parent_root_id = BTRFS_I(parent)->root->objectid;
 
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 5b9b6b6..3a1f8ee 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3037,7 +3037,7 @@ int btrfs_check_data_free_space(struct inode *inode, u64
bytes)
 	bytes = (bytes + root->sectorsize - 1) & ~((u64)root->sectorsize -
1);
 
 	if (root == root->fs_info->tree_root ||
-	    BTRFS_I(inode)->location.objectid == BTRFS_FREE_INO_OBJECTID) {
+	    BTRFS_I(inode)->inode_id == BTRFS_FREE_INO_OBJECTID) {
 		alloc_chunk = 0;
 		committed = 1;
 	}
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 02ff4a1..e01a084 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -754,7 +754,7 @@ static inline bool is_free_space_inode(struct btrfs_root
*root,
 				       struct inode *inode)
 {
 	if (root == root->fs_info->tree_root ||
-	    BTRFS_I(inode)->location.objectid == BTRFS_FREE_INO_OBJECTID)
+	    BTRFS_I(inode)->inode_id == BTRFS_FREE_INO_OBJECTID)
 		return true;
 	return false;
 }
@@ -2513,7 +2513,10 @@ static void btrfs_read_locked_inode(struct inode *inode)
 	path = btrfs_alloc_path();
 	BUG_ON(!path);
 	path->leave_spinning = 1;
-	memcpy(&location, &BTRFS_I(inode)->location, sizeof(location));
+
+	location.objectid = btrfs_ino(inode);
+	location.offset = 0;
+	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
 
 	ret = btrfs_lookup_inode(NULL, root, path, &location, 0);
 	if (ret)
@@ -2667,6 +2670,7 @@ noinline int btrfs_update_inode(struct btrfs_trans_handle
*trans,
 	struct btrfs_inode_item *inode_item;
 	struct btrfs_path *path;
 	struct extent_buffer *leaf;
+	struct btrfs_key location;
 	int ret;
 
 	/*
@@ -2687,8 +2691,12 @@ noinline int btrfs_update_inode(struct btrfs_trans_handle
*trans,
 		return -ENOMEM;
 
 	path->leave_spinning = 1;
-	ret = btrfs_lookup_inode(trans, root, path, &BTRFS_I(inode)->location,
-				 1);
+
+	location.objectid = btrfs_ino(inode);
+	location.offset = 0;
+	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
+
+	ret = btrfs_lookup_inode(trans, root, path, &location, 1);
 	if (ret) {
 		if (ret > 0)
 			ret = -ENOENT;
@@ -2839,6 +2847,7 @@ static struct btrfs_trans_handle
*__unlink_start_trans(struct inode *dir,
 	struct btrfs_path *path;
 	struct btrfs_inode_ref *ref;
 	struct btrfs_dir_item *di;
+	struct btrfs_key location;
 	struct inode *inode = dentry->d_inode;
 	u64 index;
 	int check_link = 1;
@@ -2880,8 +2889,11 @@ static struct btrfs_trans_handle
*__unlink_start_trans(struct inode *dir,
 	path->skip_locking = 1;
 	path->search_commit_root = 1;
 
-	ret = btrfs_lookup_inode(trans, root, path,
-				&BTRFS_I(dir)->location, 0);
+	location.objectid = btrfs_ino(dir);
+	location.offset = 0;
+	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
+
+	ret = btrfs_lookup_inode(trans, root, path, &location, 0);
 	if (ret < 0) {
 		err = ret;
 		goto out;
@@ -2894,8 +2906,11 @@ static struct btrfs_trans_handle
*__unlink_start_trans(struct inode *dir,
 	}
 	btrfs_release_path(path);
 
-	ret = btrfs_lookup_inode(trans, root, path,
-				&BTRFS_I(inode)->location, 0);
+	location.objectid = btrfs_ino(inode);
+	location.offset = 0;
+	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
+
+	ret = btrfs_lookup_inode(trans, root, path, &location, 0);
 	if (ret < 0) {
 		err = ret;
 		goto out;
@@ -3097,7 +3112,7 @@ static int btrfs_rmdir(struct inode *dir, struct dentry
*dentry)
 
 	if (unlikely(btrfs_ino(inode) == BTRFS_EMPTY_SUBVOL_DIR_OBJECTID)) {
 		err = btrfs_unlink_subvol(trans, root, dir,
-					  BTRFS_I(inode)->location.objectid,
+					  BTRFS_I(inode)->inode_id,
 					  dentry->d_name.name,
 					  dentry->d_name.len);
 		goto out;
@@ -3331,7 +3346,7 @@ delete:
 		if (path->slots[0] == 0 ||
 		    path->slots[0] != pending_del_slot) {
 			if (root->ref_cows &&
-			    BTRFS_I(inode)->location.objectid !+			   
BTRFS_I(inode)->inode_id ! 						BTRFS_FREE_INO_OBJECTID) {
 				err = -EAGAIN;
 				goto out;
@@ -3965,7 +3980,7 @@ struct inode *btrfs_iget(struct super_block *s, struct
btrfs_key *location,
 
 	if (inode->i_state & I_NEW) {
 		BTRFS_I(inode)->root = root;
-		memcpy(&BTRFS_I(inode)->location, location, sizeof(*location));
+		BTRFS_I(inode)->inode_id = location->objectid;
 		btrfs_read_locked_inode(inode);
 		inode_tree_add(inode);
 		unlock_new_inode(inode);
@@ -3986,7 +4001,7 @@ static struct inode *new_simple_dir(struct super_block *s,
 		return ERR_PTR(-ENOMEM);
 
 	BTRFS_I(inode)->root = root;
-	memcpy(&BTRFS_I(inode)->location, key, sizeof(*key));
+	BTRFS_I(inode)->inode_id = key->objectid;
 	BTRFS_I(inode)->dummy_inode = 1;
 
 	inode->i_ino = BTRFS_EMPTY_SUBVOL_DIR_OBJECTID;
@@ -4417,7 +4432,6 @@ static struct inode *btrfs_new_inode(struct
btrfs_trans_handle *trans,
 {
 	struct inode *inode;
 	struct btrfs_inode_item *inode_item;
-	struct btrfs_key *location;
 	struct btrfs_path *path;
 	struct btrfs_inode_ref *ref;
 	struct btrfs_key key[2];
@@ -4461,6 +4475,7 @@ static struct inode *btrfs_new_inode(struct
btrfs_trans_handle *trans,
 	BTRFS_I(inode)->generation = trans->transid;
 	inode->i_generation = BTRFS_I(inode)->generation;
 	btrfs_set_inode_space_info(root, inode);
+	BTRFS_I(inode)->inode_id = objectid;
 
 	if (mode & S_IFDIR)
 		owner = 0;
@@ -4500,11 +4515,6 @@ static struct inode *btrfs_new_inode(struct
btrfs_trans_handle *trans,
 	btrfs_mark_buffer_dirty(path->nodes[0]);
 	btrfs_free_path(path);
 
-	location = &BTRFS_I(inode)->location;
-	location->objectid = objectid;
-	location->offset = 0;
-	btrfs_set_key_type(location, BTRFS_INODE_ITEM_KEY);
-
 	btrfs_inherit_iflags(inode, dir);
 
 	if ((mode & S_IFREG)) {
@@ -7029,7 +7039,7 @@ static int btrfs_rename(struct inode *old_dir, struct
dentry *old_dentry,
 		new_inode->i_ctime = CURRENT_TIME;
 		if (unlikely(btrfs_ino(new_inode) = 			     BTRFS_EMPTY_SUBVOL_DIR_OBJECTID))
{
-			root_objectid = BTRFS_I(new_inode)->location.objectid;
+			root_objectid = BTRFS_I(new_inode)->inode_id;
 			ret = btrfs_unlink_subvol(trans, dest, new_dir,
 						root_objectid,
 						new_dentry->d_name.name,
-- 
1.6.5.2

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Liu Bo

2011-Jun-16 09:41 UTC

head link

[PATCH 2/2] Btrfs: fix ''ls -lis'' mismatch

As btrfs uses delay allocation mechanism and data=order mode, there can be
a period window, during which we sub delalloc_bytes and add_inode_bytes,
and we may get a value of ''0'' referred to inode''s
blocks via ''ls -lis''.

ino:291 blocks:198656 i_blocks:0 i_bytes:0 delalloc_bytes:101711872
ino:291 blocks:198656 i_blocks:0 i_bytes:0 delalloc_bytes:101711872
                                                       <---------
ino:291 blocks:0 i_blocks:0 i_bytes:0 delalloc_bytes:0 |  THE
ino:291 blocks:0 i_blocks:0 i_bytes:0 delalloc_bytes:0 |  WINDOW
                                                       <---------
ino:291 blocks:819200 i_blocks:819200 i_bytes:0 delalloc_bytes:0

This may make btrfs''s users confused.

Hence, we use anther counter for the number of delalloc bytes in flight
that are accounted for in coordination with inode_add_bytes to ensure
correct output results.

Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
---
 fs/btrfs/btrfs_inode.h |    8 ++++++--
 fs/btrfs/inode.c       |   15 ++++++++++++---
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 31337df..1e0dc82 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -93,9 +93,13 @@ struct btrfs_inode {
 	 */
 	u64 logged_trans;
 
-	/* total number of bytes pending delalloc, used by stat to calc the
-	 * real block usage of the file
+	/*
+	 * total number of bytes pending delalloc, used by stat to
+	 * calc the real block usage of the file.
 	 */
+	u64 pending_bytes;
+
+	/* total number of bytes pending delalloc */
 	u64 delalloc_bytes;
 
 	/* total number of bytes that may be used for this inode for
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index e01a084..70bd01d 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -106,6 +106,13 @@ static int btrfs_init_inode_security(struct
btrfs_trans_handle *trans,
 	return err;
 }
 
+static void btrfs_inode_add_bytes(struct inode *inode, loff_t bytes)
+{
+	if (BTRFS_I(inode)->pending_bytes >= bytes)
+		BTRFS_I(inode)->pending_bytes -= bytes;
+	inode_add_bytes(inode, bytes);
+}
+
 /*
  * this does all the hard work for inserting an inline extent into
  * the btree.  The caller should have done a btrfs_drop_extents so that
@@ -144,7 +151,7 @@ static noinline int insert_inline_extent(struct
btrfs_trans_handle *trans,
 	btrfs_set_key_type(&key, BTRFS_EXTENT_DATA_KEY);
 	datasize = btrfs_file_extent_calc_inline_size(cur_size);
 
-	inode_add_bytes(inode, size);
+	btrfs_inode_add_bytes(inode, size);
 	ret = btrfs_insert_empty_item(trans, root, path, &key,
 				      datasize);
 	BUG_ON(ret);
@@ -1346,6 +1353,7 @@ static int btrfs_set_bit_hook(struct inode *inode,
 
 		spin_lock(&root->fs_info->delalloc_lock);
 		BTRFS_I(inode)->delalloc_bytes += len;
+		BTRFS_I(inode)->pending_bytes += len;
 		root->fs_info->delalloc_bytes += len;
 		if (do_list && list_empty(&BTRFS_I(inode)->delalloc_inodes)) {
 			list_add_tail(&BTRFS_I(inode)->delalloc_inodes,
@@ -1685,7 +1693,7 @@ static int insert_reserved_file_extent(struct
btrfs_trans_handle *trans,
 
 	btrfs_mark_buffer_dirty(leaf);
 
-	inode_add_bytes(inode, num_bytes);
+	btrfs_inode_add_bytes(inode, num_bytes);
 
 	ins.objectid = disk_bytenr;
 	ins.offset = disk_num_bytes;
@@ -6726,6 +6734,7 @@ struct inode *btrfs_alloc_inode(struct super_block *sb)
 	ei->last_trans = 0;
 	ei->last_sub_trans = 0;
 	ei->logged_trans = 0;
+	ei->pending_bytes = 0;
 	ei->delalloc_bytes = 0;
 	ei->reserved_bytes = 0;
 	ei->disk_i_size = 0;
@@ -6901,7 +6910,7 @@ static int btrfs_getattr(struct vfsmount *mnt,
 	stat->dev = BTRFS_I(inode)->root->anon_super.s_dev;
 	stat->blksize = PAGE_CACHE_SIZE;
 	stat->blocks = (inode_get_bytes(inode) +
-			BTRFS_I(inode)->delalloc_bytes) >> 9;
+			BTRFS_I(inode)->pending_bytes) >> 9;
 	return 0;
 }
 
-- 
1.6.5.2

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

David Sterba

2011-Jun-18 15:21 UTC

head link

Re: [PATCH 1/2] Btrfs: kill location key of in-memory inode

Hi,

On Thu, Jun 16, 2011 at 05:41:10PM +0800, Liu Bo wrote:> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -29,11 +29,6 @@ struct btrfs_inode {
>  	/* which subvolume this inode belongs to */
>  	struct btrfs_root *root;
>  
> -	/* key used to find this inode on disk.  This is used by the code
> -	 * to read in roots of subvolumes
> -	 */
> -	struct btrfs_key location;
> -
>  	/* the extent_tree has caches of all the extent mappings to disk */
>  	struct extent_map_tree extent_tree;
>  
> @@ -72,6 +67,9 @@ struct btrfs_inode {
>  	/* the space_info for where this inode''s data allocations are
done */
>  	struct btrfs_space_info *space_info;
>  
> +	/* full 64 bit inode number */
> +	u64 inode_id;
I like this optimization! It saves 16 bytes (7 for padding after
locatioin and 1+8 for the removed key type and offset). However I''d
rather see this named ''ino'' or ''i_ino'',
it''s a more common name for
inode number.
> +
>  	/* full 64 bit generation number, struct vfs_inode doesn''t have
a big
>  	 * enough field for this.
>  	 */
> @@ -171,7 +169,7 @@ static inline struct btrfs_inode *BTRFS_I(struct inode
*inode)
>  
>  static inline u64 btrfs_ino(struct inode *inode)
>  {
> -	u64 ino = BTRFS_I(inode)->location.objectid;
> +	u64 ino = BTRFS_I(inode)->inode_id;
see? :)
>  
>  	if (ino <= BTRFS_FIRST_FREE_OBJECTID)
>  		ino = inode->i_ino;
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index a203d36..5d6bbb9 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1693,9 +1693,8 @@ struct btrfs_root *open_ctree(struct super_block *sb,
>  
>  	BTRFS_I(fs_info->btree_inode)->io_tree.ops =
&btree_extent_io_ops;
>  
> +	BTRFS_I(fs_info->btree_inode)->inode_id =
BTRFS_BTREE_INODE_OBJECTID;
>  	BTRFS_I(fs_info->btree_inode)->root = tree_root;
> -	memset(&BTRFS_I(fs_info->btree_inode)->location, 0,
> -	       sizeof(struct btrfs_key));
>  	BTRFS_I(fs_info->btree_inode)->dummy_inode = 1;
>  	insert_inode_hash(fs_info->btree_inode);
>  
> diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c
> index 1b8dc33..cdd3a84 100644
> --- a/fs/btrfs/export.c
> +++ b/fs/btrfs/export.c
> @@ -43,7 +43,7 @@ static int btrfs_encode_fh(struct dentry *dentry, u32
*fh, int *max_len,
>  		spin_lock(&dentry->d_lock);
>  
>  		parent = dentry->d_parent->d_inode;
> -		fid->parent_objectid = BTRFS_I(parent)->location.objectid;
> +		fid->parent_objectid = btrfs_ino(parent);
is there a reason to do btrfs_ino here instead of simple
BTRFS_I(parent)->inode_id ? Although there is just one more conditional
test whether i_ino is <= BTRFS_FREE_INO_OBJECTID, which would not hurt
so much, using btrfs_ino says this is to be expected and has a different
meaning than the original code.

The changes are not consistent, you add btrfs_ino where was a memcpy in
the original code but not everywhere:
>  		fid->parent_gen = parent->i_generation;
>  		parent_root_id = BTRFS_I(parent)->root->objectid;
>  
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 5b9b6b6..3a1f8ee 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3037,7 +3037,7 @@ int btrfs_check_data_free_space(struct inode *inode,
u64 bytes)
>  	bytes = (bytes + root->sectorsize - 1) &
~((u64)root->sectorsize - 1);
>  
>  	if (root == root->fs_info->tree_root ||
> -	    BTRFS_I(inode)->location.objectid == BTRFS_FREE_INO_OBJECTID) {
> +	    BTRFS_I(inode)->inode_id == BTRFS_FREE_INO_OBJECTID) {
>  		alloc_chunk = 0;
>  		committed = 1;
>  	}
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 02ff4a1..e01a084 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -754,7 +754,7 @@ static inline bool is_free_space_inode(struct
btrfs_root *root,
>  				       struct inode *inode)
>  {
>  	if (root == root->fs_info->tree_root ||
> -	    BTRFS_I(inode)->location.objectid == BTRFS_FREE_INO_OBJECTID)
> +	    BTRFS_I(inode)->inode_id == BTRFS_FREE_INO_OBJECTID)
>  		return true;
>  	return false;
>  }
> @@ -2513,7 +2513,10 @@ static void btrfs_read_locked_inode(struct inode
*inode)
>  	path = btrfs_alloc_path();
>  	BUG_ON(!path);
>  	path->leave_spinning = 1;
> -	memcpy(&location, &BTRFS_I(inode)->location,
sizeof(location));
> +
> +	location.objectid = btrfs_ino(inode);
> +	location.offset = 0;
> +	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
memcpy -> btrfs_ino
>  
>  	ret = btrfs_lookup_inode(NULL, root, path, &location, 0);
>  	if (ret)
> @@ -2667,6 +2670,7 @@ noinline int btrfs_update_inode(struct
btrfs_trans_handle *trans,
>  	struct btrfs_inode_item *inode_item;
>  	struct btrfs_path *path;
>  	struct extent_buffer *leaf;
> +	struct btrfs_key location;
>  	int ret;
>  
>  	/*
> @@ -2687,8 +2691,12 @@ noinline int btrfs_update_inode(struct
btrfs_trans_handle *trans,
>  		return -ENOMEM;
>  
>  	path->leave_spinning = 1;
> -	ret = btrfs_lookup_inode(trans, root, path,
&BTRFS_I(inode)->location,
> -				 1);
> +
> +	location.objectid = btrfs_ino(inode);
not sure about this, but I assume location.objectid will never be
<= FIRST_FREE_OBJECT_ID and btrfs_ino will return
BTRFS_I->location.objectid anyway. (there are more occurences of this
pattern)
> +	location.offset = 0;
> +	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
> +
> +	ret = btrfs_lookup_inode(trans, root, path, &location, 1);
>  	if (ret) {
>  		if (ret > 0)
>  			ret = -ENOENT;
> @@ -2839,6 +2847,7 @@ static struct btrfs_trans_handle
*__unlink_start_trans(struct inode *dir,
>  	struct btrfs_path *path;
>  	struct btrfs_inode_ref *ref;
>  	struct btrfs_dir_item *di;
> +	struct btrfs_key location;
>  	struct inode *inode = dentry->d_inode;
>  	u64 index;
>  	int check_link = 1;
> @@ -2880,8 +2889,11 @@ static struct btrfs_trans_handle
*__unlink_start_trans(struct inode *dir,
>  	path->skip_locking = 1;
>  	path->search_commit_root = 1;
>  
> -	ret = btrfs_lookup_inode(trans, root, path,
> -				&BTRFS_I(dir)->location, 0);
> +	location.objectid = btrfs_ino(dir);
> +	location.offset = 0;
> +	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
> +
> +	ret = btrfs_lookup_inode(trans, root, path, &location, 0);
>  	if (ret < 0) {
>  		err = ret;
>  		goto out;
> @@ -2894,8 +2906,11 @@ static struct btrfs_trans_handle
*__unlink_start_trans(struct inode *dir,
>  	}
>  	btrfs_release_path(path);
>  
> -	ret = btrfs_lookup_inode(trans, root, path,
> -				&BTRFS_I(inode)->location, 0);
> +	location.objectid = btrfs_ino(inode);
> +	location.offset = 0;
> +	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
> +
> +	ret = btrfs_lookup_inode(trans, root, path, &location, 0);
>  	if (ret < 0) {
>  		err = ret;
>  		goto out;
> @@ -3097,7 +3112,7 @@ static int btrfs_rmdir(struct inode *dir, struct
dentry *dentry)
>  
>  	if (unlikely(btrfs_ino(inode) == BTRFS_EMPTY_SUBVOL_DIR_OBJECTID)) {
>  		err = btrfs_unlink_subvol(trans, root, dir,
> -					  BTRFS_I(inode)->location.objectid,
> +					  BTRFS_I(inode)->inode_id,
>  					  dentry->d_name.name,
>  					  dentry->d_name.len);
>  		goto out;
> @@ -3331,7 +3346,7 @@ delete:
>  		if (path->slots[0] == 0 ||
>  		    path->slots[0] != pending_del_slot) {
>  			if (root->ref_cows &&
> -			    BTRFS_I(inode)->location.objectid !> +			   
BTRFS_I(inode)->inode_id !>  						BTRFS_FREE_INO_OBJECTID) {
>  				err = -EAGAIN;
>  				goto out;
> @@ -3965,7 +3980,7 @@ struct inode *btrfs_iget(struct super_block *s,
struct btrfs_key *location,
>  
>  	if (inode->i_state & I_NEW) {
>  		BTRFS_I(inode)->root = root;
> -		memcpy(&BTRFS_I(inode)->location, location, sizeof(*location));
> +		BTRFS_I(inode)->inode_id = location->objectid;
memcpy -> direct assignment
>  		btrfs_read_locked_inode(inode);
>  		inode_tree_add(inode);
>  		unlock_new_inode(inode);
> @@ -3986,7 +4001,7 @@ static struct inode *new_simple_dir(struct
super_block *s,
>  		return ERR_PTR(-ENOMEM);
>  
>  	BTRFS_I(inode)->root = root;
> -	memcpy(&BTRFS_I(inode)->location, key, sizeof(*key));
> +	BTRFS_I(inode)->inode_id = key->objectid;
memcpy -> direct assignment
>  	BTRFS_I(inode)->dummy_inode = 1;
>  
>  	inode->i_ino = BTRFS_EMPTY_SUBVOL_DIR_OBJECTID;
> @@ -4417,7 +4432,6 @@ static struct inode *btrfs_new_inode(struct
btrfs_trans_handle *trans,
>  {
>  	struct inode *inode;
>  	struct btrfs_inode_item *inode_item;
> -	struct btrfs_key *location;
>  	struct btrfs_path *path;
>  	struct btrfs_inode_ref *ref;
>  	struct btrfs_key key[2];
> @@ -4461,6 +4475,7 @@ static struct inode *btrfs_new_inode(struct
btrfs_trans_handle *trans,
>  	BTRFS_I(inode)->generation = trans->transid;
>  	inode->i_generation = BTRFS_I(inode)->generation;
>  	btrfs_set_inode_space_info(root, inode);
> +	BTRFS_I(inode)->inode_id = objectid;
>  
>  	if (mode & S_IFDIR)
>  		owner = 0;
> @@ -4500,11 +4515,6 @@ static struct inode *btrfs_new_inode(struct
btrfs_trans_handle *trans,
>  	btrfs_mark_buffer_dirty(path->nodes[0]);
>  	btrfs_free_path(path);
>  
> -	location = &BTRFS_I(inode)->location;
> -	location->objectid = objectid;
> -	location->offset = 0;
> -	btrfs_set_key_type(location, BTRFS_INODE_ITEM_KEY);
> -
>  	btrfs_inherit_iflags(inode, dir);
>  
>  	if ((mode & S_IFREG)) {
> @@ -7029,7 +7039,7 @@ static int btrfs_rename(struct inode *old_dir, struct
dentry *old_dentry,
>  		new_inode->i_ctime = CURRENT_TIME;
>  		if (unlikely(btrfs_ino(new_inode) =>  			    
BTRFS_EMPTY_SUBVOL_DIR_OBJECTID)) {
> -			root_objectid = BTRFS_I(new_inode)->location.objectid;
> +			root_objectid = BTRFS_I(new_inode)->inode_id;
direct assignment, no btrfs_ino as in the first hunk
>  			ret = btrfs_unlink_subvol(trans, dest, new_dir,
>  						root_objectid,
>  						new_dentry->d_name.name,
> -- 

david
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

liubo

2011-Jun-20 02:04 UTC

head link

Re: [PATCH 1/2] Btrfs: kill location key of in-memory inode

On 06/18/2011 11:21 PM, David Sterba wrote:> Hi,
> 
> On Thu, Jun 16, 2011 at 05:41:10PM +0800, Liu Bo wrote:
>> --- a/fs/btrfs/btrfs_inode.h
>> +++ b/fs/btrfs/btrfs_inode.h
>> @@ -29,11 +29,6 @@ struct btrfs_inode {
>>  	/* which subvolume this inode belongs to */
>>  	struct btrfs_root *root;
>>  
>> -	/* key used to find this inode on disk.  This is used by the code
>> -	 * to read in roots of subvolumes
>> -	 */
>> -	struct btrfs_key location;
>> -
>>  	/* the extent_tree has caches of all the extent mappings to disk */
>>  	struct extent_map_tree extent_tree;
>>  
>> @@ -72,6 +67,9 @@ struct btrfs_inode {
>>  	/* the space_info for where this inode''s data allocations
are done */
>>  	struct btrfs_space_info *space_info;
>>  
>> +	/* full 64 bit inode number */
>> +	u64 inode_id;
> 
> I like this optimization! It saves 16 bytes (7 for padding after
> locatioin and 1+8 for the removed key type and offset). However
I''d
> rather see this named ''ino'' or ''i_ino'',
it''s a more common name for
> inode number.
> 
This is what I''m going to improve then.
>> +
>>  	/* full 64 bit generation number, struct vfs_inode doesn''t
have a big
>>  	 * enough field for this.
>>  	 */
>> @@ -171,7 +169,7 @@ static inline struct btrfs_inode *BTRFS_I(struct
inode *inode)
>>  
>>  static inline u64 btrfs_ino(struct inode *inode)
>>  {
>> -	u64 ino = BTRFS_I(inode)->location.objectid;
>> +	u64 ino = BTRFS_I(inode)->inode_id;
> 
> see? :)
> 
>>  
>>  	if (ino <= BTRFS_FIRST_FREE_OBJECTID)
>>  		ino = inode->i_ino;
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index a203d36..5d6bbb9 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -1693,9 +1693,8 @@ struct btrfs_root *open_ctree(struct super_block
*sb,
>>  
>>  	BTRFS_I(fs_info->btree_inode)->io_tree.ops =
&btree_extent_io_ops;
>>  
>> +	BTRFS_I(fs_info->btree_inode)->inode_id =
BTRFS_BTREE_INODE_OBJECTID;
>>  	BTRFS_I(fs_info->btree_inode)->root = tree_root;
>> -	memset(&BTRFS_I(fs_info->btree_inode)->location, 0,
>> -	       sizeof(struct btrfs_key));
>>  	BTRFS_I(fs_info->btree_inode)->dummy_inode = 1;
>>  	insert_inode_hash(fs_info->btree_inode);
>>  
>> diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c
>> index 1b8dc33..cdd3a84 100644
>> --- a/fs/btrfs/export.c
>> +++ b/fs/btrfs/export.c
>> @@ -43,7 +43,7 @@ static int btrfs_encode_fh(struct dentry *dentry, u32
*fh, int *max_len,
>>  		spin_lock(&dentry->d_lock);
>>  
>>  		parent = dentry->d_parent->d_inode;
>> -		fid->parent_objectid = BTRFS_I(parent)->location.objectid;
>> +		fid->parent_objectid = btrfs_ino(parent);
> 
> is there a reason to do btrfs_ino here instead of simple
> BTRFS_I(parent)->inode_id ? Although there is just one more conditional
> test whether i_ino is <= BTRFS_FREE_INO_OBJECTID, which would not hurt
> so much, using btrfs_ino says this is to be expected and has a different
> meaning than the original code.
> 
btrfs_ino provides the 64bit ino, so if we want to get a 64bit one, just
use btrfs_ino instead.
> The changes are not consistent, you add btrfs_ino where was a memcpy in
> the original code but not everywhere:
leave those memcpy alone, what we want is to get the ino, since we have killed
the location key, right?
> 
>>  		fid->parent_gen = parent->i_generation;
>>  		parent_root_id = BTRFS_I(parent)->root->objectid;
>>  
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 5b9b6b6..3a1f8ee 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -3037,7 +3037,7 @@ int btrfs_check_data_free_space(struct inode
*inode, u64 bytes)
>>  	bytes = (bytes + root->sectorsize - 1) &
~((u64)root->sectorsize - 1);
>>  
>>  	if (root == root->fs_info->tree_root ||
>> -	    BTRFS_I(inode)->location.objectid == BTRFS_FREE_INO_OBJECTID)
{
>> +	    BTRFS_I(inode)->inode_id == BTRFS_FREE_INO_OBJECTID) {
>>  		alloc_chunk = 0;
>>  		committed = 1;
>>  	}
>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>> index 02ff4a1..e01a084 100644
>> --- a/fs/btrfs/inode.c
>> +++ b/fs/btrfs/inode.c
>> @@ -754,7 +754,7 @@ static inline bool is_free_space_inode(struct
btrfs_root *root,
>>  				       struct inode *inode)
>>  {
>>  	if (root == root->fs_info->tree_root ||
>> -	    BTRFS_I(inode)->location.objectid == BTRFS_FREE_INO_OBJECTID)
>> +	    BTRFS_I(inode)->inode_id == BTRFS_FREE_INO_OBJECTID)
>>  		return true;
>>  	return false;
>>  }
>> @@ -2513,7 +2513,10 @@ static void btrfs_read_locked_inode(struct inode
*inode)
>>  	path = btrfs_alloc_path();
>>  	BUG_ON(!path);
>>  	path->leave_spinning = 1;
>> -	memcpy(&location, &BTRFS_I(inode)->location,
sizeof(location));
>> +
>> +	location.objectid = btrfs_ino(inode);
>> +	location.offset = 0;
>> +	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
> 
> memcpy -> btrfs_ino
> 
>>  
>>  	ret = btrfs_lookup_inode(NULL, root, path, &location, 0);
>>  	if (ret)
>> @@ -2667,6 +2670,7 @@ noinline int btrfs_update_inode(struct
btrfs_trans_handle *trans,
>>  	struct btrfs_inode_item *inode_item;
>>  	struct btrfs_path *path;
>>  	struct extent_buffer *leaf;
>> +	struct btrfs_key location;
>>  	int ret;
>>  
>>  	/*
>> @@ -2687,8 +2691,12 @@ noinline int btrfs_update_inode(struct
btrfs_trans_handle *trans,
>>  		return -ENOMEM;
>>  
>>  	path->leave_spinning = 1;
>> -	ret = btrfs_lookup_inode(trans, root, path,
&BTRFS_I(inode)->location,
>> -				 1);
>> +
>> +	location.objectid = btrfs_ino(inode);
> 
> not sure about this, but I assume location.objectid will never be
> <= FIRST_FREE_OBJECT_ID and btrfs_ino will return
> BTRFS_I->location.objectid anyway. (there are more occurences of this
> pattern)
> 
>> +	location.offset = 0;
>> +	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
>> +
>> +	ret = btrfs_lookup_inode(trans, root, path, &location, 1);
>>  	if (ret) {
>>  		if (ret > 0)
>>  			ret = -ENOENT;
>> @@ -2839,6 +2847,7 @@ static struct btrfs_trans_handle
*__unlink_start_trans(struct inode *dir,
>>  	struct btrfs_path *path;
>>  	struct btrfs_inode_ref *ref;
>>  	struct btrfs_dir_item *di;
>> +	struct btrfs_key location;
>>  	struct inode *inode = dentry->d_inode;
>>  	u64 index;
>>  	int check_link = 1;
>> @@ -2880,8 +2889,11 @@ static struct btrfs_trans_handle
*__unlink_start_trans(struct inode *dir,
>>  	path->skip_locking = 1;
>>  	path->search_commit_root = 1;
>>  
>> -	ret = btrfs_lookup_inode(trans, root, path,
>> -				&BTRFS_I(dir)->location, 0);
>> +	location.objectid = btrfs_ino(dir);
>> +	location.offset = 0;
>> +	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
>> +
>> +	ret = btrfs_lookup_inode(trans, root, path, &location, 0);
>>  	if (ret < 0) {
>>  		err = ret;
>>  		goto out;
>> @@ -2894,8 +2906,11 @@ static struct btrfs_trans_handle
*__unlink_start_trans(struct inode *dir,
>>  	}
>>  	btrfs_release_path(path);
>>  
>> -	ret = btrfs_lookup_inode(trans, root, path,
>> -				&BTRFS_I(inode)->location, 0);
>> +	location.objectid = btrfs_ino(inode);
>> +	location.offset = 0;
>> +	btrfs_set_key_type(&location, BTRFS_INODE_ITEM_KEY);
>> +
>> +	ret = btrfs_lookup_inode(trans, root, path, &location, 0);
>>  	if (ret < 0) {
>>  		err = ret;
>>  		goto out;
>> @@ -3097,7 +3112,7 @@ static int btrfs_rmdir(struct inode *dir, struct
dentry *dentry)
>>  
>>  	if (unlikely(btrfs_ino(inode) == BTRFS_EMPTY_SUBVOL_DIR_OBJECTID)) {
>>  		err = btrfs_unlink_subvol(trans, root, dir,
>> -					  BTRFS_I(inode)->location.objectid,
>> +					  BTRFS_I(inode)->inode_id,
>>  					  dentry->d_name.name,
>>  					  dentry->d_name.len);
>>  		goto out;
>> @@ -3331,7 +3346,7 @@ delete:
>>  		if (path->slots[0] == 0 ||
>>  		    path->slots[0] != pending_del_slot) {
>>  			if (root->ref_cows &&
>> -			    BTRFS_I(inode)->location.objectid !>> +			   
BTRFS_I(inode)->inode_id !>>  						BTRFS_FREE_INO_OBJECTID) {
>>  				err = -EAGAIN;
>>  				goto out;
>> @@ -3965,7 +3980,7 @@ struct inode *btrfs_iget(struct super_block *s,
struct btrfs_key *location,
>>  
>>  	if (inode->i_state & I_NEW) {
>>  		BTRFS_I(inode)->root = root;
>> -		memcpy(&BTRFS_I(inode)->location, location,
sizeof(*location));
>> +		BTRFS_I(inode)->inode_id = location->objectid;
> 
> memcpy -> direct assignment
> 
>>  		btrfs_read_locked_inode(inode);
>>  		inode_tree_add(inode);
>>  		unlock_new_inode(inode);
>> @@ -3986,7 +4001,7 @@ static struct inode *new_simple_dir(struct
super_block *s,
>>  		return ERR_PTR(-ENOMEM);
>>  
>>  	BTRFS_I(inode)->root = root;
>> -	memcpy(&BTRFS_I(inode)->location, key, sizeof(*key));
>> +	BTRFS_I(inode)->inode_id = key->objectid;
> 
> memcpy -> direct assignment
> 
>>  	BTRFS_I(inode)->dummy_inode = 1;
>>  
>>  	inode->i_ino = BTRFS_EMPTY_SUBVOL_DIR_OBJECTID;
>> @@ -4417,7 +4432,6 @@ static struct inode *btrfs_new_inode(struct
btrfs_trans_handle *trans,
>>  {
>>  	struct inode *inode;
>>  	struct btrfs_inode_item *inode_item;
>> -	struct btrfs_key *location;
>>  	struct btrfs_path *path;
>>  	struct btrfs_inode_ref *ref;
>>  	struct btrfs_key key[2];
>> @@ -4461,6 +4475,7 @@ static struct inode *btrfs_new_inode(struct
btrfs_trans_handle *trans,
>>  	BTRFS_I(inode)->generation = trans->transid;
>>  	inode->i_generation = BTRFS_I(inode)->generation;
>>  	btrfs_set_inode_space_info(root, inode);
>> +	BTRFS_I(inode)->inode_id = objectid;
>>  
>>  	if (mode & S_IFDIR)
>>  		owner = 0;
>> @@ -4500,11 +4515,6 @@ static struct inode *btrfs_new_inode(struct
btrfs_trans_handle *trans,
>>  	btrfs_mark_buffer_dirty(path->nodes[0]);
>>  	btrfs_free_path(path);
>>  
>> -	location = &BTRFS_I(inode)->location;
>> -	location->objectid = objectid;
>> -	location->offset = 0;
>> -	btrfs_set_key_type(location, BTRFS_INODE_ITEM_KEY);
>> -
>>  	btrfs_inherit_iflags(inode, dir);
>>  
>>  	if ((mode & S_IFREG)) {
>> @@ -7029,7 +7039,7 @@ static int btrfs_rename(struct inode *old_dir,
struct dentry *old_dentry,
>>  		new_inode->i_ctime = CURRENT_TIME;
>>  		if (unlikely(btrfs_ino(new_inode) =>>  			    
BTRFS_EMPTY_SUBVOL_DIR_OBJECTID)) {
>> -			root_objectid = BTRFS_I(new_inode)->location.objectid;
>> +			root_objectid = BTRFS_I(new_inode)->inode_id;
> 
> direct assignment, no btrfs_ino as in the first hunk
This is a special case, where is new_inode->i_ino is
BTRFS_EMPTY_SUBVOL_DIR_OBJECTID,
while BTRFS_I(new_inode)->location.objectid is 256.

Thanks for the reviewing!

liubo
thanks,
> 
>>  			ret = btrfs_unlink_subvol(trans, dest, new_dir,
>>  						root_objectid,
>>  						new_dentry->d_name.name,
>> -- 
> 
> 
> david
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Btrfs devel - Jun 2011 - [PATCH 1/2] Btrfs: kill location key of in-memory inode

[PATCH 1/2] Btrfs: kill location key of in-memory inode

[PATCH 2/2] Btrfs: fix ''ls -lis'' mismatch

[PATCH 1/2] Btrfs: kill location key of in-memory inode

[PATCH 2/2] Btrfs: fix ''ls -lis'' mismatch

Re: [PATCH 1/2] Btrfs: kill location key of in-memory inode

Re: [PATCH 1/2] Btrfs: kill location key of in-memory inode