* [PATCH 1/2] Btrfs-progs: fsck: avoid overwritting existed space when initting csum tree
@ 2014-02-25 11:48 Wang Shilong
2014-02-25 11:48 ` [PATCH 2/2] Btrfs-progs: use bitfield instead of integer for some variants in fs_info Wang Shilong
0 siblings, 1 reply; 5+ messages in thread
From: Wang Shilong @ 2014-02-25 11:48 UTC (permalink / raw)
To: linux-btrfs
Steps to reproduce:
# mkfs.btrfs -f /dev/sda9
# btrfs check /dev/sda9 --init-extent-tree --init-csum-tree
# btrfs check /dev/sda9
During reinitting extent tree, we will pin all metadata blocks to
avoid overwritting existing metadata space. However, those space will
be unpinned after committing transaction.
If we try to reinit csum tree after reiniting extent tree, we may
overwrite existing space. Fix this problem by making reinit extent tree
and csum tree in same transaction.
Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
cmds-check.c | 74 ++++++++++++++++++++++++++----------------------------------
1 file changed, 32 insertions(+), 42 deletions(-)
diff --git a/cmds-check.c b/cmds-check.c
index 253569f..4cdeac8 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -6179,9 +6179,9 @@ out:
return ret;
}
-static int reinit_extent_tree(struct btrfs_fs_info *fs_info)
+static int reinit_extent_tree(struct btrfs_trans_handle *trans,
+ struct btrfs_fs_info *fs_info)
{
- struct btrfs_trans_handle *trans;
u64 start = 0;
int ret;
@@ -6200,12 +6200,6 @@ static int reinit_extent_tree(struct btrfs_fs_info *fs_info)
return -EINVAL;
}
- trans = btrfs_start_transaction(fs_info->extent_root, 1);
- if (IS_ERR(trans)) {
- fprintf(stderr, "Error starting transaction\n");
- return PTR_ERR(trans);
- }
-
/*
* first we need to walk all of the trees except the extent tree and pin
* down the bytes that are in use so we don't overwrite any existing
@@ -6268,11 +6262,7 @@ static int reinit_extent_tree(struct btrfs_fs_info *fs_info)
btrfs_extent_post_op(trans, fs_info->extent_root);
}
- /*
- * Ok now we commit and run the normal fsck, which will add extent
- * entries for all of the items it finds.
- */
- return btrfs_commit_transaction(trans, fs_info->extent_root);
+ return 0;
}
static int recow_extent_buffer(struct btrfs_root *root, struct extent_buffer *eb)
@@ -6475,47 +6465,47 @@ int cmd_check(int argc, char **argv)
goto close_out;
}
- if (init_extent_tree) {
- printf("Creating a new extent tree\n");
- ret = reinit_extent_tree(info);
- if (ret)
- goto close_out;
- }
- if (!extent_buffer_uptodate(info->extent_root->node)) {
- fprintf(stderr, "Critical roots corrupted, unable to fsck the FS\n");
- ret = -EIO;
- goto close_out;
- }
-
- fprintf(stderr, "checking extents\n");
- if (init_csum_tree) {
+ if (init_extent_tree || init_csum_tree) {
struct btrfs_trans_handle *trans;
- fprintf(stderr, "Reinit crc root\n");
- trans = btrfs_start_transaction(info->csum_root, 1);
+ trans = btrfs_start_transaction(info->extent_root, 0);
if (IS_ERR(trans)) {
fprintf(stderr, "Error starting transaction\n");
ret = PTR_ERR(trans);
goto close_out;
}
- ret = btrfs_fsck_reinit_root(trans, info->csum_root, 0);
- if (ret) {
- fprintf(stderr, "crc root initialization failed\n");
- ret = -EIO;
- goto close_out;
+ if (init_extent_tree) {
+ printf("Creating a new extent tree\n");
+ ret = reinit_extent_tree(trans, info);
+ if (ret)
+ goto close_out;
}
- ret = btrfs_commit_transaction(trans, info->csum_root);
- if (ret)
- exit(1);
-
- ret = check_chunks_and_extents(root);
+ if (init_csum_tree) {
+ fprintf(stderr, "Reinit crc root\n");
+ ret = btrfs_fsck_reinit_root(trans, info->csum_root, 0);
+ if (ret) {
+ fprintf(stderr, "crc root initialization failed\n");
+ ret = -EIO;
+ goto close_out;
+ }
+ }
+ /*
+ * Ok now we commit and run the normal fsck, which will add
+ * extent entries for all of the items it finds.
+ */
+ ret = btrfs_commit_transaction(trans, info->extent_root);
if (ret)
- fprintf(stderr,
- "Errors found in extent allocation tree or chunk allocation\n");
- goto out;
+ goto close_out;
+ }
+ if (!extent_buffer_uptodate(info->extent_root->node)) {
+ fprintf(stderr, "Critical roots corrupted, unable to fsck the FS\n");
+ ret = -EIO;
+ goto close_out;
}
+
+ fprintf(stderr, "checking extents\n");
ret = check_chunks_and_extents(root);
if (ret)
fprintf(stderr, "Errors found in extent allocation tree or chunk allocation\n");
--
1.9.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] Btrfs-progs: use bitfield instead of integer for some variants in fs_info
2014-02-25 11:48 [PATCH 1/2] Btrfs-progs: fsck: avoid overwritting existed space when initting csum tree Wang Shilong
@ 2014-02-25 11:48 ` Wang Shilong
2014-02-25 16:28 ` David Sterba
0 siblings, 1 reply; 5+ messages in thread
From: Wang Shilong @ 2014-02-25 11:48 UTC (permalink / raw)
To: linux-btrfs
Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
ctree.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/ctree.h b/ctree.h
index 3cc3477..9b461af 100644
--- a/ctree.h
+++ b/ctree.h
@@ -984,9 +984,11 @@ struct btrfs_fs_info {
struct btrfs_fs_devices *fs_devices;
struct list_head space_info;
int system_allocs;
- int readonly;
- int on_restoring;
- int is_chunk_recover;
+
+ unsigned int readonly:1;
+ unsigned int on_restoring:1;
+ unsigned int is_chunk_recover:1;
+
int (*free_extent_hook)(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
u64 bytenr, u64 num_bytes, u64 parent,
--
1.9.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] Btrfs-progs: use bitfield instead of integer for some variants in fs_info
2014-02-25 11:48 ` [PATCH 2/2] Btrfs-progs: use bitfield instead of integer for some variants in fs_info Wang Shilong
@ 2014-02-25 16:28 ` David Sterba
2014-02-26 2:25 ` Wang Shilong
0 siblings, 1 reply; 5+ messages in thread
From: David Sterba @ 2014-02-25 16:28 UTC (permalink / raw)
To: Wang Shilong; +Cc: linux-btrfs
On Tue, Feb 25, 2014 at 07:48:57PM +0800, Wang Shilong wrote:
> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
> ---
> ctree.h | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/ctree.h b/ctree.h
> index 3cc3477..9b461af 100644
> --- a/ctree.h
> +++ b/ctree.h
> @@ -984,9 +984,11 @@ struct btrfs_fs_info {
> struct btrfs_fs_devices *fs_devices;
> struct list_head space_info;
> int system_allocs;
> - int readonly;
> - int on_restoring;
> - int is_chunk_recover;
> +
> + unsigned int readonly:1;
> + unsigned int on_restoring:1;
> + unsigned int is_chunk_recover:1;
Well, the integers are wasteful, but there's only one instance of
fs_info per fsck run, so it saves like 8 bytes in total. I'm not sure
this patch is needed.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] Btrfs-progs: use bitfield instead of integer for some variants in fs_info
2014-02-25 16:28 ` David Sterba
@ 2014-02-26 2:25 ` Wang Shilong
2014-02-26 18:16 ` David Sterba
0 siblings, 1 reply; 5+ messages in thread
From: Wang Shilong @ 2014-02-26 2:25 UTC (permalink / raw)
To: dsterba, linux-btrfs
On 02/26/2014 12:28 AM, David Sterba wrote:
> On Tue, Feb 25, 2014 at 07:48:57PM +0800, Wang Shilong wrote:
>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>> ---
>> ctree.h | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/ctree.h b/ctree.h
>> index 3cc3477..9b461af 100644
>> --- a/ctree.h
>> +++ b/ctree.h
>> @@ -984,9 +984,11 @@ struct btrfs_fs_info {
>> struct btrfs_fs_devices *fs_devices;
>> struct list_head space_info;
>> int system_allocs;
>> - int readonly;
>> - int on_restoring;
>> - int is_chunk_recover;
>> +
>> + unsigned int readonly:1;
>> + unsigned int on_restoring:1;
>> + unsigned int is_chunk_recover:1;
> Well, the integers are wasteful, but there's only one instance of
> fs_info per fsck run, so it saves like 8 bytes in total. I'm not sure
> this patch is needed.
Originally, i wrote this patch, it is because i was going to add a new flag.
However, that approach has been replaced by a better idea.
Anyway, this patch will be a good start, however, i don't insist on it.
We can also change it when we have to add another flag later.
David, just do what you like here.
Thanks,
Wang
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] Btrfs-progs: use bitfield instead of integer for some variants in fs_info
2014-02-26 2:25 ` Wang Shilong
@ 2014-02-26 18:16 ` David Sterba
0 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2014-02-26 18:16 UTC (permalink / raw)
To: Wang Shilong; +Cc: dsterba, linux-btrfs
On Wed, Feb 26, 2014 at 10:25:24AM +0800, Wang Shilong wrote:
> On 02/26/2014 12:28 AM, David Sterba wrote:
> >On Tue, Feb 25, 2014 at 07:48:57PM +0800, Wang Shilong wrote:
> >>Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
> >>---
> >> ctree.h | 8 +++++---
> >> 1 file changed, 5 insertions(+), 3 deletions(-)
> >>
> >>diff --git a/ctree.h b/ctree.h
> >>index 3cc3477..9b461af 100644
> >>--- a/ctree.h
> >>+++ b/ctree.h
> >>@@ -984,9 +984,11 @@ struct btrfs_fs_info {
> >> struct btrfs_fs_devices *fs_devices;
> >> struct list_head space_info;
> >> int system_allocs;
> >>- int readonly;
> >>- int on_restoring;
> >>- int is_chunk_recover;
> >>+
> >>+ unsigned int readonly:1;
> >>+ unsigned int on_restoring:1;
> >>+ unsigned int is_chunk_recover:1;
> >Well, the integers are wasteful, but there's only one instance of
> >fs_info per fsck run, so it saves like 8 bytes in total. I'm not sure
> >this patch is needed.
> Originally, i wrote this patch, it is because i was going to add a new flag.
> However, that approach has been replaced by a better idea.
>
> Anyway, this patch will be a good start, however, i don't insist on it.
> We can also change it when we have to add another flag later.
Ok, let's keep it, as an example of the pattern 'bitfields for flags'.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-02-26 18:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-25 11:48 [PATCH 1/2] Btrfs-progs: fsck: avoid overwritting existed space when initting csum tree Wang Shilong
2014-02-25 11:48 ` [PATCH 2/2] Btrfs-progs: use bitfield instead of integer for some variants in fs_info Wang Shilong
2014-02-25 16:28 ` David Sterba
2014-02-26 2:25 ` Wang Shilong
2014-02-26 18:16 ` David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.