Re: [PATCH 2/5] btrfs: Refactor btrfs_can_relocate
On 10/26/2018 07:43 PM, Nikolay Borisov wrote:
btrfs_can_relocate returns 0 when it concludes the given chunk can be
relocated and -1 otherwise. Since this function is used as a predicated
and it return a binary value it makes no sense to have it's return
value as an int so change it to bool. Furthermore, remove a stale
leftover comment from e6ec716f0ddb ("Btrfs: make raid attr array more
readable").
Finally make the logic in the list_for_each_entry loop a bit more obvious. Up
until now the fact that we are returning success (0) was a bit masked by the
fact that the 0 is re-used from the return value of find_free_dev_extent.
Instead, now just increment dev_nr if we find a suitable extent and explicitly
set can_reloc to true if enough devices with unallocated space are present.
No functional changes.
Signed-off-by: Nikolay Borisov
---
fs/btrfs/ctree.h | 2 +-
fs/btrfs/extent-tree.c | 39 ++-
fs/btrfs/volumes.c | 3 +--
3 files changed, 16 insertions(+), 28 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 68ca41dbbef3..06edc4f9ceb2 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2680,7 +2680,7 @@ int btrfs_setup_space_cache(struct btrfs_trans_handle
*trans,
int btrfs_extent_readonly(struct btrfs_fs_info *fs_info, u64 bytenr);
int btrfs_free_block_groups(struct btrfs_fs_info *info);
int btrfs_read_block_groups(struct btrfs_fs_info *info);
-int btrfs_can_relocate(struct btrfs_fs_info *fs_info, u64 bytenr);
+bool btrfs_can_relocate(struct btrfs_fs_info *fs_info, u64 bytenr);
int btrfs_make_block_group(struct btrfs_trans_handle *trans,
u64 bytes_used, u64 type, u64 chunk_offset,
u64 size);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index a1febf155747..816bca482358 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -9423,10 +9423,10 @@ void btrfs_dec_block_group_ro(struct
btrfs_block_group_cache *cache)
/*
* checks to see if its even possible to relocate this block group.
*
- * @return - -1 if it's not a good idea to relocate this block group, 0 if its
- * ok to go ahead and try.
+ * @return - false if not enough space can be found for relocation, true
+ * otherwise
*/
-int btrfs_can_relocate(struct btrfs_fs_info *fs_info, u64 bytenr)
+bool btrfs_can_relocate(struct btrfs_fs_info *fs_info, u64 bytenr)
{
struct btrfs_root *root = fs_info->extent_root;
struct btrfs_block_group_cache *block_group;
@@ -9441,7 +9441,7 @@ int btrfs_can_relocate(struct btrfs_fs_info *fs_info, u64
bytenr)
int debug;
int index;
int full = 0;
- int ret = 0;
+ bool can_reloc = true;
debug = btrfs_test_opt(fs_info, ENOSPC_DEBUG);
@@ -9453,7 +9453,7 @@ int btrfs_can_relocate(struct btrfs_fs_info *fs_info, u64 bytenr)
btrfs_warn(fs_info,
"can't find block group for bytenr %llu",
bytenr);
- return -1;
+ return false;
}
min_free = btrfs_block_group_used(_group->item);
@@ -9489,16 +9489,8 @@ int btrfs_can_relocate(struct btrfs_fs_info *fs_info,
u64 bytenr)
* group is going to be restriped, run checks against the target
* profile instead of the current one.
*/
- ret = -1;
+ can_reloc = false;
- /*
-* index:
-* 0: raid10
-* 1: raid1
-* 2: dup
-* 3: raid0
-* 4: single
-*/
target = get_restripe_target(fs_info, block_group->flags);
if (target) {
index = btrfs_bg_flags_to_raid_index(extended_to_chunk(target));
@@ -9534,10 +9526,8 @@ int btrfs_can_relocate(struct btrfs_fs_info *fs_info,
u64 bytenr)
/* We need to do this so that we can look at pending chunks */
trans = btrfs_join_transaction(root);
- if (IS_ERR(trans)) {
- ret = PTR_ERR(trans);
+ if (IS_ERR(trans))
goto out;
- }
mutex_lock(_info->chunk_mutex);
list_for_each_entry(device, _devices->alloc_list, dev_alloc_list) {
@@ -9549,18 +9539,17 @@ int btrfs_can_relocate(struct btrfs_fs_info *fs_info,
u64 bytenr)
*/
if (device->total_bytes > device->bytes_used + min_free &&
!test_bit(BTRFS_DEV_STATE_REPLACE_TGT, >dev_state))
{
- ret = find_free_dev_extent(trans, device, min_free,
- _offset, NULL);
- if (!ret)
+ if (!find_free_dev_extent(trans, device, min_free,
+ _offset, NULL))
This can return -ENOMEM;
dev_nr++;
- if (dev_nr >= dev_min)
+ if (dev_nr >= dev_min) {
+
Re: [PATCH 5/5] btrfs: Replace BUG_ON with ASSERT in find_lock_delalloc_range
On 10/26/2018 07:43 PM, Nikolay Borisov wrote:
lock_delalloc_pages should only return 2 values - 0 in case of success
and -EAGAIN if the range of pages to be locked should be shrunk due to
some of gone. Manual inspections confirms that this is
indeed the case since __process_pages_contig is where lock_delalloc_pages
gets its return value. The latter always returns 0 or -EAGAIN so the
invariant holds. No functional changes.
Signed-off-by: Nikolay Borisov
Reviewed-by: Anand Jain
Thanks, Anand
---
fs/btrfs/extent_io.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 1a9a521aefe5..94bc53472031 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1606,6 +1606,7 @@ static noinline_for_stack u64
find_lock_delalloc_range(struct inode *inode,
/* step two, lock all the pages after the page that has start */
ret = lock_delalloc_pages(inode, locked_page,
delalloc_start, delalloc_end);
+ ASSERT(!ret || ret == -EAGAIN);
if (ret == -EAGAIN) {
/* some of the pages are gone, lets avoid looping by
* shortening the size of the delalloc range we're searching
@@ -1621,7 +1622,6 @@ static noinline_for_stack u64
find_lock_delalloc_range(struct inode *inode,
goto out_failed;
}
}
- BUG_ON(ret); /* Only valid values are 0 and -EAGAIN */
/* step three, lock the state bits for the whole range */
lock_extent_bits(tree, delalloc_start, delalloc_end, _state);
Re: [PATCH 4/5] btrfs: Sink find_lock_delalloc_range's 'max_bytes' argument
On 10/26/2018 07:43 PM, Nikolay Borisov wrote:
All callers of this function pass BTRFS_MAX_EXTENT_SIZE (128M) so let's
reduce the argument count and make that a local variable. No functional
changes.
Signed-off-by: Nikolay Borisov
Reviewed-by: Anand Jain
Thanks, Anand
---
fs/btrfs/extent_io.c | 11 +--
fs/btrfs/extent_io.h | 2 +-
fs/btrfs/tests/extent-io-tests.c | 10 +-
3 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 6877a74c7469..1a9a521aefe5 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1566,8 +1566,9 @@ static noinline int lock_delalloc_pages(struct inode
*inode,
static noinline_for_stack u64 find_lock_delalloc_range(struct inode *inode,
struct extent_io_tree *tree,
struct page *locked_page, u64 *start,
- u64 *end, u64 max_bytes)
+ u64 *end)
{
+ u64 max_bytes = BTRFS_MAX_EXTENT_SIZE;
u64 delalloc_start;
u64 delalloc_end;
u64 found;
@@ -1647,10 +1648,9 @@ static noinline_for_stack u64
find_lock_delalloc_range(struct inode *inode,
u64 btrfs_find_lock_delalloc_range(struct inode *inode,
struct extent_io_tree *tree,
struct page *locked_page, u64 *start,
- u64 *end, u64 max_bytes)
+ u64 *end)
{
- return find_lock_delalloc_range(inode, tree, locked_page, start, end,
- max_bytes);
+ return find_lock_delalloc_range(inode, tree, locked_page, start, end);
}
#endif
@@ -3233,8 +3233,7 @@ static noinline_for_stack int writepage_delalloc(struct inode *inode,
nr_delalloc = find_lock_delalloc_range(inode, tree,
page,
_start,
- _end,
- BTRFS_MAX_EXTENT_SIZE);
+ _end);
if (nr_delalloc == 0) {
delalloc_start = delalloc_end + 1;
continue;
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 369daa5d4f73..7ec4f93caf78 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -549,7 +549,7 @@ int free_io_failure(struct extent_io_tree *failure_tree,
u64 btrfs_find_lock_delalloc_range(struct inode *inode,
struct extent_io_tree *tree,
struct page *locked_page, u64 *start,
- u64 *end, u64 max_bytes);
+ u64 *end);
#endif
struct extent_buffer *alloc_test_extent_buffer(struct btrfs_fs_info *fs_info,
u64 start);
diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index 9e0f4a01be14..8ea8c2aa6696 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -107,7 +107,7 @@ static int test_find_delalloc(u32 sectorsize)
start = 0;
end = 0;
found = btrfs_find_lock_delalloc_range(inode, , locked_page, ,
-, max_bytes);
+);
if (!found) {
test_err("should have found at least one delalloc");
goto out_bits;
@@ -138,7 +138,7 @@ static int test_find_delalloc(u32 sectorsize)
start = test_start;
end = 0;
found = btrfs_find_lock_delalloc_range(inode, , locked_page, ,
-, max_bytes);
+);
if (!found) {
test_err("couldn't find delalloc in our range");
goto out_bits;
@@ -172,7 +172,7 @@ static int test_find_delalloc(u32 sectorsize)
start = test_start;
end = 0;
found = btrfs_find_lock_delalloc_range(inode, , locked_page, ,
-, max_bytes);
+);
if (found) {
test_err("found range when we shouldn't have");
goto out_bits;
@@ -193,7 +193,7 @@ static int test_find_delalloc(u32 sectorsize)
start = test_start;
end = 0;
found = btrfs_find_lock_delalloc_range(inode, , locked_page, ,
-, max_bytes);
+);
if (!found) {
test_err("didn't find our range");
goto out_bits;
@@ -234,7 +234,7 @@ static int test_find_delalloc(u32 sectorsize)
* tests expected behavior.
*/
found =
Re: BTRFS on production: NVR 16+ IP Cameras
On Thu, Nov 15, 2018 at 10:39 AM Juan Alberto Cirez wrote: > > Is BTRFS mature enough to be deployed on a production system to underpin > the storage layer of a 16+ ipcameras-based NVR (or VMS if you prefer)? > > Based on our limited experience with BTRFS (1+ year) under the above > scenario the answer seems to be no; but I wanted you ask the community > at large for their experience before making a final decision to hold off > on deploying BTRFS on production systems. > > Let us be clear: We think BTRFS has great potential, and as it matures > we will continue to watch its progress, so that at some future point we > can return to using it. > > The issue has been the myriad of problems we have encountered when > deploying BTRFS as the storage fs for the NVR/VMS in cases were the > camera count exceeds 10: Corrupted file systems, sudden read-only file > system, re-balance kernel panics, broken partitions, etc. Performance problems are separate from reliability problems. No matter what, there shouldn't be corruptions or failures when your process is writing through the Btrfs kernel driver. Period. So you've either got significant hardware/firmware problems as the root cause, or your use case is exposing Btrfs bugs. But you're burdened with providing sufficient details about the hardware and storage stack configuration including kernel, btrfs-progs versions and mkfs options and mount options being used. Without a way for a developer to reproduce your problem it's unlikely the source of the problem can be discovered and fixed. > So, again, the question is: is BTRFS mature enough to be used in such > use case and if so, what approach can be used to mitigate such issues. What format are the cameras writing out in? It matters if this is a continuous appending format, or if it's writing them out as individual JPEG files, one per frame, or whatever. What rate, what size, and any other concurrent operations, etc. -- Chris Murphy
Re: [PATCH 3/5] btrfs: Remove superfluous check form btrfs_remove_chunk
On 10/26/2018 07:43 PM, Nikolay Borisov wrote:
It's unnecessary to check map->stripes[i].dev for NULL given its value
is already set and dereferenced above the the check. No functional changes.
Signed-off-by: Nikolay Borisov
Reviewed-by: Anand Jain
Thanks, Anand
---
fs/btrfs/volumes.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index dc53d94a62aa..f0db43d08456 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2797,13 +2797,11 @@ int btrfs_remove_chunk(struct btrfs_trans_handle
*trans, u64 chunk_offset)
mutex_unlock(_info->chunk_mutex);
}
- if (map->stripes[i].dev) {
- ret = btrfs_update_device(trans, map->stripes[i].dev);
- if (ret) {
- mutex_unlock(_devices->device_list_mutex);
- btrfs_abort_transaction(trans, ret);
- goto out;
- }
+ ret = btrfs_update_device(trans, device);
+ if (ret) {
+ mutex_unlock(_devices->device_list_mutex);
+ btrfs_abort_transaction(trans, ret);
+ goto out;
}
}
mutex_unlock(_devices->device_list_mutex);
Re: [PATCH 7/9] btrfs: quiten warn if the replace is canceled at finish
On Fri, Nov 16, 2018 at 08:06:36PM +0800, Anand Jain wrote:
>
>
> On 11/15/2018 11:35 PM, David Sterba wrote:
> > On Sun, Nov 11, 2018 at 10:22:22PM +0800, Anand Jain wrote:
> >> When we successfully cancel the replace its scrub returns -ECANCELED,
> >> which then passed to btrfs_dev_replace_finishing(), it cleans up based
> >> on the scrub returned status and propagates the same -ECANCELED back
> >> the parent function. As of now only user can cancel the replace-scrub,
> >> so its ok to quieten the warn here.
> >>
> >> Signed-off-by: Anand Jain
> >> ---
> >> fs/btrfs/dev-replace.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> >> index 1dc8e86546db..9031a362921a 100644
> >> --- a/fs/btrfs/dev-replace.c
> >> +++ b/fs/btrfs/dev-replace.c
> >> @@ -497,7 +497,7 @@ static int btrfs_dev_replace_start(struct
> >> btrfs_fs_info *fs_info,
> >>ret = btrfs_dev_replace_finishing(fs_info, ret);
> >>if (ret == -EINPROGRESS) {
> >>ret = BTRFS_IOCTL_DEV_REPLACE_RESULT_SCRUB_INPROGRESS;
> >> - } else {
> >> + } else if (ret != -ECANCELED) {
> >>WARN_ON(ret);
> >
> > While this looks ok, can you please rework it so there are no WARN_ON at
> > random places in device-replace, poorly substituting error handling?
> >
> > The code flow in this case could be changed to make explicit checks for
> > the know codes and then a catch-all branch like:
> >
> > if (ret == -EINPROGRESS) {
> > ...
> > } else (if == -ESOMETHINGELSE) {
> > ...
> > } else {
> > unknown error, print error and do a proper cleanup
> > }
> >
>
> As below..
>
> >>}
> >>
> >> @@ -954,7 +954,7 @@ static int btrfs_dev_replace_kthread(void *data)
> >> btrfs_device_get_total_bytes(dev_replace->srcdev),
> >> _replace->scrub_progress, 0, 1);
> >>ret = btrfs_dev_replace_finishing(fs_info, ret);
> >> - WARN_ON(ret);
> >> + WARN_ON(ret && ret != -ECANCELED);
> >
> > This one too, thanks.
>
>
> btrfs_dev_scrub() can return quite a lot of errno, which is passed
> here through the btrfs_dev_replace_finishing(), so it won't be
> possible to code them all.
>
> (we use -ECANCELED only in replace and balance).
I see, filtering out only the replace error codes makes more sense.
Re: bad tree block start, want 705757184 have 82362368
On 16.11.18 г. 18:17 ч., Stephan Olbrich wrote: > Hi, > > a few days ago my root file system (simple btrfs on a SSD, no RAID or > anything) suddenly became read only. > Looking at dmsg, I found this: > > [ 19.285020] BTRFS error (device sda2): bad tree block start, want > 705757184 have 82362368 > [ 19.285042] BTRFS: error (device sda2) in __btrfs_free_extent:6804: > errno=-5 IO failure > [ 19.285048] BTRFS info (device sda2): forced readonly > [ 19.285051] BTRFS: error (device sda2) in btrfs_run_delayed_refs:2934: > errno=-5 IO failure > [ 19.287213] BTRFS error (device sda2): pending csums is 41889792 > > Late on I got the same errors for my /home partition (on the same drive) as > well. > I have snapshots of all partitions on another drive made by btrbk. To get a > working system, I made new (rw) snapshots > of the most recent backup and setup grub and fstab, so my system would boot > from the other drive. > Unfortunately now I got the "bad tree block start" error again at least once > in dmesg but I didn't save it and it's not in syslog :-( > What I remember is, that it was followed by other btrfs error messages saying > something about correcting something. > And the filesystem was still read/write this time. > At the moment I can't reproduce it. > > Is there any way to find out, which files are affected by the errors above? I > don't really trust the data on the drive I'm using at the > moment, as it has shown errors as well, but I have a less current backup on > yet another drive but at it is a few weeks old, I don't > want to use it to setup the system on the SSD again, but just copy the > relevant files if possible. > Or is it possible to repair the original file system? > > Some information about my system: > Kubuntu 18.04 > Kernel 4.19.1 when the problem occured, now 4.19.2 > btrfs-tools 4.15.1 What is the SMART status of your SSD, how old is the ssd. This really sounds like the drive going to lalal land. > > Regards, > Stephan > > > > > >
bad tree block start, want 705757184 have 82362368
Hi, a few days ago my root file system (simple btrfs on a SSD, no RAID or anything) suddenly became read only. Looking at dmsg, I found this: [ 19.285020] BTRFS error (device sda2): bad tree block start, want 705757184 have 82362368 [ 19.285042] BTRFS: error (device sda2) in __btrfs_free_extent:6804: errno=-5 IO failure [ 19.285048] BTRFS info (device sda2): forced readonly [ 19.285051] BTRFS: error (device sda2) in btrfs_run_delayed_refs:2934: errno=-5 IO failure [ 19.287213] BTRFS error (device sda2): pending csums is 41889792 Late on I got the same errors for my /home partition (on the same drive) as well. I have snapshots of all partitions on another drive made by btrbk. To get a working system, I made new (rw) snapshots of the most recent backup and setup grub and fstab, so my system would boot from the other drive. Unfortunately now I got the "bad tree block start" error again at least once in dmesg but I didn't save it and it's not in syslog :-( What I remember is, that it was followed by other btrfs error messages saying something about correcting something. And the filesystem was still read/write this time. At the moment I can't reproduce it. Is there any way to find out, which files are affected by the errors above? I don't really trust the data on the drive I'm using at the moment, as it has shown errors as well, but I have a less current backup on yet another drive but at it is a few weeks old, I don't want to use it to setup the system on the SSD again, but just copy the relevant files if possible. Or is it possible to repair the original file system? Some information about my system: Kubuntu 18.04 Kernel 4.19.1 when the problem occured, now 4.19.2 btrfs-tools 4.15.1 Regards, Stephan
Re: [PATCH 0/5] Misc cleanups in balance code
On 16.11.18 г. 17:18 ч., David Sterba wrote: > On Fri, Oct 26, 2018 at 02:43:16PM +0300, Nikolay Borisov wrote: >> While investigating the balance hang I came across various inconsistencies >> in >> the source. This series aims to fix those. >> >> The first patch is (I believe) a fix to a longstanding bug that could cause >> balance to fail due to ENOSPC. The code no properly ensures that there is >> at least 1g of unallocated space on every device being balance. >> >> Patch 2 makes btrfs_can_relocate a bit more obvious and removes leftovers >> from >> previous cleanup >> >> Patches 3/4/5 remove some redundant code from various functions. >> >> This series has survived multiple xfstest runs. >> >> Nikolay Borisov (5): >> btrfs: Ensure at least 1g is free for balance >> btrfs: Refactor btrfs_can_relocate >> btrfs: Remove superfluous check form btrfs_remove_chunk >> btrfs: Sink find_lock_delalloc_range's 'max_bytes' argument >> btrfs: Replace BUG_ON with ASSERT in find_lock_delalloc_range > > Patches 2-5 on the way to misc-next, thanks. The first one can have user > visible consequences, so I'd rather first find out why the 1MB was there > and if it's really a bug and what exactly will change when it's 1G. I'm fine with that, one thing I don't agree with, though, is the conclusion that patch 1 has user visible consequence. As a matter of fact it does not. If btrfs_shrink_device fails with ENOSPC we just break, i/e we don't try to free balance space for any of the other devices, but this doesn't stop from balance actually continuing. As it stands today I think "step one" in __btrfs_balance is more or less null-op, i.e cycles are wasted since all we do is shrink every device by 1mb, ensuring one more mb is already free. I think this is very insufficient for a balance operation and when it succeeds during normal operation it's due to the device already having enough unallocated space before the balance. Anyway, I will speak with Chris to try and find out why the code uses 1mb >
Re: [PATCH] Btrfs: allow clear_extent_dirty() to receive a cached extent state record
On 16.11.18 г. 15:04 ч., fdman...@kernel.org wrote:
> From: Filipe Manana
>
> We can have a lot freed extents during the life span of transaction, so
> the red black tree that keeps track of the ranges of each freed extent
> (fs_info->freed_extents[]) can get quite big. When finishing a transaction
> commit we find each range, process it (discard the extents, unpin them)
> and then remove it from the red black tree. We can use an extent state
> record as a cache when searching for a range, so that when we clean the
> range we can use the cached extent state we passed to the search function
> instead of iterating the red black tree again. Doing things as fast as
> possible when finishing a transaction (in state TRANS_STATE_UNBLOCKED)
> is convenient as it reduces the time we block another task that wants to
> commit the next transaction.
>
> So change clear_extent_dirty() to allow an optional extent state record to
> be passed as an argument, which will be passed down to __clear_extent_bit.
>
> Signed-off-by: Filipe Manana
Reviewed-by: Nikolay Borisov
> ---
> fs/btrfs/disk-io.c | 7 +--
> fs/btrfs/extent-tree.c | 7 +--
> fs/btrfs/extent_io.h | 4 ++--
> 3 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 05dc3c17cb62..ecf3a45490e3 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -4359,12 +4359,15 @@ static int btrfs_destroy_pinned_extent(struct
> btrfs_fs_info *fs_info,
> unpin = pinned_extents;
> again:
> while (1) {
> + struct extent_state *cached_state = NULL;
> +
> ret = find_first_extent_bit(unpin, 0, , ,
> - EXTENT_DIRTY, NULL);
> + EXTENT_DIRTY, _state);
> if (ret)
> break;
>
> - clear_extent_dirty(unpin, start, end);
> + clear_extent_dirty(unpin, start, end, _state);
> + free_extent_state(cached_state);
> btrfs_error_unpin_extent_range(fs_info, start, end);
> cond_resched();
> }
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 577878324799..33142d9c36d5 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -6615,9 +6615,11 @@ int btrfs_finish_extent_commit(struct
> btrfs_trans_handle *trans)
> unpin = _info->freed_extents[0];
>
> while (!trans->aborted) {
> + struct extent_state *cached_state = NULL;
> +
> mutex_lock(_info->unused_bg_unpin_mutex);
> ret = find_first_extent_bit(unpin, 0, , ,
> - EXTENT_DIRTY, NULL);
> + EXTENT_DIRTY, _state);
> if (ret) {
> mutex_unlock(_info->unused_bg_unpin_mutex);
> break;
> @@ -6627,9 +6629,10 @@ int btrfs_finish_extent_commit(struct
> btrfs_trans_handle *trans)
> ret = btrfs_discard_extent(fs_info, start,
> end + 1 - start, NULL);
>
> - clear_extent_dirty(unpin, start, end);
> + clear_extent_dirty(unpin, start, end, _state);
> unpin_extent_range(fs_info, start, end, true);
> mutex_unlock(_info->unused_bg_unpin_mutex);
> + free_extent_state(cached_state);
> cond_resched();
> }
>
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index b4d03e677e1d..36f7a9f87e46 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -353,11 +353,11 @@ static inline int set_extent_dirty(struct
> extent_io_tree *tree, u64 start,
> }
>
> static inline int clear_extent_dirty(struct extent_io_tree *tree, u64 start,
> - u64 end)
> + u64 end, struct extent_state **cached)
> {
> return clear_extent_bit(tree, start, end,
> EXTENT_DIRTY | EXTENT_DELALLOC |
> - EXTENT_DO_ACCOUNTING, 0, 0, NULL);
> + EXTENT_DO_ACCOUNTING, 0, 0, cached);
> }
>
> int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>
Re: [PATCH 0/5] Misc cleanups in balance code
On Fri, Oct 26, 2018 at 02:43:16PM +0300, Nikolay Borisov wrote: > While investigating the balance hang I came across various inconsistencies in > the source. This series aims to fix those. > > The first patch is (I believe) a fix to a longstanding bug that could cause > balance to fail due to ENOSPC. The code no properly ensures that there is > at least 1g of unallocated space on every device being balance. > > Patch 2 makes btrfs_can_relocate a bit more obvious and removes leftovers > from > previous cleanup > > Patches 3/4/5 remove some redundant code from various functions. > > This series has survived multiple xfstest runs. > > Nikolay Borisov (5): > btrfs: Ensure at least 1g is free for balance > btrfs: Refactor btrfs_can_relocate > btrfs: Remove superfluous check form btrfs_remove_chunk > btrfs: Sink find_lock_delalloc_range's 'max_bytes' argument > btrfs: Replace BUG_ON with ASSERT in find_lock_delalloc_range Patches 2-5 on the way to misc-next, thanks. The first one can have user visible consequences, so I'd rather first find out why the 1MB was there and if it's really a bug and what exactly will change when it's 1G.
Re: BTRFS on production: NVR 16+ IP Cameras
On 11/16/2018 03:51 AM, Nikolay Borisov wrote: On 15.11.18 г. 20:39 ч., Juan Alberto Cirez wrote: Is BTRFS mature enough to be deployed on a production system to underpin the storage layer of a 16+ ipcameras-based NVR (or VMS if you prefer)? Based on our limited experience with BTRFS (1+ year) under the above scenario the answer seems to be no; but I wanted you ask the community at large for their experience before making a final decision to hold off on deploying BTRFS on production systems. Let us be clear: We think BTRFS has great potential, and as it matures we will continue to watch its progress, so that at some future point we can return to using it. The issue has been the myriad of problems we have encountered when deploying BTRFS as the storage fs for the NVR/VMS in cases were the camera count exceeds 10: Corrupted file systems, sudden read-only file system, re-balance kernel panics, broken partitions, etc. What version of the file system, how many of those issues (if any) have you reported to the upstream mailing list? I don't remember seeing any reports from you. One specific case saw the btrfs drive pool mounted under the /var partition so that upon installation the btrfs pool contained all files under /var; /lib/mysql as well as the video storage. Needless to say this was a catastrophe... BTRFS is not suitable for random workloads, such as databases for example. In those cases it's recommended, at the very least, to disable COW on the database files. Note, disabling CoW doesn't preclude you from creating snapshots, it just means that not every 4k write (for example) will result in a CoW operation. At the other end of the spectrum is a use case where ONLY the video storage was on the btrfs pool; but in that case, the btrfs pool became read-only suddenly and would not re-mount as rw despite all the recovery trick btrfs-tools could throw at it. This, of course prevented the NVR/VMS from recording any footage. Again, you give absolutely no information about how you have configured the filesystem or versions of the software used. So, again, the question is: is BTRFS mature enough to be used in such use case and if so, what approach can be used to mitigate such issues. And.. What is the blocksize used by the application to write into btrfs? And what type of write IO? async, directio or fsync? Thanks, Anand Thank you all for your assistance -Ryan-
[PATCH] Btrfs: allow clear_extent_dirty() to receive a cached extent state record
From: Filipe Manana
We can have a lot freed extents during the life span of transaction, so
the red black tree that keeps track of the ranges of each freed extent
(fs_info->freed_extents[]) can get quite big. When finishing a transaction
commit we find each range, process it (discard the extents, unpin them)
and then remove it from the red black tree. We can use an extent state
record as a cache when searching for a range, so that when we clean the
range we can use the cached extent state we passed to the search function
instead of iterating the red black tree again. Doing things as fast as
possible when finishing a transaction (in state TRANS_STATE_UNBLOCKED)
is convenient as it reduces the time we block another task that wants to
commit the next transaction.
So change clear_extent_dirty() to allow an optional extent state record to
be passed as an argument, which will be passed down to __clear_extent_bit.
Signed-off-by: Filipe Manana
---
fs/btrfs/disk-io.c | 7 +--
fs/btrfs/extent-tree.c | 7 +--
fs/btrfs/extent_io.h | 4 ++--
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 05dc3c17cb62..ecf3a45490e3 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4359,12 +4359,15 @@ static int btrfs_destroy_pinned_extent(struct
btrfs_fs_info *fs_info,
unpin = pinned_extents;
again:
while (1) {
+ struct extent_state *cached_state = NULL;
+
ret = find_first_extent_bit(unpin, 0, , ,
- EXTENT_DIRTY, NULL);
+ EXTENT_DIRTY, _state);
if (ret)
break;
- clear_extent_dirty(unpin, start, end);
+ clear_extent_dirty(unpin, start, end, _state);
+ free_extent_state(cached_state);
btrfs_error_unpin_extent_range(fs_info, start, end);
cond_resched();
}
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 577878324799..33142d9c36d5 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -6615,9 +6615,11 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle
*trans)
unpin = _info->freed_extents[0];
while (!trans->aborted) {
+ struct extent_state *cached_state = NULL;
+
mutex_lock(_info->unused_bg_unpin_mutex);
ret = find_first_extent_bit(unpin, 0, , ,
- EXTENT_DIRTY, NULL);
+ EXTENT_DIRTY, _state);
if (ret) {
mutex_unlock(_info->unused_bg_unpin_mutex);
break;
@@ -6627,9 +6629,10 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle
*trans)
ret = btrfs_discard_extent(fs_info, start,
end + 1 - start, NULL);
- clear_extent_dirty(unpin, start, end);
+ clear_extent_dirty(unpin, start, end, _state);
unpin_extent_range(fs_info, start, end, true);
mutex_unlock(_info->unused_bg_unpin_mutex);
+ free_extent_state(cached_state);
cond_resched();
}
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index b4d03e677e1d..36f7a9f87e46 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -353,11 +353,11 @@ static inline int set_extent_dirty(struct extent_io_tree
*tree, u64 start,
}
static inline int clear_extent_dirty(struct extent_io_tree *tree, u64 start,
- u64 end)
+u64 end, struct extent_state **cached)
{
return clear_extent_bit(tree, start, end,
EXTENT_DIRTY | EXTENT_DELALLOC |
- EXTENT_DO_ACCOUNTING, 0, 0, NULL);
+ EXTENT_DO_ACCOUNTING, 0, 0, cached);
}
int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
--
2.11.0
Re: BTRFS on production: NVR 16+ IP Cameras
On 2018-11-15 13:39, Juan Alberto Cirez wrote: Is BTRFS mature enough to be deployed on a production system to underpin the storage layer of a 16+ ipcameras-based NVR (or VMS if you prefer)? For NVR, I'd say no. BTRFS does pretty horribly with append-only workloads, even if they are WORM style. It also does a really bad job with most relational database systems that you would likely use for indexing. If you can suggest your reasoning for wanting to use BTRFS though, I can probably point you at alternatives that would work more reliably for your use case.
Re: [PATCH 7/9] btrfs: quiten warn if the replace is canceled at finish
On 11/15/2018 11:35 PM, David Sterba wrote:
On Sun, Nov 11, 2018 at 10:22:22PM +0800, Anand Jain wrote:
When we successfully cancel the replace its scrub returns -ECANCELED,
which then passed to btrfs_dev_replace_finishing(), it cleans up based
on the scrub returned status and propagates the same -ECANCELED back
the parent function. As of now only user can cancel the replace-scrub,
so its ok to quieten the warn here.
Signed-off-by: Anand Jain
---
fs/btrfs/dev-replace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 1dc8e86546db..9031a362921a 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -497,7 +497,7 @@ static int btrfs_dev_replace_start(struct btrfs_fs_info
*fs_info,
ret = btrfs_dev_replace_finishing(fs_info, ret);
if (ret == -EINPROGRESS) {
ret = BTRFS_IOCTL_DEV_REPLACE_RESULT_SCRUB_INPROGRESS;
- } else {
+ } else if (ret != -ECANCELED) {
WARN_ON(ret);
While this looks ok, can you please rework it so there are no WARN_ON at
random places in device-replace, poorly substituting error handling?
The code flow in this case could be changed to make explicit checks for
the know codes and then a catch-all branch like:
if (ret == -EINPROGRESS) {
...
} else (if == -ESOMETHINGELSE) {
...
} else {
unknown error, print error and do a proper cleanup
}
As below..
}
@@ -954,7 +954,7 @@ static int btrfs_dev_replace_kthread(void *data)
btrfs_device_get_total_bytes(dev_replace->srcdev),
_replace->scrub_progress, 0, 1);
ret = btrfs_dev_replace_finishing(fs_info, ret);
- WARN_ON(ret);
+ WARN_ON(ret && ret != -ECANCELED);
This one too, thanks.
btrfs_dev_scrub() can return quite a lot of errno, which is passed
here through the btrfs_dev_replace_finishing(), so it won't be
possible to code them all.
(we use -ECANCELED only in replace and balance).
Thanks, Anand
[PATCH] Btrfs: bring back key search optimization to btrfs_search_old_slot()
From: Filipe Manana
Commit d7396f07358a ("Btrfs: optimize key searches in btrfs_search_slot"),
dated from August 2013, introduced an optimization to search for keys in a
node/leaf to both btrfs_search_slot() and btrfs_search_old_slot(). For the
later, it ended up being reverted in commit d4b4087c43cc ("Btrfs: do a
full search everytime in btrfs_search_old_slot"), from September 2013,
because the content of extent buffers were often inconsistent during
replay. It turned out that the reason why they were often inconsistent was
because the extent buffer replay stopped being done atomically, and got
broken after commit c8cc63416537 ("Btrfs: stop using GFP_ATOMIC for the
tree mod log allocations"), introduced in July 2013. The extent buffer
replay issue was then found and fixed by commit 5de865eebb83 ("Btrfs: fix
tree mod logging"), dated from December 2013.
So bring back the optimization to btrfs_search_old_slot() as skipping it
and its comment about disabling it confusing. After all, if unwinding
extent buffers resulted in some inconsistency, the normal searches (binary
searches) would also not always work.
Signed-off-by: Filipe Manana
---
fs/btrfs/ctree.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 089b46c4d97f..cf5487a79c96 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2966,7 +2966,7 @@ int btrfs_search_old_slot(struct btrfs_root *root, const
struct btrfs_key *key,
int level;
int lowest_unlock = 1;
u8 lowest_level = 0;
- int prev_cmp = -1;
+ int prev_cmp;
lowest_level = p->lowest_level;
WARN_ON(p->nodes[0] != NULL);
@@ -2977,6 +2977,7 @@ int btrfs_search_old_slot(struct btrfs_root *root, const
struct btrfs_key *key,
}
again:
+ prev_cmp = -1;
b = get_old_root(root, time_seq);
level = btrfs_header_level(b);
p->locks[level] = BTRFS_READ_LOCK;
@@ -2994,11 +2995,6 @@ int btrfs_search_old_slot(struct btrfs_root *root, const
struct btrfs_key *key,
*/
btrfs_unlock_up_safe(p, level + 1);
- /*
-* Since we can unwind ebs we want to do a real search every
-* time.
-*/
- prev_cmp = -1;
ret = key_search(b, key, level, _cmp, );
if (level != 0) {
--
2.11.0
Re: [PATCH 8/9] btrfs: user requsted replace cancel is not an error
On 11/16/2018 06:29 PM, Anand Jain wrote:
On 11/15/2018 11:31 PM, David Sterba wrote:
On Sun, Nov 11, 2018 at 10:22:23PM +0800, Anand Jain wrote:
As of now only user requested replace cancel can cancel the
replace-scrub
so no need to log error for it.
This has probably some user visible effect or threre are steps to
reproduce the message even if it's not expected (ie. the case that this
patch fixes). Please update the changelog, thanks.
before the patch [1]
[1]
btrfs: fix use-after-free due to race between replace start and cancel
We used to set the replace_state to CANCELED [2] and then call
btrfs_scrub_cancel(), but the problem with this approach is
if the scrub isn't running yet, then things get messier.
[2]
- dev_replace->replace_state =
BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED;
So to fix, [1] shall set replace_state to CANCELED only after the
replace cancel thread has successfully canceled the replace using
btrfs_scrub_cancel().
And about the cleanup process for the replace target..
we call
btrfs_dev_replace_finishing(.. ret)
after
ret= btrfs_scrub_dev()
now ret is -ECANCELED due to replace cancel request by the user.
(its not set to -ECANCELED for any other reason).
btrfs_dev_replace_finishing() has the following code [3] which it
earlier blocked processing of the cleanup process after the cancel,
because the replace_state was already updated to
BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED.
[3]
--
static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
int scrub_ret)
{
::
/* was the operation canceled, or is it finished? */
if (dev_replace->replace_state !=
BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED) {
btrfs_dev_replace_read_unlock(dev_replace);
mutex_unlock(_replace->lock_finishing_cancel_unmount);
return 0;
}
---
In fact before this patch [1] the code wasn't sync-ing the IO
when replace was canceled. Which this patch also fixes by using
the btrfs_dev_replace_finishing()
---
btrfs_dev_replace_finishing
::
/*
* flush all outstanding I/O and inode extent mappings before the
* copy operation is declared as being finished
*/
ret = btrfs_start_delalloc_roots(fs_info, -1);
if (ret) {
mutex_unlock(_replace->lock_finishing_cancel_unmount);
return ret;
}
btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
---
So to answer above question... these warn and error log wasn't
reported before when replace was canceled and now since I am
using the btrfs_dev_replace_finishing() to finish the job
of cancel.. it shall be reported which is ok to quite.
Do you think we still need to update the change log?
OR I think more appropriately this patch should be merged with
[PATCH 4/9] btrfs: fix UAF due to race between replace start and cancel
-Anand
HTH.
Thanks, Anand
Re: [PATCH 8/9] btrfs: user requsted replace cancel is not an error
On 11/15/2018 11:31 PM, David Sterba wrote:
On Sun, Nov 11, 2018 at 10:22:23PM +0800, Anand Jain wrote:
As of now only user requested replace cancel can cancel the replace-scrub
so no need to log error for it.
This has probably some user visible effect or threre are steps to
reproduce the message even if it's not expected (ie. the case that this
patch fixes). Please update the changelog, thanks.
before the patch [1]
[1]
btrfs: fix use-after-free due to race between replace start and cancel
We used to set the replace_state to CANCELED [2] and then call
btrfs_scrub_cancel(), but the problem with this approach is
if the scrub isn't running yet, then things get messier.
[2]
- dev_replace->replace_state = BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED;
So to fix, [1] shall set replace_state to CANCELED only after the
replace cancel thread has successfully canceled the replace using
btrfs_scrub_cancel().
And about the cleanup process for the replace target..
we call
btrfs_dev_replace_finishing(.. ret)
after
ret= btrfs_scrub_dev()
now ret is -ECANCELED due to replace cancel request by the user.
(its not set to -ECANCELED for any other reason).
btrfs_dev_replace_finishing() has the following code [3] which it
earlier blocked processing of the cleanup process after the cancel,
because the replace_state was already updated to
BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED.
[3]
--
static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
int scrub_ret)
{
::
/* was the operation canceled, or is it finished? */
if (dev_replace->replace_state !=
BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED) {
btrfs_dev_replace_read_unlock(dev_replace);
mutex_unlock(_replace->lock_finishing_cancel_unmount);
return 0;
}
---
In fact before this patch [1] the code wasn't sync-ing the IO
when replace was canceled. Which this patch also fixes by using
the btrfs_dev_replace_finishing()
---
btrfs_dev_replace_finishing
::
/*
* flush all outstanding I/O and inode extent mappings before the
* copy operation is declared as being finished
*/
ret = btrfs_start_delalloc_roots(fs_info, -1);
if (ret) {
mutex_unlock(_replace->lock_finishing_cancel_unmount);
return ret;
}
btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
---
So to answer above question... these warn and error log wasn't
reported before when replace was canceled and now since I am
using the btrfs_dev_replace_finishing() to finish the job
of cancel.. it shall be reported which is ok to quite.
Do you think we still need to update the change log?
HTH.
Thanks, Anand
Re: [PATCH 0/9 v2] fix replace-start and replace-cancel racing
On 11/15/2018 11:41 PM, David Sterba wrote: On Sun, Nov 11, 2018 at 10:22:15PM +0800, Anand Jain wrote: v1->v2: 2/9: Drop writeback required 3/9: Drop writeback required 7/9: Use the condition within the WARN_ON() 6/9: Use the condition within the ASSERT() Replace-start and replace-cancel threads can race to create a messy situation leading to UAF. We use the scrub code to write the blocks on the replace target. So if we haven't have set the replace-scrub-running yet, without this patch we just ignore the error and free the target device. When this happens the system panics with UAF error. Its nice to see that btrfs_dev_replace_finishing() already handles the ECANCELED (replace canceled) situation, but for an unknown reason we aren't using it to cleanup the replace cancel situation, instead we just let the replace cancel ioctl thread to cleanup the target device and return and out of synchronous with the scrub code. This patch 4/9, 5/9 and 6/9 uses the return code of btrfs_scrub_cancel() to check if the scrub was really running. And if its not then shall return an error to the user (replace not started error) so that user can retry replace cancel. And uses btrfs_dev_replace_finishing() code to cleanup after successful cancel of the replace scrub. Further, a suspended replace, when tries to restart, and if it fails (for example target device missing, or excl ops running) it goes to the started state, and so the cli 'btrfs replace status /mnt' hangs with no progress. So patches 2/9 and 3/9 fixes that. As the originals code idea of ECANCELED was limited to the situation of the error only and not user requested, there are unnecessary error log and warn log which 7/9 and 8/9 patches fixes. Patches 1/9 and 9/9 are good to have fixes. Makes a function static and code readability good. Testing: (I did some attempt to convert these into xfstests but need a mechanism where kernel thread can wait for user land script. I thought I could do it using ebfp, but needs more digging on how). As of now hand tested with using procfs to hold kernel thread at (wait_for_user(..)) until user land issues go. This could be tricky to get implemented but would be of course useful. I saw the crash about once a week so will watch if this still happens. That will be nice. Anand Jain (9): btrfs: mark btrfs_dev_replace_start() as static btrfs: replace go back to suspended if target missing btrfs: replace back to suspend state if EXCL OP is running btrfs: fix UAF due to race between replace start and cancel btrfs: replace cancel is successful if scrub cancel is successful btrfs: replace's scrub must not be running in replace suspended state btrfs: quiten warn if the replace is canceled at finish btrfs: user requsted replace cancel is not an error btrfs: add explicit check for replace result no error The above is merged to misc-next, except: btrfs: quiten warn if the replace is canceled at finish btrfs: user requsted replace cancel is not an error with replies under the patches what could be improved. The changes can be sent independently if you need to do that in several patches. Thanks. We need these patch otherwise you will see WARN_ON and btrfs_err after a successful replace cancel. Will send revised patch. Thanks, Anand
Re: [PATCH v1.1 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning
On 16.11.18 г. 10:22 ч., Qu Wenruo wrote:
> GCC 8.2.1 will report the following warning with "make W=1":
>
> ctree.c: In function 'btrfs_next_sibling_tree_block':
> ctree.c:2990:21: warning: 'slot' may be used uninitialized in this function
> [-Wmaybe-uninitialized]
> path->slots[level] = slot;
> ~~~^~
>
> The culprit is the following code:
>
> int slot; << Not initialized
> int level = path->lowest_level + 1;
> BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
> while(level < BTRFS_MAX_LEVEL) {
> slot = path->slots[level] + 1;
> ^^ but we initialize @slot here.
> ...
> }
> path->slots[level] = slot;
>
> Again, it's the stupid compiler needs some hint for the fact that
> we will always enter the while loop for at least once, thus @slot should
> always be initialized.
>
> Fix it by assign level after the BUG_ON(), so the stupid compiler knows
> we will always go into the while loop.
>
> Signed-off-by: Qu Wenruo
Much better, thank you:
Reviewed-by: Nikolay Borisov
> ---
> changelog:
> v1.1:
>Better commit message, with the original warning report and
>how compiler thinks this could be a problem.
> ---
> ctree.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/ctree.c b/ctree.c
> index 46e2ccedc0bf..9c9cb0dfdbf2 100644
> --- a/ctree.c
> +++ b/ctree.c
> @@ -2961,11 +2961,12 @@ int btrfs_next_sibling_tree_block(struct
> btrfs_fs_info *fs_info,
> struct btrfs_path *path)
> {
> int slot;
> - int level = path->lowest_level + 1;
> + int level;
> struct extent_buffer *c;
> struct extent_buffer *next = NULL;
>
> BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
> + level = path->lowest_level + 1;
> while(level < BTRFS_MAX_LEVEL) {
> if (!path->nodes[level])
> return 1;
>
[PATCH v1.1 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning
GCC 8.2.1 will report the following warning with "make W=1":
ctree.c: In function 'btrfs_next_sibling_tree_block':
ctree.c:2990:21: warning: 'slot' may be used uninitialized in this function
[-Wmaybe-uninitialized]
path->slots[level] = slot;
~~~^~
The culprit is the following code:
int slot; << Not initialized
int level = path->lowest_level + 1;
BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
while(level < BTRFS_MAX_LEVEL) {
slot = path->slots[level] + 1;
^^ but we initialize @slot here.
...
}
path->slots[level] = slot;
Again, it's the stupid compiler needs some hint for the fact that
we will always enter the while loop for at least once, thus @slot should
always be initialized.
Fix it by assign level after the BUG_ON(), so the stupid compiler knows
we will always go into the while loop.
Signed-off-by: Qu Wenruo
---
changelog:
v1.1:
Better commit message, with the original warning report and
how compiler thinks this could be a problem.
---
ctree.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ctree.c b/ctree.c
index 46e2ccedc0bf..9c9cb0dfdbf2 100644
--- a/ctree.c
+++ b/ctree.c
@@ -2961,11 +2961,12 @@ int btrfs_next_sibling_tree_block(struct btrfs_fs_info
*fs_info,
struct btrfs_path *path)
{
int slot;
- int level = path->lowest_level + 1;
+ int level;
struct extent_buffer *c;
struct extent_buffer *next = NULL;
BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
+ level = path->lowest_level + 1;
while(level < BTRFS_MAX_LEVEL) {
if (!path->nodes[level])
return 1;
--
2.19.1
Re: [PATCH 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning
On 2018/11/16 下午4:13, Nikolay Borisov wrote:
>
>
> On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
>> The only location is the following code:
>>
>> int level = path->lowest_level + 1;
>> BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
>> while(level < BTRFS_MAX_LEVEL) {
>> slot = path->slots[level] + 1;
>> ...
>> }
>> path->slots[level] = slot;
>>
>> Again, it's the stupid compiler needs some hint for the fact that
>> we will always enter the while loop for at least once, thus @slot should
>> always be initialized.
>>
>> Fix it by assign level after the BUG_ON(), so the stupid compiler knows
>> we will always go into the while loop.
>>
>> Signed-off-by: Qu Wenruo
>
> I don't get this warning on gcc 7.3. Also from your changelog it's not
> really clear what the compiler is confused about.
My gcc 8.2.1 reports it with "make W=1"
I'll just update the commit message with the original report:
ctree.c: In function 'btrfs_next_sibling_tree_block':
ctree.c:2990:21: warning: 'slot' may be used uninitialized in this
function [-Wmaybe-uninitialized]
path->slots[level] = slot;
~~~^~
Thanks,
Qu
>
> Codewise lgtm:
>
> Reviewed-by: Nikolay Borisov
>
>> ---
>> ctree.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/ctree.c b/ctree.c
>> index 46e2ccedc0bf..9c9cb0dfdbf2 100644
>> --- a/ctree.c
>> +++ b/ctree.c
>> @@ -2961,11 +2961,12 @@ int btrfs_next_sibling_tree_block(struct
>> btrfs_fs_info *fs_info,
>>struct btrfs_path *path)
>> {
>> int slot;
>> -int level = path->lowest_level + 1;
>> +int level;
>> struct extent_buffer *c;
>> struct extent_buffer *next = NULL;
>>
>> BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
>> +level = path->lowest_level + 1;
>> while(level < BTRFS_MAX_LEVEL) {
>> if (!path->nodes[level])
>> return 1;
>>
Re: [PATCH v1.1 9/9] btrfs-progs: Cleanup warning reported by -Wmissing-prototypes
On 16.11.18 г. 10:04 ч., Qu Wenruo wrote:
> The following missing prototypes will be fixed:
> 1) btrfs.c::handle_special_globals()
> 2) check/mode-lowmem.c::repair_ternary_lowmem()
> 3) extent-tree.c::btrfs_search_overlap_extent()
> Above 3 can be fixed by making them static
>
> 4) utils.c::btrfs_check_nodesize()
> Fixed by moving it to fsfeatures.c
>
> 5) chunk-recover.c::btrfs_recover_chunk_tree()
> 6) super-recover.c::btrfs_recover_superblocks()
> Fixed by moving the declaration from cmds-rescue.c to rescue.h
>
> 7) utils-lib.c::arg_strtou64()
> 8) utils-lib.c::lookup_path_rootid()
> Fixed by include "utils.h"
>
> 9) free-space-tree.c::set_free_space_tree_thresholds()
> Fixed by deleting it, as there is no user.
>
> 10) free-space-tree.c::convert_free_space_to_bitmaps()
> 11) free-space-tree.c::convert_free_space_to_extents()
> 12) free-space-tree.c::__remove_from_free_space_tree()
> 13) free-space-tree.c::__add_to_free_space_tree()
> 14) free-space-tree.c::btrfs_create_tree()
> Fixed by making them static.
>
> Signed-off-by: Qu Wenruo
Reviewed-by: Nikolay Borisov
> ---
> changelog:
> v1.1
>Update the subject, as we free space tree will also be cleaned up.
> ---
> btrfs.c | 2 +-
> check/mode-lowmem.c | 6 ++---
> chunk-recover.c | 1 +
> cmds-rescue.c | 4 +--
> extent-tree.c | 2 +-
> free-space-tree.c | 59 -
> fsfeatures.c| 22 +
> rescue.h| 14 +++
> super-recover.c | 1 +
> utils-lib.c | 1 +
> utils.c | 23 --
> 11 files changed, 60 insertions(+), 75 deletions(-)
> create mode 100644 rescue.h
>
> diff --git a/btrfs.c b/btrfs.c
> index 2d39f2ced3e8..78c468d2e050 100644
> --- a/btrfs.c
> +++ b/btrfs.c
> @@ -210,7 +210,7 @@ static int handle_global_options(int argc, char **argv)
> return shift;
> }
>
> -void handle_special_globals(int shift, int argc, char **argv)
> +static void handle_special_globals(int shift, int argc, char **argv)
> {
> int has_help = 0;
> int has_full = 0;
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 14bbc9ee6cb6..f56b5e8d45dc 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -953,9 +953,9 @@ out:
> * returns 0 means success.
> * returns not 0 means on error;
> */
> -int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
> - u64 index, char *name, int name_len, u8 filetype,
> - int err)
> +static int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64
> ino,
> + u64 index, char *name, int name_len,
> + u8 filetype, int err)
> {
> struct btrfs_trans_handle *trans;
> int stage = 0;
> diff --git a/chunk-recover.c b/chunk-recover.c
> index 1d30db51d8ed..1e554b8e8750 100644
> --- a/chunk-recover.c
> +++ b/chunk-recover.c
> @@ -40,6 +40,7 @@
> #include "utils.h"
> #include "btrfsck.h"
> #include "commands.h"
> +#include "rescue.h"
>
> struct recover_control {
> int verbose;
> diff --git a/cmds-rescue.c b/cmds-rescue.c
> index 2bc50c0841ed..36e9e1277e40 100644
> --- a/cmds-rescue.c
> +++ b/cmds-rescue.c
> @@ -26,15 +26,13 @@
> #include "commands.h"
> #include "utils.h"
> #include "help.h"
> +#include "rescue.h"
>
> static const char * const rescue_cmd_group_usage[] = {
> "btrfs rescue [options] ",
> NULL
> };
>
> -int btrfs_recover_chunk_tree(const char *path, int verbose, int yes);
> -int btrfs_recover_superblocks(const char *path, int verbose, int yes);
> -
> static const char * const cmd_rescue_chunk_recover_usage[] = {
> "btrfs rescue chunk-recover [options] ",
> "Recover the chunk tree by scanning the devices one by one.",
> diff --git a/extent-tree.c b/extent-tree.c
> index cd98633992ac..8c9cdeff3b02 100644
> --- a/extent-tree.c
> +++ b/extent-tree.c
> @@ -3749,7 +3749,7 @@ static void __get_extent_size(struct btrfs_root *root,
> struct btrfs_path *path,
> * Return >0 for not found.
> * Return <0 for err
> */
> -int btrfs_search_overlap_extent(struct btrfs_root *root,
> +static int btrfs_search_overlap_extent(struct btrfs_root *root,
> struct btrfs_path *path, u64 bytenr, u64 len)
> {
> struct btrfs_key key;
> diff --git a/free-space-tree.c b/free-space-tree.c
> index 6641cdfa42ba..af141e6e611a 100644
> --- a/free-space-tree.c
> +++ b/free-space-tree.c
> @@ -24,35 +24,6 @@
> #include "bitops.h"
> #include "internal.h"
>
> -void set_free_space_tree_thresholds(struct btrfs_block_group_cache *cache,
> - u64 sectorsize)
> -{
> - u32 bitmap_range;
> - size_t bitmap_size;
> - u64 num_bitmaps, total_bitmap_size;
> -
> - /*
> - * We convert to bitmaps when the disk space required for using extents
> - * exceeds
Re: [PATCH 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning
On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> The only location is the following code:
>
> int level = path->lowest_level + 1;
> BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
> while(level < BTRFS_MAX_LEVEL) {
> slot = path->slots[level] + 1;
> ...
> }
> path->slots[level] = slot;
>
> Again, it's the stupid compiler needs some hint for the fact that
> we will always enter the while loop for at least once, thus @slot should
> always be initialized.
>
> Fix it by assign level after the BUG_ON(), so the stupid compiler knows
> we will always go into the while loop.
>
> Signed-off-by: Qu Wenruo
I don't get this warning on gcc 7.3. Also from your changelog it's not
really clear what the compiler is confused about.
Codewise lgtm:
Reviewed-by: Nikolay Borisov
> ---
> ctree.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/ctree.c b/ctree.c
> index 46e2ccedc0bf..9c9cb0dfdbf2 100644
> --- a/ctree.c
> +++ b/ctree.c
> @@ -2961,11 +2961,12 @@ int btrfs_next_sibling_tree_block(struct
> btrfs_fs_info *fs_info,
> struct btrfs_path *path)
> {
> int slot;
> - int level = path->lowest_level + 1;
> + int level;
> struct extent_buffer *c;
> struct extent_buffer *next = NULL;
>
> BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
> + level = path->lowest_level + 1;
> while(level < BTRFS_MAX_LEVEL) {
> if (!path->nodes[level])
> return 1;
>
Re: [PATCH 5/9] btrfs-progs: Fix Wimplicit-fallthrough warning
On 2018/11/16 下午4:04, Nikolay Borisov wrote:
>
>
> On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
>> Although most fallthrough case is pretty obvious, we still need to teach
>> the dumb compiler that it's an explicit fallthrough.
>>
>> Also reformat the code to use common indent.
>>
>> Signed-off-by: Qu Wenruo
>
> Reviewed-by: Nikolay Borisov
>
> Is this attribute dependent on a particular compiler version? Does
> gcc4.4 for example support it?
IIRC unsupported attribute should just be ignored by the compiler, so it
shouldn't cause any problem.
However I'm not 100% perfect sure, as my distribution doesn't provide
older gcc.
Thanks,
Qu
>
>> ---
>> utils.c | 30 ++
>> 1 file changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/utils.c b/utils.c
>> index a310300829eb..b274f46fdd9d 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -1134,15 +1134,25 @@ int pretty_size_snprintf(u64 size, char *str, size_t
>> str_size, unsigned unit_mod
>> num_divs = 0;
>> last_size = size;
>> switch (unit_mode & UNITS_MODE_MASK) {
>> -case UNITS_TBYTES: base *= mult; num_divs++;
>> -case UNITS_GBYTES: base *= mult; num_divs++;
>> -case UNITS_MBYTES: base *= mult; num_divs++;
>> -case UNITS_KBYTES: num_divs++;
>> - break;
>> +case UNITS_TBYTES:
>> +base *= mult;
>> +num_divs++;
>> +__attribute__ ((fallthrough));
>> +case UNITS_GBYTES:
>> +base *= mult;
>> +num_divs++;
>> +__attribute__ ((fallthrough));
>> +case UNITS_MBYTES:
>> +base *= mult;
>> +num_divs++;
>> +__attribute__ ((fallthrough));
>> +case UNITS_KBYTES:
>> +num_divs++;
>> +break;
>> case UNITS_BYTES:
>> - base = 1;
>> - num_divs = 0;
>> - break;
>> +base = 1;
>> +num_divs = 0;
>> +break;
>> default:
>> if (negative) {
>> s64 ssize = (s64)size;
>> @@ -1907,13 +1917,17 @@ int test_num_disk_vs_raid(u64 metadata_profile, u64
>> data_profile,
>> default:
>> case 4:
>> allowed |= BTRFS_BLOCK_GROUP_RAID10;
>> +__attribute__ ((fallthrough));
>> case 3:
>> allowed |= BTRFS_BLOCK_GROUP_RAID6;
>> +__attribute__ ((fallthrough));
>> case 2:
>> allowed |= BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1 |
>> BTRFS_BLOCK_GROUP_RAID5;
>> +__attribute__ ((fallthrough));
>> case 1:
>> allowed |= BTRFS_BLOCK_GROUP_DUP;
>> +__attribute__ ((fallthrough));
>> }
>>
>> if (dev_cnt > 1 && profile & BTRFS_BLOCK_GROUP_DUP) {
>>
Re: [PATCH 8/9] btrfs-progs: Fix Wtype-limits warning
On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> The only hit is the following code:
>
> tlv_len = le16_to_cpu(tlv_hdr->tlv_len);
>
> if (tlv_type == 0 || tlv_type > BTRFS_SEND_A_MAX
> || tlv_len > BTRFS_SEND_BUF_SIZE) {
> error("invalid tlv in cmd tlv_type = %hu, tlv_len =
> %hu",
> tlv_type, tlv_len);
>
> @tlv_len is u16, while BTRFS_SEND_BUF_SIZE is 64K.
> u16 MAX is 64K - 1, so the final check is always false.
>
> Just remove it.
>
> Signed-off-by: Qu Wenruo
I had an identical patch:
Reviewed-by: Nikolay Borisov
> ---
> send-stream.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/send-stream.c b/send-stream.c
> index 3b8e39c9486a..25461e92c37b 100644
> --- a/send-stream.c
> +++ b/send-stream.c
> @@ -157,8 +157,7 @@ static int read_cmd(struct btrfs_send_stream *sctx)
> tlv_type = le16_to_cpu(tlv_hdr->tlv_type);
> tlv_len = le16_to_cpu(tlv_hdr->tlv_len);
>
> - if (tlv_type == 0 || tlv_type > BTRFS_SEND_A_MAX
> - || tlv_len > BTRFS_SEND_BUF_SIZE) {
> + if (tlv_type == 0 || tlv_type > BTRFS_SEND_A_MAX) {
> error("invalid tlv in cmd tlv_type = %hu, tlv_len =
> %hu",
> tlv_type, tlv_len);
> ret = -EINVAL;
>
Re: [PATCH 6/9] btrfs-progs: Fix Wsuggest-attribute=format warning
On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> Add __attribute__ ((format (printf, 4, 0))) to fix the vprintf calling
> function.
>
> Signed-off-by: Qu Wenruo
Reviewed-by: Nikolay Borisov
> ---
> string-table.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/string-table.c b/string-table.c
> index 95833768960d..455285702d51 100644
> --- a/string-table.c
> +++ b/string-table.c
> @@ -48,6 +48,7 @@ struct string_table *table_create(int columns, int rows)
> * '>' the text is right aligned. If fmt is equal to '=' the text will
> * be replaced by a '=' dimensioned on the basis of the column width
> */
> +__attribute__ ((format (printf, 4, 0)))
> char *table_vprintf(struct string_table *tab, int column, int row,
> const char *fmt, va_list ap)
> {
>
Re: [PATCH 5/9] btrfs-progs: Fix Wimplicit-fallthrough warning
On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> Although most fallthrough case is pretty obvious, we still need to teach
> the dumb compiler that it's an explicit fallthrough.
>
> Also reformat the code to use common indent.
>
> Signed-off-by: Qu Wenruo
Reviewed-by: Nikolay Borisov
Is this attribute dependent on a particular compiler version? Does
gcc4.4 for example support it?
> ---
> utils.c | 30 ++
> 1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/utils.c b/utils.c
> index a310300829eb..b274f46fdd9d 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1134,15 +1134,25 @@ int pretty_size_snprintf(u64 size, char *str, size_t
> str_size, unsigned unit_mod
> num_divs = 0;
> last_size = size;
> switch (unit_mode & UNITS_MODE_MASK) {
> - case UNITS_TBYTES: base *= mult; num_divs++;
> - case UNITS_GBYTES: base *= mult; num_divs++;
> - case UNITS_MBYTES: base *= mult; num_divs++;
> - case UNITS_KBYTES: num_divs++;
> -break;
> + case UNITS_TBYTES:
> + base *= mult;
> + num_divs++;
> + __attribute__ ((fallthrough));
> + case UNITS_GBYTES:
> + base *= mult;
> + num_divs++;
> + __attribute__ ((fallthrough));
> + case UNITS_MBYTES:
> + base *= mult;
> + num_divs++;
> + __attribute__ ((fallthrough));
> + case UNITS_KBYTES:
> + num_divs++;
> + break;
> case UNITS_BYTES:
> -base = 1;
> -num_divs = 0;
> -break;
> + base = 1;
> + num_divs = 0;
> + break;
> default:
> if (negative) {
> s64 ssize = (s64)size;
> @@ -1907,13 +1917,17 @@ int test_num_disk_vs_raid(u64 metadata_profile, u64
> data_profile,
> default:
> case 4:
> allowed |= BTRFS_BLOCK_GROUP_RAID10;
> + __attribute__ ((fallthrough));
> case 3:
> allowed |= BTRFS_BLOCK_GROUP_RAID6;
> + __attribute__ ((fallthrough));
> case 2:
> allowed |= BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1 |
> BTRFS_BLOCK_GROUP_RAID5;
> + __attribute__ ((fallthrough));
> case 1:
> allowed |= BTRFS_BLOCK_GROUP_DUP;
> + __attribute__ ((fallthrough));
> }
>
> if (dev_cnt > 1 && profile & BTRFS_BLOCK_GROUP_DUP) {
>
[PATCH v1.1 9/9] btrfs-progs: Cleanup warning reported by -Wmissing-prototypes
The following missing prototypes will be fixed:
1) btrfs.c::handle_special_globals()
2) check/mode-lowmem.c::repair_ternary_lowmem()
3) extent-tree.c::btrfs_search_overlap_extent()
Above 3 can be fixed by making them static
4) utils.c::btrfs_check_nodesize()
Fixed by moving it to fsfeatures.c
5) chunk-recover.c::btrfs_recover_chunk_tree()
6) super-recover.c::btrfs_recover_superblocks()
Fixed by moving the declaration from cmds-rescue.c to rescue.h
7) utils-lib.c::arg_strtou64()
8) utils-lib.c::lookup_path_rootid()
Fixed by include "utils.h"
9) free-space-tree.c::set_free_space_tree_thresholds()
Fixed by deleting it, as there is no user.
10) free-space-tree.c::convert_free_space_to_bitmaps()
11) free-space-tree.c::convert_free_space_to_extents()
12) free-space-tree.c::__remove_from_free_space_tree()
13) free-space-tree.c::__add_to_free_space_tree()
14) free-space-tree.c::btrfs_create_tree()
Fixed by making them static.
Signed-off-by: Qu Wenruo
---
changelog:
v1.1
Update the subject, as we free space tree will also be cleaned up.
---
btrfs.c | 2 +-
check/mode-lowmem.c | 6 ++---
chunk-recover.c | 1 +
cmds-rescue.c | 4 +--
extent-tree.c | 2 +-
free-space-tree.c | 59 -
fsfeatures.c| 22 +
rescue.h| 14 +++
super-recover.c | 1 +
utils-lib.c | 1 +
utils.c | 23 --
11 files changed, 60 insertions(+), 75 deletions(-)
create mode 100644 rescue.h
diff --git a/btrfs.c b/btrfs.c
index 2d39f2ced3e8..78c468d2e050 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -210,7 +210,7 @@ static int handle_global_options(int argc, char **argv)
return shift;
}
-void handle_special_globals(int shift, int argc, char **argv)
+static void handle_special_globals(int shift, int argc, char **argv)
{
int has_help = 0;
int has_full = 0;
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 14bbc9ee6cb6..f56b5e8d45dc 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -953,9 +953,9 @@ out:
* returns 0 means success.
* returns not 0 means on error;
*/
-int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
- u64 index, char *name, int name_len, u8 filetype,
- int err)
+static int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
+u64 index, char *name, int name_len,
+u8 filetype, int err)
{
struct btrfs_trans_handle *trans;
int stage = 0;
diff --git a/chunk-recover.c b/chunk-recover.c
index 1d30db51d8ed..1e554b8e8750 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -40,6 +40,7 @@
#include "utils.h"
#include "btrfsck.h"
#include "commands.h"
+#include "rescue.h"
struct recover_control {
int verbose;
diff --git a/cmds-rescue.c b/cmds-rescue.c
index 2bc50c0841ed..36e9e1277e40 100644
--- a/cmds-rescue.c
+++ b/cmds-rescue.c
@@ -26,15 +26,13 @@
#include "commands.h"
#include "utils.h"
#include "help.h"
+#include "rescue.h"
static const char * const rescue_cmd_group_usage[] = {
"btrfs rescue [options] ",
NULL
};
-int btrfs_recover_chunk_tree(const char *path, int verbose, int yes);
-int btrfs_recover_superblocks(const char *path, int verbose, int yes);
-
static const char * const cmd_rescue_chunk_recover_usage[] = {
"btrfs rescue chunk-recover [options] ",
"Recover the chunk tree by scanning the devices one by one.",
diff --git a/extent-tree.c b/extent-tree.c
index cd98633992ac..8c9cdeff3b02 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -3749,7 +3749,7 @@ static void __get_extent_size(struct btrfs_root *root,
struct btrfs_path *path,
* Return >0 for not found.
* Return <0 for err
*/
-int btrfs_search_overlap_extent(struct btrfs_root *root,
+static int btrfs_search_overlap_extent(struct btrfs_root *root,
struct btrfs_path *path, u64 bytenr, u64 len)
{
struct btrfs_key key;
diff --git a/free-space-tree.c b/free-space-tree.c
index 6641cdfa42ba..af141e6e611a 100644
--- a/free-space-tree.c
+++ b/free-space-tree.c
@@ -24,35 +24,6 @@
#include "bitops.h"
#include "internal.h"
-void set_free_space_tree_thresholds(struct btrfs_block_group_cache *cache,
- u64 sectorsize)
-{
- u32 bitmap_range;
- size_t bitmap_size;
- u64 num_bitmaps, total_bitmap_size;
-
- /*
-* We convert to bitmaps when the disk space required for using extents
-* exceeds that required for using bitmaps.
-*/
- bitmap_range = sectorsize * BTRFS_FREE_SPACE_BITMAP_BITS;
- num_bitmaps = div_u64(cache->key.offset + bitmap_range - 1,
- bitmap_range);
- bitmap_size = sizeof(struct btrfs_item) + BTRFS_FREE_SPACE_BITMAP_SIZE;
-
Re: [PATCH 4/9] btrfs-progs: Fix Wempty-body warning
On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> messages.h:49:24: warning: suggest braces around empty body in an 'if'
> statement [-Wempty-body]
> PRINT_TRACE_ON_ERROR;\
>
> Just extra braces would solve the problem.
>
> Signed-off-by: Qu Wenruo
Reviewed-by: Nikolay Borisov
> ---
> messages.h | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/messages.h b/messages.h
> index ec7d93381a36..16f650d19a4b 100644
> --- a/messages.h
> +++ b/messages.h
> @@ -45,13 +45,16 @@
>
> #define error_on(cond, fmt, ...) \
> do {\
> - if ((cond)) \
> + if ((cond)) { \
> PRINT_TRACE_ON_ERROR; \
> - if ((cond)) \
> + } \
> + if ((cond)) { \
> PRINT_VERBOSE_ERROR;\
> + } \
> __btrfs_error_on((cond), (fmt), ##__VA_ARGS__); \
> - if ((cond)) \
> + if ((cond)) { \
> DO_ABORT_ON_ERROR; \
> + } \
> } while (0)
>
> #define error_btrfs_util(err)
> \
> @@ -76,10 +79,12 @@
>
> #define warning_on(cond, fmt, ...) \
> do {\
> - if ((cond)) \
> + if ((cond)) { \
> PRINT_TRACE_ON_ERROR; \
> - if ((cond)) \
> + } \
> + if ((cond)) { \
> PRINT_VERBOSE_ERROR;\
> + } \
> __btrfs_warning_on((cond), (fmt), ##__VA_ARGS__); \
> } while (0)
>
>
[PATCH v1.1 3/9] btrfs-progs: Makefile.extrawarn: Don't warn on sign compare
Under most case, we are just using 'int' for 'unsigned int', and doesn't care about the sign. The Wsign-compare is causing tons of false alerts. Suppressing it would make W=1 less noisy. Signed-off-by: Qu Wenruo --- changelog: v1.1: Use cc-disable-warning to provide much better compatibility against older compiler --- Makefile.extrawarn | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.extrawarn b/Makefile.extrawarn index bbb2d5173846..736bee82759f 100644 --- a/Makefile.extrawarn +++ b/Makefile.extrawarn @@ -54,6 +54,7 @@ warning-1 += $(call cc-option, -Wmissing-include-dirs) warning-1 += $(call cc-option, -Wunused-but-set-variable) warning-1 += $(call cc-disable-warning, missing-field-initializers) warning-1 += $(call cc-disable-warning, format-truncation) +warning-1 += $(call cc-disable-warning, sign-compare) warning-2 := -Waggregate-return warning-2 += -Wcast-align -- 2.19.1
