[PATCH] Btrfs: avoid building inode cache repeatly

[PATCH] Btrfs: avoid building inode cache repeatly

[Liu Bo]

Inode cache is similar to free space cache and in fact shares the same
code, however, we don't load inode cache unless we're about to allocate
inode id, then there is a case where we only commit the transaction during
other operations, such as snapshot creation, we now update fs roots' generation
to the new transaction id, after that when we want to load the inode cache,
we'll find that it's not valid thanks to the mismatch of generation, and we
have to push btrfs-ino-cache thread to build inode cache from disk, and
this operation is sometimes time-costing.

So to fix the above, we load inode cache into memory during reading fs root.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/disk-io.c   | 9 +++++----
 fs/btrfs/inode-map.c | 2 ++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 9c9667d..93b4473 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1525,14 +1525,15 @@ int btrfs_init_fs_root(struct btrfs_root *root)
 		goto fail;
 	}
 
-	btrfs_init_free_ino_ctl(root);
+	ret = get_anon_bdev(&root->anon_dev);
+	if (ret)
+		goto fail;
+
 	mutex_init(&root->fs_commit_mutex);
 	spin_lock_init(&root->cache_lock);
 	init_waitqueue_head(&root->cache_wait);
+	btrfs_init_free_ino_ctl(root);
 
-	ret = get_anon_bdev(&root->anon_dev);
-	if (ret)
-		goto fail;
 	return 0;
 fail:
 	kfree(root->free_ino_ctl);
diff --git a/fs/btrfs/inode-map.c b/fs/btrfs/inode-map.c
index ab485e5..6c8d7bb 100644
--- a/fs/btrfs/inode-map.c
+++ b/fs/btrfs/inode-map.c
@@ -388,6 +388,8 @@ void btrfs_init_free_ino_ctl(struct btrfs_root *root)
 	pinned->private = NULL;
 	pinned->extents_thresh = 0;
 	pinned->op = &pinned_free_ino_op;
+
+	start_caching(root);
 }
 
 int btrfs_save_ino_cache(struct btrfs_root *root,
-- 
1.8.2.1

Re: [PATCH] Btrfs: avoid building inode cache repeatly

[Chris Mason]

Quoting Liu Bo (2013-12-09 04:34:31)
> Inode cache is similar to free space cache and in fact shares the same
> code, however, we don't load inode cache unless we're about to allocate
> inode id, then there is a case where we only commit the transaction during
> other operations, such as snapshot creation, we now update fs roots' generation
> to the new transaction id, after that when we want to load the inode cache,
> we'll find that it's not valid thanks to the mismatch of generation, and we
> have to push btrfs-ino-cache thread to build inode cache from disk, and
> this operation is sometimes time-costing.
> 
> So to fix the above, we load inode cache into memory during reading fs root.

Thanks Liu.  Have you tested this with orphan replay?  I'd like to make
sure the new ordering of starting caching isn't causing problems with
finding and processing the orphan items.

-chris

Re: [PATCH] Btrfs: avoid building inode cache repeatly

[Liu Bo]

On Mon, Dec 09, 2013 at 09:20:05AM -0500, Chris Mason wrote:
> Quoting Liu Bo (2013-12-09 04:34:31)
> > Inode cache is similar to free space cache and in fact shares the same
> > code, however, we don't load inode cache unless we're about to allocate
> > inode id, then there is a case where we only commit the transaction during
> > other operations, such as snapshot creation, we now update fs roots' generation
> > to the new transaction id, after that when we want to load the inode cache,
> > we'll find that it's not valid thanks to the mismatch of generation, and we
> > have to push btrfs-ino-cache thread to build inode cache from disk, and
> > this operation is sometimes time-costing.
> > 
> > So to fix the above, we load inode cache into memory during reading fs root.
> 
> Thanks Liu.  Have you tested this with orphan replay?  I'd like to make
> sure the new ordering of starting caching isn't causing problems with
> finding and processing the orphan items.

I hacked the code with the following change in order to test orphan
replay with inode cache enabled, it's ok on my side on mounting and
using, I'll make some tests to make sure xfstests is ok, too.

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 431ba6f..ba5abd7 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3515,7 +3515,7 @@ static int btrfs_orphan_del(struct btrfs_trans_handle *trans,
 
 	if (delete_item) {
 		atomic_dec(&root->orphan_inodes);
-		if (trans)
+		if (0 && trans)
 			ret = btrfs_del_orphan_item(trans, root,
 						    btrfs_ino(inode));
 	}



I'll be glad if anyone tells me he/she has a better way to test it :)

-liubo

Re: [PATCH] Btrfs: avoid building inode cache repeatly

[Liu Bo]

On Tue, Dec 10, 2013 at 04:51:41PM +0800, Liu Bo wrote:
> On Mon, Dec 09, 2013 at 09:20:05AM -0500, Chris Mason wrote:
> > Quoting Liu Bo (2013-12-09 04:34:31)
> > > Inode cache is similar to free space cache and in fact shares the same
> > > code, however, we don't load inode cache unless we're about to allocate
> > > inode id, then there is a case where we only commit the transaction during
> > > other operations, such as snapshot creation, we now update fs roots' generation
> > > to the new transaction id, after that when we want to load the inode cache,
> > > we'll find that it's not valid thanks to the mismatch of generation, and we
> > > have to push btrfs-ino-cache thread to build inode cache from disk, and
> > > this operation is sometimes time-costing.
> > > 
> > > So to fix the above, we load inode cache into memory during reading fs root.
> > 
> > Thanks Liu.  Have you tested this with orphan replay?  I'd like to make
> > sure the new ordering of starting caching isn't causing problems with
> > finding and processing the orphan items.
> 
> I hacked the code with the following change in order to test orphan
> replay with inode cache enabled, it's ok on my side on mounting and
> using, I'll make some tests to make sure xfstests is ok, too.

Oops, xfstests/btrfs/012 complains with a 'general protection", I need to look into that.

-liubo