summaryrefslogtreecommitdiff
path: root/fs
AgeCommit message (Collapse)Author
2025-05-15btrfs: exit after state split error at set_extent_bit()Filipe Manana
If split_state() returned an error we call extent_io_tree_panic() which will trigger a BUG() call. However if CONFIG_BUG is disabled, which is an uncommon and exotic scenario, then we fallthrough and hit a use after free when calling set_state_bits() since the extent state record which the local variable 'prealloc' points to was freed by split_state(). So jump to the label 'out' after calling extent_io_tree_panic() and set the 'prealloc' pointer to NULL since split_state() has already freed it when it hit an error. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: exit after state insertion failure at set_extent_bit()Filipe Manana
If insert_state() state failed it returns an error pointer and we call extent_io_tree_panic() which will trigger a BUG() call. However if CONFIG_BUG is disabled, which is an uncommon and exotic scenario, then we fallthrough and call cache_state() which will dereference the error pointer, resulting in an invalid memory access. So jump to the 'out' label after calling extent_io_tree_panic(), it also makes the code more clear besides dealing with the exotic scenario where CONFIG_BUG is disabled. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: simplify last record detection at btrfs_convert_extent_bit()Filipe Manana
There's no need to compare the current extent state's end offset to (u64)-1 to check if we have the last possible record and to check as as well if after updating the start offset to the end offset of the current record plus one we are still inside the target range. Instead we can simplify and exit if the current extent state ends at or after the target range and then remove the check for the (u64)-1 as well as the check to see if the updated start offset (to last_end + 1) is still inside the target range. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: avoid re-searching tree when converting bits in an extent rangeFilipe Manana
When converting bits for an extent range (btrfs_convert_extent_bit()), if the current extent state record starts after the target range, we always do a jump to the 'search_again' label, which will cause us to do a full tree search for the next state if the current state ends before the target range. Unless we need to reschedule, we can just grab the next state and process it, avoiding a full tree search, even if that next state is not contiguous, as we'll allocate and insert a new prealloc state if needed. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: avoid repeated extent state processing when converting extent bitsFilipe Manana
When converting bits for an extent range, if we find an extent state with its start offset greater than current start offset, we insert a new extent state to cover the gap, with its end offset computed and stored in the @this_end local variable, and after the insertion we update the current start offset to @this_end + 1. However if the insert_state() call resulted in an extent state merge then the end offset of the merged extent may be greater than @this_end and if that's the case, since we jump to the 'search_again' label, we'll do a full tree search that will leave us in the same extent state - this is harmless but wastes time by doing a pointless tree search and extent state processing. So improve on this by updating the current start offset to the end offset of the inserted state plus 1. This also removes the use of the @this_end variable and directly set the value in the prealloc extent state to avoid any confusion and misuse in the future. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: avoid unnecessary next node searches when clearing bits from extent rangeFilipe Manana
When clearing bits for a range in an io tree, at clear_state_bit(), we always go search for the next node (through next_state() -> rb_next()) and return it. However if the current extent state record ends at or after the target range passed to btrfs_clear_extent_bit_changeset() or btrfs_convert_extent_bit(), we are just wasting time finding that next node since we won't use it in those functions. Improve on this by skipping the next node search if the current node ends at or after the target range. Reviewed-by: Boris Burkov <boris@bur.io> Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: exit after state insertion failure at btrfs_convert_extent_bit()Filipe Manana
If insert_state() state failed it returns an error pointer and we call extent_io_tree_panic() which will trigger a BUG() call. However if CONFIG_BUG is disabled, which is an uncommon and exotic scenario, then we fallthrough and call cache_state() which will dereference the error pointer, resulting in an invalid memory access. So jump to the 'out' label after calling extent_io_tree_panic(), it also makes the code more clear besides dealing with the exotic scenario where CONFIG_BUG is disabled. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: exit after state split error at btrfs_convert_extent_bit()Filipe Manana
If split_state() returned an error we call extent_io_tree_panic() which will trigger a BUG() call. However if CONFIG_BUG is disabled, which is an uncommon and exotic scenario, then we fallthrough and hit a use after free when calling set_state_bits() since the extent state record which the local variable 'prealloc' points to was freed by split_state(). So jump to the label 'out' after calling extent_io_tree_panic() and set the 'prealloc' pointer to NULL since split_state() has already freed it when it hit an error. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: remove duplicate error check at btrfs_convert_extent_bit()Filipe Manana
There's no need to check if split_state() returned an error twice, instead unify into a single if statement after setting 'prealloc' to NULL, because on error split_state() frees the 'prealloc' extent state record. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: simplify last record detection at btrfs_clear_extent_bit_changeset()Filipe Manana
Instead of checking for an end offset of (u64)-1 (U64_MAX) for the current extent state's end, and then checking after updating the current start offset if it's now beyond the range's end offset, we can simply stop if the current extent state's end is greater than or equals to our range's end offset. This helps remove one comparison under the 'next' label and allows to remove the if statement that checks if the start offset is greater than the end offset under the 'search_again' label. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: avoid extra tree search at btrfs_clear_extent_bit_changeset()Filipe Manana
When we find an extent state that starts before our range's start we split it and jump into the 'search_again' label with our start offset remaining the same, making us then go to the 'again' label and search again for an extent state that contains the 'start' offset, and this time it finds the same extent state but with its start offset set to our range's start offset (due to the split). This is because we have consumed the preallocated extent state record and we may need to split again, and by jumping to 'again' we release the spinlock, allocate a new prealloc state and restart the search. However we may not need to restart and allocate a new extent state in case we don't find extent states that cross our end offset, therefore no need for further extent state splits, or we may be able to do an atomic allocation (which is quick even if it fails). In these cases it's a waste to restart the search. So change the behaviour to do the restart only if we need to reschedule, otherwise fall through - if we need to allocate an extent state for split operations, we will try an atomic allocation and if that fails we will do the restart as before. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: use bools for local variables at btrfs_clear_extent_bit_changeset()Filipe Manana
Several variables are defined as integers but used as booleans, and the 'delete' variable can be made const since it's not changed after being declared. So change them to proper booleans and simplify setting their value. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: add missing error return to btrfs_clear_extent_bit_changeset()Filipe Manana
We have a couple error branches where we have an error stored in the 'err' variable and then jump to the 'out' label, however we don't return that error, we just return 0. Normally this is not a problem since those error branches call extent_io_tree_panic() which triggers a BUG() call, however it's possible to have rather exotic kernel config with CONFIG_BUG disabled in which case the BUG() call does nothing and we fallthrough. So make sure to return the error, not just to fix that exotic case but also to make the code less confusing. While at it also rename the 'err' variable to 'ret' since this is the style we prefer and use more widely. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: exit after state split error at btrfs_clear_extent_bit_changeset()Filipe Manana
If split_state() returned an error we call extent_io_tree_panic() which will trigger a BUG() call. However if CONFIG_BUG is disabled, which is an uncommon and exotic scenario, then we fallthrough and hit a use after free when calling clear_state_bit() since the extent state record which the local variable 'prealloc' points to was freed by split_state(). So jump to the label 'out' after calling extent_io_tree_panic() and set the 'prealloc' pointer to NULL since split_state() has already freed it when it hit an error. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: remove duplicate error check at btrfs_clear_extent_bit_changeset()Filipe Manana
There's no need to check if split_state() returned an error twice, instead unify into a single if statement after setting 'prealloc' to NULL, because on error split_state() frees the 'prealloc' extent state record. Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: get rid of btrfs_read_dev_super()Qu Wenruo
The function is introduced by commit a512bbf855ff ("Btrfs: superblock duplication") at the beginning of btrfs. It leaved a comment saying we'd need a special mount option to read all super blocks, but it's never been implemented and there was not need/request for it. The check/rescue tools are able to start from a specific copy and use it as primary eventually. This means btrfs_read_dev_super() is always reading the first super block, making all the code finding the latest super block unnecessary. Just remove that function and replace all call sites with btrfs_read_disk_super(bdev, 0, false). Signed-off-by: Qu Wenruo <wqu@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: merge btrfs_read_dev_one_super() into btrfs_read_disk_super()Qu Wenruo
We have two functions to read a super block from a block device: - btrfs_read_dev_one_super() Exported from disk-io.c - btrfs_read_disk_super() Local to volumes.c And they have some minor differences: - btrfs_read_dev_one_super() uses @copy_num Meanwhile btrfs_read_disk_super() relies on the physical and expected bytenr passed from the caller. The parameter list of btrfs_read_dev_one_super() is more user friendly. - btrfs_read_disk_super() makes sure the label is NUL terminated We do not need two different functions doing the same job, so merge the behavior into btrfs_read_disk_super() by: - Remove btrfs_read_dev_one_super() - Export btrfs_read_disk_super() The name pairs with btrfs_release_disk_super() perfectly. - Change the parameter list of btrfs_read_disk_super() to mimic btrfs_read_dev_one_super() All existing callers are calculating the physical address and expect bytenr before calling btrfs_read_disk_super() already. Signed-off-by: Qu Wenruo <wqu@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: get rid of goto in alloc_test_extent_buffer()Daniel Vacek
The `free_eb` label is used only once. Simplify by moving the code inplace. Signed-off-by: Daniel Vacek <neelx@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: use buffer xarray for extent buffer writeback operationsJosef Bacik
Currently we have this ugly back and forth with the btree writeback where we find the folio, find the eb associated with that folio, and then attempt to writeback. This results in two different paths for subpage ebs and >= page size ebs. Clean this up by adding our own infrastructure around looking up tagged ebs and writing the ebs out directly. This allows us to unify the subpage and >= pagesize IO paths, resulting in a much cleaner writeback path for extent buffers. I ran this through fsperf on a VM with 8 CPUs and 16GiB of RAM. I used smallfiles100k, but reduced the files to 1k to make it run faster, the results are as follows, with the statistically significant improvements marked with *, there were no regressions. fsperf was run with -n 10 for both runs, so the baseline is the average 10 runs and the test is the average of 10 runs. smallfiles100k results metric baseline current stdev diff ================================================================================ avg_commit_ms 68.58 58.44 3.35 -14.79% * commits 270.60 254.70 16.24 -5.88% dev_read_iops 48 48 0 0.00% dev_read_kbytes 1044 1044 0 0.00% dev_write_iops 866117.90 850028.10 14292.20 -1.86% dev_write_kbytes 10939976.40 10605701.20 351330.32 -3.06% elapsed 49.30 33 1.64 -33.06% * end_state_mount_ns 41251498.80 35773220.70 2531205.32 -13.28% * end_state_umount_ns 1.90e+09 1.50e+09 14186226.85 -21.38% * max_commit_ms 139 111.60 9.72 -19.71% * sys_cpu 4.90 3.86 0.88 -21.29% write_bw_bytes 42935768.20 64318451.10 1609415.05 49.80% * write_clat_ns_mean 366431.69 243202.60 14161.98 -33.63% * write_clat_ns_p50 49203.20 20992 264.40 -57.34% * write_clat_ns_p99 827392 653721.60 65904.74 -20.99% * write_io_kbytes 2035940 2035940 0 0.00% write_iops 10482.37 15702.75 392.92 49.80% * write_lat_ns_max 1.01e+08 90516129 3910102.06 -10.29% * write_lat_ns_mean 366556.19 243308.48 14154.51 -33.62% * As you can see we get about a 33% decrease runtime, with a 50% throughput increase, which is pretty significant. Reviewed-by: Filipe Manana <fdmanana@suse.com> Signed-off-by: Josef Bacik <josef@toxicpanda.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: set DIRTY and WRITEBACK tags on the buffer_treeJosef Bacik
In preparation for changing how we do writeout of extent buffers, start tagging the extent buffer xarray with DIRTY and WRITEBACK to make it easier to find extent buffers that are in either state. Reviewed-by: Filipe Manana <fdmanana@suse.com> Signed-off-by: Josef Bacik <josef@toxicpanda.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: convert the buffer_radix to an xarrayJosef Bacik
In order to fully utilize xarray tagging to improve writeback we need to convert the buffer_radix to a proper xarray. This conversion is relatively straightforward as the radix code uses the xarray underneath. Using xarray directly allows for quite a lot less code. 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>
2025-05-15btrfs: rename btrfs_discard workqueue to btrfs-discardDavid Sterba
We use the "btrfs-" prefix for our workqueues, the discard has underscore instead of dash, so unify it. Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: on unknown chunk allocation policy fallback to regularDavid Sterba
We have only two chunk allocation policies right now and the switch/cases don't handle an unknown one properly. The error is in the impossible category (the policy is stored only in memory), we don't have to BUG(), falling back to regular policy should be safe. Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: reformat comments in acls_after_inode_item()David Sterba
Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: switch int dev_replace_is_ongoing variables/parameters to boolDavid Sterba
Both the variable and the parameter are used as logical indicators so convert them to bool. Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: trivial conversion to return bool instead of intDavid Sterba
Old code has a lot of int for bool return values, bool is recommended and done in new code. Convert the trivial cases that do simple 0/false and 1/true. Functions comment are updated if needed. Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: subpage: reject tree blocks which are not nodesize alignedQu Wenruo
When btrfs subpage support (fs block < page size) was introduced, a subpage filesystem will only reject tree blocks which cross page boundaries. This used to be a compromise to simplify the tree block handling and still allowing subpage cases to read some old converted filesystems which did not have proper chunk alignment. But in practice, suppose we have the following unaligned tree block on a 64K page sized system: 0 32K 44K 60K 64K | |///////////////| | Although btrfs has no problem reading the tree block at [44K, 60K), if extent allocator is allocating another tree block, it may choose the range [60K, 74K), as extent allocator has no awareness if it's a subpage metadata request or not. Then we'd get -EINVAL from the following sequence: btrfs_alloc_tree_block() |- btrfs_reserve_extent() | Which returned range [60K, 74K) |- btrfs_init_new_buffer() |- btrfs_find_create_tree_block() |- alloc_extent_buffer() |- check_eb_alignment() Which returned -EINVAL, because the range crosses page boundary. This situation will not fix itself and should mostly mark the fs read-only. Thankfully we didn't really get such reports in the real world because: - The original unaligned tree block is only caused by older btrfs-convert It's before the btrfs-convert rework was done in v4.6, where converted btrfs filesystem can have metadata block groups which are not aligned to nodesize nor stripe size (64K). But after btrfs-progs v4.6, all chunks allocated will be stripe (64K) aligned, thus no more such problem. Considering how old the fix is (v4.6 was released almost 10 years ago), subpage support for btrfs was introduced in v5.15, it should be safe to reject those unaligned tree blocks. Signed-off-by: Qu Wenruo <wqu@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: move folio initialization to one place in attach_eb_folio_to_filemap()Daniel Vacek
This is just a trivial change. The code looks a bit more readable this way, IMO. Move initialization of existing_folio to the beginning of the retry loop so it's set to NULL at one place. Signed-off-by: Daniel Vacek <neelx@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: raid56: rename parameter err to status in endio helpersDavid Sterba
Trivial renames to unify the naming of blk_status_t variables/parameters. Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: change return type of btrfs_alloc_dummy_sum() to intDavid Sterba
The type blk_status_t is from block layer and not related to checksums in our context. Use int internally and do the conversions to blk_status_t as needed in btrfs_submit_chunk(). Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: rename ret2 to ret in btrfs_submit_compressed_read()David Sterba
We can now rename 'ret2' to 'ret' and use it for generic errors. Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: rename ret to status in btrfs_submit_compressed_read()David Sterba
We're using 'status' for the blk_status_t variables, rename 'ret' so we can use it for generic errors. Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: simplify reading bio status in end_compressed_writeback()David Sterba
We don't need to have a separate variable to read the bio status, 'ret' works for that just fine so remove 'error'. Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: rename error to ret in btrfs_submit_chunk()David Sterba
We can now rename 'error' to 'ret' and use it for generic errors. Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: rename ret to status in btrfs_submit_chunk()David Sterba
We're using 'status' for the blk_status_t variables, rename 'ret' so we can use it for proper return type. Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: change return type of btrfs_bio_csum() to intDavid Sterba
The type blk_status_t is from block layer and not related to checksums in our context. Use int internally and do the conversions to blk_status_t as needed. Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: change return type of btree_csum_one_bio() to intDavid Sterba
The type blk_status_t is from block layer and not related to checksums in our context. Use int internally and do the conversions to blk_status_t as needed in btrfs_bio_csum(). Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: change return type of btrfs_csum_one_bio() to intDavid Sterba
The type blk_status_t is from block layer and not related to checksums in our context. Use int internally and do the conversions to blk_status_t as needed in btrfs_bio_csum(). Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: change return type of btrfs_lookup_bio_sums() to intDavid Sterba
The type blk_status_t is from block layer and not related to checksums in our context. Use int internally and do the conversions to blk_status_t as needed in btrfs_submit_chunk(). Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: drop redundant local variable in raid_wait_write_end_io()David Sterba
The bio status is read only once, no variable needed for that. Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: merge __setup_root() to btrfs_alloc_root()David Sterba
There's only one caller of __setup_root() so merge it there. Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: use unsigned types for constants defined as bit shiftsDavid Sterba
The unsigned type is a recommended practice (CWE-190, CWE-194) for bit shifts to avoid problems with potential unwanted sign extensions. Although there are no such cases in btrfs codebase, follow the recommendation. Reviewed-by: Boris Burkov <boris@bur.io> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: remove unused btrfs_io_stripe::lengthDavid Sterba
First added (but not effectively used) in 02c372e1f016e5 ("btrfs: add support for inserting raid stripe extents"). The structure is initialized to zeros so the only use in btrfs_insert_one_raid_extent() u64 length = bioc->stripes[i].length; struct btrfs_raid_stride *raid_stride = &stripe_extent->strides[i]; if (length == 0) length = bioc->size; the 'if' always happens. Last use in 4016358e852861 ("btrfs: remove unused variable length in btrfs_insert_one_raid_extent()") was an obvious cleanup. It seems to be safe to remove, raid-stripe-tree works without using it since 6.6. This was found by tool https://github.com/jirislaby/clang-struct . Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: use list_first_entry() everywhereDavid Sterba
Using the helper makes it a bit more clear that we're accessing the first list entry. Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: convert ASSERT(0) with handled errors to DEBUG_WARN()David Sterba
The use of ASSERT(0) is maybe useful for some cases but more like a notice for developers. Assertions can be compiled in independently so convert it to a debugging helper. The difference is that it's just a warning and will not end up in BUG(). The converted cases are in connection with proper error handling. Reviewed-by: Josef Bacik <josef@toxicpanda.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: convert WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG)) to DEBUG_WARNDavid Sterba
Use the conditional warning instead of typing the whole condition. Optional message is printed where it seems clear what could be the problem. Conversion is left out in btree_csum_one_bio() because of the additional condition. Reviewed-by: Josef Bacik <josef@toxicpanda.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: add debug build only WARNDavid Sterba
Add conditional WARN() wrapper that's enabled only in debug build. It should be used for unexpected conditions that should be noisy. Use it instead of ASSERT(0). As it will not lead to BUG() make sure that continuing is still possible, e.g. the error is handled anyway. Reviewed-by: Josef Bacik <josef@toxicpanda.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: use verbose ASSERT() in volumes.cDavid Sterba
The file volumes.c has about 40 assertions and half of them are suitable for ASSERT() with additional data. Reviewed-by: Josef Bacik <josef@toxicpanda.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: enhance ASSERT() to take optional format stringDavid Sterba
Currently ASSERT() prints the stringified condition and without macro expansions so simple constants like BTRFS_MAX_METADATA_BLOCKSIZE remain readable in the output. There are expressions where we'd like to see the exact values but all we get is something like: assertion failed: em->start <= start && start < extent_map_end(em), in fs/btrfs/extent_map.c:613 It would be nice to be able to print any additional information to help understand the problem. With some preprocessor magic and compile-time optimizations we can enhance ASSERT to work like that as well: ASSERT(value > limit, "value=%llu limit=%llu", value, limit); with free-form printk arguments that will be part of the assertion message. Pros: - helps debugging and understanding reported problems - the optional format is verified at compile-time Cons: - increases the .ko size - writing the assertion code is repetitive (condition, format, values) - format and variable type must match (extra lookup) - needs gcc 8.x and newer, otherwise it's the short format Recommended use is for non-trivial expressions, so basic ASSERT(value) can be used for pointers or sometimes integers. The format has been slightly updated to also print the result of the evaluation of the condition, appended to the stringified condition as "condition :: <value>". Reviewed-by: Josef Bacik <josef@toxicpanda.com> Signed-off-by: David Sterba <dsterba@suse.com>
2025-05-15btrfs: remove BTRFS_REF_LAST from enum btrfs_ref_typeYangtao Li
Commit b28b1f0ce44c ("btrfs: delayed-ref: Introduce better documented delayed ref structures") introduced BTRFS_REF_LAST, which can be used for sanity checking, e.g. in switch/case or for loops. In btrfs_ref_type() there is an assertion ASSERT(ref->type == BTRFS_REF_DATA || ref->type == BTRFS_REF_METADATA); to validate the values so we don't need the ending enum. Signed-off-by: Yangtao Li <frank.li@vivo.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>