- 25 Jul, 2019 1 commit
-
-
Nikolay Borisov authored
Commit 06297d8c ("btrfs: switch extent_buffer blocking_writers from atomic to int") changed the type of blocking_writers but forgot to adjust relevant code in btrfs_tree_unlock by converting the smp_mb__after_atomic to smp_mb. This opened up the possibility of a deadlock due to re-ordering of setting blocking_writers and checking/waking up the waiter. This particular lockup is explained in a comment above waitqueue_active() function. Fix it by converting the memory barrier to a full smp_mb, accounting for the fact that blocking_writers is a simple integer. Fixes: 06297d8c ("btrfs: switch extent_buffer blocking_writers from atomic to int") Tested-by:
Johannes Thumshirn <jthumshirn@suse.com> Signed-off-by:
Nikolay Borisov <nborisov@suse.com> Reviewed-by:
David Sterba <dsterba@suse.com> Signed-off-by:
-
- 17 Jul, 2019 4 commits
-
-
Johannes Thumshirn authored
btrfs_get_io_geometry() calls btrfs_get_chunk_map() to acquire a reference on a extent_map, but on normal operation it does not drop this reference anymore. This leads to excessive kmemleak reports. Always call free_extent_map(), not just in the error case. Fixes: 5f141126 ("btrfs: Introduce btrfs_io_geometry infrastructure") Reviewed-by:
Johannes Thumshirn <jthumshirn@suse.de> Reviewed-by:
-
Johannes Thumshirn authored
fs_info::csum_hash gets initialized in btrfs_init_csum_hash() which is called by open_ctree(). But it only gets freed if open_ctree() fails, not on normal operation. This leads to a memory leak like the following found by kmemleak: unreferenced object 0xffff888132cb8720 (size 96): comm "mount", pid 450, jiffies 4294912436 (age 17.584s) hex dump (first 32 bytes): 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [<000000000c9643d4>] crypto_create_tfm+0x2d/0xd0 [<00000000ae577f68>] crypto_alloc_tfm+0x4b/0xb0 [<000000002b5cdf30>] open_ctree+0xb84/0x2060 [btrfs] [<0000000043204297>] btrfs_mount_root+0x552/0x640 [btrfs] [<00000000c99b10ea>] legacy_get_tree+0x22/0x40 [<0000000071a6495f>] vfs_get_tree+0x1f/0xc0 [<00000000f180080e>] fc_mount+0x9/0x30 [<000000009e36cebd>] vfs_kern_mount.part.11+0x6a/0x80 [<0000000004594c05>] btrfs_mount+0x174/0x910 [btrfs] [<00000000c99b10ea>] legacy_get_tree+0x22/0x40 [<0000000071a6495f>] vfs_get_tree+0x1f/0xc0 [<00000000b86e92c5>] do_mount+0x6b0/0x940 [<0000000097464494>] ksys_mount+0x7b/0xd0 [<0000000057213c80>] __x64_sys_mount+0x1c/0x20 [<00000000cb689b5e>] do_syscall_64+0x43/0x130 [<000000002194e289>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Free fs_info::csum_hash in close_ctree() to avoid the memory leak. Fixes: 6d97c6e3 ("btrfs: add boilerplate code for directly including the crypto framework") Reviewed-by:Qu Wenruo <wqu@suse.com> Signed-off-by:
-
YueHaibing authored
If CONFIG_BTRFS_FS is y and CONFIG_LIBCRC32C is m, building fails: fs/btrfs/super.o: In function `btrfs_mount_root': super.c:(.text+0xb7f9): undefined reference to `crc32c_impl' fs/btrfs/super.o: In function `init_btrfs_fs': super.c:(.init.text+0x3465): undefined reference to `crc32c_impl' fs/btrfs/extent-tree.o: In function `hash_extent_data_ref': extent-tree.c:(.text+0xe60): undefined reference to `crc32c' extent-tree.c:(.text+0xe78): undefined reference to `crc32c' extent-tree.c:(.text+0xe8b): undefined reference to `crc32c' fs/btrfs/dir-item.o: In function `btrfs_insert_xattr_item': dir-item.c:(.text+0x291): undefined reference to `crc32c' fs/btrfs/dir-item.o: In function `btrfs_insert_dir_item': dir-item.c:(.text+0x429): undefined reference to `crc32c' Select LIBCRC32C to fix it. Reported-by:
Hulk Robot <hulkci@huawei.com> Fixes: d5178578 ("btrfs: directly call into crypto framework for checksumming") Reviewed-by:
YueHaibing <yuehaibing@huawei.com> Reviewed-by:
-
Qu Wenruo authored
As btrfs(5) specified: Note If nodatacow or nodatasum are enabled, compression is disabled. If NODATASUM or NODATACOW set, we should not compress the extent. Normally NODATACOW is detected properly in run_delalloc_range() so compression won't happen for NODATACOW. However for NODATASUM we don't have any check, and it can cause compressed extent without csum pretty easily, just by: mkfs.btrfs -f $dev mount $dev $mnt -o nodatasum touch $mnt/foobar mount -o remount,datasum,compress $mnt xfs_io -f -c "pwrite 0 128K" $mnt/foobar And in fact, we have a bug report about corrupted compressed extent without proper data checksum so even RAID1 can't recover the corruption. (https://bugzilla.kernel.org/show_bug.cgi?id=199707) Running compression without proper checksum could cause more damage when corruption happens, as compressed data could make the whole extent unreadable, so there is no need to allow compression for NODATACSUM. The fix will refactor the inode compression check into two parts: - inode_can_compress() As the hard requirement, checked at btrfs_run_delalloc_range(), so no compression will happen for NODATASUM inode at all. - inode_need_compress() As the soft requirement, checked at btrfs_run_delalloc_range() and compress_file_range(). Reported-by:
James Harvey <jamespharvey20@gmail.com> CC: stable@vger.kernel.org # 4.4+ Signed-off-by:
-
- 05 Jul, 2019 1 commit
-
-
Colin Ian King authored
Currently if the allocation of roots or tmp_ulist fails the error handling does not free up the allocation of path causing a memory leak. Fix this and other similar leaks by moving the call of btrfs_free_path from label out to label out_free_ulist. Kudos to David Sterba for spotting the issue in my original fix and suggesting the correct way to fix the leak and Anand Jain for spotting a double free issue. Addresses-Coverity: ("Resource leak") Fixes: 5911c8fe ("btrfs: fiemap: preallocate ulists for btrfs_check_shared") Reviewed-by:Colin Ian King <colin.king@canonical.com> Reviewed-by:
-
- 04 Jul, 2019 5 commits
-
-
Josef Bacik authored
This is just two functions, put it in root-tree.c since it involves root items. Signed-off-by:
Josef Bacik <josef@toxicpanda.com> Signed-off-by:
-
Josef Bacik authored
We have code for data and metadata reservations for delalloc. There's quite a bit of code here, and it's used in a lot of places so I've separated it out to it's own file. inode.c and file.c are already pretty large, and this code is complicated enough to live in its own space. Signed-off-by:
-
Josef Bacik authored
Move this into transaction.c with the rest of the transaction related code. Signed-off-by:
-
Josef Bacik authored
These belong with the delayed refs related code, not in extent-tree.c. Signed-off-by:
-
Goldwyn Rodrigues authored
Simplification. No point passing the tree variable when it can be evaluated from inode. The tests now use the io_tree from btrfs_inode as opposed to creating one. Signed-off-by:
Goldwyn Rodrigues <rgoldwyn@suse.com> Reviewed-by:
-
- 02 Jul, 2019 29 commits
-
-
Josef Bacik authored
These helpers belong in block-rsv.c Signed-off-by:
-
Josef Bacik authored
This moves everything out of extent-tree.c to block-rsv.c. Signed-off-by:
-
Josef Bacik authored
block_rsv_release_bytes() is the internal to the block_rsv code, and shouldn't be called directly by anything else. Switch all users to the exported helpers. Signed-off-by:
-
Josef Bacik authored
This works for all callers already, but if we wanted to use the helper for the global_block_rsv it would end up trying to refill itself. Fix the logic to be able to be used no matter which block rsv is passed in to this helper. Signed-off-by:
-
Josef Bacik authored
The delalloc reserve stuff calls this directly because it cares about the qgroup accounting stuff, so export it so we can move it around. Fix btrfs_block_rsv_release() to just be a static inline since it just calls __btrfs_block_rsv_release() with NULL for the qgroup stuff. Signed-off-by:
-
Josef Bacik authored
This is used in a few places, we need to make sure it's exported so we can move it around. Signed-off-by:
-
Josef Bacik authored
Prep work for separating out all of the block_rsv related code into its own file. Signed-off-by:
-
Goldwyn Rodrigues authored
We don't need an if-else-if chain where we can use a simple OR since both conditions are performing the same action. The short-circuit for OR will ensure that if the first condition is true, can_overcommit() is not called. Reviewed-by:
-
Josef Bacik authored
Now that we've moved all of the users to space-info.c, unexport it and name it back to can_overcommit. Reviewed-by:
-
Josef Bacik authored
This moves all of the metadata reservation code into space-info.c. Reviewed-by:
-
Josef Bacik authored
We'll need this exported so we can use it in all the various was we need to use it. This is prep work to move reserve_metadata_bytes. Reviewed-by:
-
Josef Bacik authored
We are going to need this to move the metadata reservation stuff to space_info.c. Reviewed-by:
-
Josef Bacik authored
Now that we've moved all the pre-requisite stuff, move these two functions. Reviewed-by:
-
Josef Bacik authored
Also rename it to btrfs_space_info_update_* so it's clear what we're updating. Reviewed-by:
-
Josef Bacik authored
This is the first piece of moving the space reservation code to space-info.c Reviewed-by:
-
Josef Bacik authored
These are the basic init and lookup functions and some helper functions, fairly straightforward before the bad stuff starts. Reviewed-by:
-
Josef Bacik authored
Prep work for consolidating all of the space_info code into one file. We need to export these so multiple files can use them. Reviewed-by:
-
Josef Bacik authored
Really we just need the enum, but as we break more things up it'll help to have this external to extent-tree.c. Reviewed-by:
-
Josef Bacik authored
Migrate the struct definition and the one helper that's in ctree.h into space-info.h Reviewed-by:
-
David Sterba authored
The block device is passed around for the only purpose to set it in new bios. Move the assignment one level up. This is a preparatory patch for further bdev cleanups. Signed-off-by: -
David Sterba authored
Minimum stripe count matches the minimum devices required for a given profile. The open coded assignments match the raid_attr table. What's changed here is the meaning for RAID5/6. Previously their min_stripes would be 1, while newly it's devs_min. This however shold be the same as before because it's not possible to create filesystem on fewer devices than the raid_attr table allows. There's no adjustment regarding the parity stripes (like calc_data_stripes does), because we're interested in overall space that would fit on the devices. Missing devices make no difference for the whole calculation, we have the size stored in the structures. Signed-off-by: -
David Sterba authored
Special case for DUP can be replaced by lookup to the attribute table, where the dev_stripes is the right coefficient. Signed-off-by: -
David Sterba authored
A few more instances whre we don't need to specify the values as long as they are the same that enum assigns automatically. All of the enums are in-memory only and nothing relies on the exact values. Signed-off-by: -
David Sterba authored
Print the error messages using the helpers that also print the filesystem identification. Signed-off-by: -
Josef Bacik authored
We have been seeing issues in production where a cleaner script will end up unlinking a bunch of files that have pending iputs. This means they will get their final iput's run at btrfs-cleaner time and thus are not throttled, which impacts the workload. Since we are unlinking these files we can just drop the delayed iput at unlink time. We are already holding a reference to the inode so this will not be the final iput and thus is completely safe to do at this point. Doing this means we are more likely to be doing the final iput at unlink time, and thus will get the IO charged to the caller and get throttled appropriately without affecting the main workload. Reviewed-by:
-
Filipe Manana authored
If the range for which we are punching a hole covers only part of a page, we end up updating the inode item but we skip the update of the inode's iversion, mtime and ctime. Fix that by ensuring we update those properties of the inode. A patch for fstests test case generic/059 that tests this as been sent along with this fix. Fixes: 2aaa6655 ("Btrfs: add hole punching") Fixes: e8c1c76e ("Btrfs: add missing inode update when punching hole") CC: stable@vger.kernel.org # 4.4+ Signed-off-by:
Filipe Manana <fdmanana@suse.com> Signed-off-by:
-
Filipe Manana authored
In order to avoid searches on a log tree when unlinking an inode, we check if the inode being unlinked was logged in the current transaction, as well as the inode of its parent directory. When any of the inodes are logged, we proceed to delete directory items and inode reference items from the log, to ensure that if a subsequent fsync of only the inode being unlinked or only of the parent directory when the other is not fsync'ed as well, does not result in the entry still existing after a power failure. That check however is not reliable when one of the inodes involved (the one being unlinked or its parent directory's inode) is evicted, since the logged_trans field is transient, that is, it is not stored on disk, so it is lost when the inode is evicted and loaded into memory again (which is set to zero on load). As a consequence the checks currently being done by btrfs_del_dir_entries_in_log() and btrfs_del_inode_ref_in_log() always return true if the inode was evicted before, regardless of the inode having been logged or not before (and in the current transaction), this results in the dentry being unlinked still existing after a log replay if after the unlink operation only one of the inodes involved is fsync'ed. Example: $ mkfs.btrfs -f /dev/sdb $ mount /dev/sdb /mnt $ mkdir /mnt/dir $ touch /mnt/dir/foo $ xfs_io -c fsync /mnt/dir/foo # Keep an open file descriptor on our directory while we evict inodes. # We just want to evict the file's inode, the directory's inode must not # be evicted. $ ( cd /mnt/dir; while true; do :; done ) & $ pid=$! # Wait a bit to give time to background process to chdir to our test # directory. $ sleep 0.5 # Trigger eviction of the file's inode. $ echo 2 > /proc/sys/vm/drop_caches # Unlink our file and fsync the parent directory. After a power failure # we don't expect to see the file anymore, since we fsync'ed the parent # directory. $ rm -f $SCRATCH_MNT/dir/foo $ xfs_io -c fsync /mnt/dir <power failure> $ mount /dev/sdb /mnt $ ls /mnt/dir foo $ --> file still there, unlink not persisted despite explicit fsync on dir Fix this by checking if the inode has the full_sync bit set in its runtime flags as well, since that bit is set everytime an inode is loaded from disk, or for other less common cases such as after a shrinking truncate or failure to allocate extent maps for holes, and gets cleared after the first fsync. Also consider the inode as possibly logged only if it was last modified in the current transaction (besides having the full_fsync flag set). Fixes: 3a5f1d45 ("Btrfs: Optimize btree walking while logging inodes") CC: stable@vger.kernel.org # 4.4+ Signed-off-by:
-
Nikolay Borisov authored
Presently btrfs_map_block is used not only to do everything necessary to map a bio to the underlying allocation profile but it's also used to identify how much data could be written based on btrfs' stripe logic without actually submitting anything. This is achieved by passing NULL for 'bbio_ret' parameter. This patch refactors all callers that require just the mapping length by switching them to using btrfs_io_geometry instead of calling btrfs_map_block with a special NULL value for 'bbio_ret'. No functional change. Signed-off-by:
-
Nikolay Borisov authored
Add a structure that holds various parameters for IO calculations and a helper that fills the values. This will help further refactoring and reduction of functions that in some way open-coded the calculations. Signed-off-by:
-
