OSDN Git Service
Filipe Manana [Wed, 15 May 2019 15:03:17 +0000 (16:03 +0100)]
Btrfs: fix race updating log root item during fsync
When syncing the log, the final phase of a fsync operation, we need to
either create a log root's item or update the existing item in the log
tree of log roots, and that depends on the current value of the log
root's log_transid - if it's 1 we need to create the log root item,
otherwise it must exist already and we update it. Since there is no
synchronization between updating the log_transid and checking it for
deciding whether the log root's item needs to be created or updated, we
end up with a tiny race window that results in attempts to update the
item to fail because the item was not yet created:
CPU 1 CPU 2
btrfs_sync_log()
lock root->log_mutex
set log root's log_transid to 1
unlock root->log_mutex
btrfs_sync_log()
lock root->log_mutex
sets log root's
log_transid to 2
unlock root->log_mutex
update_log_root()
sees log root's log_transid
with a value of 2
calls btrfs_update_root(),
which fails with -EUCLEAN
and causes transaction abort
Until recently the race lead to a BUG_ON at btrfs_update_root(), but after
the recent commit
7ac1e464c4d47 ("btrfs: Don't panic when we can't find a
root key") we just abort the current transaction.
A sample trace of the BUG_ON() on a SLE12 kernel:
------------[ cut here ]------------
kernel BUG at ../fs/btrfs/root-tree.c:157!
Oops: Exception in kernel mode, sig: 5 [#1]
SMP NR_CPUS=2048 NUMA pSeries
(...)
Supported: Yes, External
CPU: 78 PID: 76303 Comm: rtas_errd Tainted: G X 4.4.156-94.57-default #1
task:
c00000ffa906d010 ti:
c00000ff42b08000 task.ti:
c00000ff42b08000
NIP:
d000000036ae5cdc LR:
d000000036ae5cd8 CTR:
0000000000000000
REGS:
c00000ff42b0b860 TRAP: 0700 Tainted: G X (4.4.156-94.57-default)
MSR:
8000000002029033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR:
22444484 XER:
20000000
CFAR:
d000000036aba66c SOFTE: 1
GPR00:
d000000036ae5cd8 c00000ff42b0bae0 d000000036bda220 0000000000000054
GPR04:
0000000000000001 0000000000000000 c00007ffff8d37c8 0000000000000000
GPR08:
c000000000e19c00 0000000000000000 0000000000000000 3736343438312079
GPR12:
3930373337303434 c000000007a3a800 00000000007fffff 0000000000000023
GPR16:
c00000ffa9d26028 c00000ffa9d261f8 0000000000000010 c00000ffa9d2ab28
GPR20:
c00000ff42b0bc48 0000000000000001 c00000ff9f0d9888 0000000000000001
GPR24:
c00000ffa9d26000 c00000ffa9d261e8 c00000ffa9d2a800 c00000ff9f0d9888
GPR28:
c00000ffa9d26028 c00000ffa9d2aa98 0000000000000001 c00000ffa98f5b20
NIP [
d000000036ae5cdc] btrfs_update_root+0x25c/0x4e0 [btrfs]
LR [
d000000036ae5cd8] btrfs_update_root+0x258/0x4e0 [btrfs]
Call Trace:
[
c00000ff42b0bae0] [
d000000036ae5cd8] btrfs_update_root+0x258/0x4e0 [btrfs] (unreliable)
[
c00000ff42b0bba0] [
d000000036b53610] btrfs_sync_log+0x2d0/0xc60 [btrfs]
[
c00000ff42b0bce0] [
d000000036b1785c] btrfs_sync_file+0x44c/0x4e0 [btrfs]
[
c00000ff42b0bd80] [
c00000000032e300] vfs_fsync_range+0x70/0x120
[
c00000ff42b0bdd0] [
c00000000032e44c] do_fsync+0x5c/0xb0
[
c00000ff42b0be10] [
c00000000032e8dc] SyS_fdatasync+0x2c/0x40
[
c00000ff42b0be30] [
c000000000009488] system_call+0x3c/0x100
Instruction dump:
7f43d378 4bffebb9 60000000 88d90008 3d220000 e8b90000 3b390009 e87a01f0
e8898e08 e8f90000 4bfd48e5 60000000 <
0fe00000>
e95b0060 39200004 394a0ea0
---[ end trace
8f2dc8f919cabab8 ]---
So fix this by doing the check of log_transid and updating or creating the
log root's item while holding the root's log_mutex.
Fixes:
7237f1833601d ("Btrfs: fix tree logs parallel sync")
CC: stable@vger.kernel.org # 4.4+
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Wed, 15 May 2019 15:02:47 +0000 (16:02 +0100)]
Btrfs: fix wrong ctime and mtime of a directory after log replay
When replaying a log that contains a new file or directory name that needs
to be added to its parent directory, we end up updating the mtime and the
ctime of the parent directory to the current time after we have set their
values to the correct ones (set at fsync time), efectivelly losing them.
Sample reproducer:
$ mkfs.btrfs -f /dev/sdb
$ mount /dev/sdb /mnt
$ mkdir /mnt/dir
$ touch /mnt/dir/file
# fsync of the directory is optional, not needed
$ xfs_io -c fsync /mnt/dir
$ xfs_io -c fsync /mnt/dir/file
$ stat -c %Y /mnt/dir
1557856079
<power failure>
$ sleep 3
$ mount /dev/sdb /mnt
$ stat -c %Y /mnt/dir
1557856082
--> should have been
1557856079, the mtime is updated to the current
time when replaying the log
Fix this by not updating the mtime and ctime to the current time at
btrfs_add_link() when we are replaying a log tree.
This could be triggered by my recent fsync fuzz tester for fstests, for
which an fstests patch exists titled "fstests: generic, fsync fuzz tester
with fsstress".
Fixes:
e02119d5a7b43 ("Btrfs: Add a write ahead tree log to optimize synchronous operations")
CC: stable@vger.kernel.org # 4.4+
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Thu, 16 May 2019 14:48:55 +0000 (15:48 +0100)]
Btrfs: fix fsync not persisting changed attributes of a directory
While logging an inode we follow its ancestors and for each one we mark
it as logged in the current transaction, even if we have not logged it.
As a consequence if we change an attribute of an ancestor, such as the
UID or GID for example, and then explicitly fsync it, we end up not
logging the inode at all despite returning success to user space, which
results in the attribute being lost if a power failure happens after
the fsync.
Sample reproducer:
$ mkfs.btrfs -f /dev/sdb
$ mount /dev/sdb /mnt
$ mkdir /mnt/dir
$ chown 6007:6007 /mnt/dir
$ sync
$ chown 9003:9003 /mnt/dir
$ touch /mnt/dir/file
$ xfs_io -c fsync /mnt/dir/file
# fsync our directory after fsync'ing the new file, should persist the
# new values for the uid and gid.
$ xfs_io -c fsync /mnt/dir
<power failure>
$ mount /dev/sdb /mnt
$ stat -c %u:%g /mnt/dir
6007:6007
--> should be 9003:9003, the uid and gid were not persisted, despite
the explicit fsync on the directory prior to the power failure
Fix this by not updating the logged_trans field of ancestor inodes when
logging an inode, since we have not logged them. Let only future calls to
btrfs_log_inode() to mark inodes as logged.
This could be triggered by my recent fsync fuzz tester for fstests, for
which an fstests patch exists titled "fstests: generic, fsync fuzz tester
with fsstress".
Fixes:
12fcfd22fe5b ("Btrfs: tree logging unlink/rename fixes")
CC: stable@vger.kernel.org # 4.4+
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Tue, 21 May 2019 11:28:08 +0000 (19:28 +0800)]
btrfs: qgroup: Check bg while resuming relocation to avoid NULL pointer dereference
[BUG]
When mounting a fs with reloc tree and has qgroup enabled, it can cause
NULL pointer dereference at mount time:
BUG: kernel NULL pointer dereference, address:
00000000000000a8
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: 0000 [#1] PREEMPT SMP NOPTI
RIP: 0010:btrfs_qgroup_add_swapped_blocks+0x186/0x300 [btrfs]
Call Trace:
replace_path.isra.23+0x685/0x900 [btrfs]
merge_reloc_root+0x26e/0x5f0 [btrfs]
merge_reloc_roots+0x10a/0x1a0 [btrfs]
btrfs_recover_relocation+0x3cd/0x420 [btrfs]
open_ctree+0x1bc8/0x1ed0 [btrfs]
btrfs_mount_root+0x544/0x680 [btrfs]
legacy_get_tree+0x34/0x60
vfs_get_tree+0x2d/0xf0
fc_mount+0x12/0x40
vfs_kern_mount.part.12+0x61/0xa0
vfs_kern_mount+0x13/0x20
btrfs_mount+0x16f/0x860 [btrfs]
legacy_get_tree+0x34/0x60
vfs_get_tree+0x2d/0xf0
do_mount+0x81f/0xac0
ksys_mount+0xbf/0xe0
__x64_sys_mount+0x25/0x30
do_syscall_64+0x65/0x240
entry_SYSCALL_64_after_hwframe+0x49/0xbe
[CAUSE]
In btrfs_recover_relocation(), we don't have enough info to determine
which block group we're relocating, but only to merge existing reloc
trees.
Thus in btrfs_recover_relocation(), rc->block_group is NULL.
btrfs_qgroup_add_swapped_blocks() hasn't taken this into consideration,
and causes a NULL pointer dereference.
The bug is introduced by commit
3d0174f78e72 ("btrfs: qgroup: Only trace
data extents in leaves if we're relocating data block group"), and
later qgroup refactoring still keeps this optimization.
[FIX]
Thankfully in the context of btrfs_recover_relocation(), there is no
other progress can modify tree blocks, thus those swapped tree blocks
pair will never affect qgroup numbers, no matter whatever we set for
block->trace_leaf.
So we only need to check if @bg is NULL before accessing @bg->flags.
Reported-by: Juan Erbes <jerbes@gmail.com>
Link: https://bugzilla.opensuse.org/show_bug.cgi?id=1134806
Fixes:
3d0174f78e72 ("btrfs: qgroup: Only trace data extents in leaves if we're relocating data block group")
CC: stable@vger.kernel.org # 4.20+
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Wed, 22 May 2019 08:33:11 +0000 (16:33 +0800)]
btrfs: reloc: Also queue orphan reloc tree for cleanup to avoid BUG_ON()
[BUG]
When a fs has orphan reloc tree along with unfinished balance:
...
item 16 key (TREE_RELOC ROOT_ITEM FS_TREE) itemoff 12090 itemsize 439
generation 12 root_dirid 256 bytenr
300400640 level 1 refs 0 <<<
lastsnap 8 byte_limit 0 bytes_used
1359872 flags 0x0(none)
uuid
7c48d938-33a3-4aae-ab19-
6e5c9d406e46
item 17 key (BALANCE TEMPORARY_ITEM 0) itemoff 11642 itemsize 448
temporary item objectid BALANCE offset 0
balance status flags 14
Then at mount time, we can hit the following kernel BUG_ON():
BTRFS info (device dm-3): relocating block group
298844160 flags metadata|dup
------------[ cut here ]------------
kernel BUG at fs/btrfs/relocation.c:1413!
invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
CPU: 1 PID: 897 Comm: btrfs-balance Tainted: G O 5.2.0-rc1-custom #15
RIP: 0010:create_reloc_root+0x1eb/0x200 [btrfs]
Call Trace:
btrfs_init_reloc_root+0x96/0xb0 [btrfs]
record_root_in_trans+0xb2/0xe0 [btrfs]
btrfs_record_root_in_trans+0x55/0x70 [btrfs]
select_reloc_root+0x7e/0x230 [btrfs]
do_relocation+0xc4/0x620 [btrfs]
relocate_tree_blocks+0x592/0x6a0 [btrfs]
relocate_block_group+0x47b/0x5d0 [btrfs]
btrfs_relocate_block_group+0x183/0x2f0 [btrfs]
btrfs_relocate_chunk+0x4e/0xe0 [btrfs]
btrfs_balance+0x864/0xfa0 [btrfs]
balance_kthread+0x3b/0x50 [btrfs]
kthread+0x123/0x140
ret_from_fork+0x27/0x50
[CAUSE]
In btrfs, reloc trees are used to record swapped tree blocks during
balance.
Reloc tree either get merged (replace old tree blocks of its parent
subvolume) in next transaction if its ref is 1 (fresh).
Or is already merged and will be cleaned up if its ref is 0 (orphan).
After commit
d2311e698578 ("btrfs: relocation: Delay reloc tree deletion
after merge_reloc_roots"), reloc tree cleanup is delayed until one block
group is balanced.
Since fresh reloc roots are recorded during merge, as long as there
is no power loss, those orphan reloc roots converted from fresh ones are
handled without problem.
However when power loss happens, orphan reloc roots can be recorded
on-disk, thus at next mount time, we will have orphan reloc roots from
on-disk data directly, and ignored by clean_dirty_subvols() routine.
Then when background balance starts to balance another block group, and
needs to create new reloc root for the same root, btrfs_insert_item()
returns -EEXIST, and trigger that BUG_ON().
[FIX]
For orphan reloc roots, also queue them to rc->dirty_subvol_roots, so
all reloc roots no matter orphan or not, can be cleaned up properly and
avoid above BUG_ON().
And to cooperate with above change, clean_dirty_subvols() will check if
the queued root is a reloc root or a subvol root.
For a subvol root, do the old work, and for a orphan reloc root, clean it
up.
Fixes:
d2311e698578 ("btrfs: relocation: Delay reloc tree deletion after merge_reloc_roots")
CC: stable@vger.kernel.org # 5.1
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Mon, 20 May 2019 08:57:00 +0000 (09:57 +0100)]
Btrfs: incremental send, fix emission of invalid clone operations
When doing an incremental send we can now issue clone operations with a
source range that ends at the source's file eof and with a destination
range that ends at an offset smaller then the destination's file eof.
If the eof of the source file is not aligned to the sector size of the
filesystem, the receiver will get a -EINVAL error when trying to do the
operation or, on older kernels, silently corrupt the destination file.
The corruption happens on kernels without commit
ac765f83f1397646
("Btrfs: fix data corruption due to cloning of eof block"), while the
failure to clone happens on kernels with that commit.
Example reproducer:
$ mkfs.btrfs -f /dev/sdb
$ mount /dev/sdb /mnt/sdb
$ xfs_io -f -c "pwrite -S 0xb1 0 2M" /mnt/sdb/foo
$ xfs_io -f -c "pwrite -S 0xc7 0 2M" /mnt/sdb/bar
$ xfs_io -f -c "pwrite -S 0x4d 0 2M" /mnt/sdb/baz
$ xfs_io -f -c "pwrite -S 0xe2 0 2M" /mnt/sdb/zoo
$ btrfs subvolume snapshot -r /mnt/sdb /mnt/sdb/base
$ btrfs send -f /tmp/base.send /mnt/sdb/base
$ xfs_io -c "reflink /mnt/sdb/bar 1560K 500K 100K" /mnt/sdb/bar
$ xfs_io -c "reflink /mnt/sdb/bar 1560K 0 100K" /mnt/sdb/zoo
$ xfs_io -c "truncate 550K" /mnt/sdb/bar
$ btrfs subvolume snapshot -r /mnt/sdb /mnt/sdb/incr
$ btrfs send -f /tmp/incr.send -p /mnt/sdb/base /mnt/sdb/incr
$ mkfs.btrfs -f /dev/sdc
$ mount /dev/sdc /mnt/sdc
$ btrfs receive -f /tmp/base.send /mnt/sdc
$ btrfs receive -vv -f /tmp/incr.send /mnt/sdc
(...)
truncate bar size=563200
utimes bar
clone zoo - source=bar source offset=512000 offset=0 length=51200
ERROR: failed to clone extents to zoo
Invalid argument
The failure happens because the clone source range ends at the eof of file
bar, 563200, which is not aligned to the filesystems sector size (4Kb in
this case), and the destination range ends at offset 0 + 51200, which is
less then the size of the file zoo (2Mb).
So fix this by detecting such case and instead of issuing a clone
operation for the whole range, do a clone operation for smaller range
that is sector size aligned followed by a write operation for the block
containing the eof. Here we will always be pessimistic and assume the
destination filesystem of the send stream has the largest possible sector
size (64Kb), since we have no way of determining it.
This fixes a recent regression introduced in kernel 5.2-rc1.
Fixes:
040ee6120cb6706 ("Btrfs: send, improve clone range")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Mon, 20 May 2019 08:55:42 +0000 (09:55 +0100)]
Btrfs: incremental send, fix file corruption when no-holes feature is enabled
When using the no-holes feature, if we have a file with prealloc extents
with a start offset beyond the file's eof, doing an incremental send can
cause corruption of the file due to incorrect hole detection. Such case
requires that the prealloc extent(s) exist in both the parent and send
snapshots, and that a hole is punched into the file that covers all its
extents that do not cross the eof boundary.
Example reproducer:
$ mkfs.btrfs -f -O no-holes /dev/sdb
$ mount /dev/sdb /mnt/sdb
$ xfs_io -f -c "pwrite -S 0xab 0 500K" /mnt/sdb/foobar
$ xfs_io -c "falloc -k 1200K 800K" /mnt/sdb/foobar
$ btrfs subvolume snapshot -r /mnt/sdb /mnt/sdb/base
$ btrfs send -f /tmp/base.snap /mnt/sdb/base
$ xfs_io -c "fpunch 0 500K" /mnt/sdb/foobar
$ btrfs subvolume snapshot -r /mnt/sdb /mnt/sdb/incr
$ btrfs send -p /mnt/sdb/base -f /tmp/incr.snap /mnt/sdb/incr
$ md5sum /mnt/sdb/incr/foobar
816df6f64deba63b029ca19d880ee10a /mnt/sdb/incr/foobar
$ mkfs.btrfs -f /dev/sdc
$ mount /dev/sdc /mnt/sdc
$ btrfs receive -f /tmp/base.snap /mnt/sdc
$ btrfs receive -f /tmp/incr.snap /mnt/sdc
$ md5sum /mnt/sdc/incr/foobar
cf2ef71f4a9e90c2f6013ba3b2257ed2 /mnt/sdc/incr/foobar
--> Different checksum, because the prealloc extent beyond the
file's eof confused the hole detection code and it assumed
a hole starting at offset 0 and ending at the offset of the
prealloc extent (1200Kb) instead of ending at the offset
500Kb (the file's size).
Fix this by ensuring we never cross the file's size when issuing the
write operations for a hole.
Fixes:
16e7549f045d33 ("Btrfs: incompatible format change to remove hole extents")
CC: stable@vger.kernel.org # 3.14+
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Dennis Zhou [Fri, 17 May 2019 23:16:26 +0000 (19:16 -0400)]
btrfs: correct zstd workspace manager lock to use spin_lock_bh()
The btrfs zstd workspace manager uses a background timer to reclaim not
recently used workspaces. I used spin_lock() from this context which
should have been caught with lockdep, but was not. This deadlock was
reported in bugzilla. The fix is to switch the zstd wsm lock to use
spin_lock_bh() from the softirq context.
This happened quite relibably on ppc64, unlike on other architectures.
[ 313.402874] ================================
[ 313.402875] WARNING: inconsistent lock state
[ 313.402879] 5.1.0-rc7 #1 Not tainted
[ 313.402880] --------------------------------
[ 313.402882] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
[ 313.402885] swapper/5/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
[ 313.402888]
0000000080d1120c (&(&wsm.lock)->rlock){+.?.}, at: .zstd_reclaim_timer_fn+0x40/0x230
[ 313.402895] {SOFTIRQ-ON-W} state was registered at:
[ 313.402899] .lock_acquire+0xd0/0x240
[ 313.402903] ._raw_spin_lock+0x34/0x60
[ 313.402906] .zstd_get_workspace+0xd0/0x360
[ 313.402908] .end_compressed_bio_read+0x3b8/0x540
[ 313.402911] .bio_endio+0x174/0x2c0
[ 313.402914] .end_workqueue_fn+0x4c/0x70
[ 313.402917] .normal_work_helper+0x138/0x7e0
[ 313.402920] .process_one_work+0x324/0x790
[ 313.402922] .worker_thread+0x68/0x570
[ 313.402925] .kthread+0x19c/0x1b0
[ 313.402928] .ret_from_kernel_thread+0x58/0x78
[ 313.402930] irq event stamp:
2629216
[ 313.402933] hardirqs last enabled at (
2629216): [<
c0000000009da738>] ._raw_spin_unlock_irq+0x38/0x60
[ 313.402936] hardirqs last disabled at (
2629215): [<
c0000000009da4c4>] ._raw_spin_lock_irq+0x24/0x70
[ 313.402939] softirqs last enabled at (
2629212): [<
c0000000000af9fc>] .irq_enter+0x8c/0xd0
[ 313.402942] softirqs last disabled at (
2629213): [<
c0000000000afb58>] .irq_exit+0x118/0x170
[ 313.402944]
other info that might help us debug this:
[ 313.402945] Possible unsafe locking scenario:
[ 313.402947] CPU0
[ 313.402948] ----
[ 313.402949] lock(&(&wsm.lock)->rlock);
[ 313.402951] <Interrupt>
[ 313.402952] lock(&(&wsm.lock)->rlock);
[ 313.402954]
*** DEADLOCK ***
[ 313.402957] 1 lock held by swapper/5/0:
[ 313.402958] #0:
000000004b612042 ((&wsm.timer)){+.-.}, at: .call_timer_fn+0x0/0x3c0
[ 313.402963]
stack backtrace:
[ 313.402967] CPU: 5 PID: 0 Comm: swapper/5 Not tainted 5.1.0-rc7 #1
[ 313.402968] Call Trace:
[ 313.402972] [
c0000007fa262e70] [
c0000000009b3294] .dump_stack+0xe0/0x15c (unreliable)
[ 313.402975] [
c0000007fa262f10] [
c000000000125548] .print_usage_bug+0x348/0x390
[ 313.402978] [
c0000007fa262fd0] [
c000000000125cb4] .mark_lock+0x724/0x930
[ 313.402981] [
c0000007fa263080] [
c000000000126c20] .__lock_acquire+0xc90/0x16a0
[ 313.402984] [
c0000007fa2631b0] [
c000000000128040] .lock_acquire+0xd0/0x240
[ 313.402987] [
c0000007fa263280] [
c0000000009da2b4] ._raw_spin_lock+0x34/0x60
[ 313.402990] [
c0000007fa263300] [
c00000000054b0b0] .zstd_reclaim_timer_fn+0x40/0x230
[ 313.402993] [
c0000007fa2633d0] [
c000000000158b38] .call_timer_fn+0xc8/0x3c0
[ 313.402996] [
c0000007fa2634a0] [
c000000000158f74] .expire_timers+0x144/0x260
[ 313.402999] [
c0000007fa263550] [
c000000000159178] .run_timer_softirq+0xe8/0x230
[ 313.403002] [
c0000007fa263680] [
c0000000009db288] .__do_softirq+0x188/0x5d4
[ 313.403004] [
c0000007fa263790] [
c0000000000afb58] .irq_exit+0x118/0x170
[ 313.403008] [
c0000007fa263800] [
c000000000028d88] .timer_interrupt+0x158/0x430
[ 313.403012] [
c0000007fa2638b0] [
c0000000000091d4] decrementer_common+0x134/0x140
[ 313.403017] --- interrupt: 901 at replay_interrupt_return+0x0/0x4
LR = .arch_local_irq_restore.part.0+0x68/0x80
[ 313.403020] [
c0000007fa263bb0] [
c00000000001a3ac] .arch_local_irq_restore.part.0+0x2c/0x80 (unreliable)
[ 313.403024] [
c0000007fa263c30] [
c0000000007bbbcc] .cpuidle_enter_state+0xec/0x670
[ 313.403027] [
c0000007fa263d00] [
c0000000000f5130] .call_cpuidle+0x40/0x90
[ 313.403031] [
c0000007fa263d70] [
c0000000000f554c] .do_idle+0x2dc/0x3a0
[ 313.403034] [
c0000007fa263e30] [
c0000000000f59ac] .cpu_startup_entry+0x2c/0x30
[ 313.403037] [
c0000007fa263ea0] [
c000000000045674] .start_secondary+0x644/0x650
[ 313.403041] [
c0000007fa263f90] [
c00000000000ad5c] start_secondary_prolog+0x10/0x14
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203517
Fixes:
3f93aef535c8 ("btrfs: add zstd compression level support")
CC: stable@vger.kernel.org # 5.1+
Signed-off-by: Dennis Zhou <dennis@kernel.org>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Fri, 17 May 2019 07:44:25 +0000 (10:44 +0300)]
btrfs: Ensure replaced device doesn't have pending chunk allocation
Recent FITRIM work, namely
bbbf7243d62d ("btrfs: combine device update
operations during transaction commit") combined the way certain
operations are recoded in a transaction. As a result an ASSERT was added
in dev_replace_finish to ensure the new code works correctly.
Unfortunately I got reports that it's possible to trigger the assert,
meaning that during a device replace it's possible to have an unfinished
chunk allocation on the source device.
This is supposed to be prevented by the fact that a transaction is
committed before finishing the replace oepration and alter acquiring the
chunk mutex. This is not sufficient since by the time the transaction is
committed and the chunk mutex acquired it's possible to allocate a chunk
depending on the workload being executed on the replaced device. This
bug has been present ever since device replace was introduced but there
was never code which checks for it.
The correct way to fix is to ensure that there is no pending device
modification operation when the chunk mutex is acquire and if there is
repeat transaction commit. Unfortunately it's not possible to just
exclude the source device from btrfs_fs_devices::dev_alloc_list since
this causes ENOSPC to be hit in transaction commit.
Fixing that in another way would need to add special cases to handle the
last writes and forbid new ones. The looped transaction fix is more
obvious, and can be easily backported. The runtime of dev-replace is
long so there's no noticeable delay caused by that.
Reported-by: David Sterba <dsterba@suse.com>
Fixes:
391cd9df81ac ("Btrfs: fix unprotected alloc list insertion during the finishing procedure of replace")
CC: stable@vger.kernel.org # 4.4+
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Mon, 6 May 2019 15:44:12 +0000 (16:44 +0100)]
Btrfs: tree-checker: detect file extent items with overlapping ranges
Having file extent items with ranges that overlap each other is a
serious issue that leads to all sorts of corruptions and crashes (like a
BUG_ON() during the course of __btrfs_drop_extents() when it traims file
extent items). Therefore teach the tree checker to detect such cases.
This is motivated by a recently fixed bug (race between ranged full
fsync and writeback or adjacent ranges).
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Mon, 6 May 2019 15:44:02 +0000 (16:44 +0100)]
Btrfs: fix race between ranged fsync and writeback of adjacent ranges
When we do a full fsync (the bit BTRFS_INODE_NEEDS_FULL_SYNC is set in the
inode) that happens to be ranged, which happens during a msync() or writes
for files opened with O_SYNC for example, we can end up with a corrupt log,
due to different file extent items representing ranges that overlap with
each other, or hit some assertion failures.
When doing a ranged fsync we only flush delalloc and wait for ordered
exents within that range. If while we are logging items from our inode
ordered extents for adjacent ranges complete, we end up in a race that can
make us insert the file extent items that overlap with others we logged
previously and the assertion failures.
For example, if tree-log.c:copy_items() receives a leaf that has the
following file extents items, all with a length of 4K and therefore there
is an implicit hole in the range 68K to 72K - 1:
(257 EXTENT_ITEM 64K), (257 EXTENT_ITEM 72K), (257 EXTENT_ITEM 76K), ...
It copies them to the log tree. However due to the need to detect implicit
holes, it may release the path, in order to look at the previous leaf to
detect an implicit hole, and then later it will search again in the tree
for the first file extent item key, with the goal of locking again the
leaf (which might have changed due to concurrent changes to other inodes).
However when it locks again the leaf containing the first key, the key
corresponding to the extent at offset 72K may not be there anymore since
there is an ordered extent for that range that is finishing (that is,
somewhere in the middle of btrfs_finish_ordered_io()), and it just
removed the file extent item but has not yet replaced it with a new file
extent item, so the part of copy_items() that does hole detection will
decide that there is a hole in the range starting from 68K to 76K - 1,
and therefore insert a file extent item to represent that hole, having
a key offset of 68K. After that we now have a log tree with 2 different
extent items that have overlapping ranges:
1) The file extent item copied before copy_items() released the path,
which has a key offset of 72K and a length of 4K, representing the
file range 72K to 76K - 1.
2) And a file extent item representing a hole that has a key offset of
68K and a length of 8K, representing the range 68K to 76K - 1. This
item was inserted after releasing the path, and overlaps with the
extent item inserted before.
The overlapping extent items can cause all sorts of unpredictable and
incorrect behaviour, either when replayed or if a fast (non full) fsync
happens later, which can trigger a BUG_ON() when calling
btrfs_set_item_key_safe() through __btrfs_drop_extents(), producing a
trace like the following:
[61666.783269] ------------[ cut here ]------------
[61666.783943] kernel BUG at fs/btrfs/ctree.c:3182!
[61666.784644] invalid opcode: 0000 [#1] PREEMPT SMP
(...)
[61666.786253] task:
ffff880117b88c40 task.stack:
ffffc90008168000
[61666.786253] RIP: 0010:btrfs_set_item_key_safe+0x7c/0xd2 [btrfs]
[61666.786253] RSP: 0018:
ffffc9000816b958 EFLAGS:
00010246
[61666.786253] RAX:
0000000000000000 RBX:
000000000000000f RCX:
0000000000030000
[61666.786253] RDX:
0000000000000000 RSI:
ffffc9000816ba4f RDI:
ffffc9000816b937
[61666.786253] RBP:
ffffc9000816b998 R08:
ffff88011dae2428 R09:
0000000000001000
[61666.786253] R10:
0000160000000000 R11:
6db6db6db6db6db7 R12:
ffff88011dae2418
[61666.786253] R13:
ffffc9000816ba4f R14:
ffff8801e10c4118 R15:
ffff8801e715c000
[61666.786253] FS:
00007f6060a18700(0000) GS:
ffff88023f5c0000(0000) knlGS:
0000000000000000
[61666.786253] CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
[61666.786253] CR2:
00007f6060a28000 CR3:
0000000213e69000 CR4:
00000000000006e0
[61666.786253] Call Trace:
[61666.786253] __btrfs_drop_extents+0x5e3/0xaad [btrfs]
[61666.786253] ? time_hardirqs_on+0x9/0x14
[61666.786253] btrfs_log_changed_extents+0x294/0x4e0 [btrfs]
[61666.786253] ? release_extent_buffer+0x38/0xb4 [btrfs]
[61666.786253] btrfs_log_inode+0xb6e/0xcdc [btrfs]
[61666.786253] ? lock_acquire+0x131/0x1c5
[61666.786253] ? btrfs_log_inode_parent+0xee/0x659 [btrfs]
[61666.786253] ? arch_local_irq_save+0x9/0xc
[61666.786253] ? btrfs_log_inode_parent+0x1f5/0x659 [btrfs]
[61666.786253] btrfs_log_inode_parent+0x223/0x659 [btrfs]
[61666.786253] ? arch_local_irq_save+0x9/0xc
[61666.786253] ? lockref_get_not_zero+0x2c/0x34
[61666.786253] ? rcu_read_unlock+0x3e/0x5d
[61666.786253] btrfs_log_dentry_safe+0x60/0x7b [btrfs]
[61666.786253] btrfs_sync_file+0x317/0x42c [btrfs]
[61666.786253] vfs_fsync_range+0x8c/0x9e
[61666.786253] SyS_msync+0x13c/0x1c9
[61666.786253] entry_SYSCALL_64_fastpath+0x18/0xad
A sample of a corrupt log tree leaf with overlapping extents I got from
running btrfs/072:
item 14 key (295 108 200704) itemoff 2599 itemsize 53
extent data disk bytenr 0 nr 0
extent data offset 0 nr 458752 ram 458752
item 15 key (295 108 659456) itemoff 2546 itemsize 53
extent data disk bytenr
4343541760 nr 770048
extent data offset 606208 nr 163840 ram 770048
item 16 key (295 108 663552) itemoff 2493 itemsize 53
extent data disk bytenr
4343541760 nr 770048
extent data offset 610304 nr 155648 ram 770048
item 17 key (295 108 819200) itemoff 2440 itemsize 53
extent data disk bytenr
4334788608 nr 4096
extent data offset 0 nr 4096 ram 4096
The file extent item at offset 659456 (item 15) ends at offset 823296
(659456 + 163840) while the next file extent item (item 16) starts at
offset 663552.
Another different problem that the race can trigger is a failure in the
assertions at tree-log.c:copy_items(), which expect that the first file
extent item key we found before releasing the path exists after we have
released path and that the last key we found before releasing the path
also exists after releasing the path:
$ cat -n fs/btrfs/tree-log.c
4080 if (need_find_last_extent) {
4081 /* btrfs_prev_leaf could return 1 without releasing the path */
4082 btrfs_release_path(src_path);
4083 ret = btrfs_search_slot(NULL, inode->root, &first_key,
4084 src_path, 0, 0);
4085 if (ret < 0)
4086 return ret;
4087 ASSERT(ret == 0);
(...)
4103 if (i >= btrfs_header_nritems(src_path->nodes[0])) {
4104 ret = btrfs_next_leaf(inode->root, src_path);
4105 if (ret < 0)
4106 return ret;
4107 ASSERT(ret == 0);
4108 src = src_path->nodes[0];
4109 i = 0;
4110 need_find_last_extent = true;
4111 }
(...)
The second assertion implicitly expects that the last key before the path
release still exists, because the surrounding while loop only stops after
we have found that key. When this assertion fails it produces a stack like
this:
[139590.037075] assertion failed: ret == 0, file: fs/btrfs/tree-log.c, line: 4107
[139590.037406] ------------[ cut here ]------------
[139590.037707] kernel BUG at fs/btrfs/ctree.h:3546!
[139590.038034] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC PTI
[139590.038340] CPU: 1 PID: 31841 Comm: fsstress Tainted: G W 5.0.0-btrfs-next-46 #1
(...)
[139590.039354] RIP: 0010:assfail.constprop.24+0x18/0x1a [btrfs]
(...)
[139590.040397] RSP: 0018:
ffffa27f48f2b9b0 EFLAGS:
00010282
[139590.040730] RAX:
0000000000000041 RBX:
ffff897c635d92c8 RCX:
0000000000000000
[139590.041105] RDX:
0000000000000000 RSI:
ffff897d36a96868 RDI:
ffff897d36a96868
[139590.041470] RBP:
ffff897d1b9a0708 R08:
0000000000000000 R09:
0000000000000000
[139590.041815] R10:
0000000000000008 R11:
0000000000000000 R12:
0000000000000013
[139590.042159] R13:
0000000000000227 R14:
ffff897cffcbba88 R15:
0000000000000001
[139590.042501] FS:
00007f2efc8dee80(0000) GS:
ffff897d36a80000(0000) knlGS:
0000000000000000
[139590.042847] CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
[139590.043199] CR2:
00007f8c064935e0 CR3:
0000000232252002 CR4:
00000000003606e0
[139590.043547] DR0:
0000000000000000 DR1:
0000000000000000 DR2:
0000000000000000
[139590.043899] DR3:
0000000000000000 DR6:
00000000fffe0ff0 DR7:
0000000000000400
[139590.044250] Call Trace:
[139590.044631] copy_items+0xa3f/0x1000 [btrfs]
[139590.045009] ? generic_bin_search.constprop.32+0x61/0x200 [btrfs]
[139590.045396] btrfs_log_inode+0x7b3/0xd70 [btrfs]
[139590.045773] btrfs_log_inode_parent+0x2b3/0xce0 [btrfs]
[139590.046143] ? do_raw_spin_unlock+0x49/0xc0
[139590.046510] btrfs_log_dentry_safe+0x4a/0x70 [btrfs]
[139590.046872] btrfs_sync_file+0x3b6/0x440 [btrfs]
[139590.047243] btrfs_file_write_iter+0x45b/0x5c0 [btrfs]
[139590.047592] __vfs_write+0x129/0x1c0
[139590.047932] vfs_write+0xc2/0x1b0
[139590.048270] ksys_write+0x55/0xc0
[139590.048608] do_syscall_64+0x60/0x1b0
[139590.048946] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[139590.049287] RIP: 0033:0x7f2efc4be190
(...)
[139590.050342] RSP: 002b:
00007ffe743243a8 EFLAGS:
00000246 ORIG_RAX:
0000000000000001
[139590.050701] RAX:
ffffffffffffffda RBX:
0000000000008d58 RCX:
00007f2efc4be190
[139590.051067] RDX:
0000000000008d58 RSI:
00005567eca0f370 RDI:
0000000000000003
[139590.051459] RBP:
0000000000000024 R08:
0000000000000003 R09:
0000000000008d60
[139590.051863] R10:
0000000000000078 R11:
0000000000000246 R12:
0000000000000003
[139590.052252] R13:
00000000003d3507 R14:
00005567eca0f370 R15:
0000000000000000
(...)
[139590.055128] ---[ end trace
193f35d0215cdeeb ]---
So fix this race between a full ranged fsync and writeback of adjacent
ranges by flushing all delalloc and waiting for all ordered extents to
complete before logging the inode. This is the simplest way to solve the
problem because currently the full fsync path does not deal with ranges
at all (it assumes a full range from 0 to LLONG_MAX) and it always needs
to look at adjacent ranges for hole detection. For use cases of ranged
fsyncs this can make a few fsyncs slower but on the other hand it can
make some following fsyncs to other ranges do less work or no need to do
anything at all. A full fsync is rare anyway and happens only once after
loading/creating an inode and once after less common operations such as a
shrinking truncate.
This is an issue that exists for a long time, and was often triggered by
generic/127, because it does mmap'ed writes and msync (which triggers a
ranged fsync). Adding support for the tree checker to detect overlapping
extents (next patch in the series) and trigger a WARN() when such cases
are found, and then calling btrfs_check_leaf_full() at the end of
btrfs_insert_file_extent() made the issue much easier to detect. Running
btrfs/072 with that change to the tree checker and making fsstress open
files always with O_SYNC made it much easier to trigger the issue (as
triggering it with generic/127 is very rare).
CC: stable@vger.kernel.org # 3.16+
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Mon, 6 May 2019 15:43:51 +0000 (16:43 +0100)]
Btrfs: avoid fallback to transaction commit during fsync of files with holes
When we are doing a full fsync (bit BTRFS_INODE_NEEDS_FULL_SYNC set) of a
file that has holes and has file extent items spanning two or more leafs,
we can end up falling to back to a full transaction commit due to a logic
bug that leads to failure to insert a duplicate file extent item that is
meant to represent a hole between the last file extent item of a leaf and
the first file extent item in the next leaf. The failure (EEXIST error)
leads to a transaction commit (as most errors when logging an inode do).
For example, we have the two following leafs:
Leaf N:
-----------------------------------------------
| ..., ..., ..., (257, FILE_EXTENT_ITEM, 64K) |
-----------------------------------------------
The file extent item at the end of leaf N has a length of 4Kb,
representing the file range from 64K to 68K - 1.
Leaf N + 1:
-----------------------------------------------
| (257, FILE_EXTENT_ITEM, 72K), ..., ..., ... |
-----------------------------------------------
The file extent item at the first slot of leaf N + 1 has a length of
4Kb too, representing the file range from 72K to 76K - 1.
During the full fsync path, when we are at tree-log.c:copy_items() with
leaf N as a parameter, after processing the last file extent item, that
represents the extent at offset 64K, we take a look at the first file
extent item at the next leaf (leaf N + 1), and notice there's a 4K hole
between the two extents, and therefore we insert a file extent item
representing that hole, starting at file offset 68K and ending at offset
72K - 1. However we don't update the value of *last_extent, which is used
to represent the end offset (plus 1, non-inclusive end) of the last file
extent item inserted in the log, so it stays with a value of 68K and not
with a value of 72K.
Then, when copy_items() is called for leaf N + 1, because the value of
*last_extent is smaller then the offset of the first extent item in the
leaf (68K < 72K), we look at the last file extent item in the previous
leaf (leaf N) and see it there's a 4K gap between it and our first file
extent item (again, 68K < 72K), so we decide to insert a file extent item
representing the hole, starting at file offset 68K and ending at offset
72K - 1, this insertion will fail with -EEXIST being returned from
btrfs_insert_file_extent() because we already inserted a file extent item
representing a hole for this offset (68K) in the previous call to
copy_items(), when processing leaf N.
The -EEXIST error gets propagated to the fsync callback, btrfs_sync_file(),
which falls back to a full transaction commit.
Fix this by adjusting *last_extent after inserting a hole when we had to
look at the next leaf.
Fixes:
4ee3fad34a9c ("Btrfs: fix fsync after hole punching when using no-holes feature")
Cc: stable@vger.kernel.org # 4.14+
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Fri, 10 May 2019 04:45:05 +0000 (12:45 +0800)]
btrfs: extent-tree: Fix a bug that btrfs is unable to add pinned bytes
Commit
ddf30cf03fb5 ("btrfs: extent-tree: Use btrfs_ref to refactor
add_pinned_bytes()") refactored add_pinned_bytes(), but during that
refactor, there are two callers which add the pinned bytes instead
of subtracting.
That refactor misses those two caller, causing incorrect pinned bytes
calculation and resulting unexpected ENOSPC error.
Fix it by adding a new parameter @sign to restore the original behavior.
Reported-by: kernel test robot <rong.a.chen@intel.com>
Fixes:
ddf30cf03fb5 ("btrfs: extent-tree: Use btrfs_ref to refactor add_pinned_bytes()")
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Tobin C. Harding [Mon, 13 May 2019 03:39:12 +0000 (13:39 +1000)]
btrfs: sysfs: don't leak memory when failing add fsid
A failed call to kobject_init_and_add() must be followed by a call to
kobject_put(). Currently in the error path when adding fs_devices we
are missing this call. This could be fixed by calling
btrfs_sysfs_remove_fsid() if btrfs_sysfs_add_fsid() returns an error or
by adding a call to kobject_put() directly in btrfs_sysfs_add_fsid().
Here we choose the second option because it prevents the slightly
unusual error path handling requirements of kobject from leaking out
into btrfs functions.
Add a call to kobject_put() in the error path of kobject_add_and_init().
This causes the release method to be called if kobject_init_and_add()
fails. open_tree() is the function that calls btrfs_sysfs_add_fsid()
and the error code in this function is already written with the
assumption that the release method is called during the error path of
open_tree() (as seen by the call to btrfs_sysfs_remove_fsid() under the
fail_fsdev_sysfs label).
Cc: stable@vger.kernel.org # v4.4+
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Tobin C. Harding <tobin@kernel.org>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Tobin C. Harding [Mon, 13 May 2019 03:39:11 +0000 (13:39 +1000)]
btrfs: sysfs: Fix error path kobject memory leak
If a call to kobject_init_and_add() fails we must call kobject_put()
otherwise we leak memory.
Calling kobject_put() when kobject_init_and_add() fails drops the
refcount back to 0 and calls the ktype release method (which in turn
calls the percpu destroy and kfree).
Add call to kobject_put() in the error path of call to
kobject_init_and_add().
Cc: stable@vger.kernel.org # v4.4+
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Tobin C. Harding <tobin@kernel.org>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Mon, 29 Apr 2019 12:08:14 +0000 (13:08 +0100)]
Btrfs: do not abort transaction at btrfs_update_root() after failure to COW path
Currently when we fail to COW a path at btrfs_update_root() we end up
always aborting the transaction. However all the current callers of
btrfs_update_root() are able to deal with errors returned from it, many do
end up aborting the transaction themselves (directly or not, such as the
transaction commit path), other BUG_ON() or just gracefully cancel whatever
they were doing.
When syncing the fsync log, we call btrfs_update_root() through
tree-log.c:update_log_root(), and if it returns an -ENOSPC error, the log
sync code does not abort the transaction, instead it gracefully handles
the error and returns -EAGAIN to the fsync handler, so that it falls back
to a transaction commit. Any other error different from -ENOSPC, makes the
log sync code abort the transaction.
So remove the transaction abort from btrfs_update_log() when we fail to
COW a path to update the root item, so that if an -ENOSPC failure happens
we avoid aborting the current transaction and have a chance of the fsync
succeeding after falling back to a transaction commit.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203413
Fixes:
79787eaab46121 ("btrfs: replace many BUG_ONs with proper error handling")
Cc: stable@vger.kernel.org # 4.4+
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Josef Bacik [Fri, 26 Apr 2019 14:25:50 +0000 (16:25 +0200)]
btrfs: use the existing reserved items for our first prop for inheritance
We're now reserving an extra items worth of space for property
inheritance. We only have one property at the moment so this covers us,
but if we add more in the future this will allow us to not get bitten by
the extra space reservation. If we do add more properties in the future
we should re-visit how we calculate the space reservation needs by the
callers.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
[ refreshed on top of prop/xattr cleanups ]
Signed-off-by: David Sterba <dsterba@suse.com>
Josef Bacik [Fri, 3 May 2019 15:10:06 +0000 (11:10 -0400)]
btrfs: don't double unlock on error in btrfs_punch_hole
If we have an error writing out a delalloc range in
btrfs_punch_hole_lock_range we'll unlock the inode and then goto
out_only_mutex, where we will again unlock the inode. This is bad,
don't do this.
Fixes:
f27451f22996 ("Btrfs: add support for fallocate's zero range operation")
CC: stable@vger.kernel.org # 4.19+
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Johnny Chang [Fri, 26 Apr 2019 03:01:05 +0000 (11:01 +0800)]
btrfs: Check the compression level before getting a workspace
When a file's compression property is set as zlib or zstd but leave
the compression mount option not be set, that means btrfs will try
to compress the file with default compression level. But in
btrfs_compress_pages(), it calls get_workspace() with level = 0.
This will return a workspace with a wrong compression level.
For zlib, the compression level in the workspace will be 0
(that means "store only"). And for zstd, the compression in the
workspace will be 1, not the default level 3.
How to reproduce:
mkfs -t btrfs /dev/sdb
mount /dev/sdb /mnt/
mkdir /mnt/zlib
btrfs property set /mnt/zlib/ compression zlib
dd if=/dev/zero of=/mnt/zlib/compression-friendly-file-10M bs=1M count=10
sync
btrfs-debugfs -f /mnt/zlib/compression-friendly-file-10M
btrfs-debugfs output:
* before:
...
(258
9961472): ram 524288 disk
1106247680 disk_size 524288
file: ... extents 20 disk size
10485760 logical size
10485760 ratio 1.00
* after:
...
(258
10354688): ram 131072 disk
14217216 disk_size 4096
file: ... extents 80 disk size 327680 logical size
10485760 ratio 32.00
The steps for zstd are similar, but need to put a debugging message to
show the level of the return workspace in zstd_get_workspace().
This commit adds a check of the compression level before getting a
workspace by set_level().
CC: stable@vger.kernel.org # 5.1+
Signed-off-by: Johnny Chang <johnnyc@synology.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Mon, 1 Apr 2019 08:29:57 +0000 (11:29 +0300)]
btrfs: Use kvmalloc for allocating compressed path context
Recent refactoring of cow_file_range_async means it's now possible to
request a rather large physically contiguous memory via kmalloc. The
size is dependent on the number of 512k chunks that the compressed range
consists of. David reported multiple OOM messages on such large
allocations. Fix it by switching to using kvmalloc.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Tue, 12 Mar 2019 15:20:30 +0000 (17:20 +0200)]
btrfs: Factor out common extent locking code in submit_compressed_extents
Irrespective of whether the compress code fell back to uncompressed or
a compressed extent has to be submitted, the extent range is always
locked. So factor out the common lock_extent call at the beginning of
the loop. No functional changes just removes one duplicate lock_extent
call.
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Tue, 12 Mar 2019 15:20:29 +0000 (17:20 +0200)]
btrfs: Set io_tree only once in submit_compressed_extents
The inode never changes so it's sufficient to dereference it and get
the iotree only once, before the execution of the main loop. No
functional changes, only the size of the function is decreased:
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-44 (-44)
Function old new delta
submit_compressed_extents 1240 1196 -44
Total: Before=88476, After=88432, chg -0.05%
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Tue, 12 Mar 2019 15:20:28 +0000 (17:20 +0200)]
btrfs: Replace clear_extent_bit with unlock_extent
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Tue, 12 Mar 2019 15:20:27 +0000 (17:20 +0200)]
btrfs: Make compress_file_range take only struct async_chunk
All context this function needs is held within struct async_chunk.
Currently we not only pass the struct but also every individual member.
This is redundant, simplify it by only passing struct async_chunk and
leaving it to compress_file_range to extract the values it requires.
No functional changes.
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Tue, 12 Mar 2019 15:20:26 +0000 (17:20 +0200)]
btrfs: Remove fs_info from struct async_chunk
The associated btrfs_work already contains a reference to the fs_info so
use that instead of passing it via async_chunk. No functional changes.
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Tue, 12 Mar 2019 15:20:25 +0000 (17:20 +0200)]
btrfs: Rename async_cow to async_chunk
Now that we have an explicit async_chunk struct rename references to
variables of this type to async_chunk. No functional changes.
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Tue, 12 Mar 2019 15:20:24 +0000 (17:20 +0200)]
btrfs: Preallocate chunks in cow_file_range_async
This commit changes the implementation of cow_file_range_async in order
to get rid of the BUG_ON in the middle of the loop. Additionally it
reworks the inner loop in the hopes of making it more understandable.
The idea is to make async_cow be a top-level structured, shared amongst
all chunks being sent for compression. This allows to perform one memory
allocation at the beginning and gracefully fail the IO if there isn't
enough memory. Now, each chunk is going to be described by an
async_chunk struct. It's the responsibility of the final chunk
to actually free the memory.
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Josef Bacik [Wed, 10 Apr 2019 19:56:10 +0000 (15:56 -0400)]
btrfs: reserve delalloc metadata differently
With the per-inode block reserves we started refilling the reserve based
on the calculated size of the outstanding csum bytes and extents for the
inode, including the amount we were adding with the new operation.
However, generic/224 exposed a problem with this approach. With 1000
files all writing at the same time we ended up with a bunch of bytes
being reserved but unusable.
When you write to a file we reserve space for the csum leaves for those
bytes, the number of extent items required to cover those bytes, and a
single transaction item for updating the inode at ordered extent finish
for that range of bytes. This is held until the ordered extent finishes
and we release all of the reserved space.
If a second write comes in at this point we would add a single
reservation for the new outstanding extent and however many reservations
for the csum leaves. At this point we find the delta of how much we
have reserved and how much outstanding size this is and attempt to
reserve this delta. If the first write finishes it will not release any
space, because the space it had reserved for the initial write is still
needed for the second write. However some space would have been used,
as we have added csums, extent items, and dirtied the inode. Our
reserved space would be > 0 but less than the total needed reserved
space.
This is just for a single inode, now consider generic/224. This has
1000 inodes writing in parallel to a very small file system, 1GiB. In
my testing this usually means we get about a 120MiB metadata area to
work with, more than enough to allow the writes to continue, but not
enough if all of the inodes are stuck trying to reserve the slack space
while continuing to hold their leftovers from their initial writes.
Fix this by pre-reserved _only_ for the space we are currently trying to
add. Then once that is successful modify our inodes csum count and
outstanding extents, and then add the newly reserved space to the inodes
block_rsv. This allows us to actually pass generic/224 without running
out of metadata space.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Josef Bacik [Wed, 10 Apr 2019 19:56:09 +0000 (15:56 -0400)]
btrfs: track DIO bytes in flight
When diagnosing a slowdown of generic/224 I noticed we were not doing
anything when calling into shrink_delalloc(). This is because all
writes in 224 are O_DIRECT, not delalloc, and thus our delalloc_bytes
counter is 0, which short circuits most of the work inside of
shrink_delalloc(). However O_DIRECT writes still consume metadata
resources and generate ordered extents, which we can still wait on.
Fix this by tracking outstanding DIO write bytes, and use this as well
as the delalloc bytes counter to decide if we need to lookup and wait on
any ordered extents. If we have more DIO writes than delalloc bytes
we'll go ahead and wait on any ordered extents regardless of our flush
state as flushing delalloc is likely to not gain us anything.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
[ use dio instead of odirect in identifiers ]
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Sat, 20 Apr 2019 11:49:00 +0000 (19:49 +0800)]
btrfs: merge calls of btrfs_setxattr and btrfs_setxattr_trans in btrfs_set_prop
Since now the trans argument is never NULL in btrfs_set_prop we don't
have to check. So delete it and use btrfs_setxattr that makes use of
that.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Sat, 20 Apr 2019 11:48:59 +0000 (19:48 +0800)]
btrfs: delete unused function btrfs_set_prop_trans
The last consumer of btrfs_set_prop_trans() was taken away by the patch
("btrfs: start transaction in xattr_handler_set_prop") so now this
function can be deleted.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Sat, 20 Apr 2019 11:48:58 +0000 (19:48 +0800)]
btrfs: start transaction in xattr_handler_set_prop
btrfs specific extended attributes on the inode are set using
btrfs_xattr_handler_set_prop(), and the required transaction for this
update is started by btrfs_setxattr(). For better visibility of the
transaction start and end, do this in btrfs_xattr_handler_set_prop().
For which this patch copied code of btrfs_setxattr() as it is in the
original, which needs proper error handling.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Sat, 20 Apr 2019 11:48:57 +0000 (19:48 +0800)]
btrfs: drop local copy of inode i_mode
There isn't real use of making struct inode::i_mode a local copy, it
saves a dereference one time, not much. Just use it directly.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Sat, 20 Apr 2019 11:48:56 +0000 (19:48 +0800)]
btrfs: drop old_fsflags in btrfs_ioctl_setflags
btrfs_inode_flags_to_fsflags() is copied into @old_fsflags and used only
once. Instead used it directly.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Sat, 20 Apr 2019 11:48:55 +0000 (19:48 +0800)]
btrfs: modify local copy of btrfs_inode flags
Instead of updating the binode::flags directly, update a local copy, and
then at the point of no error, store copy it to the binode::flags.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Sat, 20 Apr 2019 11:48:54 +0000 (19:48 +0800)]
btrfs: drop useless inode i_flags copy and restore
The patch ("btrfs: start transaction in btrfs_ioctl_setflags()") used
btrfs_set_prop() instead of btrfs_set_prop_trans() by which now the
inode::i_flags update functions such as
btrfs_sync_inode_flags_to_i_flags() and btrfs_update_inode() is called
in btrfs_ioctl_setflags() instead of
btrfs_set_prop_trans()->btrfs_setxattr() as earlier. So the
inode::i_flags remains unmodified until the thread has checked all the
conditions. So drop the saved inode::i_flags in out_i_flags.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Sat, 20 Apr 2019 11:48:53 +0000 (19:48 +0800)]
btrfs: start transaction in btrfs_ioctl_setflags()
Inode attribute can be set through the FS_IOC_SETFLAGS ioctl. This
flags also includes compression attribute for which we would set/reset
the compression extended attribute. While doing this there is a bit of
duplicate code, the following things happens twice:
- start/end_transaction
- inode_inc_iversion()
- current_time update to inode->i_ctime
- and btrfs_update_inode()
These are updated both at btrfs_ioctl_setflags() and btrfs_set_props()
as well. This patch merges these two duplicate codes at
btrfs_ioctl_setflags().
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Sat, 20 Apr 2019 11:48:52 +0000 (19:48 +0800)]
btrfs: export btrfs_set_prop
Make btrfs_set_prop() a non-static function, so that it can be called
from btrfs_ioctl_setflags(). We need btrfs_set_prop() instead of
btrfs_set_prop_trans() so that we can use the transaction which is
already started in the current thread.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Sat, 20 Apr 2019 11:48:51 +0000 (19:48 +0800)]
btrfs: refactor btrfs_set_props to validate externally
In preparation to merge multiple transactions when setting the
compression flags, split btrfs_set_props() validation part outside of
it.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Thu, 25 Apr 2019 00:55:53 +0000 (08:55 +0800)]
btrfs: ctree: Dump the leaf before BUG_ON in btrfs_set_item_key_safe
We have a long standing problem with reversed keys that's detected by
btrfs_set_item_key_safe. This is hard to reproduce so we'd like to
capture more information for later analysis.
Let's dump the leaf content before triggering BUG_ON() so that we can
have some clue on what's going wrong. The output of tree locks should
help us to debug such problem.
Sample stacktrace:
generic/522 [00:07:05]
[26946.113381] run fstests generic/522 at 2019-04-16 00:07:05
[27161.474720] kernel BUG at fs/btrfs/ctree.c:3192!
[27161.475923] invalid opcode: 0000 [#1] PREEMPT SMP
[27161.477167] CPU: 0 PID: 15676 Comm: fsx Tainted: G W 5.1.0-rc5-default+ #562
[27161.478932] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.12.0-0-ga698c89-prebuilt.qemu.org 04/01/2014
[27161.481099] RIP: 0010:btrfs_set_item_key_safe+0x146/0x1c0 [btrfs]
[27161.485369] RSP: 0018:
ffffb087499e39b0 EFLAGS:
00010286
[27161.486464] RAX:
00000000ffffffff RBX:
ffff941534d80e70 RCX:
0000000000024000
[27161.487929] RDX:
0000000000013039 RSI:
ffffb087499e3aa5 RDI:
ffffb087499e39c7
[27161.489289] RBP:
000000000000000e R08:
ffff9414e0f49008 R09:
0000000000001000
[27161.490807] R10:
0000000000000000 R11:
0000000000000003 R12:
ffff9414e0f48e70
[27161.492305] R13:
ffffb087499e3aa5 R14:
0000000000000000 R15:
0000000000071000
[27161.493845] FS:
00007f8ea58d0b80(0000) GS:
ffff94153d400000(0000) knlGS:
0000000000000000
[27161.495608] CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
[27161.496717] CR2:
00007f8ea57a9000 CR3:
0000000016a33000 CR4:
00000000000006f0
[27161.498100] Call Trace:
[27161.498771] __btrfs_drop_extents+0x6ec/0xdf0 [btrfs]
[27161.499872] btrfs_log_changed_extents.isra.26+0x3a2/0x9e0 [btrfs]
[27161.501114] btrfs_log_inode+0x7ff/0xdc0 [btrfs]
[27161.502114] ? __mutex_unlock_slowpath+0x4b/0x2b0
[27161.503172] btrfs_log_inode_parent+0x237/0x9c0 [btrfs]
[27161.504348] btrfs_log_dentry_safe+0x4a/0x70 [btrfs]
[27161.505374] btrfs_sync_file+0x1b7/0x480 [btrfs]
[27161.506371] __x64_sys_msync+0x180/0x210
[27161.507208] do_syscall_64+0x54/0x180
[27161.507932] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[27161.508839] RIP: 0033:0x7f8ea5aa9c61
[27161.512616] RSP: 002b:
00007ffea2a06498 EFLAGS:
00000246 ORIG_RAX:
000000000000001a
[27161.514161] RAX:
ffffffffffffffda RBX:
000000000002a938 RCX:
00007f8ea5aa9c61
[27161.515376] RDX:
0000000000000004 RSI:
000000000001c9b2 RDI:
00007f8ea578d000
[27161.516572] RBP:
000000000001c07a R08:
fffffffffffffff8 R09:
000000000002a000
[27161.517883] R10:
00007f8ea57a99b2 R11:
0000000000000246 R12:
0000000000000938
[27161.519080] R13:
00007f8ea578d000 R14:
000000000001c9b2 R15:
0000000000000000
[27161.520281] Modules linked in: btrfs libcrc32c xor zstd_decompress zstd_compress xxhash raid6_pq loop [last unloaded: scsi_debug]
[27161.522272] ---[ end trace
d5afec7ccac6a252 ]---
[27161.523111] RIP: 0010:btrfs_set_item_key_safe+0x146/0x1c0 [btrfs]
[27161.527253] RSP: 0018:
ffffb087499e39b0 EFLAGS:
00010286
[27161.528192] RAX:
00000000ffffffff RBX:
ffff941534d80e70 RCX:
0000000000024000
[27161.529392] RDX:
0000000000013039 RSI:
ffffb087499e3aa5 RDI:
ffffb087499e39c7
[27161.530607] RBP:
000000000000000e R08:
ffff9414e0f49008 R09:
0000000000001000
[27161.531802] R10:
0000000000000000 R11:
0000000000000003 R12:
ffff9414e0f48e70
[27161.533018] R13:
ffffb087499e3aa5 R14:
0000000000000000 R15:
0000000000071000
[27161.534405] FS:
00007f8ea58d0b80(0000) GS:
ffff94153d400000(0000) knlGS:
0000000000000000
[27161.536048] CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
[27161.537210] CR2:
00007f8ea57a9000 CR3:
0000000016a33000 CR4:
00000000000006f0
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Wed, 24 Apr 2019 07:22:53 +0000 (15:22 +0800)]
btrfs: tree-checker: Allow error injection for tree-checker
Allowing error injection for btrfs_check_leaf_full() and
btrfs_check_node() is useful to test the failure path of btrfs write
time tree check.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Mon, 22 Apr 2019 13:07:31 +0000 (16:07 +0300)]
btrfs: Document btrfs_csum_one_bio
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Wed, 17 Apr 2019 10:31:06 +0000 (11:31 +0100)]
Btrfs: improve performance on fsync of files with multiple hardlinks
Commit
41bd6067692382 ("Btrfs: fix fsync of files with multiple hard links
in new directories") introduced a path that makes fsync fallback to a full
transaction commit in order to avoid losing hard links and new ancestors
of the fsynced inode. That path is triggered only when the inode has more
than one hard link and either has a new hard link created in the current
transaction or the inode was evicted and reloaded in the current
transaction.
That path ends up getting triggered very often (hundreds of times) during
the course of pgbench benchmarks, resulting in performance drops of about
20%.
This change restores the performance by not triggering the full transaction
commit in those cases, and instead iterate the fs/subvolume tree in search
of all possible new ancestors, for all hard links, to log them.
Reported-by: Zhao Yuhu <zyuhu@suse.com>
Tested-by: James Wang <jnwang@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Mon, 22 Apr 2019 15:43:42 +0000 (16:43 +0100)]
Btrfs: fix race between send and deduplication that lead to failures and crashes
Send operates on read only trees and expects them to never change while it
is using them. This is part of its initial design, and this expection is
due to two different reasons:
1) When it was introduced, no operations were allowed to modifiy read-only
subvolumes/snapshots (including defrag for example).
2) It keeps send from having an impact on other filesystem operations.
Namely send does not need to keep locks on the trees nor needs to hold on
to transaction handles and delay transaction commits. This ends up being
a consequence of the former reason.
However the deduplication feature was introduced later (on September 2013,
while send was introduced in July 2012) and it allowed for deduplication
with destination files that belong to read-only trees (subvolumes and
snapshots).
That means that having a send operation (either full or incremental) running
in parallel with a deduplication that has the destination inode in one of
the trees used by the send operation, can result in tree nodes and leaves
getting freed and reused while send is using them. This problem is similar
to the problem solved for the root nodes getting freed and reused when a
snapshot is made against one tree that is currenly being used by a send
operation, fixed in commits [1] and [2]. These commits explain in detail
how the problem happens and the explanation is valid for any node or leaf
that is not the root of a tree as well. This problem was also discussed
and explained recently in a thread [3].
The problem is very easy to reproduce when using send with large trees
(snapshots) and just a few concurrent deduplication operations that target
files in the trees used by send. A stress test case is being sent for
fstests that triggers the issue easily. The most common error to hit is
the send ioctl return -EIO with the following messages in dmesg/syslog:
[
1631617.204075] BTRFS error (device sdc): did not find backref in send_root. inode=63292, offset=0, disk_byte=
5228134400 found extent=
5228134400
[
1631633.251754] BTRFS error (device sdc): parent transid verify failed on
32243712 wanted 24 found 27
The first one is very easy to hit while the second one happens much less
frequently, except for very large trees (in that test case, snapshots
with 100000 files having large xattrs to get deep and wide trees).
Less frequently, at least one BUG_ON can be hit:
[
1631742.130080] ------------[ cut here ]------------
[
1631742.130625] kernel BUG at fs/btrfs/ctree.c:1806!
[
1631742.131188] invalid opcode: 0000 [#6] SMP DEBUG_PAGEALLOC PTI
[
1631742.131726] CPU: 1 PID: 13394 Comm: btrfs Tainted: G B D W 5.0.0-rc8-btrfs-next-45 #1
[
1631742.132265] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.11.2-0-gf9626ccb91-prebuilt.qemu-project.org 04/01/2014
[
1631742.133399] RIP: 0010:read_node_slot+0x122/0x130 [btrfs]
(...)
[
1631742.135061] RSP: 0018:
ffffb530021ebaa0 EFLAGS:
00010246
[
1631742.135615] RAX:
ffff93ac8912e000 RBX:
000000000000009d RCX:
0000000000000002
[
1631742.136173] RDX:
000000000000009d RSI:
ffff93ac564b0d08 RDI:
ffff93ad5b48c000
[
1631742.136759] RBP:
ffffb530021ebb7d R08:
0000000000000001 R09:
ffffb530021ebb7d
[
1631742.137324] R10:
ffffb530021eba70 R11:
0000000000000000 R12:
ffff93ac87d0a708
[
1631742.137900] R13:
0000000000000000 R14:
0000000000000000 R15:
0000000000000001
[
1631742.138455] FS:
00007f4cdb1528c0(0000) GS:
ffff93ad76a80000(0000) knlGS:
0000000000000000
[
1631742.139010] CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
[
1631742.139568] CR2:
00007f5acb3d0420 CR3:
000000012be3e006 CR4:
00000000003606e0
[
1631742.140131] DR0:
0000000000000000 DR1:
0000000000000000 DR2:
0000000000000000
[
1631742.140719] DR3:
0000000000000000 DR6:
00000000fffe0ff0 DR7:
0000000000000400
[
1631742.141272] Call Trace:
[
1631742.141826] ? do_raw_spin_unlock+0x49/0xc0
[
1631742.142390] tree_advance+0x173/0x1d0 [btrfs]
[
1631742.142948] btrfs_compare_trees+0x268/0x690 [btrfs]
[
1631742.143533] ? process_extent+0x1070/0x1070 [btrfs]
[
1631742.144088] btrfs_ioctl_send+0x1037/0x1270 [btrfs]
[
1631742.144645] _btrfs_ioctl_send+0x80/0x110 [btrfs]
[
1631742.145161] ? trace_sched_stick_numa+0xe0/0xe0
[
1631742.145685] btrfs_ioctl+0x13fe/0x3120 [btrfs]
[
1631742.146179] ? account_entity_enqueue+0xd3/0x100
[
1631742.146662] ? reweight_entity+0x154/0x1a0
[
1631742.147135] ? update_curr+0x20/0x2a0
[
1631742.147593] ? check_preempt_wakeup+0x103/0x250
[
1631742.148053] ? do_vfs_ioctl+0xa2/0x6f0
[
1631742.148510] ? btrfs_ioctl_get_supported_features+0x30/0x30 [btrfs]
[
1631742.148942] do_vfs_ioctl+0xa2/0x6f0
[
1631742.149361] ? __fget+0x113/0x200
[
1631742.149767] ksys_ioctl+0x70/0x80
[
1631742.150159] __x64_sys_ioctl+0x16/0x20
[
1631742.150543] do_syscall_64+0x60/0x1b0
[
1631742.150931] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[
1631742.151326] RIP: 0033:0x7f4cd9f5add7
(...)
[
1631742.152509] RSP: 002b:
00007ffe91017708 EFLAGS:
00000202 ORIG_RAX:
0000000000000010
[
1631742.152892] RAX:
ffffffffffffffda RBX:
0000000000000105 RCX:
00007f4cd9f5add7
[
1631742.153268] RDX:
00007ffe91017790 RSI:
0000000040489426 RDI:
0000000000000007
[
1631742.153633] RBP:
0000000000000007 R08:
00007f4cd9e79700 R09:
00007f4cd9e79700
[
1631742.153999] R10:
00007f4cd9e799d0 R11:
0000000000000202 R12:
0000000000000003
[
1631742.154365] R13:
0000555dfae53020 R14:
0000000000000000 R15:
0000000000000001
(...)
[
1631742.156696] ---[ end trace
5dac9f96dcc3fd6b ]---
That BUG_ON happens because while send is using a node, that node is COWed
by a concurrent deduplication, gets freed and gets reused as a leaf (because
a transaction commit happened in between), so when it attempts to read a
slot from the extent buffer, at ctree.c:read_node_slot(), the extent buffer
contents were wiped out and it now matches a leaf (which can even belong to
some other tree now), hitting the BUG_ON(level == 0).
Fix this concurrency issue by not allowing send and deduplication to run
in parallel if both operate on the same readonly trees, returning EAGAIN
to user space and logging an exlicit warning in dmesg/syslog.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=
be6821f82c3cc36e026f5afd10249988852b35ea
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=
6f2f0b394b54e2b159ef969a0b5274e9bbf82ff2
[3] https://lore.kernel.org/linux-btrfs/CAL3q7H7iqSEEyFaEtpRZw3cp613y+4k2Q8b4W7mweR3tZA05bQ@mail.gmail.com/
CC: stable@vger.kernel.org # 4.4+
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Mon, 15 Apr 2019 08:29:36 +0000 (09:29 +0100)]
Btrfs: send, flush dellaloc in order to avoid data loss
When we set a subvolume to read-only mode we do not flush dellaloc for any
of its inodes (except if the filesystem is mounted with -o flushoncommit),
since it does not affect correctness for any subsequent operations - except
for a future send operation. The send operation will not be able to see the
delalloc data since the respective file extent items, inode item updates,
backreferences, etc, have not hit yet the subvolume and extent trees.
Effectively this means data loss, since the send stream will not contain
any data from existing delalloc. Another problem from this is that if the
writeback starts and finishes while the send operation is in progress, we
have the subvolume tree being being modified concurrently which can result
in send failing unexpectedly with EIO or hitting runtime errors, assertion
failures or hitting BUG_ONs, etc.
Simple reproducer:
$ mkfs.btrfs -f /dev/sdb
$ mount /dev/sdb /mnt
$ btrfs subvolume create /mnt/sv
$ xfs_io -f -c "pwrite -S 0xea 0 108K" /mnt/sv/foo
$ btrfs property set /mnt/sv ro true
$ btrfs send -f /tmp/send.stream /mnt/sv
$ od -t x1 -A d /mnt/sv/foo
0000000 ea ea ea ea ea ea ea ea ea ea ea ea ea ea ea ea
*
0110592
$ umount /mnt
$ mkfs.btrfs -f /dev/sdc
$ mount /dev/sdc /mnt
$ btrfs receive -f /tmp/send.stream /mnt
$ echo $?
0
$ od -t x1 -A d /mnt/sv/foo
0000000
# ---> empty file
Since this a problem that affects send only, fix it in send by flushing
dellaloc for all the roots used by the send operation before send starts
to process the commit roots.
This is a problem that affects send since it was introduced (commit
31db9f7c23fbf7 ("Btrfs: introduce BTRFS_IOC_SEND for btrfs send/receive"))
but backporting it to older kernels has some dependencies:
- For kernels between 3.19 and 4.20, it depends on commit
3cd24c698004d2
("btrfs: use tagged writepage to mitigate livelock of snapshot") because
the function btrfs_start_delalloc_snapshot() does not exist before that
commit. So one has to either pick that commit or replace the calls to
btrfs_start_delalloc_snapshot() in this patch with calls to
btrfs_start_delalloc_inodes().
- For kernels older than 3.19 it also requires commit
e5fa8f865b3324
("Btrfs: ensure send always works on roots without orphans") because
it depends on the function ensure_commit_roots_uptodate() which that
commits introduced.
- No dependencies for 5.0+ kernels.
A test case for fstests follows soon.
CC: stable@vger.kernel.org # 3.19+
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Mon, 15 Apr 2019 13:50:51 +0000 (14:50 +0100)]
Btrfs: do not start a transaction during fiemap
During fiemap, for regular extents (non inline) we need to check if they
are shared and if they are, set the shared bit. Checking if an extent is
shared requires checking the delayed references of the currently running
transaction, since some reference might have not yet hit the extent tree
and be only in the in-memory delayed references.
However we were using a transaction join for this, which creates a new
transaction when there is no transaction currently running. That means
that two more potential failures can happen: creating the transaction and
committing it. Further, if no write activity is currently happening in the
system, and fiemap calls keep being done, we end up creating and
committing transactions that do nothing.
In some extreme cases this can result in the commit of the transaction
created by fiemap to fail with ENOSPC when updating the root item of a
subvolume tree because a join does not reserve any space, leading to a
trace like the following:
heisenberg kernel: ------------[ cut here ]------------
heisenberg kernel: BTRFS: Transaction aborted (error -28)
heisenberg kernel: WARNING: CPU: 0 PID: 7137 at fs/btrfs/root-tree.c:136 btrfs_update_root+0x22b/0x320 [btrfs]
(...)
heisenberg kernel: CPU: 0 PID: 7137 Comm: btrfs-transacti Not tainted 4.19.0-4-amd64 #1 Debian 4.19.28-2
heisenberg kernel: Hardware name: FUJITSU LIFEBOOK U757/FJNB2A5, BIOS Version 1.21 03/19/2018
heisenberg kernel: RIP: 0010:btrfs_update_root+0x22b/0x320 [btrfs]
(...)
heisenberg kernel: RSP: 0018:
ffffb5448828bd40 EFLAGS:
00010286
heisenberg kernel: RAX:
0000000000000000 RBX:
ffff8ed56bccef50 RCX:
0000000000000006
heisenberg kernel: RDX:
0000000000000007 RSI:
0000000000000092 RDI:
ffff8ed6bda166a0
heisenberg kernel: RBP:
00000000ffffffe4 R08:
00000000000003df R09:
0000000000000007
heisenberg kernel: R10:
0000000000000000 R11:
0000000000000001 R12:
ffff8ed63396a078
heisenberg kernel: R13:
ffff8ed092d7c800 R14:
ffff8ed64f5db028 R15:
ffff8ed6bd03d068
heisenberg kernel: FS:
0000000000000000(0000) GS:
ffff8ed6bda00000(0000) knlGS:
0000000000000000
heisenberg kernel: CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
heisenberg kernel: CR2:
00007f46f75f8000 CR3:
0000000310a0a002 CR4:
00000000003606f0
heisenberg kernel: DR0:
0000000000000000 DR1:
0000000000000000 DR2:
0000000000000000
heisenberg kernel: DR3:
0000000000000000 DR6:
00000000fffe0ff0 DR7:
0000000000000400
heisenberg kernel: Call Trace:
heisenberg kernel: commit_fs_roots+0x166/0x1d0 [btrfs]
heisenberg kernel: ? _cond_resched+0x15/0x30
heisenberg kernel: ? btrfs_run_delayed_refs+0xac/0x180 [btrfs]
heisenberg kernel: btrfs_commit_transaction+0x2bd/0x870 [btrfs]
heisenberg kernel: ? start_transaction+0x9d/0x3f0 [btrfs]
heisenberg kernel: transaction_kthread+0x147/0x180 [btrfs]
heisenberg kernel: ? btrfs_cleanup_transaction+0x530/0x530 [btrfs]
heisenberg kernel: kthread+0x112/0x130
heisenberg kernel: ? kthread_bind+0x30/0x30
heisenberg kernel: ret_from_fork+0x35/0x40
heisenberg kernel: ---[ end trace
05de912e30e012d9 ]---
Since fiemap (and btrfs_check_shared()) is a read-only operation, do not do
a transaction join to avoid the overhead of creating a new transaction (if
there is currently no running transaction) and introducing a potential
point of failure when the new transaction gets committed, instead use a
transaction attach to grab a handle for the currently running transaction
if any.
Reported-by: Christoph Anton Mitterer <calestyo@scientia.net>
Link: https://lore.kernel.org/linux-btrfs/b2a668d7124f1d3e410367f587926f622b3f03a4.camel@scientia.net/
Fixes:
afce772e87c36c ("btrfs: fix check_shared for fiemap ioctl")
CC: stable@vger.kernel.org # 4.14+
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 10:43:36 +0000 (11:43 +0100)]
btrfs: remove unused parameter fs_info from btrfs_set_disk_extent_flags
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 10:42:34 +0000 (11:42 +0100)]
btrfs: remove unused parameter fs_info from btrfs_add_delayed_extent_op
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 10:29:46 +0000 (11:29 +0100)]
btrfs: remove unused parameter fs_info from emit_last_fiemap_cache
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 14:59:22 +0000 (15:59 +0100)]
btrfs: remove unused parameter fs_info from CHECK_FE_ALIGNED
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 14:03:48 +0000 (15:03 +0100)]
btrfs: remove unused parameter fs_info from from tree_advance
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 14:02:46 +0000 (15:02 +0100)]
btrfs: remove unused parameter fs_info from tree_move_down
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 13:51:10 +0000 (14:51 +0100)]
btrfs: remove unused parameter fs_info from btrfs_extend_item
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 13:49:12 +0000 (14:49 +0100)]
btrfs: remove unused parameter fs_info from btrfs_truncate_item
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 13:44:57 +0000 (14:44 +0100)]
btrfs: remove unused parameter fs_info from split_item
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Thu, 4 Apr 2019 06:45:37 +0000 (14:45 +0800)]
btrfs: qgroup: Don't scan leaf if we're modifying reloc tree
Since reloc tree doesn't contribute to qgroup numbers, just skip them.
This should catch the final cause of unnecessary data ref processing
when running balance of metadata with qgroups on.
The 4G data 16 snapshots test (*) should explain it pretty well:
| delayed subtree | refactor delayed ref | this patch
---------------------------------------------------------------------
relocated | 22653 | 22673 | 22744
qgroup dirty | 122792 | 48360 | 70
time | 24.494 | 11.606 | 3.944
Finally, we're at the stage where qgroup + metadata balance cost no
obvious overhead.
Test environment:
Test VM:
- vRAM 8G
- vCPU 8
- block dev vitrio-blk, 'unsafe' cache mode
- host block 850evo
Test workload:
- Copy 4G data from /usr/ to one subvolume
- Create 16 snapshots of that subvolume, and modify 3 files in each
snapshot
- Enable quota, rescan
- Time "btrfs balance start -m"
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Thu, 4 Apr 2019 06:45:36 +0000 (14:45 +0800)]
btrfs: extent-tree: Use btrfs_ref to refactor btrfs_free_extent()
Similar to btrfs_inc_extent_ref(), use btrfs_ref to replace the long
parameter list and the confusing @owner parameter.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Thu, 4 Apr 2019 06:45:35 +0000 (14:45 +0800)]
btrfs: extent-tree: Use btrfs_ref to refactor btrfs_inc_extent_ref()
Use the new btrfs_ref structure and replace parameter list to clean up
the usage of owner and level to distinguish the extent types.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Thu, 4 Apr 2019 06:45:34 +0000 (14:45 +0800)]
btrfs: extent-tree: Use btrfs_ref to refactor add_pinned_bytes()
Since add_pinned_bytes() only needs to know if the extent is metadata
and if it's a chunk tree extent, btrfs_ref is a perfect match for it, as
we don't need various owner/level trick to determine extent type.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Thu, 4 Apr 2019 06:45:33 +0000 (14:45 +0800)]
btrfs: ref-verify: Use btrfs_ref to refactor btrfs_ref_tree_mod()
It's a perfect match for btrfs_ref_tree_mod() to use btrfs_ref, as
btrfs_ref describes a metadata/data reference update comprehensively.
Now we have one less function use confusing owner/level trick.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Thu, 4 Apr 2019 06:45:32 +0000 (14:45 +0800)]
btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_data_ref()
Just like btrfs_add_delayed_tree_ref(), use btrfs_ref to refactor
btrfs_add_delayed_data_ref().
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Thu, 4 Apr 2019 06:45:31 +0000 (14:45 +0800)]
btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_tree_ref()
btrfs_add_delayed_tree_ref() has a longer and longer parameter list, and
some callers like btrfs_inc_extent_ref() are using @owner as level for
delayed tree ref.
Instead of making the parameter list longer, use btrfs_ref to refactor
it, so each parameter assignment should be self-explaining without dirty
level/owner trick, and provides the basis for later refactoring.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Thu, 4 Apr 2019 06:45:30 +0000 (14:45 +0800)]
btrfs: extent-tree: Open-code process_func in __btrfs_mod_ref
The process_func function pointer is local to __btrfs_mod_ref() and
points to either btrfs_inc_extent_ref() or btrfs_free_extent().
Open code it to make later delayed ref refactor easier, so we can
refactor btrfs_inc_extent_ref() and btrfs_free_extent() in different
patches.
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Thu, 4 Apr 2019 06:45:29 +0000 (14:45 +0800)]
btrfs: delayed-ref: Introduce better documented delayed ref structures
Current delayed ref interface has several problems:
- Longer and longer parameter lists
bytenr
num_bytes
parent
---------- so far so good
ref_root
owner
offset
---------- I don't feel good now
- Different interpretation of the same parameter
Above @owner for data ref is inode number (u64),
while for tree ref, it's level (int).
They are even in different size range.
For level we only need 0 ~ 8, while for ino it's
BTRFS_FIRST_FREE_OBJECTID ~ BTRFS_LAST_FREE_OBJECTID.
And @offset doesn't even make sense for tree ref.
Such parameter reuse may look clever as an hidden union, but it
destroys code readability.
To solve both problems, we introduce a new structure, btrfs_ref to solve
them:
- Structure instead of long parameter list
This makes later expansion easier, and is better documented.
- Use btrfs_ref::type to distinguish data and tree ref
- Use proper union to store data/tree ref specific structures.
- Use separate functions to fill data/tree ref data, with a common generic
function to fill common bytenr/num_bytes members.
All parameters will find its place in btrfs_ref, and an extra member,
@real_root, inspired by ref-verify code, is newly introduced for later
qgroup code, to record which tree is triggered by this extent modification.
This patch doesn't touch any code, but provides the basis for further
refactoring.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Wed, 17 Apr 2019 10:30:30 +0000 (11:30 +0100)]
Btrfs: do not start a transaction at iterate_extent_inodes()
When finding out which inodes have references on a particular extent, done
by backref.c:iterate_extent_inodes(), from the BTRFS_IOC_LOGICAL_INO (both
v1 and v2) ioctl and from scrub we use the transaction join API to grab a
reference on the currently running transaction, since in order to give
accurate results we need to inspect the delayed references of the currently
running transaction.
However, if there is currently no running transaction, the join operation
will create a new transaction. This is inefficient as the transaction will
eventually be committed, doing unnecessary IO and introducing a potential
point of failure that will lead to a transaction abort due to -ENOSPC, as
recently reported [1].
That's because the join, creates the transaction but does not reserve any
space, so when attempting to update the root item of the root passed to
btrfs_join_transaction(), during the transaction commit, we can end up
failling with -ENOSPC. Users of a join operation are supposed to actually
do some filesystem changes and reserve space by some means, which is not
the case of iterate_extent_inodes(), it is a read-only operation for all
contextes from which it is called.
The reported [1] -ENOSPC failure stack trace is the following:
heisenberg kernel: ------------[ cut here ]------------
heisenberg kernel: BTRFS: Transaction aborted (error -28)
heisenberg kernel: WARNING: CPU: 0 PID: 7137 at fs/btrfs/root-tree.c:136 btrfs_update_root+0x22b/0x320 [btrfs]
(...)
heisenberg kernel: CPU: 0 PID: 7137 Comm: btrfs-transacti Not tainted 4.19.0-4-amd64 #1 Debian 4.19.28-2
heisenberg kernel: Hardware name: FUJITSU LIFEBOOK U757/FJNB2A5, BIOS Version 1.21 03/19/2018
heisenberg kernel: RIP: 0010:btrfs_update_root+0x22b/0x320 [btrfs]
(...)
heisenberg kernel: RSP: 0018:
ffffb5448828bd40 EFLAGS:
00010286
heisenberg kernel: RAX:
0000000000000000 RBX:
ffff8ed56bccef50 RCX:
0000000000000006
heisenberg kernel: RDX:
0000000000000007 RSI:
0000000000000092 RDI:
ffff8ed6bda166a0
heisenberg kernel: RBP:
00000000ffffffe4 R08:
00000000000003df R09:
0000000000000007
heisenberg kernel: R10:
0000000000000000 R11:
0000000000000001 R12:
ffff8ed63396a078
heisenberg kernel: R13:
ffff8ed092d7c800 R14:
ffff8ed64f5db028 R15:
ffff8ed6bd03d068
heisenberg kernel: FS:
0000000000000000(0000) GS:
ffff8ed6bda00000(0000) knlGS:
0000000000000000
heisenberg kernel: CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
heisenberg kernel: CR2:
00007f46f75f8000 CR3:
0000000310a0a002 CR4:
00000000003606f0
heisenberg kernel: DR0:
0000000000000000 DR1:
0000000000000000 DR2:
0000000000000000
heisenberg kernel: DR3:
0000000000000000 DR6:
00000000fffe0ff0 DR7:
0000000000000400
heisenberg kernel: Call Trace:
heisenberg kernel: commit_fs_roots+0x166/0x1d0 [btrfs]
heisenberg kernel: ? _cond_resched+0x15/0x30
heisenberg kernel: ? btrfs_run_delayed_refs+0xac/0x180 [btrfs]
heisenberg kernel: btrfs_commit_transaction+0x2bd/0x870 [btrfs]
heisenberg kernel: ? start_transaction+0x9d/0x3f0 [btrfs]
heisenberg kernel: transaction_kthread+0x147/0x180 [btrfs]
heisenberg kernel: ? btrfs_cleanup_transaction+0x530/0x530 [btrfs]
heisenberg kernel: kthread+0x112/0x130
heisenberg kernel: ? kthread_bind+0x30/0x30
heisenberg kernel: ret_from_fork+0x35/0x40
heisenberg kernel: ---[ end trace
05de912e30e012d9 ]---
So fix that by using the attach API, which does not create a transaction
when there is currently no running transaction.
[1] https://lore.kernel.org/linux-btrfs/
b2a668d7124f1d3e410367f587926f622b3f03a4.camel@scientia.net/
Reported-by: Zygo Blaxell <ce3g8jdj@umail.furryterror.org>
CC: stable@vger.kernel.org # 4.4+
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 15:34:54 +0000 (16:34 +0100)]
btrfs: get fs_info from device in btrfs_rm_dev_replace_free_srcdev
We can read fs_info from the device and can drop it from the parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 15:32:55 +0000 (16:32 +0100)]
btrfs: get fs_info from device in btrfs_scrub_cancel_dev
We can read fs_info from the device and can drop it from the parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 15:31:53 +0000 (16:31 +0100)]
btrfs: get fs_info from device in btrfs_rm_dev_item
We can read fs_info from the device and can drop it from the parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 13:40:41 +0000 (14:40 +0100)]
btrfs: get fs_info from eb in __push_leaf_left
We can read fs_info from extent buffer and can drop it from the
parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 13:39:45 +0000 (14:39 +0100)]
btrfs: get fs_info from eb in __push_leaf_right
We can read fs_info from extent buffer and can drop it from the
parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Wed, 10 Apr 2019 16:46:04 +0000 (19:46 +0300)]
btrfs: Remove bio_offset argument from submit_bio_hook
None of the implementers of the submit_bio_hook use the bio_offset
parameter, simply remove it. No functional changes.
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Wed, 10 Apr 2019 14:24:42 +0000 (17:24 +0300)]
btrfs: Always pass 0 bio_offset for btree_submit_bio_start
The btree submit hook queues the async csum and forwards the bio_offset
parameter passed to btree_submit_bio_hook. This is redundant since
btree_submit_bio_start calls btree_csum_one_bio which doesn't use the
offset at all. No functional changes.
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Wed, 10 Apr 2019 14:24:41 +0000 (17:24 +0300)]
btrfs: Pass 0 for bio_offset to btrfs_wq_submit_bio
Buffered writeback always calls btrfs_csum_one_bio with the last 2
arguments being 0 irrespective of what the bio_offset has been passed to
btrfs_submit_bio_start. Make this apparent by explicitly passing 0 for
bio_offset when calling btrfs_wq_submit_bio from btrfs_submit_bio_hook.
This will allow for further simplifications down the line. No functional
changes.
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Wed, 10 Apr 2019 14:24:40 +0000 (17:24 +0300)]
btrfs: Remove 'tree' argument from read_extent_buffer_pages
This function always uses the btree inode's io_tree. Stop taking the
tree as a function argument and instead access it internally from
read_extent_buffer_pages. No functional changes.
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Wed, 10 Apr 2019 14:24:39 +0000 (17:24 +0300)]
btrfs: Change submit_bio_hook to taking an inode directly
The only possible 'private_data' that is passed to this function is
actually an inode. Make that explicit by changing the signature of the
call back. No functional changes.
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Nikolay Borisov [Wed, 10 Apr 2019 14:24:38 +0000 (17:24 +0300)]
btrfs: Define submit_bio_hook's type directly
There is no need to use a typedef to define the type of the function and
then use that to define the respective member in extent_io_ops. Define
struct's member directly. No functional changes.
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 13:11:21 +0000 (14:11 +0100)]
btrfs: get fs_info from block group in search_free_space_info
We can read fs_info from the block group cache structure and can drop it
from the parameters. Though the transaction is also availabe, it's not
guaranteed to be non-NULL.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 12:53:49 +0000 (13:53 +0100)]
btrfs: get fs_info from block group in btrfs_find_space_cluster
We can read fs_info from the block group cache structure and can drop it
from the parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 12:49:09 +0000 (13:49 +0100)]
btrfs: get fs_info from block group in write_pinned_extent_entries
We can read fs_info from the block group cache structure and can drop it
from the parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 12:47:15 +0000 (13:47 +0100)]
btrfs: get fs_info from block group in load_free_space_cache
We can read fs_info from the block group cache structure and can drop it
from the parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 12:40:19 +0000 (13:40 +0100)]
btrfs: get fs_info from block group in lookup_free_space_inode
We can read fs_info from the block group cache structure and can drop it
from the parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 11:12:32 +0000 (12:12 +0100)]
btrfs: get fs_info from block group in pin_down_extent
We can read fs_info from the block group cache structure and can drop it
from the parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 11:01:07 +0000 (12:01 +0100)]
btrfs: get fs_info from block group in next_block_group
We can read fs_info from the block group cache structure and can drop it
from the parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Wed, 17 Apr 2019 09:28:47 +0000 (10:28 +0100)]
Btrfs: remove no longer used function to run delayed refs asynchronously
It used to be called from only two places (truncate path and releasing a
transaction handle), but commits
28bad2125767c5 ("btrfs: fix truncate
throttling") and
db2462a6ad3dc4 ("btrfs: don't run delayed refs in the end
transaction logic") removed their calls to this function, so it's not used
anymore. Just remove it and all its helpers.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Fri, 12 Apr 2019 08:02:59 +0000 (16:02 +0800)]
btrfs: cleanup btrfs_setxattr_trans and drop transaction parameter
Previous patch made sure that btrfs_setxattr_trans() is called only when
transaction NULL. Clean up btrfs_setxattr_trans() and drop the
parameter.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Fri, 12 Apr 2019 08:02:58 +0000 (16:02 +0800)]
btrfs: split btrfs_setxattr calls regarding transaction
When the caller has already created the transaction handle,
btrfs_setxattr() will use it. Also adds assert in btrfs_setxattr().
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Fri, 12 Apr 2019 08:02:57 +0000 (16:02 +0800)]
btrfs: remove redundant readonly root check in btrfs_setxattr_trans
btrfs_setxattr_trans() is called by 5 functions as below and all of them
do updates. None of them would be roun on a read-only root.
So its ok to remove the readonly root check here as it's a high-level
conditon.
1.
__btrfs_set_acl()
btrfs_init_acl()
btrfs_init_inode_security()
2.
__btrfs_set_acl()
btrfs_set_acl()
3.
btrfs_set_prop()
btrfs_set_prop_trans()
/ \
btrfs_ioctl_setflags() btrfs_xattr_handler_set_prop()
4.
btrfs_xattr_handler_set()
5.
btrfs_initxattrs()
btrfs_xattr_security_init()
btrfs_init_inode_security()
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Fri, 12 Apr 2019 08:02:56 +0000 (16:02 +0800)]
btrfs: export btrfs_setxattr
Preparatory patch, as we are going split the calls with and without
transaction to use the respective btrfs_setxattr() and
btrfs_setxattr_trans() functions. Export btrfs_setxattr() for calls
outside of xattr.c.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Fri, 12 Apr 2019 08:02:55 +0000 (16:02 +0800)]
btrfs: rename do_setxattr to btrfs_setxattr
When trans is not NULL btrfs_setxattr() calls do_setxattr() directly
with a check for readonly root. Rename do_setxattr() btrfs_setxattr() in
preparation to call do_setxattr() directly instead. Preparatory patch,
no functional changes.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Anand Jain [Fri, 12 Apr 2019 08:02:54 +0000 (16:02 +0800)]
btrfs: rename btrfs_setxattr to btrfs_setxattr_trans
Rename btrfs_setxattr() to btrfs_setxattr_trans(), so that do_setxattr()
can be renamed to btrfs_setxattr().
Preparatory patch, no functional changes.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Mon, 15 Apr 2019 13:15:25 +0000 (21:15 +0800)]
btrfs: trace: Introduce trace events for all btrfs tree locking events
Unlike btrfs_tree_lock() and btrfs_tree_read_lock(), the remaining
functions in locking.c will not sleep, thus doesn't make much sense to
record their execution time.
Those events are introduced mainly for user space tool to audit and
detect lock leakage or dead lock.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo [Mon, 15 Apr 2019 13:15:24 +0000 (21:15 +0800)]
btrfs: trace: Introduce trace events for sleepable tree lock
There are two tree lock events which can sleep:
- btrfs_tree_read_lock()
- btrfs_tree_lock()
Sometimes we may need to look into the concurrency picture of the fs.
For that case, we need the execution time of above two functions and the
owner of @eb.
Here we introduce a trace events for user space tools like bcc, to get
the execution time of above two functions, and get detailed owner info
where eBPF code can't.
All the overhead is hidden behind the trace events, so if events are not
enabled, there is no overhead.
These trace events also output bytenr and generation, allow them to be
pared with unlock events to pin down deadlock.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Filipe Manana [Mon, 15 Apr 2019 08:29:19 +0000 (09:29 +0100)]
Btrfs: remove no longer used member num_dirty_bgs from transaction
The member num_dirty_bgs of struct btrfs_transaction is not used anymore,
it is set and incremented but nothing reads its value anymore. Its last
read use was removed by commit
64403612b73a94 ("btrfs: rework
btrfs_check_space_for_delayed_refs"). So just remove that member.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 15:51:44 +0000 (16:51 +0100)]
btrfs: get fs_info from trans in btrfs_run_dev_replace
We can read fs_info from the transaction and can drop it from the
parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 15:50:38 +0000 (16:50 +0100)]
btrfs: get fs_info from trans in btrfs_run_dev_stats
We can read fs_info from the transaction and can drop it from the
parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 15:36:39 +0000 (16:36 +0100)]
btrfs: get fs_info from trans in btrfs_finish_sprout
We can read fs_info from the transaction and can drop it from the
parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 15:29:13 +0000 (16:29 +0100)]
btrfs: get fs_info from trans in init_first_rw_device
We can read fs_info from the transaction and can drop it from the
parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 13:42:33 +0000 (14:42 +0100)]
btrfs: get fs_info from trans in copy_for_split
We can read fs_info from the transaction and can drop it from the
parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 13:32:45 +0000 (14:32 +0100)]
btrfs: get fs_info from trans in insert_ptr
We can read fs_info from the transaction and can drop it from the
parameters.
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba [Wed, 20 Mar 2019 13:18:06 +0000 (14:18 +0100)]
btrfs: get fs_info from trans in balance_node_right
We can read fs_info from the transaction and can drop it from the
parameters.
Signed-off-by: David Sterba <dsterba@suse.com>