On 2020/12/10 下午11:39, Nikolay Borisov wrote:
On 10.12.20 г. 8:38 ч., Qu Wenruo wrote:
For subpage case, grab_extent_buffer_from_page() can't really get an
extent buffer just from btrfs_subpage.
Although we have btrfs_subpage::tree_block_bitmap, which can be used to
grab the bytenr of an e
On 2020/12/10 下午11:30, Nikolay Borisov wrote:
On 10.12.20 г. 8:38 ч., Qu Wenruo wrote:
For subpage case, we need to allocate new memory for each metadata page.
So we need to:
- Allow attach_extent_buffer_page() to return int
To indicate allocation failure
- Prealloc page->private for a
On 2020/12/17 下午1:59, Nikolay Borisov wrote:
On 17.12.20 г. 7:55 ч., Nikolay Borisov wrote:
On 17.12.20 г. 6:57 ч., Qu Wenruo wrote:
In btrfs_invalidatepage() we re-declare @tree variable as
btrfs_ordered_inode_tree.
Remove such variable shadowing which can be very confusing.
You can'
On Thu 17 Dec 2020 at 13:42, Qu Wenruo
wrote:
On 2020/12/17 下午1:38, Su Yue wrote:
On Thu 17 Dec 2020 at 12:57, Qu Wenruo wrote:
In btrfs_invalidatepage() we re-declare @tree variable as
btrfs_ordered_inode_tree.
Remove such variable shadowing which can be very confusing.
Signed-off-b
On 17.12.20 г. 7:55 ч., Nikolay Borisov wrote:
>
>
> On 17.12.20 г. 6:57 ч., Qu Wenruo wrote:
>> In btrfs_invalidatepage() we re-declare @tree variable as
>> btrfs_ordered_inode_tree.
>>
>> Remove such variable shadowing which can be very confusing.
>
> You can't do that, because lock_extent_
On 17.12.20 г. 6:57 ч., Qu Wenruo wrote:
> In btrfs_invalidatepage() we re-declare @tree variable as
> btrfs_ordered_inode_tree.
>
> Remove such variable shadowing which can be very confusing.
You can't do that, because lock_extent_bits expects extent_io_tree !
On 2020/12/17 下午1:38, Su Yue wrote:
On Thu 17 Dec 2020 at 12:57, Qu Wenruo wrote:
In btrfs_invalidatepage() we re-declare @tree variable as
btrfs_ordered_inode_tree.
Remove such variable shadowing which can be very confusing.
Signed-off-by: Qu Wenruo
---
fs/btrfs/inode.c | 9 +++--
On Thu 17 Dec 2020 at 12:57, Qu Wenruo wrote:
In btrfs_invalidatepage() we re-declare @tree variable as
btrfs_ordered_inode_tree.
Remove such variable shadowing which can be very confusing.
Signed-off-by: Qu Wenruo
---
fs/btrfs/inode.c | 9 +++--
1 file changed, 3 insertions(+), 6 del
In btrfs_invalidatepage() we introduce a temporary variable, new_len, to
update ordered->truncated_len.
But we can use min() to replace it completely and no need for the
variable.
Signed-off-by: Qu Wenruo
---
fs/btrfs/inode.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --g
[BUG]
With current subpage RW patchset, the following script can lead to
filesystem hang:
# mkfs.btrfs -f -s 4k $dev
# mount $dev -o nospace_cache $mnt
# fsstress -w -n 100 -p 1 -s 1608140256 -v -d $mnt
The file system will hang at wait_event() of
btrfs_start_ordered_extent().
[CAUSE]
The r
For current sectorsize == PAGE_ZIE case, there will only be at most one
ordered extent for one page.
But for subpage case, one page can contain several ordered extents, thus
current TestClearPagePrivate2() call will only finish ordered IO for the
first ordered extent, the remaining ones will be sk
This small patchset contains 3 refactors which are subpage independent.
And the last patch is RFC where I'm not certain about the existing code,
but it solves the problem for subpage during test.
Thus I'm here asking for help on the btrfs_invalidatepage() behavior.
Qu Wenruo (4):
btrfs: inode:
In btrfs_invalidatepage() we re-declare @tree variable as
btrfs_ordered_inode_tree.
Remove such variable shadowing which can be very confusing.
Signed-off-by: Qu Wenruo
---
fs/btrfs/inode.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/
On 2020/12/17 上午3:28, Ross Vandegrift wrote:
Hello,
I recently saw these in my dmesg, on 4.19.160:
BTRFS warning (device dm-20): csum failed root -9 ino 1303 off 0 csum
0x47abffd5 expected csum 0xd5f0d335 mirror 1
BTRFS warning (device dm-20): csum failed root -9 ino 1303 off 0 csum
0
On Wed, Dec 16, 2020 at 10:07:38PM +, Nick Terrell wrote:
[...]
> It is very large. If it helps, in the commit message I’ve provided this link
> [0],
> which provides the diff between upstream zstd as-is and the imported zstd,
> which has been modified by the automated tooling to work in the k
> On Dec 16, 2020, at 10:50 AM, David Sterba wrote:
>
> On Wed, Dec 16, 2020 at 11:58:07AM +1100, Herbert Xu wrote:
>> On Wed, Dec 16, 2020 at 12:48:51AM +, Nick Terrell wrote:
>>>
>>> Thanks for the advice! The first zstd patches went through Herbert’s tree,
>>> which is
>>> why I’ve sen
On 14:13 15/12, Darrick J. Wong wrote:
> On Tue, Dec 15, 2020 at 12:06:36PM -0600, Goldwyn Rodrigues wrote:
> > From: Goldwyn Rodrigues
> >
> > btrfs_direct_write() fallsback to buffered write in case btrfs is not
> > able to perform or complete a direct I/O. During the fallback
> > inode lock is
On Wed, Dec 16, 2020 at 11:26:16AM -0500, Josef Bacik wrote:
> v6->v7:
> - Broke up the series into 3 series, 1 for cosmetic things, 1 for all the
> major
> issues (including those reported on v6 of this set), and this new set which
> is
> solely the error handling related patches for relocat
Hello,
I recently saw these in my dmesg, on 4.19.160:
BTRFS warning (device dm-20): csum failed root -9 ino 1303 off 0 csum
0x47abffd5 expected csum 0xd5f0d335 mirror 1
BTRFS warning (device dm-20): csum failed root -9 ino 1303 off 0 csum
0x47abffd5 expected csum 0xd5f0d335 mirror 1
I'm con
On Wed, Dec 16, 2020 at 11:58:07AM +1100, Herbert Xu wrote:
> On Wed, Dec 16, 2020 at 12:48:51AM +, Nick Terrell wrote:
> >
> > Thanks for the advice! The first zstd patches went through Herbert’s tree,
> > which is
> > why I’ve sent them this way.
>
> Sorry, but I'm not touch these patches a
On Wed, Dec 16, 2020 at 5:40 PM David Sterba wrote:
> On Wed, Nov 18, 2020 at 11:23:42AM +0100, Ondrej Mosnacek wrote:
> > When SELinux security options are passed to btrfs via fsconfig(2) rather
> > than via mount(2), the operation aborts with an error. What happens is
> > roughly this sequence:
On Wed, Dec 16, 2020 at 06:23:50PM +0100, David Sterba wrote:
> On Fri, Dec 11, 2020 at 06:09:11PM +, Sidong Yang wrote:
> > On Fri, Dec 11, 2020 at 06:46:29PM +0100, David Sterba wrote:
> > > On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote:
> > > > On Fri, Dec 11, 2020 at 04:48:12
On Fri, Dec 11, 2020 at 06:09:11PM +, Sidong Yang wrote:
> On Fri, Dec 11, 2020 at 06:46:29PM +0100, David Sterba wrote:
> > On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote:
> > > On Fri, Dec 11, 2020 at 04:48:12PM +, Sidong Yang wrote:
> > > > Example json format:
> > > >
> >
On Wed, Dec 16, 2020 at 08:52:08PM +0800, Su Yue wrote:
> > > The new line made filter produce the '+1'.
> >
> > Thanks for testing this patch.
> > I checked the fmt_end() and there is an additional newline.
> > I think that fmt_end() should be used for formatting. so it seems that
>
> Yes, it's f
On Wed, Dec 16, 2020 at 02:30:04PM +0800, Su Yue wrote:
> On Sat, Dec 12, 2020 at 3:04 AM David Sterba wrote:
> >
> > On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote:
> > > On Fri, Dec 11, 2020 at 04:48:12PM +, Sidong Yang wrote:
> > > > Example json format:
> > > >
> > > > {
> >
We love running delayed refs in commit_cowonly_roots, but it is a bit
excessive. I was seeing cases of running 3 or 4 refs a few times in a
row during this time. Instead simply update all of the roots first,
then run delayed refs, then handle the empty block groups case, and then
if we have any m
This was added a very long time ago to work around issues with delayed
refs and with qgroups. Both of these issues have since been properly
fixed, so all this does is cause a lot of lock contention with anybody
else who is running delayed refs.
Signed-off-by: Josef Bacik
---
fs/btrfs/transactio
I've been running a stress test that runs 20 workers in their own
subvolume, which are running an fsstress instance with 4 threads per
worker, which is 80 total fsstress threads. In addition to this I'm
running balance in the background as well as creating and deleting
snapshots. This test takes
Previously our delayed ref running used the total number of items as the
items to run. However we changed that to number of heads to run with
the delayed_refs_rsv, as generally we want to run all of the operations
for one bytenr.
But with btrfs_run_delayed_refs(trans, 0) we set our count to 2x th
We try to pre-flush the delayed refs when committing, because we want to
do as little work as possible in the critical section of the transaction
commit.
However doing this twice can lead to very long transaction commit delays
as other threads are allowed to continue to generate more delayed refs,
While running some stress tests I started getting hung task messages.
This is because the delete unused bg's code has to take the
delete_unused_bgs_mutex to do it's work, which is taken by balance to
make sure we don't delete block groups while we're balancing.
The problem is a balance can take a
v2->v3:
- Added Nikolay's reviewed by for the second patch.
- Rebased onto the latest misc-next.
v1->v2:
- Fixed the log messages that Nikolay pointed out.
- Added Nikolay's reviewed by for the first patch.
- Removed the unneeded mb for flushing.
--- Original email ---
Hello,
I've been running s
v1->v2:
- Rebase onto latest misc-next.
- Added Nikolay's reviewed-by for the first patch.
--- Original email ---
Hello,
Nikolay discovered a regression with generic/371 with my delayed refs patches
applied. This isn't actually a regression caused by those patches, but rather
those patches uncov
Currently we pass things around to figure out if we maybe free'ing data
based on the state of the delayed refs head. This makes the accounting
sort of confusing and hard to follow, as it's distinctly separate from
the delayed ref heads stuff, but also depends on it entirely.
Fix this by explicitl
My recent set of patches to reduce lock contention on the extent root by
running delayed refs resulted in a regression in generic/371. This test
fallocate()'s the fs until it's full, deletes all the files, and then
tries to fallocate() until full again.
Before my delayed refs patches we would run
On Wed, Nov 18, 2020 at 11:23:42AM +0100, Ondrej Mosnacek wrote:
> When SELinux security options are passed to btrfs via fsconfig(2) rather
> than via mount(2), the operation aborts with an error. What happens is
> roughly this sequence:
>
> 1. vfs_parse_fs_param() eats away the LSM options and pa
We can already deal with errors appropriately from do_relocation, simply
handle any errors that come from changing the refs at this point
cleanly. We have to abort the transaction if we fail here as we've
modified metadata at this point.
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 9
We already handle some errors in this function, and the callers do the
correct error handling, so clean up the rest of the function to do the
appropriate error handling.
There's a little extra work that needs to be done here, as we create the
inode item before we create the orphan item. We could
This can fail for any number of reasons, why bring the whole box down
with it?
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index d43603ae5570..8101
record_root_in_trans can currently fail, so handle this failure
properly.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/transaction.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 4efdb87df2
This will be able to return errors in the future, so change it to return
an error and handle the error appropriately.
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocatio
We call btrfs_update_root in btrfs_update_reloc_root, which can fail for
all sorts of reasons, including IO errors. Instead of panicing the box
lets return the error, now that all callers properly handle those
errors.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c
A few BUG_ON()'s in replace_path are purely to keep us from making
logical mistakes, so replace them with ASSERT()'s.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/relocation.c b/fs/
btrfs_update_reloc_root will will return errors in the future, so handle
the error properly in commit_fs_roots.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/transaction.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/t
This probably can't happen even with a corrupt file system, because we
would have failed much earlier on than here. However there's no reason
we can't just check and bail out as appropriate, so do that and convert
the correctness BUG_ON() to an ASSERT().
Reviewed-by: Qu Wenruo
Signed-off-by: Jos
We can already handle errors appropriately from this function, deal with
an error coming from __add_reloc_root appropriately.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/re
create_reloc_root will return errors in the future, and __add_reloc_root
can return -ENOMEM or -EEXIST, so handle these errors properly.
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs
We have a BUG_ON() if we get an error back from btrfs_get_fs_root().
This honestly should never fail, as at this point we have a solid
coordination of fs root to reloc root, and these roots will all be in
memory. But in the name of killing BUG_ON()'s remove these and handle
the error condition pro
There's a few places where we don't check the return value of
btrfs_commit_transaction in relocation.c. Thankfully all these places
have straightforward error handling, so simply change all of the sites
at once.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 9 ++
In corruption cases we could have paths from a block up to no root at
all, and thus we'll BUG_ON(!root) in select_one_root. Handle this by
adding an ASSERT() for developers, and returning an error for normal
users.
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 20 +---
If any of the reference count manipulation stuff fails in replace_path
we need to abort the transaction, as we've modified the blocks already.
We can simply break at this point and everything will be cleaned up.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 20 ++
record_root_in_trans can fail currently, so handle this failure
properly.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/transaction.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 307a73abe86d..5b
These checks are all taken care of for us by the tree checker code.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 29 +
1 file changed, 1 insertion(+), 28 deletions(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 6
We need to validate that a data extent item does not have the
FULL_BACKREF flag set on it's flags.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/tree-checker.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 028e7
If we error out cow'ing the root node when doing a replace_path then we
simply unlock and free the buffer and return the error.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/
btrfs_update_reloc_root will will return errors in the future, so handle
an error properly in prepare_to_merge.
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
ind
btrfs_update_reloc_root will will return errors in the future, so handle
the error properly in insert_dirty_subvol.
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 13d
We do memory allocations here, read blocks from disk, all sorts of
operations that could easily fail at any given point. Instead of
panicing the box, simply return the error back up the chain, all callers
at this point have proper error handling.
Signed-off-by: Josef Bacik
---
fs/btrfs/relocati
If we have a duplicate entry for a reloc root then we could have fs
corruption that resulted in a double allocation. This shouldn't happen
generally so leave an ASSERT() for this case, but return an error
instead of panicing in the normal user case.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Ba
btrfs_record_root_in_trans will return errors in the future, so handle
the error properly in create_subvol.
Signed-off-by: Josef Bacik
---
fs/btrfs/ioctl.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index af8d01659562..2d0782f7e0
record_root_in_trans can fail currently, handle this failure properly.
Signed-off-by: Josef Bacik
---
fs/btrfs/transaction.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 5b3008ec4e13..4efdb87df27d 100644
--- a/fs/b
If we fail to setup a ->reloc_root in a different thread that path will
error out, however it still leaves root->reloc_root NULL but would still
appear set up in the transaction. Subsequent calls to
btrfs_record_root_in_transaction would succeed without attempting to
create the reloc root, as the
btrfs_record_root_in_trans will return errors in the future, so handle
the error properly in btrfs_recover_log_trees.
This appears tricky, however we have a reference count on the
destination root, so if this fails we need to continue on in the loop to
make sure the properly cleanup is done.
Revi
btrfs_record_root_in_trans will return errors in the future, so handle
the error properly in relocate_tree_block.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/r
Generally speaking this shouldn't ever fail, the corresponding fs root
for the reloc root will already be in memory, so we won't get -ENOMEM
here.
However if there is no corresponding root for the reloc root then we
could get -ENOMEM when we try to allocate it or we could get -ENOENT
when we look
btrfs_record_root_in_trans will return errors in the future, so handle
the error properly in start_transaction.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/transaction.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs
btrfs_record_root_in_trans will return errors in the future, so handle
the error properly in btrfs_delete_subvolume.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/inode.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.
btrfs_record_root_in_trans will return errors in the future, so handle
the error properly in btrfs_rename.
Signed-off-by: Josef Bacik
---
fs/btrfs/inode.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2f8bb8405ac6..bcbae8b460
btrfs_record_root_in_trans will return errors in the future, so handle
the error properly in btrfs_rename_exchange.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/inode.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode
We will record the fs root or the reloc root in the trans in
select_reloc_root. These will actually return errors in the following
patches, so check their return value here and return it up the stack.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/relocation.c | 9 +++--
1
We have several BUG_ON()'s in select_reloc_root() that can be tripped if
you have extent tree corruption. Convert these to ASSERT()'s, because
if we hit it during testing it really is bad, or could indicate a
problem with the backref walking code.
However if users hit these problems it generally
Currently select_reloc_root() doesn't return an error, but followup
patches will make it possible for it to return an error. We do have
proper error recovery in do_relocation however, so handle the
possibility of select_reloc_root() having an error properly instead of
BUG_ON(!root). I've also adj
We can create a reloc root when we record the root in the trans, which
can fail for all sorts of different reasons. Propagate this error up
the chain of callers. Future patches will fix the callers of
btrfs_record_root_in_trans() to handle the error.
Reviewed-by: Qu Wenruo
Reviewed-by: Johannes
We have a couple of BUG_ON()'s in relocate_tree_block() that can be
tripped if we have file system corruption. Convert these to ASSERT()'s
so developers still get yelled at when they break the backref code, but
error out nicely for users so the whole box doesn't go down.
Reviewed-by: Qu Wenruo
S
v6->v7:
- Broke up the series into 3 series, 1 for cosmetic things, 1 for all the major
issues (including those reported on v6 of this set), and this new set which is
solely the error handling related patches for relocation. It's still a lot of
patches, sorry about that.
v5->v6:
- Reworked
Zygo reported the following panic when testing my error handling patches
for relocation
[ cut here ]
kernel BUG at fs/btrfs/backref.c:2545!
invalid opcode: [#1] SMP KASAN PTI CPU: 3 PID: 8472 Comm: btrfs Tainted: G
W 14
Hardware name: QEMU Standard PC (i440FX +
Qu pointed out a bug in one of my error handling patches, which made me
notice that we modify the new_root->highest_objectid _after_ we've
dropped the ref to the new_root. This could lead to a possible UAF, fix
this by modifying the ->highest_objectid before we drop our reference to
the new_root.
If we fail to update a block group item in the loop we'll break, however
we'll do btrfs_run_delayed_refs and lose our error value in ret, and
thus not clean up properly. Fix this by only running the delayed refs
if there was no failure.
Reviewed-by: Qu Wenruo
Reviewed-by: Johannes Thumshirn
Sig
While testing the error paths of relocation I hit the following lockdep
splat
==
WARNING: possible circular locking dependency detected
5.10.0-rc6+ #217 Not tainted
--
mount/779 is trying to acq
While doing error injection testing with my relocation patches I hit the
following ASSERT()
assertion failed: list_empty(&block_group->dirty_list), in
fs/btrfs/block-group.c:3356
[ cut here ]
kernel BUG at fs/btrfs/ctree.h:3357!
invalid opcode: [#1] SMP NOPTI
CPU: 0 P
Any number of things could have gone wrong, like ENOMEM or EIO, so don't
WARN_ON() if we're unable to find the reloc root in the backref code.
Reported-by: Zygo Blaxell
Signed-off-by: Josef Bacik
---
fs/btrfs/backref.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs
I hit a pretty ugly corruption when doing some error injection, it turns
out this is because we do not abort the transaction if we fail to adjust
the reference counts in btrfs_copy_root(). This makes us miss updating
the references for the new root that has already been allocated and
inserted, whi
A weird KASAN problem that Zygo reported could have been easily caught
if we checked for basic things in our backref freeing code. We have two
methods of freeing a backref node
- btrfs_backref_free_node: this just is kfree() essentially.
- btrfs_backref_drop_node: this actually unlinks the node a
Zygo reported the following KASAN splat
BUG: KASAN: use-after-free in btrfs_backref_cleanup_node+0x18a/0x420
Read of size 8 at addr 888112402950 by task btrfs/28836
CPU: 0 PID: 28836 Comm: btrfs Tainted: GW
5.10.0-e35f27394290-for-next+ #23
Hardware name: QEMU Standard PC (i4
When recovering a relocation, if we run into a reloc root that has 0
refs we simply add it to the reloc_control->reloc_roots list, and then
clean it up later. The problem with this is __del_reloc_root() doesn't
do anything if the root isn't in the radix tree, which in this case it
won't be because
While testing the error paths in relocation, I hit the following lockdep
splat
==
WARNING: possible circular locking dependency detected
5.10.0-rc3+ #206 Not tainted
--
btrfs-balance/1571 is try
On Wed, Nov 18, 2020 at 11:18:44AM -0800, Omar Sandoval wrote:
> From: Omar Sandoval
>
> The long options array for send is missing the zero terminator, so
> unknown options result in a crash:
>
> # btrfs send --foo
> Segmentation fault (core dumped)
>
> Signed-off-by: Omar Sandoval
As th
This was partially fixed by f3e3d9cc35252, however it missed a spot when
we restart a trans handle because we need to end the transaction. The
fix is the same, simply use btrfs_join_transaction() instead of
btrfs_start_transaction() when deleting reloc roots.
Signed-off-by: Josef Bacik
---
fs/b
While testing other things I was noticing that sometimes my VM would
fail to load the btrfs module because the self test failed like this
BTRFS: selftest: fs/btrfs/tests/inode-tests.c:963 miscount, wanted 1, got 0
This turned out to be because sometimes the btrfs ino would be the btree
inode numb
Hello,
A lot of these were in previous versions of the relocation error handling
patches. I added a few since the last go around. All of these do not rely on
the error handling patches, and some of them are quite important otherwise we
get corruption if we get errors in certain spots. There's a
I noticed that shared ref entries in ref-verify didn't have the proper
owner set, which caused me to think there was something seriously wrong.
However the problem is if we have a parent we simply weren't filling out
the owner part of the reference, even though we have it. Fix this by
making sure
I noticed that sometimes I would have the wrong level printed out with
ref-verify while testing some error injection related problems. This is
because we only get the level from the main extent item, but our
references could go off the current leaf into another, and at that point
we lose our level
We're supposed to print the root_key.offset in btrfs_root_name in the
case of a reloc root, not the objectid. Fix this helper to take the key
so we have access to the offset when we need it.
Reviewed-by: Qu Wenruo
Signed-off-by: Josef Bacik
---
fs/btrfs/disk-io.c| 2 +-
fs/btrfs/print-tre
I was attempting to reproduce a problem that Zygo hit, but my error
injection wasn't firing for a few of the common calls to
btrfs_should_cancel_balance. This is because the compiler decided to
inline it at these spots. Keep this from happening by explicitly
noinline'ing the function so that erro
The following patches are going to address error handling in relocation,
in order to test those patches I need to be able to inject errors in
btrfs_search_slot and btrfs_cow_block, as we call both of these pretty
often in different cases during relocation.
Reviewed-by: Qu Wenruo
Reviewed-by: Joha
Hello,
These patches were originally in my reloc error handling patches that have been
broken out on their own. They stand on their own and are simple and don't
affect the code in a real way. Simply fixing some cosmetic stuff, or allowing
error injection in certain places. They were patches I n
On Wed, Dec 16, 2020 at 6:52 PM Sidong Yang wrote:
>
> On Wed, Dec 16, 2020 at 02:30:04PM +0800, Su Yue wrote:
> > On Sat, Dec 12, 2020 at 3:04 AM David Sterba wrote:
> > >
> > > On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote:
> > > > On Fri, Dec 11, 2020 at 04:48:12PM +, Sidong
>>> Zygo Blaxell schrieb am 15.12.2020 um
>>> 19:18 in
Nachricht <20201215181828.gn31...@hungrycats.org>:
> On Fri, Dec 11, 2020 at 03:25:47PM +0100, Ulrich Windl wrote:
>> Hi!
>>
>> While configuring a VM environment in a cluster I had setup an SLES15 SP2
> test VM using BtrFS. Due to some pro
On Wed, Dec 16, 2020 at 02:30:04PM +0800, Su Yue wrote:
> On Sat, Dec 12, 2020 at 3:04 AM David Sterba wrote:
> >
> > On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote:
> > > On Fri, Dec 11, 2020 at 04:48:12PM +, Sidong Yang wrote:
> > > > Example json format:
> > > >
> > > > {
> >
99 matches
Mail list logo
