btrfs: implement delayed inode items operation

PATCH Superseded Miao Xie 4 participants 8 messages Mar 3, 2011 – Mar 11, 2011 Btrfs file system

Share & tools

Miao Xie

author

2011-03-03 06:06 UTC #0 lore raw

PATCH v3 1/1 btrfs: implement delayed inode items operation

Changelog V2 -> V3:
- Fix the race between the delayed worker and the task which does delayed items balance, which is reported by Tsutomu Itoh.
- Modify the patch address David Sterba's comment.
- Fix the bug of the cpu recursion spinlock, reported by Chris Mason
- introduce a list to manage the delayed nodes which is waiting to be dealt with by the work thread.

Changelog V1 -> V2:
- break up the global rb-tree, use a list to manage the delayed nodes, which is created for every directory and file, and used to manage the delayed directory name index items and the delayed inode item.
- introduce a worker to deal with the delayed nodes.

Compare with Ext3/4, the performance of file creation and deletion on btrfs is very poor. the reason is that btrfs must do a lot of b+ tree insertions, such as inode item, directory name item, directory name index and so on.

If we can do some delayed b+ tree insertion or deletion, we can improve the performance, so we made this patch which implemented delayed directory name index insertion/deletion and delayed inode update.

Implementation:
- introduce a delayed root object into the filesystem, that use two lists to manage the delayed nodes which are created for every file/directory. One is used to manage all the delayed nodes that have delayed items. And the other is used to manage the delayed nodes which is waiting to be dealt with by the work thread.
- Every delayed node has two rb-tree, one is used to manage the directory name index which is going to be inserted into b+ tree, and the other is used to manage the directory name index which is going to be deleted from b+ tree.
- introduce a worker to deal with the delayed operation. This worker is used to deal with the works of the delayed directory name index items insertion and deletion and the delayed inode update.
When the delayed items is beyond the lower limit, we create works for some delayed nodes and insert them into the work queue of the worker, and then go back.
When the delayed items is beyond the upper bound, we create works for all the delayed nodes that haven't been dealt with, and insert them into the work queue of the worker, and then wait for that the untreated items is below some threshold value.
- When we want to insert a directory name index into b+ tree, we just add the information into the delayed inserting rb-tree.
And then we check the number of the delayed items and do delayed items balance. (The balance policy is above.)
- When we want to delete a directory name index from the b+ tree, we search it in the inserting rb-tree at first. If we look it up, just drop it. If not, add the key of it into the delayed deleting rb-tree.
Similar to the delayed inserting rb-tree, we also check the number of the delayed items and do delayed items balance.
(The same to inserting manipulation)
- When we want to update the metadata of some inode, we cached the data of the inode into the delayed node. the worker will flush it into the b+ tree after dealing with the delayed insertion and deletion.
- We will move the delayed node to the tail of the list after we access the delayed node, By this way, we can cache more delayed items and merge more inode updates.
- If we want to commit transaction, we will deal with all the delayed node.
- the delayed node will be freed when we free the btrfs inode.
- Before we log the inode items, we commit all the directory name index items and the delayed inode update.

I did a quick test by the benchmark tool[1] and found we can improve the performance of file creation by ~15%, and file deletion by ~20%.

Before applying this patch:
Create files:

	Total files: 50000
	Total time: 1.096108
	Average time: 0.000022
Delete files:
	Total files: 50000
	Total time: 1.510403
	Average time: 0.000030

After applying this patch:
Create files:

	Total files: 50000
	Total time: 0.932899
	Average time: 0.000019
Delete files:
	Total files: 50000
	Total time: 1.215732
	Average time: 0.000024

[1] http://marc.info/?l=linux-btrfs&m=128212635122920&q=p3

Signed-off-by Miao Xie <[email protected]>

Reviewed-by David Sterba <[email protected]>

Tested-by Tsutomu Itoh <[email protected]>

---
 fs/btrfs/Makefile        |    2 +-
 fs/btrfs/btrfs_inode.h   |    5 +
 fs/btrfs/ctree.c         |   14 +-
 fs/btrfs/ctree.h         |   24 +-
 fs/btrfs/delayed-inode.c | 1614 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/delayed-inode.h |  139 ++++
 fs/btrfs/dir-item.c      |   32 +-
 fs/btrfs/disk-io.c       |   26 +-
 fs/btrfs/extent-tree.c   |   18 +-
 fs/btrfs/inode.c         |  109 ++--
 fs/btrfs/ioctl.c         |    2 +-
 fs/btrfs/super.c         |   10 +-
 fs/btrfs/transaction.c   |   49 ++-
 fs/btrfs/transaction.h   |    2 +
 fs/btrfs/tree-log.c      |    7 +
 15 files changed, 1945 insertions(+), 108 deletions(-)
 create mode 100644 fs/btrfs/delayed-inode.c
 create mode 100644 fs/btrfs/delayed-inode.h

fs/btrfs/Makefile +1 -1

index 31610ea..a8411c2 100644
@@ -7,4 +7,4 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o root-tree.o dir-item.o \
 	   extent_map.o sysfs.o struct-funcs.o xattr.o ordered-data.o \
 	   extent_io.o volumes.o async-thread.o ioctl.o locking.o orphan.o \
 	   export.o tree-log.o acl.o free-space-cache.o zlib.o lzo.o \
-	   compression.o delayed-ref.o relocation.o
+	   compression.o delayed-ref.o relocation.o delayed-inode.o

fs/btrfs/btrfs_inode.h +5 -0

index ccc991c..ef3baa8 100644
@@ -22,6 +22,7 @@
 #include "extent_map.h"
 #include "extent_io.h"
 #include "ordered-data.h"
+#include "delayed-inode.h"
 
 /* in memory btrfs inode */
 struct btrfs_inode {

@@ -159,9 +160,13 @@ struct btrfs_inode {
 	 */
 	unsigned force_compress:4;
 
+	struct btrfs_delayed_node *delayed_node;
+
 	struct inode vfs_inode;
 };
 
+extern unsigned char btrfs_filetype_table[];
+
 static inline struct btrfs_inode *BTRFS_I(struct inode *inode)
 {
 	return container_of(inode, struct btrfs_inode, vfs_inode);

fs/btrfs/ctree.c +4 ~~-10~~

index b5baff0..6e3e4c3 100644
@@ -38,11 +38,6 @@ static int balance_node_right(struct btrfs_trans_handle *trans,
 			      struct extent_buffer *src_buf);
 static int del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		   struct btrfs_path *path, int level, int slot);
-static int setup_items_for_insert(struct btrfs_trans_handle *trans,
-			struct btrfs_root *root, struct btrfs_path *path,
-			struct btrfs_key *cpu_key, u32 *data_size,
-			u32 total_data, u32 total_size, int nr);
-
 
 struct btrfs_path *btrfs_alloc_path(void)
 {

@@ -3688,11 +3683,10 @@ out:
  * to save stack depth by doing the bulk of the work in a function
  * that doesn't call btrfs_search_slot
  */
-static noinline_for_stack int
-setup_items_for_insert(struct btrfs_trans_handle *trans,
-		      struct btrfs_root *root, struct btrfs_path *path,
-		      struct btrfs_key *cpu_key, u32 *data_size,
-		      u32 total_data, u32 total_size, int nr)
+int setup_items_for_insert(struct btrfs_trans_handle *trans,
+			   struct btrfs_root *root, struct btrfs_path *path,
+			   struct btrfs_key *cpu_key, u32 *data_size,
+			   u32 total_data, u32 total_size, int nr)
 {
 	struct btrfs_item *item;
 	int i;

fs/btrfs/ctree.h +22 -2

index 28188a7..bafca2e 100644
@@ -859,6 +859,7 @@ struct btrfs_block_group_cache {
 struct reloc_control;
 struct btrfs_device;
 struct btrfs_fs_devices;
+struct btrfs_delayed_root;
 struct btrfs_fs_info {
 	u8 fsid[BTRFS_FSID_SIZE];
 	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];

@@ -885,7 +886,10 @@ struct btrfs_fs_info {
 	/* logical->physical extent mapping */
 	struct btrfs_mapping_tree mapping_tree;
 
-	/* block reservation for extent, checksum and root tree */
+	/*
+	 * block reservation for extent, checksum, root tree and
+	 * delayed dir index item
+	 */
 	struct btrfs_block_rsv global_block_rsv;
 	/* block reservation for delay allocation */
 	struct btrfs_block_rsv delalloc_block_rsv;

@@ -1012,6 +1016,7 @@ struct btrfs_fs_info {
 	 * for the sys_munmap function call path
 	 */
 	struct btrfs_workers fixup_workers;
+	struct btrfs_workers delayed_workers;
 	struct task_struct *transaction_kthread;
 	struct task_struct *cleaner_kthread;
 	int thread_pool_size;

@@ -1069,6 +1074,8 @@ struct btrfs_fs_info {
 
 	/* filesystem state */
 	u64 fs_state;
+
+	struct btrfs_delayed_root *delayed_root;
 };
 
 /*

@@ -2086,6 +2093,13 @@ static inline bool btrfs_mixed_space_info(struct btrfs_space_info *space_info)
 }
 
 /* extent-tree.c */
+static inline u64 btrfs_calc_trans_metadata_size(struct btrfs_root *root,
+						 int num_items)
+{
+	return (root->leafsize + root->nodesize * (BTRFS_MAX_LEVEL - 1)) *
+		3 * num_items;
+}
+
 void btrfs_put_block_group(struct btrfs_block_group_cache *cache);
 int btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
 			   struct btrfs_root *root, unsigned long count);

@@ -2277,6 +2291,8 @@ void btrfs_release_path(struct btrfs_root *root, struct btrfs_path *p);
 struct btrfs_path *btrfs_alloc_path(void);
 void btrfs_free_path(struct btrfs_path *p);
 void btrfs_set_path_blocking(struct btrfs_path *p);
+void btrfs_clear_path_blocking(struct btrfs_path *p,
+			       struct extent_buffer *held);
 void btrfs_unlock_up_safe(struct btrfs_path *p, int level);
 
 int btrfs_del_items(struct btrfs_trans_handle *trans, struct btrfs_root *root,

@@ -2288,6 +2304,10 @@ static inline int btrfs_del_item(struct btrfs_trans_handle *trans,
 	return btrfs_del_items(trans, root, path, path->slots[0], 1);
 }
 
+int setup_items_for_insert(struct btrfs_trans_handle *trans,
+			   struct btrfs_root *root, struct btrfs_path *path,
+			   struct btrfs_key *cpu_key, u32 *data_size,
+			   u32 total_data, u32 total_size, int nr);
 int btrfs_insert_item(struct btrfs_trans_handle *trans, struct btrfs_root
 		      *root, struct btrfs_key *key, void *data, u32 data_size);
 int btrfs_insert_some_items(struct btrfs_trans_handle *trans,

@@ -2349,7 +2369,7 @@ int btrfs_set_root_node(struct btrfs_root_item *item,
 /* dir-item.c */
 int btrfs_insert_dir_item(struct btrfs_trans_handle *trans,
 			  struct btrfs_root *root, const char *name,
-			  int name_len, u64 dir,
+			  int name_len, struct inode *dir,
 			  struct btrfs_key *location, u8 type, u64 index);
 struct btrfs_dir_item *btrfs_lookup_dir_item(struct btrfs_trans_handle *trans,
 					     struct btrfs_root *root,

fs/btrfs/delayed-inode.c new file +1614 -0

1,614 lines not on this page · raw message

fs/btrfs/delayed-inode.h new file +139 -0

139 lines not on this page · raw message

fs/btrfs/dir-item.c +10 ~~-22~~

index f0cad5a..696c793 100644
@@ -124,8 +124,9 @@ int btrfs_insert_xattr_item(struct btrfs_trans_handle *trans,
  * to use for the second index (if one is created).
  */
 int btrfs_insert_dir_item(struct btrfs_trans_handle *trans, struct btrfs_root
-			  *root, const char *name, int name_len, u64 dir,
-			  struct btrfs_key *location, u8 type, u64 index)
+			  *root, const char *name, int name_len,
+			  struct inode *dir, struct btrfs_key *location,
+			  u8 type, u64 index)
 {
 	int ret = 0;
 	int ret2 = 0;

@@ -137,13 +138,17 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle *trans, struct btrfs_root
 	struct btrfs_disk_key disk_key;
 	u32 data_size;
 
-	key.objectid = dir;
+	key.objectid = dir->i_ino;
 	btrfs_set_key_type(&key, BTRFS_DIR_ITEM_KEY);
 	key.offset = btrfs_name_hash(name, name_len);
 
 	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
 	path->leave_spinning = 1;
 
+	btrfs_cpu_key_to_disk(&disk_key, location);
+
 	data_size = sizeof(*dir_item) + name_len;
 	dir_item = insert_with_overflow(trans, root, path, &key, data_size,
 					name, name_len);

@@ -155,7 +160,6 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle *trans, struct btrfs_root
 	}
 
 	leaf = path->nodes[0];
-	btrfs_cpu_key_to_disk(&disk_key, location);
 	btrfs_set_dir_item_key(leaf, dir_item, &disk_key);
 	btrfs_set_dir_type(leaf, dir_item, type);
 	btrfs_set_dir_data_len(leaf, dir_item, 0);

@@ -174,24 +178,8 @@ second_insert:
 	}
 	btrfs_release_path(root, path);
 
-	btrfs_set_key_type(&key, BTRFS_DIR_INDEX_KEY);
-	key.offset = index;
-	dir_item = insert_with_overflow(trans, root, path, &key, data_size,
-					name, name_len);
-	if (IS_ERR(dir_item)) {
-		ret2 = PTR_ERR(dir_item);
-		goto out;
-	}
-	leaf = path->nodes[0];
-	btrfs_cpu_key_to_disk(&disk_key, location);
-	btrfs_set_dir_item_key(leaf, dir_item, &disk_key);
-	btrfs_set_dir_type(leaf, dir_item, type);
-	btrfs_set_dir_data_len(leaf, dir_item, 0);
-	btrfs_set_dir_name_len(leaf, dir_item, name_len);
-	btrfs_set_dir_transid(leaf, dir_item, trans->transid);
-	name_ptr = (unsigned long)(dir_item + 1);
-	write_extent_buffer(leaf, name, name_ptr, name_len);
-	btrfs_mark_buffer_dirty(leaf);
+	ret2 = btrfs_insert_delayed_dir_index(trans, root, name, name_len, dir,
+					      &disk_key, type, index);
 out:
 	btrfs_free_path(path);
 	if (ret)

fs/btrfs/disk-io.c +21 -5

index 3e1ea3e..ea3c145 100644
@@ -1676,6 +1676,13 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 
 	INIT_LIST_HEAD(&fs_info->ordered_extents);
 	spin_lock_init(&fs_info->ordered_extent_lock);
+	fs_info->delayed_root = kmalloc(sizeof(struct btrfs_delayed_root),
+					GFP_NOFS);
+	if (!fs_info->delayed_root) {
+		err = -ENOMEM;
+		goto fail_iput;
+	}
+	btrfs_init_delayed_root(fs_info->delayed_root);
 
 	sb->s_blocksize = 4096;
 	sb->s_blocksize_bits = blksize_bits(4096);

@@ -1743,7 +1750,7 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 	bh = btrfs_read_dev_super(fs_devices->latest_bdev);
 	if (!bh) {
 		err = -EINVAL;
-		goto fail_iput;
+		goto fail_alloc;
 	}
 
 	memcpy(&fs_info->super_copy, bh->b_data, sizeof(fs_info->super_copy));

@@ -1755,7 +1762,7 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 
 	disk_super = &fs_info->super_copy;
 	if (!btrfs_super_root(disk_super))
-		goto fail_iput;
+		goto fail_alloc;
 
 	/* check FS state, whether FS is broken. */
 	fs_info->fs_state |= btrfs_super_flags(disk_super);

@@ -1765,7 +1772,7 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 	ret = btrfs_parse_options(tree_root, options);
 	if (ret) {
 		err = ret;
-		goto fail_iput;
+		goto fail_alloc;
 	}
 
 	features = btrfs_super_incompat_flags(disk_super) &

@@ -1775,7 +1782,7 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 		       "unsupported optional features (%Lx).\n",
 		       (unsigned long long)features);
 		err = -EINVAL;
-		goto fail_iput;
+		goto fail_alloc;
 	}
 
 	features = btrfs_super_incompat_flags(disk_super);

@@ -1791,7 +1798,7 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 		       "unsupported option features (%Lx).\n",
 		       (unsigned long long)features);
 		err = -EINVAL;
-		goto fail_iput;
+		goto fail_alloc;
 	}
 
 	btrfs_init_workers(&fs_info->generic_worker,

@@ -1838,6 +1845,9 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 			   &fs_info->generic_worker);
 	btrfs_init_workers(&fs_info->endio_freespace_worker, "freespace-write",
 			   1, &fs_info->generic_worker);
+	btrfs_init_workers(&fs_info->delayed_workers, "delayed-meta",
+			   fs_info->thread_pool_size,
+			   &fs_info->generic_worker);
 
 	/*
 	 * endios are largely parallel and should have a very

@@ -1859,6 +1869,7 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 	btrfs_start_workers(&fs_info->endio_meta_write_workers, 1);
 	btrfs_start_workers(&fs_info->endio_write_workers, 1);
 	btrfs_start_workers(&fs_info->endio_freespace_worker, 1);
+	btrfs_start_workers(&fs_info->delayed_workers, 1);
 
 	fs_info->bdi.ra_pages *= btrfs_super_num_devices(disk_super);
 	fs_info->bdi.ra_pages = max(fs_info->bdi.ra_pages,

@@ -2104,6 +2115,9 @@ fail_sb_buffer:
 	btrfs_stop_workers(&fs_info->endio_write_workers);
 	btrfs_stop_workers(&fs_info->endio_freespace_worker);
 	btrfs_stop_workers(&fs_info->submit_workers);
+	btrfs_stop_workers(&fs_info->delayed_workers);
+fail_alloc:
+	kfree(fs_info->delayed_root);
 fail_iput:
 	invalidate_inode_pages2(fs_info->btree_inode->i_mapping);
 	iput(fs_info->btree_inode);

@@ -2551,6 +2565,7 @@ int close_ctree(struct btrfs_root *root)
 	del_fs_roots(fs_info);
 
 	iput(fs_info->btree_inode);
+	kfree(fs_info->delayed_root);
 
 	btrfs_stop_workers(&fs_info->generic_worker);
 	btrfs_stop_workers(&fs_info->fixup_workers);

@@ -2562,6 +2577,7 @@ int close_ctree(struct btrfs_root *root)
 	btrfs_stop_workers(&fs_info->endio_write_workers);
 	btrfs_stop_workers(&fs_info->endio_freespace_worker);
 	btrfs_stop_workers(&fs_info->submit_workers);
+	btrfs_stop_workers(&fs_info->delayed_workers);
 
 	btrfs_close_devices(fs_info->fs_devices);
 	btrfs_mapping_tree_free(&fs_info->mapping_tree);

fs/btrfs/extent-tree.c +6 ~~-12~~

index 100e409..aed763f 100644
@@ -3897,12 +3897,6 @@ static void release_global_block_rsv(struct btrfs_fs_info *fs_info)
 	WARN_ON(fs_info->chunk_block_rsv.reserved > 0);
 }
 
-static u64 calc_trans_metadata_size(struct btrfs_root *root, int num_items)
-{
-	return (root->leafsize + root->nodesize * (BTRFS_MAX_LEVEL - 1)) *
-		3 * num_items;
-}
-
 int btrfs_trans_reserve_metadata(struct btrfs_trans_handle *trans,
 				 struct btrfs_root *root,
 				 int num_items)

@@ -3913,7 +3907,7 @@ int btrfs_trans_reserve_metadata(struct btrfs_trans_handle *trans,
 	if (num_items == 0 || root->fs_info->chunk_root == root)
 		return 0;
 
-	num_bytes = calc_trans_metadata_size(root, num_items);
+	num_bytes = btrfs_calc_trans_metadata_size(root, num_items);
 	ret = btrfs_block_rsv_add(trans, root, &root->fs_info->trans_block_rsv,
 				  num_bytes);
 	if (!ret) {

@@ -3952,14 +3946,14 @@ int btrfs_orphan_reserve_metadata(struct btrfs_trans_handle *trans,
 	 * If all of the metadata space is used, we can commit
 	 * transaction and use space it freed.
 	 */
-	u64 num_bytes = calc_trans_metadata_size(root, 4);
+	u64 num_bytes = btrfs_calc_trans_metadata_size(root, 4);
 	return block_rsv_migrate_bytes(src_rsv, dst_rsv, num_bytes);
 }
 
 void btrfs_orphan_release_metadata(struct inode *inode)
 {
 	struct btrfs_root *root = BTRFS_I(inode)->root;
-	u64 num_bytes = calc_trans_metadata_size(root, 4);
+	u64 num_bytes = btrfs_calc_trans_metadata_size(root, 4);
 	btrfs_block_rsv_release(root, root->orphan_block_rsv, num_bytes);
 }

@@ -3973,7 +3967,7 @@ int btrfs_snap_reserve_metadata(struct btrfs_trans_handle *trans,
 	 * two for root back/forward refs, two for directory entries
 	 * and one for root of the snapshot.
 	 */
-	u64 num_bytes = calc_trans_metadata_size(root, 5);
+	u64 num_bytes = btrfs_calc_trans_metadata_size(root, 5);
 	dst_rsv->space_info = src_rsv->space_info;
 	return block_rsv_migrate_bytes(src_rsv, dst_rsv, num_bytes);
 }

@@ -4000,7 +3994,7 @@ int btrfs_delalloc_reserve_metadata(struct inode *inode, u64 num_bytes)
 	nr_extents = atomic_read(&BTRFS_I(inode)->outstanding_extents) + 1;
 	if (nr_extents > BTRFS_I(inode)->reserved_extents) {
 		nr_extents -= BTRFS_I(inode)->reserved_extents;
-		to_reserve = calc_trans_metadata_size(root, nr_extents);
+		to_reserve = btrfs_calc_trans_metadata_size(root, nr_extents);
 	} else {
 		nr_extents = 0;
 		to_reserve = 0;

@@ -4047,7 +4041,7 @@ void btrfs_delalloc_release_metadata(struct inode *inode, u64 num_bytes)
 
 	to_free = calc_csum_metadata_size(inode, num_bytes);
 	if (nr_extents > 0)
-		to_free += calc_trans_metadata_size(root, nr_extents);
+		to_free += btrfs_calc_trans_metadata_size(root, nr_extents);
 
 	btrfs_block_rsv_release(root, &root->fs_info->delalloc_block_rsv,
 				to_free);

fs/btrfs/inode.c +59 ~~-50~~

221 lines not on this page · raw message

fs/btrfs/ioctl.c +1 -1

index 5fdb2ab..b200e07 100644
@@ -333,7 +333,7 @@ static noinline int create_subvol(struct btrfs_root *root,
 	BUG_ON(ret);
 
 	ret = btrfs_insert_dir_item(trans, root,
-				    name, namelen, dir->i_ino, &key,
+				    name, namelen, dir, &key,
 				    BTRFS_FT_DIR, index);
 	if (ret)
 		goto fail;

fs/btrfs/super.c +9 -1

index db0a827..593ce8a 100644
@@ -40,6 +40,7 @@
 #include <linux/magic.h>
 #include <linux/slab.h>
 #include "compat.h"
+#include "delayed-inode.h"
 #include "ctree.h"
 #include "disk-io.h"
 #include "transaction.h"

@@ -1163,10 +1164,14 @@ static int __init init_btrfs_fs(void)
 	if (err)
 		goto free_extent_io;
 
-	err = btrfs_interface_init();
+	err = btrfs_delayed_inode_init();
 	if (err)
 		goto free_extent_map;
 
+	err = btrfs_interface_init();
+	if (err)
+		goto free_delayed_inode;
+
 	err = register_filesystem(&btrfs_fs_type);
 	if (err)
 		goto unregister_ioctl;

@@ -1176,6 +1181,8 @@ static int __init init_btrfs_fs(void)
 
 unregister_ioctl:
 	btrfs_interface_exit();
+free_delayed_inode:
+	btrfs_delayed_inode_exit();
 free_extent_map:
 	extent_map_exit();
 free_extent_io:

@@ -1192,6 +1199,7 @@ free_sysfs:
 static void __exit exit_btrfs_fs(void)
 {
 	btrfs_destroy_cachep();
+	btrfs_delayed_inode_exit();
 	extent_map_exit();
 	extent_io_exit();
 	btrfs_interface_exit();

fs/btrfs/transaction.c +45 -4

89 lines not on this page · raw message

fs/btrfs/transaction.h +2 -0

index 229a594..a4a441f 100644
@@ -115,6 +115,8 @@ int btrfs_commit_transaction_async(struct btrfs_trans_handle *trans,
 				   int wait_for_unblock);
 int btrfs_end_transaction_throttle(struct btrfs_trans_handle *trans,
 				   struct btrfs_root *root);
+int btrfs_end_transaction_dmeta(struct btrfs_trans_handle *trans,
+				struct btrfs_root *root);
 int btrfs_should_end_transaction(struct btrfs_trans_handle *trans,
 				 struct btrfs_root *root);
 void btrfs_throttle(struct btrfs_root *root);

fs/btrfs/tree-log.c +7 -0

index a4bbb85..e8a2e95 100644
@@ -2775,6 +2775,13 @@ static int btrfs_log_inode(struct btrfs_trans_handle *trans,
 		max_key.type = (u8)-1;
 	max_key.offset = (u64)-1;
 
+	ret = btrfs_commit_inode_delayed_items(trans, root, inode);
+	if (ret) {
+		btrfs_free_path(path);
+		btrfs_free_path(dst_path);
+		return ret;
+	}
+
 	mutex_lock(&BTRFS_I(inode)->log_mutex);
 
 	/*

Signature · 1 line

1.7.3.1

Itaru Kitayama

2011-03-05 07:01 UTC #1 lore raw

Hi Miao,

The V3 patch on top of the next-rc fails to pass an xfstests test 13. In the btrfs link path, we need to reserve one more metadata in the trans_block_rsv for the delayed inode update (if needed) to complete.

Here's the log from the test:

------------[ cut here ]------------
kernel BUG at fs/btrfs/delayed-inode.c:1565!
invalid opcode: 0000 [#2] PREEMPT SMP DEBUG_PAGEALLOC
last sysfs file: /sys/module/floppy/initstate
CPU 1 
Modules linked in: nfsd exportfs nfs lockd fscache nfs_acl auth_rpcgss sunrpc ipv6 uinput snd_ens1371 gameport snd_rawmidi ]

Pid: 6948, comm: fsstress Tainted: G D W 2.6.36-preempt+ #72 440BX Desktop Reference Platform/VMware Virtual Platform

RIP: 0010:[<ffffffff8120de65>]  [<ffffffff8120de65>] btrfs_delayed_update_inode+0xfd/0x101
RSP: 0018:ffff88003b073d68  EFLAGS: 00010286
RAX: 00000000ffffffe4 RBX: ffff88002a876e48 RCX: ffff88004f3702a0
RDX: 00000000000003b9 RSI: 0000000000000001 RDI: ffff88003b072000
RBP: ffff88003b073da8 R08: 0000000000018000 R09: 0000000000000000
R10: ffff88004f3703f0 R11: 09f911029d74e35b R12: ffff88006861cb90
R13: ffff88002ec91c18 R14: 0000000000018000 R15: ffff88002a876ea0
FS:  00007fcad4905720(0000) GS:ffff880005000000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f46eb380000 CR3: 00000000708b4000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process fsstress (pid: 6948, threadinfo ffff88003b072000, task ffff88004f324710)
Stack:
 ffff88003b073d88 ffff88004f3702a0 0000000000000000 ffff88002ec91c18
<0> ffff88006861cb90 ffff880036783c18 0000000000000000 ffff88006fcaf000
<0> ffff88003b073dc8 ffffffff811d8489 ffff88002ec91c18 ffff88006861cb90
Call Trace:
 [<ffffffff811d8489>] btrfs_update_inode+0x17/0x44
 [<ffffffff811da3e0>] btrfs_link+0xff/0x17a
 [<ffffffff8112300d>] vfs_link+0xda/0x144
 [<ffffffff811257ac>] sys_linkat+0xc4/0x121
 [<ffffffff81118925>] ? fsnotify_modify+0x66/0x6e
 [<ffffffff811190f8>] ? vfs_write+0xd2/0x10a
 [<ffffffff81125827>] sys_link+0x1e/0x20
 [<ffffffff81002ddb>] system_call_fastpath+0x16/0x1b
Code: f0 ff 40 60 eb 02 eb fe 4c 89 ff e8 7d 29 2b 00 31 f6 48 89 df e8 31 fb ff ff 31 c0 48 83 c4 18 5b 41 5c 41 5d 41 5e  
RIP  [<ffffffff8120de65>] btrfs_delayed_update_inode+0xfd/0x101
 RSP <ffff88003b073d68>
---[ end trace 38596ade53c80d11 ]---

Tsutomu Itoh

2011-03-06 23:25 UTC #2 lore raw

Quoted 5 lines · Itaru Kitayama

Hi Miao,
The V3 patch on top of the next-rc fails to pass an xfstests test 13. In the btrfs link path, we need to reserve one more metadata in the trans_block_rsv for the delayed inode update (if needed) to complete.

Miao's patch (http://marc.info/?l=linux-btrfs&m=129802068318176&w=2)
fixed this problem, maybe.

Thanks,
Itoh

Quoted 42 lines

Here's the log from the test:

------------[ cut here ]------------
kernel BUG at fs/btrfs/delayed-inode.c:1565!
invalid opcode: 0000 [#2] PREEMPT SMP DEBUG_PAGEALLOC
last sysfs file: /sys/module/floppy/initstate
CPU 1 
Modules linked in: nfsd exportfs nfs lockd fscache nfs_acl auth_rpcgss sunrpc ipv6 uinput snd_ens1371 gameport snd_rawmidi ]

Pid: 6948, comm: fsstress Tainted: G D W 2.6.36-preempt+ #72 440BX Desktop Reference Platform/VMware Virtual Platform

RIP: 0010:[<ffffffff8120de65>]  [<ffffffff8120de65>] btrfs_delayed_update_inode+0xfd/0x101
RSP: 0018:ffff88003b073d68  EFLAGS: 00010286
RAX: 00000000ffffffe4 RBX: ffff88002a876e48 RCX: ffff88004f3702a0
RDX: 00000000000003b9 RSI: 0000000000000001 RDI: ffff88003b072000
RBP: ffff88003b073da8 R08: 0000000000018000 R09: 0000000000000000
R10: ffff88004f3703f0 R11: 09f911029d74e35b R12: ffff88006861cb90
R13: ffff88002ec91c18 R14: 0000000000018000 R15: ffff88002a876ea0
FS:  00007fcad4905720(0000) GS:ffff880005000000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f46eb380000 CR3: 00000000708b4000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process fsstress (pid: 6948, threadinfo ffff88003b072000, task ffff88004f324710)
Stack:
 ffff88003b073d88 ffff88004f3702a0 0000000000000000 ffff88002ec91c18
<0> ffff88006861cb90 ffff880036783c18 0000000000000000 ffff88006fcaf000
<0> ffff88003b073dc8 ffffffff811d8489 ffff88002ec91c18 ffff88006861cb90
Call Trace:
 [<ffffffff811d8489>] btrfs_update_inode+0x17/0x44
 [<ffffffff811da3e0>] btrfs_link+0xff/0x17a
 [<ffffffff8112300d>] vfs_link+0xda/0x144
 [<ffffffff811257ac>] sys_linkat+0xc4/0x121
 [<ffffffff81118925>] ? fsnotify_modify+0x66/0x6e
 [<ffffffff811190f8>] ? vfs_write+0xd2/0x10a
 [<ffffffff81125827>] sys_link+0x1e/0x20
 [<ffffffff81002ddb>] system_call_fastpath+0x16/0x1b
Code: f0 ff 40 60 eb 02 eb fe 4c 89 ff e8 7d 29 2b 00 31 f6 48 89 df e8 31 fb ff ff 31 c0 48 83 c4 18 5b 41 5c 41 5d 41 5e  
RIP  [<ffffffff8120de65>] btrfs_delayed_update_inode+0xfd/0x101
 RSP <ffff88003b073d68>
---[ end trace 38596ade53c80d11 ]---

Miao Xie

author

2011-03-07 01:57 UTC #3 lore raw

Quoted 9 lines · Tsutomu Itoh

Quoted 5 lines · Itaru Kitayama
Hi Miao,
The V3 patch on top of the next-rc fails to pass an xfstests test 13. In the btrfs link path, we need to reserve one more metadata in the trans_block_rsv for the delayed inode update (if needed) to complete.
Miao's patch (http://marc.info/?l=linux-btrfs&m=129802068318176&w=2)
fixed this problem, maybe.

Yes, the above patch fixed this problem.
I think this problem is a bug of btrfs_link(), and hope the fix patch can be received as early as possible. So I sent the above patch separately, not with the delayed inode items operation patch.

Thanks
Miao

Quoted 52 lines

Thanks,
Itoh

Quoted 33 lines

Here's the log from the test:

------------[ cut here ]------------
kernel BUG at fs/btrfs/delayed-inode.c:1565!
invalid opcode: 0000 [#2] PREEMPT SMP DEBUG_PAGEALLOC
last sysfs file: /sys/module/floppy/initstate
CPU 1 
Modules linked in: nfsd exportfs nfs lockd fscache nfs_acl auth_rpcgss sunrpc ipv6 uinput snd_ens1371 gameport snd_rawmidi ]

Pid: 6948, comm: fsstress Tainted: G D W 2.6.36-preempt+ #72 440BX Desktop Reference Platform/VMware Virtual Platform

RIP: 0010:[<ffffffff8120de65>]  [<ffffffff8120de65>] btrfs_delayed_update_inode+0xfd/0x101
RSP: 0018:ffff88003b073d68  EFLAGS: 00010286
RAX: 00000000ffffffe4 RBX: ffff88002a876e48 RCX: ffff88004f3702a0
RDX: 00000000000003b9 RSI: 0000000000000001 RDI: ffff88003b072000
RBP: ffff88003b073da8 R08: 0000000000018000 R09: 0000000000000000
R10: ffff88004f3703f0 R11: 09f911029d74e35b R12: ffff88006861cb90
R13: ffff88002ec91c18 R14: 0000000000018000 R15: ffff88002a876ea0
FS:  00007fcad4905720(0000) GS:ffff880005000000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f46eb380000 CR3: 00000000708b4000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process fsstress (pid: 6948, threadinfo ffff88003b072000, task ffff88004f324710)
Stack:
 ffff88003b073d88 ffff88004f3702a0 0000000000000000 ffff88002ec91c18
<0> ffff88006861cb90 ffff880036783c18 0000000000000000 ffff88006fcaf000
<0> ffff88003b073dc8 ffffffff811d8489 ffff88002ec91c18 ffff88006861cb90
Call Trace:
 [<ffffffff811d8489>] btrfs_update_inode+0x17/0x44
 [<ffffffff811da3e0>] btrfs_link+0xff/0x17a
 [<ffffffff8112300d>] vfs_link+0xda/0x144
 [<ffffffff811257ac>] sys_linkat+0xc4/0x121

Quoted 9 lines

 [<ffffffff81118925>] ? fsnotify_modify+0x66/0x6e
 [<ffffffff811190f8>] ? vfs_write+0xd2/0x10a
 [<ffffffff81125827>] sys_link+0x1e/0x20
 [<ffffffff81002ddb>] system_call_fastpath+0x16/0x1b
Code: f0 ff 40 60 eb 02 eb fe 4c 89 ff e8 7d 29 2b 00 31 f6 48 89 df e8 31 fb ff ff 31 c0 48 83 c4 18 5b 41 5c 41 5d 41 5e  
RIP  [<ffffffff8120de65>] btrfs_delayed_update_inode+0xfd/0x101
 RSP <ffff88003b073d68>
---[ end trace 38596ade53c80d11 ]---

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

Itaru Kitayama

2011-03-10 03:07 UTC #4 lore raw

Thank you both for your help. I agree that we need to reserve 5 items for that operation.

Quoted 7 lines

Quoted 2 lines
Miao's patch (http://marc.info/?l=linux-btrfs&m=129802068318176&w=2)
fixed this problem, maybe.
Yes, the above patch fixed this problem.
I think this problem is a bug of btrfs_link(), and hope the fix patch can be received as early as possible. So I sent the above patch separately, not with the delayed inode items operation patch.

David Sterba

2011-03-08 16:56 UTC #5 lore raw

In reply to Miao Xie

Hi,

I did some testing, the speedup results match yours. I was watching kmem cache stats of the delayed_node, seem to behave well. Increase and decrease of number of active objects, from 3 to about ~400 (creat_unlink 50000) and went back to initial values after a few seconds. On my setup, there are 13 delayed_node objects per page, which gives a good chance of quick allocation of the spare objects.

Size of the delayed_node is 304 bytes, which would slack 208 bytes (hitting the nearest kmalloc bucket of 512 bytes) per node.

So to justify the recommendation of kmem_cache use.

I was thinking about reducing size of the structure by a few bytes, to 292 bytes. In this case it would make 14 objects per slab page. There are padding holes after
* atomic_t count (4 bytes)
* bool inode_dirty (2 bytes)

you may want to reorder the fileds like this:
* [no change up to inode_item]
* index_cnt
* refs
* count
* the bool fields

Still needed to reduce by 7 from 304-6=298. Does not seem to be possible, the bool flags provide only 1 byte. Under these conditions, I suggest to make the bool fields int again to reduce instruction count which manage these flags. Compiler did not make them int automatically (altough it is free to do it).

dave

Miao Xie

author

2011-03-09 05:40 UTC #6 lore raw

Quoted 20 lines · David Sterba

Hi,
I did some testing, the speedup results match yours. I was watching kmem cache stats of the delayed_node, seem to behave well. Increase and decrease of number of active objects, from 3 to about ~400 (creat_unlink 50000) and went back to initial values after a few seconds. On my setup, there are 13 delayed_node objects per page, which gives a good chance of quick allocation of the spare objects.
Size of the delayed_node is 304 bytes, which would slack 208 bytes (hitting the nearest kmalloc bucket of 512 bytes) per node.
So to justify the recommendation of kmem_cache use.
I was thinking about reducing size of the structure by a few bytes, to 292 bytes. In this case it would make 14 objects per slab page. There are padding holes after
* atomic_t count (4 bytes)
* bool inode_dirty (2 bytes)

Quoted 6 lines

you may want to reorder the fileds like this:
* [no change up to inode_item]
* index_cnt
* refs
* count
* the bool fields

Thanks for your advice. But AFAIK, reordering the fields like what you said can not reduce the space of delayed_nodes, because a hole is placed on the end to make the structure big enough to pack tightly into arrays and maintain proper alignment. So although we reorder the fields, the size of delayed_node is still 304 bytes.

Quoted 4 lines

Still needed to reduce by 7 from 304-6=298. Does not seem to be possible, the bool flags provide only 1 byte. Under these conditions, I suggest to make the bool fields int again to reduce instruction count which manage these flags. Compiler did not make them int automatically (altough it is free to do it).

Sorry, I don't understand what you said above clearly, Did you mean we can not access bool variables by one instruction?

Thanks
Miao

Quoted 7 lines

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

David Sterba

2011-03-11 15:24 UTC #7 lore raw

Quoted 5 lines · Miao Xie

Thanks for your advice. But AFAIK, reordering the fields like what you said can not reduce the space of delayed_nodes, because a hole is placed on the end to make the structure big enough to pack tightly into arrays and maintain proper alignment. So although we reorder the fields, the size of delayed_node is still 304 bytes.

Let me redo it explicitly:

Your patch does not specify alignment of the cache object

+int __init btrfs_delayed_inode_init(void)
+{
+ delayed_node_cache = kmem_cache_create("delayed_node",
+ sizeof(struct btrfs_delayed_node),
+ 0,

^^

+ SLAB_RECLAIM_ACCOUNT | SLAB_MEM_SPREAD,
+ NULL);
+ if (!delayed_node_cache)
+ return -ENOMEM;
+ return 0;
+}

and default BYTES_PER_WORD == sizeof(void*) will be used. Let's assume it's 8 (eg. on x86_64). The delayed_node is 304, which is multiple of 8 and the objects will have no padding at the end. A quick test, comparing adresses of 2 adjacent allocated objects, confirmed that.

If I did the math correctly, we are able to reduce size by 7 bytes:
reorder fileds (6b) and merge bools into one-byte bitfield (1b). This is not enough, padding to next object alignment will consume this (as you wrote above).

Hm, nice exercise, no improvement.

Quoted 7 lines

Quoted 4 lines
Still needed to reduce by 7 from 304-6=298. Does not seem to be possible, the bool flags provide only 1 byte. Under these conditions, I suggest to make the bool fields int again to reduce instruction count which manage these flags. Compiler did not make them int automatically (altough it is free to do it).
Sorry, I don't understand what you said above clearly, Did you mean we can not access bool variables by one instruction?

Ah, no I confused myself, I was thinking of bitfields. Checked that gcc emits one instruction on x86_64, I did not check other arches.

dave