* [PATCH 1/6] btrfs-progs: fix floating point exception for btrfs-calc-size
@ 2015-10-29 9:31 Zhao Lei
2015-10-29 9:31 ` [PATCH 2/6] btrfs-progs: Fix negative eb's ref_cnt in btrfs-calc-size Zhao Lei
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Zhao Lei @ 2015-10-29 9:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: Zhao Lei
Current code exit with floating point exception on a blank fs:
# btrfs-calc-size -b /dev/sda6
Calculating size of root tree
Total size: 16384
Inline data: 0
Total seeks: 0
Forward seeks: 0
Backward seeks: 0
Floating point exception
This patch add a condition check for above case.
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
btrfs-calc-size.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/btrfs-calc-size.c b/btrfs-calc-size.c
index b756693..17d44ae 100644
--- a/btrfs-calc-size.c
+++ b/btrfs-calc-size.c
@@ -372,8 +372,8 @@ out_print:
printf("\tTotal seeks: %Lu\n", stat.total_seeks);
printf("\t\tForward seeks: %Lu\n", stat.forward_seeks);
printf("\t\tBackward seeks: %Lu\n", stat.backward_seeks);
- printf("\t\tAvg seek len: %Lu\n", stat.total_seek_len /
- stat.total_seeks);
+ printf("\t\tAvg seek len: %llu\n", stat.total_seeks ?
+ stat.total_seek_len / stat.total_seeks : 0);
print_seek_histogram(&stat);
printf("\tTotal clusters: %Lu\n", stat.total_clusters);
printf("\t\tAvg cluster size: %Lu\n", stat.total_cluster_size /
--
1.8.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/6] btrfs-progs: Fix negative eb's ref_cnt in btrfs-calc-size
2015-10-29 9:31 [PATCH 1/6] btrfs-progs: fix floating point exception for btrfs-calc-size Zhao Lei
@ 2015-10-29 9:31 ` Zhao Lei
2015-10-29 9:31 ` [PATCH 3/6] btrfs-progs: free fslabel for btrfs-convert Zhao Lei
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Zhao Lei @ 2015-10-29 9:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: Zhao Lei
btrfs-calc-size show following warning:
# btrfs-calc-size /dev/sda6
Calculating size of root tree
...
extent_io.c:582: free_extent_buffer: Assertion `eb->refs < 0` failed.
./btrfs-calc-size[0x41d642]
./btrfs-calc-size(free_extent_buffer+0x70)[0x41e1c1]
./btrfs-calc-size(btrfs_free_fs_root+0x11)[0x40e1e8]
./btrfs-calc-size[0x40e215]
./btrfs-calc-size(rb_free_nodes+0x1d)[0x4326fe]
./btrfs-calc-size(close_ctree+0x3f3)[0x40f9ea]
./btrfs-calc-size(main+0x200)[0x431b4e]
/lib64/libc.so.6(__libc_start_main+0xf5)[0x3858621d65]
./btrfs-calc-size[0x407009]
Reason:
path in calc_root_size() is only used to save node data,
it don't hold ref_cnt for each eb in.
Using btrfs_free_path() to free path will reduce these eb
again, and cause many problems, as negative ref_cnt or
invalid memory access.
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
btrfs-calc-size.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/btrfs-calc-size.c b/btrfs-calc-size.c
index 17d44ae..b84cda9 100644
--- a/btrfs-calc-size.c
+++ b/btrfs-calc-size.c
@@ -417,7 +417,14 @@ out:
free(seek);
}
- btrfs_free_path(path);
+ /*
+ * We only use path to save node data in iterating,
+ * without holding eb's ref_cnt in path.
+ * Don't use btrfs_free_path() here, it will free these
+ * eb again, and cause many problems, as negative ref_cnt
+ * or invalid memory access.
+ */
+ free(path);
return ret;
}
--
1.8.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/6] btrfs-progs: free fslabel for btrfs-convert
2015-10-29 9:31 [PATCH 1/6] btrfs-progs: fix floating point exception for btrfs-calc-size Zhao Lei
2015-10-29 9:31 ` [PATCH 2/6] btrfs-progs: Fix negative eb's ref_cnt in btrfs-calc-size Zhao Lei
@ 2015-10-29 9:31 ` Zhao Lei
2015-10-29 13:08 ` David Sterba
2015-10-29 9:31 ` [PATCH 4/6] btrfs-progs: Fix uninitialized key.type for btrfs_find_free_objectid Zhao Lei
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Zhao Lei @ 2015-10-29 9:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: Zhao Lei
fslabel need to be freed before exit.
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
btrfs-convert.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/btrfs-convert.c b/btrfs-convert.c
index 5b9171e..1693d03 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -3027,6 +3027,9 @@ int main(int argc, char *argv[])
ret = do_convert(file, datacsum, packing, noxattr, nodesize,
copylabel, fslabel, progress, features);
}
+
+ free(fslabel);
+
if (ret)
return 1;
return 0;
--
1.8.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/6] btrfs-progs: Fix uninitialized key.type for btrfs_find_free_objectid
2015-10-29 9:31 [PATCH 1/6] btrfs-progs: fix floating point exception for btrfs-calc-size Zhao Lei
2015-10-29 9:31 ` [PATCH 2/6] btrfs-progs: Fix negative eb's ref_cnt in btrfs-calc-size Zhao Lei
2015-10-29 9:31 ` [PATCH 3/6] btrfs-progs: free fslabel for btrfs-convert Zhao Lei
@ 2015-10-29 9:31 ` Zhao Lei
2015-10-29 9:31 ` [PATCH 5/6] btrfs-progs: free comparer_set in cmd_qgroup_show Zhao Lei
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Zhao Lei @ 2015-10-29 9:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: Zhao Lei
To avoid using uninitialized value in btrfs_search_slot().
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
inode-map.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/inode-map.c b/inode-map.c
index 1321bfb..346952b 100644
--- a/inode-map.c
+++ b/inode-map.c
@@ -44,6 +44,7 @@ int btrfs_find_free_objectid(struct btrfs_trans_handle *trans,
BTRFS_FIRST_FREE_OBJECTID);
search_key.objectid = search_start;
search_key.offset = 0;
+ search_key.type = 0;
btrfs_init_path(path);
start_found = 0;
--
1.8.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/6] btrfs-progs: free comparer_set in cmd_qgroup_show
2015-10-29 9:31 [PATCH 1/6] btrfs-progs: fix floating point exception for btrfs-calc-size Zhao Lei
` (2 preceding siblings ...)
2015-10-29 9:31 ` [PATCH 4/6] btrfs-progs: Fix uninitialized key.type for btrfs_find_free_objectid Zhao Lei
@ 2015-10-29 9:31 ` Zhao Lei
2015-10-30 13:35 ` David Sterba
2015-10-29 9:31 ` [PATCH 6/6] btrfs-progs: Avoid use pointer in handle_options Zhao Lei
2015-10-29 13:26 ` [PATCH 1/6] btrfs-progs: fix floating point exception for btrfs-calc-size David Sterba
5 siblings, 1 reply; 11+ messages in thread
From: Zhao Lei @ 2015-10-29 9:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: Zhao Lei
comparer_set, which was allocated by malloc(), should be free before
function return.
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
cmds-qgroup.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index a64b716..f069d32 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -290,7 +290,7 @@ static int cmd_qgroup_show(int argc, char **argv)
int filter_flag = 0;
unsigned unit_mode;
- struct btrfs_qgroup_comparer_set *comparer_set;
+ struct btrfs_qgroup_comparer_set *comparer_set = NULL;
struct btrfs_qgroup_filter_set *filter_set;
filter_set = btrfs_qgroup_alloc_filter_set();
comparer_set = btrfs_qgroup_alloc_comparer_set();
@@ -372,6 +372,8 @@ static int cmd_qgroup_show(int argc, char **argv)
fprintf(stderr, "ERROR: can't list qgroups: %s\n",
strerror(e));
+ free(comparer_set);
+
return !!ret;
}
--
1.8.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 6/6] btrfs-progs: Avoid use pointer in handle_options
2015-10-29 9:31 [PATCH 1/6] btrfs-progs: fix floating point exception for btrfs-calc-size Zhao Lei
` (3 preceding siblings ...)
2015-10-29 9:31 ` [PATCH 5/6] btrfs-progs: free comparer_set in cmd_qgroup_show Zhao Lei
@ 2015-10-29 9:31 ` Zhao Lei
2015-10-29 13:15 ` David Sterba
2015-10-29 13:26 ` [PATCH 1/6] btrfs-progs: fix floating point exception for btrfs-calc-size David Sterba
5 siblings, 1 reply; 11+ messages in thread
From: Zhao Lei @ 2015-10-29 9:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: Zhao Lei
We use pointer of argc and argv in handle_options() because they
are necessary in very old code which are not exist now.
This patch move to use argc and argv directly in handle_options(),
alone with following update:
1: rename handle_options() to check_options()
to fit its function.
2: cleanup for condition in handle_options() to make line short.
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
btrfs.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/btrfs.c b/btrfs.c
index 9416a29..f881c18 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -172,20 +172,22 @@ static int cmd_version(int argc, char **argv)
return 0;
}
-static void handle_options(int *argc, char ***argv)
+static void check_options(int argc, char **argv)
{
- if (*argc > 0) {
- const char *arg = (*argv)[0];
- if (arg[0] != '-' ||
- !strcmp(arg, "--help") ||
- !strcmp(arg, "--version"))
- return;
- fprintf(stderr, "Unknown option: %s\n", arg);
- fprintf(stderr, "usage: %s\n",
- btrfs_cmd_group.usagestr[0]);
- exit(129);
- }
- return;
+ if (argc == 0)
+ return;
+
+ const char *arg = argv[0];
+
+ if (arg[0] != '-' ||
+ !strcmp(arg, "--help") ||
+ !strcmp(arg, "--version"))
+ return;
+
+ fprintf(stderr, "Unknown option: %s\n", arg);
+ fprintf(stderr, "usage: %s\n",
+ btrfs_cmd_group.usagestr[0]);
+ exit(129);
}
static const struct cmd_group btrfs_cmd_group = {
@@ -227,7 +229,7 @@ int main(int argc, char **argv)
} else {
argc--;
argv++;
- handle_options(&argc, &argv);
+ check_options(argc, argv);
if (argc > 0) {
if (!prefixcmp(argv[0], "--"))
argv[0] += 2;
--
1.8.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/6] btrfs-progs: free fslabel for btrfs-convert
2015-10-29 9:31 ` [PATCH 3/6] btrfs-progs: free fslabel for btrfs-convert Zhao Lei
@ 2015-10-29 13:08 ` David Sterba
0 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2015-10-29 13:08 UTC (permalink / raw)
To: Zhao Lei; +Cc: linux-btrfs
On Thu, Oct 29, 2015 at 05:31:45PM +0800, Zhao Lei wrote:
> fslabel need to be freed before exit.
>
> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> ---
> btrfs-convert.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/btrfs-convert.c b/btrfs-convert.c
> index 5b9171e..1693d03 100644
> --- a/btrfs-convert.c
> +++ b/btrfs-convert.c
> @@ -3027,6 +3027,9 @@ int main(int argc, char *argv[])
> ret = do_convert(file, datacsum, packing, noxattr, nodesize,
> copylabel, fslabel, progress, features);
> }
> +
> + free(fslabel);
fslabel is on stack:
btrfs-convert.c: In function 'main':
btrfs-convert.c:3031:6: warning: attempt to free a non-heap object 'fslabel' [-Wfree-nonheap-object]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 6/6] btrfs-progs: Avoid use pointer in handle_options
2015-10-29 9:31 ` [PATCH 6/6] btrfs-progs: Avoid use pointer in handle_options Zhao Lei
@ 2015-10-29 13:15 ` David Sterba
0 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2015-10-29 13:15 UTC (permalink / raw)
To: Zhao Lei; +Cc: linux-btrfs
On Thu, Oct 29, 2015 at 05:31:48PM +0800, Zhao Lei wrote:
> +static void check_options(int argc, char **argv)
> {
> + if (argc == 0)
> + return;
> +
> + const char *arg = argv[0];
Declaration after statements, fixed at commit time.
> +
> + if (arg[0] != '-' ||
> + !strcmp(arg, "--help") ||
> + !strcmp(arg, "--version"))
> + return;
> +
> + fprintf(stderr, "Unknown option: %s\n", arg);
> + fprintf(stderr, "usage: %s\n",
> + btrfs_cmd_group.usagestr[0]);
> + exit(129);
> }
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/6] btrfs-progs: fix floating point exception for btrfs-calc-size
2015-10-29 9:31 [PATCH 1/6] btrfs-progs: fix floating point exception for btrfs-calc-size Zhao Lei
` (4 preceding siblings ...)
2015-10-29 9:31 ` [PATCH 6/6] btrfs-progs: Avoid use pointer in handle_options Zhao Lei
@ 2015-10-29 13:26 ` David Sterba
5 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2015-10-29 13:26 UTC (permalink / raw)
To: Zhao Lei; +Cc: linux-btrfs
Hi,
paches 1,2,4,5,6 applied, thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/6] btrfs-progs: free comparer_set in cmd_qgroup_show
2015-10-29 9:31 ` [PATCH 5/6] btrfs-progs: free comparer_set in cmd_qgroup_show Zhao Lei
@ 2015-10-30 13:35 ` David Sterba
2015-11-02 1:16 ` Zhao Lei
0 siblings, 1 reply; 11+ messages in thread
From: David Sterba @ 2015-10-30 13:35 UTC (permalink / raw)
To: Zhao Lei; +Cc: linux-btrfs
On Thu, Oct 29, 2015 at 05:31:47PM +0800, Zhao Lei wrote:
> comparer_set, which was allocated by malloc(), should be free before
> function return.
>
> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> ---
> cmds-qgroup.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/cmds-qgroup.c b/cmds-qgroup.c
> index a64b716..f069d32 100644
> --- a/cmds-qgroup.c
> +++ b/cmds-qgroup.c
> @@ -290,7 +290,7 @@ static int cmd_qgroup_show(int argc, char **argv)
> int filter_flag = 0;
> unsigned unit_mode;
>
> - struct btrfs_qgroup_comparer_set *comparer_set;
> + struct btrfs_qgroup_comparer_set *comparer_set = NULL;
> struct btrfs_qgroup_filter_set *filter_set;
> filter_set = btrfs_qgroup_alloc_filter_set();
> comparer_set = btrfs_qgroup_alloc_comparer_set();
> @@ -372,6 +372,8 @@ static int cmd_qgroup_show(int argc, char **argv)
> fprintf(stderr, "ERROR: can't list qgroups: %s\n",
> strerror(e));
>
> + free(comparer_set);
Doh, coverity correctly found that comparer_set is freed inside
btrfs_show_qgroups() a few lines above. Patch dropped.
> +
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 5/6] btrfs-progs: free comparer_set in cmd_qgroup_show
2015-10-30 13:35 ` David Sterba
@ 2015-11-02 1:16 ` Zhao Lei
0 siblings, 0 replies; 11+ messages in thread
From: Zhao Lei @ 2015-11-02 1:16 UTC (permalink / raw)
To: dsterba; +Cc: linux-btrfs
Hi, David Sterba
> -----Original Message-----
> From: David Sterba [mailto:dsterba@suse.cz]
> Sent: Friday, October 30, 2015 9:36 PM
> To: Zhao Lei <zhaolei@cn.fujitsu.com>
> Cc: linux-btrfs@vger.kernel.org
> Subject: Re: [PATCH 5/6] btrfs-progs: free comparer_set in cmd_qgroup_show
>
> On Thu, Oct 29, 2015 at 05:31:47PM +0800, Zhao Lei wrote:
> > comparer_set, which was allocated by malloc(), should be free before
> > function return.
> >
> > Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> > ---
> > cmds-qgroup.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmds-qgroup.c b/cmds-qgroup.c index a64b716..f069d32
> > 100644
> > --- a/cmds-qgroup.c
> > +++ b/cmds-qgroup.c
> > @@ -290,7 +290,7 @@ static int cmd_qgroup_show(int argc, char **argv)
> > int filter_flag = 0;
> > unsigned unit_mode;
> >
> > - struct btrfs_qgroup_comparer_set *comparer_set;
> > + struct btrfs_qgroup_comparer_set *comparer_set = NULL;
> > struct btrfs_qgroup_filter_set *filter_set;
> > filter_set = btrfs_qgroup_alloc_filter_set();
> > comparer_set = btrfs_qgroup_alloc_comparer_set();
> > @@ -372,6 +372,8 @@ static int cmd_qgroup_show(int argc, char **argv)
> > fprintf(stderr, "ERROR: can't list qgroups: %s\n",
> > strerror(e));
> >
> > + free(comparer_set);
>
> Doh, coverity correctly found that comparer_set is freed inside
> btrfs_show_qgroups() a few lines above. Patch dropped.
>
My bad.
This problem is find in my node by valgrind memckeck, maybe it
is not freed in some case, or a valgrind misreport.
I'll check it deeply.
Thanks
Zhaolei
> > +
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-11-02 1:17 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-29 9:31 [PATCH 1/6] btrfs-progs: fix floating point exception for btrfs-calc-size Zhao Lei
2015-10-29 9:31 ` [PATCH 2/6] btrfs-progs: Fix negative eb's ref_cnt in btrfs-calc-size Zhao Lei
2015-10-29 9:31 ` [PATCH 3/6] btrfs-progs: free fslabel for btrfs-convert Zhao Lei
2015-10-29 13:08 ` David Sterba
2015-10-29 9:31 ` [PATCH 4/6] btrfs-progs: Fix uninitialized key.type for btrfs_find_free_objectid Zhao Lei
2015-10-29 9:31 ` [PATCH 5/6] btrfs-progs: free comparer_set in cmd_qgroup_show Zhao Lei
2015-10-30 13:35 ` David Sterba
2015-11-02 1:16 ` Zhao Lei
2015-10-29 9:31 ` [PATCH 6/6] btrfs-progs: Avoid use pointer in handle_options Zhao Lei
2015-10-29 13:15 ` David Sterba
2015-10-29 13:26 ` [PATCH 1/6] btrfs-progs: fix floating point exception for btrfs-calc-size 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.