* [PATCH 1/2] Btrfs: fix for patch "cleanup: don't check the same thing twice"
@ 2013-08-23 8:34 Stefan Behrens
2013-08-23 8:34 ` [PATCH 2/2] Btrf: cleanup: don't check for root_refs == 0 twice Stefan Behrens
2013-08-23 9:03 ` [PATCH 1/2] Btrfs: fix for patch "cleanup: don't check the same thing twice" Miao Xie
0 siblings, 2 replies; 4+ messages in thread
From: Stefan Behrens @ 2013-08-23 8:34 UTC (permalink / raw)
To: linux-btrfs
Mitch Harder noticed that the patch 3c64a1a mentioned in the subject
line was causing a kernel BUG() on snapshot deletion.
The patch was wrong. It did not handle cached roots correctly. The
check for root_refs == 0 was removed everywhere where
btrfs_read_fs_root_no_name() had been used to retrieve the root,
because this check was already dealt with in
btrfs_read_fs_root_no_name(). But in the case when the root was
found in the cache, there was no such check.
This patch adds the missing check in the case where the root is
found in the cache.
Reported-by: Mitch Harder <mitch.harder@sabayonlinux.org>
Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de>
---
fs/btrfs/disk-io.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 43ec3c6..7078554 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1583,8 +1583,11 @@ struct btrfs_root *btrfs_read_fs_root_no_name(struct btrfs_fs_info *fs_info,
ERR_PTR(-ENOENT);
again:
root = btrfs_lookup_fs_root(fs_info, location->objectid);
- if (root)
+ if (root) {
+ if (btrfs_root_refs(&root->root_item) == 0)
+ return ERR_PTR(-ENOENT);
return root;
+ }
root = btrfs_read_fs_root(fs_info->tree_root, location);
if (IS_ERR(root))
--
1.8.3.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] Btrf: cleanup: don't check for root_refs == 0 twice
2013-08-23 8:34 [PATCH 1/2] Btrfs: fix for patch "cleanup: don't check the same thing twice" Stefan Behrens
@ 2013-08-23 8:34 ` Stefan Behrens
2013-08-23 9:03 ` [PATCH 1/2] Btrfs: fix for patch "cleanup: don't check the same thing twice" Miao Xie
1 sibling, 0 replies; 4+ messages in thread
From: Stefan Behrens @ 2013-08-23 8:34 UTC (permalink / raw)
To: linux-btrfs
btrfs_read_fs_root_no_name() already checks if btrfs_root_refs()
is zero and returns ENOENT in this case. There is no need to do
it again in three more places.
Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de>
---
fs/btrfs/file.c | 5 -----
fs/btrfs/relocation.c | 3 ---
fs/btrfs/scrub.c | 5 -----
3 files changed, 13 deletions(-)
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 5c63229..fce83dd 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -310,11 +310,6 @@ static int __btrfs_run_defrag_inode(struct btrfs_fs_info *fs_info,
goto cleanup;
}
- if (btrfs_root_refs(&inode_root->root_item) == 0) {
- ret = -ENOENT;
- goto cleanup;
- }
-
key.objectid = defrag->ino;
btrfs_set_key_type(&key, BTRFS_INODE_ITEM_KEY);
key.offset = 0;
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index cf5e30f..aacc212 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -2354,9 +2354,6 @@ again:
if (IS_ERR(root))
continue;
- if (btrfs_root_refs(&root->root_item) == 0)
- continue;
-
trans = btrfs_join_transaction(root);
BUG_ON(IS_ERR(trans));
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index ec6a33a..0afcd45 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -3217,11 +3217,6 @@ static int copy_nocow_pages_for_inode(u64 inum, u64 offset, u64 root, void *ctx)
return PTR_ERR(local_root);
}
- if (btrfs_root_refs(&local_root->root_item) == 0) {
- srcu_read_unlock(&fs_info->subvol_srcu, srcu_index);
- return -ENOENT;
- }
-
key.type = BTRFS_INODE_ITEM_KEY;
key.objectid = inum;
key.offset = 0;
--
1.8.3.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/2] Btrfs: fix for patch "cleanup: don't check the same thing twice"
2013-08-23 8:34 [PATCH 1/2] Btrfs: fix for patch "cleanup: don't check the same thing twice" Stefan Behrens
2013-08-23 8:34 ` [PATCH 2/2] Btrf: cleanup: don't check for root_refs == 0 twice Stefan Behrens
@ 2013-08-23 9:03 ` Miao Xie
2013-08-23 16:24 ` Mitch Harder
1 sibling, 1 reply; 4+ messages in thread
From: Miao Xie @ 2013-08-23 9:03 UTC (permalink / raw)
To: Stefan Behrens; +Cc: linux-btrfs
On fri, 23 Aug 2013 10:34:42 +0200, Stefan Behrens wrote:
> Mitch Harder noticed that the patch 3c64a1a mentioned in the subject
> line was causing a kernel BUG() on snapshot deletion.
>
> The patch was wrong. It did not handle cached roots correctly. The
> check for root_refs == 0 was removed everywhere where
> btrfs_read_fs_root_no_name() had been used to retrieve the root,
> because this check was already dealt with in
> btrfs_read_fs_root_no_name(). But in the case when the root was
> found in the cache, there was no such check.
>
> This patch adds the missing check in the case where the root is
> found in the cache.
>
> Reported-by: Mitch Harder <mitch.harder@sabayonlinux.org>
> Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de>
> ---
> fs/btrfs/disk-io.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 43ec3c6..7078554 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1583,8 +1583,11 @@ struct btrfs_root *btrfs_read_fs_root_no_name(struct btrfs_fs_info *fs_info,
> ERR_PTR(-ENOENT);
> again:
> root = btrfs_lookup_fs_root(fs_info, location->objectid);
> - if (root)
> + if (root) {
> + if (btrfs_root_refs(&root->root_item) == 0)
> + return ERR_PTR(-ENOENT);
> return root;
> + }
It seems good to me.
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
>
> root = btrfs_read_fs_root(fs_info->tree_root, location);
> if (IS_ERR(root))
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 1/2] Btrfs: fix for patch "cleanup: don't check the same thing twice"
2013-08-23 9:03 ` [PATCH 1/2] Btrfs: fix for patch "cleanup: don't check the same thing twice" Miao Xie
@ 2013-08-23 16:24 ` Mitch Harder
0 siblings, 0 replies; 4+ messages in thread
From: Mitch Harder @ 2013-08-23 16:24 UTC (permalink / raw)
To: Miao Xie; +Cc: Stefan Behrens, linux-btrfs
On Fri, Aug 23, 2013 at 4:03 AM, Miao Xie <miaox@cn.fujitsu.com> wrote:
> On fri, 23 Aug 2013 10:34:42 +0200, Stefan Behrens wrote:
>> Mitch Harder noticed that the patch 3c64a1a mentioned in the subject
>> line was causing a kernel BUG() on snapshot deletion.
>>
>> The patch was wrong. It did not handle cached roots correctly. The
>> check for root_refs == 0 was removed everywhere where
>> btrfs_read_fs_root_no_name() had been used to retrieve the root,
>> because this check was already dealt with in
>> btrfs_read_fs_root_no_name(). But in the case when the root was
>> found in the cache, there was no such check.
>>
>> This patch adds the missing check in the case where the root is
>> found in the cache.
>>
>> Reported-by: Mitch Harder <mitch.harder@sabayonlinux.org>
>> Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de>
>> ---
>> fs/btrfs/disk-io.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 43ec3c6..7078554 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -1583,8 +1583,11 @@ struct btrfs_root *btrfs_read_fs_root_no_name(struct btrfs_fs_info *fs_info,
>> ERR_PTR(-ENOENT);
>> again:
>> root = btrfs_lookup_fs_root(fs_info, location->objectid);
>> - if (root)
>> + if (root) {
>> + if (btrfs_root_refs(&root->root_item) == 0)
>> + return ERR_PTR(-ENOENT);
>> return root;
>> + }
>
> It seems good to me.
>
> Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
>
>>
>> root = btrfs_read_fs_root(fs_info->tree_root, location);
>> if (IS_ERR(root))
>>
Tested-by: Mitch Harder <mitch.harder@sabayonlinux.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-08-23 16:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-23 8:34 [PATCH 1/2] Btrfs: fix for patch "cleanup: don't check the same thing twice" Stefan Behrens
2013-08-23 8:34 ` [PATCH 2/2] Btrf: cleanup: don't check for root_refs == 0 twice Stefan Behrens
2013-08-23 9:03 ` [PATCH 1/2] Btrfs: fix for patch "cleanup: don't check the same thing twice" Miao Xie
2013-08-23 16:24 ` Mitch Harder
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).