* [PATCH 0/3] Btrfs: inode cache fixes
@ 2013-12-13 9:16 Liu Bo
2013-12-13 9:16 ` [PATCH 1/3] Btrfs: avoid building inode cache repeatly Liu Bo
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Liu Bo @ 2013-12-13 9:16 UTC (permalink / raw)
To: linux-btrfs
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] Btrfs: avoid building inode cache repeatly
2013-12-13 9:16 [PATCH 0/3] Btrfs: inode cache fixes Liu Bo
@ 2013-12-13 9:16 ` Liu Bo
2013-12-13 9:38 ` Miao Xie
2013-12-13 9:16 ` [PATCH 2/3] Btrfs: don't build inode cache for orphan root Liu Bo
2013-12-13 9:16 ` [PATCH 3/3] Btrfs: fix EEXIST error when creating new file in subvolume/snapshot Liu Bo
2 siblings, 1 reply; 7+ messages in thread
From: Liu Bo @ 2013-12-13 9:16 UTC (permalink / raw)
To: linux-btrfs
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] Btrfs: don't build inode cache for orphan root
2013-12-13 9:16 [PATCH 0/3] Btrfs: inode cache fixes Liu Bo
2013-12-13 9:16 ` [PATCH 1/3] Btrfs: avoid building inode cache repeatly Liu Bo
@ 2013-12-13 9:16 ` Liu Bo
2013-12-13 9:43 ` Miao Xie
2013-12-13 9:16 ` [PATCH 3/3] Btrfs: fix EEXIST error when creating new file in subvolume/snapshot Liu Bo
2 siblings, 1 reply; 7+ messages in thread
From: Liu Bo @ 2013-12-13 9:16 UTC (permalink / raw)
To: linux-btrfs
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] Btrfs: fix EEXIST error when creating new file in subvolume/snapshot
2013-12-13 9:16 [PATCH 0/3] Btrfs: inode cache fixes Liu Bo
2013-12-13 9:16 ` [PATCH 1/3] Btrfs: avoid building inode cache repeatly Liu Bo
2013-12-13 9:16 ` [PATCH 2/3] Btrfs: don't build inode cache for orphan root Liu Bo
@ 2013-12-13 9:16 ` Liu Bo
2013-12-13 9:46 ` Miao Xie
2 siblings, 1 reply; 7+ messages in thread
From: Liu Bo @ 2013-12-13 9:16 UTC (permalink / raw)
To: linux-btrfs
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] Btrfs: avoid building inode cache repeatly
2013-12-13 9:16 ` [PATCH 1/3] Btrfs: avoid building inode cache repeatly Liu Bo
@ 2013-12-13 9:38 ` Miao Xie
0 siblings, 0 replies; 7+ messages in thread
From: Miao Xie @ 2013-12-13 9:38 UTC (permalink / raw)
To: Liu Bo, linux-btrfs
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,
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] Btrfs: don't build inode cache for orphan root
2013-12-13 9:16 ` [PATCH 2/3] Btrfs: don't build inode cache for orphan root Liu Bo
@ 2013-12-13 9:43 ` Miao Xie
0 siblings, 0 replies; 7+ messages in thread
From: Miao Xie @ 2013-12-13 9:43 UTC (permalink / raw)
To: Liu Bo, linux-btrfs
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);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] Btrfs: fix EEXIST error when creating new file in subvolume/snapshot
2013-12-13 9:16 ` [PATCH 3/3] Btrfs: fix EEXIST error when creating new file in subvolume/snapshot Liu Bo
@ 2013-12-13 9:46 ` Miao Xie
0 siblings, 0 replies; 7+ messages in thread
From: Miao Xie @ 2013-12-13 9:46 UTC (permalink / raw)
To: Liu Bo, linux-btrfs
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:
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-12-13 9:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-13 9:16 [PATCH 0/3] Btrfs: inode cache fixes Liu Bo
2013-12-13 9:16 ` [PATCH 1/3] Btrfs: avoid building inode cache repeatly Liu Bo
2013-12-13 9:38 ` Miao Xie
2013-12-13 9:16 ` [PATCH 2/3] Btrfs: don't build inode cache for orphan root Liu Bo
2013-12-13 9:43 ` Miao Xie
2013-12-13 9:16 ` [PATCH 3/3] Btrfs: fix EEXIST error when creating new file in subvolume/snapshot Liu Bo
2013-12-13 9:46 ` Miao Xie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).