thr3ads.net - Btrfs devel - [PATCH 0/3] Btrfs: inode cache fixes [Dec 2013]

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

Liu Bo

2013-Dec-13 09:16 UTC

[PATCH 0/3] Btrfs: inode cache fixes

The first patch aims to fix the bug of repeatly building inode cache.
The next two patches fix problems with the first one applied.

For better review, I decided to have three patches instead of folding them into
one.

Liu Bo (3):
  Btrfs: avoid building inode cache repeatly
  Btrfs: don''t build inode cache for orphan root
  Btrfs: fix EEXIST error when creating new file in subvolume/snapshot

 fs/btrfs/disk-io.c   |  9 +++++----
 fs/btrfs/inode-map.c | 21 +++++++++++++++++----
 2 files changed, 22 insertions(+), 8 deletions(-)

-- 
1.8.2.1

--
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

2013-Dec-13 09:16 UTC

head link

[PATCH 1/3] Btrfs: avoid building inode cache repeatly

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 8072cfa..cb0b12b 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1524,14 +1524,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

--
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

2013-Dec-13 09:16 UTC

head link

[PATCH 2/3] Btrfs: don''t build inode cache for orphan root

We check if we have orphan roots when mounting btrfs, but orphan roots
are those who are already dead and about to be freed, so don''t start
building
inode cache for them, otherwise we''ll get an ugly crash.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/inode-map.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/inode-map.c b/fs/btrfs/inode-map.c
index 6c8d7bb..493694f 100644
--- a/fs/btrfs/inode-map.c
+++ b/fs/btrfs/inode-map.c
@@ -141,7 +141,9 @@ static void start_caching(struct btrfs_root *root)
 	int ret;
 	u64 objectid;
 
-	if (!btrfs_test_opt(root, INODE_MAP_CACHE))
+	/* Don''t even start if this is an orphan root. */
+	if (!btrfs_test_opt(root, INODE_MAP_CACHE) ||
+	    btrfs_root_refs(&root->root_item) == 0)
 		return;
 
 	spin_lock(&root->cache_lock);
-- 
1.8.2.1

--
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

2013-Dec-13 09:16 UTC

head link

[PATCH 3/3] Btrfs: fix EEXIST error when creating new file in subvolume/snapshot

While creating a subvolume/snapshot, we don''t use inode cache to
allocate
an inode id for the root dir "..", so inode cache doesn''t
mark that id as
used, and when we create a new file, it''ll find that fact and throw out
-EEXIST.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/inode-map.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/inode-map.c b/fs/btrfs/inode-map.c
index 493694f..bcff910 100644
--- a/fs/btrfs/inode-map.c
+++ b/fs/btrfs/inode-map.c
@@ -528,6 +528,16 @@ static int btrfs_find_highest_objectid(struct btrfs_root
*root, u64 *objectid)
 	struct btrfs_key search_key;
 	struct btrfs_key found_key;
 	int slot;
+	u64 min_objectid;
+
+	/*
+	 * For fs/file tree, FIRST_FREE_OBJECTID is reserved for
+	 * root dir ".."
+	 */
+	if (is_fstree(root->root_key.objectid))
+		min_objectid = BTRFS_FIRST_FREE_OBJECTID;
+	else
+		min_objectid = BTRFS_FIRST_FREE_OBJECTID - 1;
 
 	path = btrfs_alloc_path();
 	if (!path)
@@ -544,10 +554,9 @@ static int btrfs_find_highest_objectid(struct btrfs_root
*root, u64 *objectid)
 		slot = path->slots[0] - 1;
 		l = path->nodes[0];
 		btrfs_item_key_to_cpu(l, &found_key, slot);
-		*objectid = max_t(u64, found_key.objectid,
-				  BTRFS_FIRST_FREE_OBJECTID - 1);
+		*objectid = max_t(u64, found_key.objectid, min_objectid);
 	} else {
-		*objectid = BTRFS_FIRST_FREE_OBJECTID - 1;
+		*objectid = min_objectid;
 	}
 	ret = 0;
 error:
-- 
1.8.2.1

--
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

Miao Xie

2013-Dec-13 09:38 UTC

head link

Re: [PATCH 1/3] Btrfs: avoid building inode cache repeatly

On fri, 13 Dec 2013 17:16:21 +0800, Liu Bo wrote:> 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.
This patch will introduce a problem that if some tasks load the same fs root
at the same time, they will building inode cache for it respectively. So I will
NACK this patch. Why not build the inode cache after the fs root is inserted
into the radix tree successfully.

Thanks
Miao
> 
> 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 8072cfa..cb0b12b 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1524,14 +1524,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,
> 
--
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

Miao Xie

2013-Dec-13 09:43 UTC

head link

Re: [PATCH 2/3] Btrfs: don''t build inode cache for orphan root

On 	fri, 13 Dec 2013 17:16:22 +0800, Liu Bo wrote:> We check if we have orphan roots when mounting btrfs, but orphan roots
> are those who are already dead and about to be freed, so don''t
start building
> inode cache for them, otherwise we''ll get an ugly crash.
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
Acked-by: Miao Xie <miaox@cn.fujitsu.com>
> ---
>  fs/btrfs/inode-map.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/inode-map.c b/fs/btrfs/inode-map.c
> index 6c8d7bb..493694f 100644
> --- a/fs/btrfs/inode-map.c
> +++ b/fs/btrfs/inode-map.c
> @@ -141,7 +141,9 @@ static void start_caching(struct btrfs_root *root)
>  	int ret;
>  	u64 objectid;
>  
> -	if (!btrfs_test_opt(root, INODE_MAP_CACHE))
> +	/* Don''t even start if this is an orphan root. */
> +	if (!btrfs_test_opt(root, INODE_MAP_CACHE) ||
> +	    btrfs_root_refs(&root->root_item) == 0)
>  		return;
>  
>  	spin_lock(&root->cache_lock);
> 
--
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

Miao Xie

2013-Dec-13 09:46 UTC

head link

Re: [PATCH 3/3] Btrfs: fix EEXIST error when creating new file in subvolume/snapshot

On 	fri, 13 Dec 2013 17:16:23 +0800, Liu Bo wrote:> While creating a subvolume/snapshot, we don''t use inode cache to
allocate
> an inode id for the root dir "..", so inode cache
doesn''t mark that id as
FIRST_FREE_OBJECTID should be the root dir ".", not "..".
The other is OK for me.

Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
> used, and when we create a new file, it''ll find that fact and
throw out
> -EEXIST.
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ---
>  fs/btrfs/inode-map.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/inode-map.c b/fs/btrfs/inode-map.c
> index 493694f..bcff910 100644
> --- a/fs/btrfs/inode-map.c
> +++ b/fs/btrfs/inode-map.c
> @@ -528,6 +528,16 @@ static int btrfs_find_highest_objectid(struct
btrfs_root *root, u64 *objectid)
>  	struct btrfs_key search_key;
>  	struct btrfs_key found_key;
>  	int slot;
> +	u64 min_objectid;
> +
> +	/*
> +	 * For fs/file tree, FIRST_FREE_OBJECTID is reserved for
> +	 * root dir ".."
> +	 */
> +	if (is_fstree(root->root_key.objectid))
> +		min_objectid = BTRFS_FIRST_FREE_OBJECTID;
> +	else
> +		min_objectid = BTRFS_FIRST_FREE_OBJECTID - 1;
>  
>  	path = btrfs_alloc_path();
>  	if (!path)
> @@ -544,10 +554,9 @@ static int btrfs_find_highest_objectid(struct
btrfs_root *root, u64 *objectid)
>  		slot = path->slots[0] - 1;
>  		l = path->nodes[0];
>  		btrfs_item_key_to_cpu(l, &found_key, slot);
> -		*objectid = max_t(u64, found_key.objectid,
> -				  BTRFS_FIRST_FREE_OBJECTID - 1);
> +		*objectid = max_t(u64, found_key.objectid, min_objectid);
>  	} else {
> -		*objectid = BTRFS_FIRST_FREE_OBJECTID - 1;
> +		*objectid = min_objectid;
>  	}
>  	ret = 0;
>  error:
> 
--
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 - Dec 2013 - [PATCH 0/3] Btrfs: inode cache fixes

[PATCH 0/3] Btrfs: inode cache fixes

[PATCH 1/3] Btrfs: avoid building inode cache repeatly

[PATCH 2/3] Btrfs: don''t build inode cache for orphan root

[PATCH 3/3] Btrfs: fix EEXIST error when creating new file in subvolume/snapshot

Re: [PATCH 1/3] Btrfs: avoid building inode cache repeatly

Re: [PATCH 2/3] Btrfs: don''t build inode cache for orphan root

Re: [PATCH 3/3] Btrfs: fix EEXIST error when creating new file in subvolume/snapshot