- 11 Jul, 2024 40 commits
-
-
Qu Wenruo authored
[HICCUP] Kernels can create file extent items with incorrect ram_bytes like this: item 6 key (257 EXTENT_DATA 0) itemoff 15816 itemsize 53 generation 7 type 1 (regular) extent data disk byte 13631488 nr 32768 extent data offset 0 nr 4096 ram 4096 extent compression 0 (none) Thankfully kernel can handle them properly, as in that case ram_bytes is not utilized at all. [ENHANCEMENT] Since the hiccup is not going to cause any data-loss and is only a minor violation of on-disk format, here we only need to ignore the incorrect ram_bytes value, and use the correct one from btrfs_file_extent_item::disk_num_bytes. 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 authored
[HICCUP] Before commit 85de2be7129c ("btrfs: remove extent_map::block_start member"), we utilized @bytenr variable inside btrfs_extent_item_to_extent_map() to calculate block_start. But that commit removed block_start completely, we have no need to advance @bytenr at all. [ENHANCEMENT] - Rename @bytenr as @disk_bytenr - Only declare @disk_bytenr inside the if branch - Make @disk_bytenr const and remove the modification on it Reviewed-by: -
Mark Harmstone authored
There's a typo in an error message when checking the block group tree feature, it mentions fres-space-tree instead of free-space-tree. Fix that. Signed-off-by:
Mark Harmstone <maharmstone@fb.com> Reviewed-by:
-
Filipe Manana authored
The direct IO code is over a thousand lines and it's currently spread between file.c and inode.c, which makes it not easy to locate some parts of it sometimes. Also inode.c is about 11 thousand lines and file.c about 4 thousand lines, both too big. So move all the direct IO code into a dedicated file, so that it's easy to locate all its code and reduce the sizes of inode.c and file.c. This is a pure move of code without any other changes except export a a couple functions from inode.c (get_extent_allocation_hint() and create_io_em()) because they are used in inode.c and the new direct-io.c file, and a couple functions from file.c (btrfs_buffered_write() and btrfs_write_check()) because they are used both in file.c and in the new direct-io.c file. Reviewed-by:
Boris Burkov <boris@bur.io> Reviewed-by:
-
David Sterba authored
Pass a struct btrfs_inode to btrfs_set_prop() as it's an internal interface, allowing to remove some use of BTRFS_I. Reviewed-by:
-
David Sterba authored
Pass a struct btrfs_inode to btrfs_compress_heuristic() as it's an internal interface, allowing to remove some use of BTRFS_I. Reviewed-by:
-
David Sterba authored
The structure is internal so we should use struct btrfs_inode for that, allowing to remove some use of BTRFS_I. Reviewed-by:
-
David Sterba authored
The structure is internal so we should use struct btrfs_inode for that. Reviewed-by:
-
David Sterba authored
Pass a struct btrfs_inode to btrfs_ioctl_send() and _btrfs_ioctl_send() as it's an internal interface, allowing to remove some use of BTRFS_I. Reviewed-by:
-
David Sterba authored
The structure is internal so we should use struct btrfs_inode for that. Reviewed-by:
-
David Sterba authored
Pass a struct btrfs_inode to is_data_inode() as it's an internal interface, allowing to remove some use of BTRFS_I. Reviewed-by:
-
David Sterba authored
Pass a struct btrfs_inode to btrfs_readdir_get_delayed_items() as it's an internal interface, allowing to remove some use of BTRFS_I. Reviewed-by:
-
David Sterba authored
Pass a struct btrfs_inode to btrfs_readdir_put_delayed_items() as it's an internal interface, allowing to remove some use of BTRFS_I. Reviewed-by:
-
Johannes Thumshirn authored
Remove the encoding field from 'struct btrfs_stripe_extent'. It was originally intended to encode the RAID type as well as if we're a data or a parity stripe. But the RAID type can be inferred form the block-group and the data vs. parity differentiation can be done easier with adding a new key type for parity stripes in the RAID stripe tree. Signed-off-by:
Johannes Thumshirn <johannes.thumshirn@wdc.com> Signed-off-by:
-
Qu Wenruo authored
When debugging the recent ram_bytes mismatch bug, I can hit it with enhanced tree-checker for file extent items at write time. But the bug is not that easy to trigger (mostly triggered with btrfs/06*, which uses 20 threads fsstress), and when I hit it, the only info is the kernel leaf dump, but it doesn't include things like the file extent type (REGULAR or PREALLOC). Add the dump for generation and type (although only numeric output) to make debugging a little easier. Signed-off-by:
-
Boris Burkov authored
Periodic reclaim attempts to avoid block_groups seeing active use with a sweep mark that gets cleared on allocation and set on a sweep. In urgent conditions where we have very little unallocated space (less than one chunk used by the threshold calculation for the unallocated target), we want to be able to override this mechanism. Introduce a second pass that only happens if we fail to find a reclaim candidate and reclaim is urgent. In that case, do a second pass where all block groups are eligible. Reviewed-by:
Josef Bacik <josef@toxicpanda.com> Signed-off-by:
-
Boris Burkov authored
Periodic reclaim runs the risk of getting stuck in a state where it keeps reclaiming the same block group over and over. This can happen if 1. reclaiming that block_group fails 2. reclaiming that block_group fails to move any extents into existing block_groups and just allocates a fresh chunk and moves everything. Currently, 1. is a very tight loop inside the reclaim worker. That is critical for edge triggered reclaim or else we risk forgetting about a reclaimable group. On the other hand, with level triggered reclaim we can break out of that loop and get it later. With that fixed, 2. applies to both failures and "successes" with no progress. If we have done a periodic reclaim on a space_info and nothing has changed in that space_info, there is not much point to trying again, so don't, until enough space gets free, which we capture with a heuristic of needing to net free 1 chunk. Reviewed-by:
-
Boris Burkov authored
We currently employ a edge-triggered block group reclaim strategy which marks block groups for reclaim as they free down past a threshold. With a dynamic threshold, this is worse than doing it in a level-triggered fashion periodically. That is because the reclaim itself happens periodically, so the threshold at that point in time is what really matters, not the threshold at freeing time. If we mark the reclaim in a big pass, then sort by usage and do reclaim, we also benefit from a negative feedback loop preventing unnecessary reclaims as we crunch through the "best" candidates. Since this is quite a different model, it requires some additional support. The edge triggered reclaim has a good heuristic for not reclaiming fresh block groups, so we need to replace that with a typical GC sweep mark which skips block groups that have seen an allocation since the last sweep. Reviewed-by:
-
Boris Burkov authored
We can currently recover allocated block_groups by: - explicitly starting balance operations - "auto reclaim" via bg_reclaim_threshold The latter works by checking against a fixed threshold on frees. If we pass from above the threshold to below, relocation triggers and the block group will get reclaimed by the cleaner thread (assuming it is still eligible) Picking a threshold is challenging. Too high, and you end up trying to reclaim very full block_groups which is quite costly, and you don't do reclaim on block_groups that don't get quite THAT full, but could still be quite fragmented and stranding a lot of space. Too low, and you similarly miss out on reclaim even if you badly need it to avoid running out of unallocated space, if you have heavily fragmented block groups living above the threshold. No matter the threshold, it suffers from a workload that happens to bounce around that threshold, which can introduce arbitrary amounts of reclaim waste. To improve this situation, introduce a dynamic threshold. The basic idea behind this threshold is that it should be very lax when there is plenty of unallocated space, and increasingly aggressive as we approach zero unallocated space. To that end, it sets a target for unallocated space (10 chunks) and then linearly increases the threshold as the amount of space short of the target we are increases. The formula is: (target - unalloc) / target I tested this by running it on three interesting workloads: 1. bounce allocations around X% full. 2. fill up all the way and introduce full fragmentation. 3. write in a fragmented way until the filesystem is just about full. 1. and 2. attack the weaknesses of a fixed threshold; fixed either works perfectly or fully falls apart, depending on the threshold. Dynamic always handles these cases well. 3. attacks dynamic by checking whether it is too zealous to reclaim in conditions with low unallocated and low unused. It tends to claw back 1GiB of unallocated fairly aggressively, but not much more. Early versions of dynamic threshold struggled on this test. Additional work could be done to intelligently ratchet up the urgency of reclaim in very low unallocated conditions. Existing mechanisms are already useless in that case anyway. Reviewed-by:
-
Boris Burkov authored
This is handy when computing space_info dynamic reclaim thresholds where we do not have access to a block group. We could add it to the various functions as a parameter, but it seems reasonable for space_info to have an fs_info pointer. Reviewed-by:
-
Boris Burkov authored
When evaluating various reclaim strategies/thresholds against each other, it is useful to collect data about the amount of reclaim happening. Expose a count, error count, and byte count via sysfs per space_info. Note that this is only for automatic reclaim, not manually invoked balances or other codepaths that use "relocate_block_group" Reviewed-by:
-
David Sterba authored
Calling btrfs_handle_fs_error() after btrfs_run_qgroups() fails to update the qgroup status is probably not necessary, this would turn the filesystem to read-only. For the same reason aborting the transaction is also not a good option. The state is left inconsistent and can be fixed by rescan, printing a warning should be sufficient. Return code reflects the status of adding/deleting the relation and if the transaction was ended properly. Reviewed-by:
-
David Sterba authored
There's a transaction joined in the qgroup relation add/remove ioctl and any error will lead to abort/error. We could lift the allocation from btrfs_add_qgroup_relation() and move it outside of the transaction context. The relation deletion does not need that. The ownership of the structure is moved to the add relation handler. Reviewed-by:
-
David Sterba authored
The errors during removing a chunk item are fatal, we expect to have a matching item in the chunk map from which the chunk_offset is taken. Handle that by transaction abort. Reviewed-by:
-
David Sterba authored
The extent item used to have a v0 that was removed in 6.6. There's a check for minimum expected size that could lead to btrfs_handle_fs_error() that would make the filesystem read-only. As we don't have v0 anymore (and haven't seen any reports in the deprecation period), handle this in a less intrusive way. Reviewed-by:
-
David Sterba authored
When an extended ref is deleted we do a sanity check right before removing the item, if we can't find it then handle the error. This is done by btrfs_handle_fs_error() but this is from the time before we had the transaction abort infrastructure, so switch to that. The end result is the same, the error is reported and switched to read-only. We newly return the -ENOENT error code as this better represents what happened. Reviewed-by:
-
Filipe Manana authored
We always allocate a delayed extent op structure when allocating a tree block (except for log trees), but most of the time we don't need it as we only need to set the BTRFS_BLOCK_FLAG_FULL_BACKREF if we're dealing with a relocation tree and we only need to set the key of a tree block in a btrfs_tree_block_info structure if we are not using skinny metadata (feature enabled by default since btrfs-progs 3.18 and available as of kernel 3.10). In these cases, where we don't need neither to update flags nor to set the key, we only use the delayed extent op structure to set the tree block's level. This is a waste of memory and besides that, the memory allocation can fail and can add additional latency. Instead of using a delayed extent op structure to store the level of the tree block, use the delayed ref head to store it. This doesn't change the size of neither structure and helps us avoid allocating delayed extent ops structures when using the skinny metadata feature and there's no relocation going on. This also gets rid of a BUG_ON(). For example, for a fs_mark run, with 5 iterations, 8 threads and 100K files per iteration, before this patch there were 118109 allocations of delayed extent op structures and after it there were none. Reviewed-by:
-
Boris Burkov authored
When qgroups are enabled, during data reservation, we allocate the ulist_nodes that track the exact reserved extents with GFP_ATOMIC unconditionally. This is unnecessary, and we can follow the model already employed by the struct extent_state we preallocate in the non qgroups case, which should reduce the risk of allocation failures with GFP_ATOMIC. Add a prealloc node to struct ulist which ulist_add will grab when it is present, and try to allocate it before taking the tree lock while we can still take advantage of a less strict gfp mask. The lifetime of that node belongs to the new prealloc field, until it is used, at which point it belongs to the ulist linked list. Reviewed-by:
-
Filipe Manana authored
Instead of doing a BUG_ON() handle the error by returning -EUCLEAN, aborting the transaction and logging an error message. Reviewed-by:
-
Filipe Manana authored
Instead of using an if-else statement when processing the extent item at btrfs_lookup_extent_info(), use a single if statement for the error case since it does a goto at the end and leave the success (expected) case following the if statement, reducing indentation and making the logic a bit easier to follow. Also make the if statement's condition as unlikely since it's not expected to ever happen, as it signals some corruption, making it clear and hint the compiler to generate more efficient code. Reviewed-by:
-
Filipe Manana authored
If we didn't found an extent item with the initial btrfs_search_slot() call, it's pointless to test if the "metadata" variable is "true", because right after we check if the key type is BTRFS_METADATA_ITEM_KEY and that is the case only when "metadata" is set to "true". So remove the redundant check. Reviewed-by:
-
Filipe Manana authored
Instead of a BUG_ON() just return an error, log an error message and abort the transaction in case we find an extent buffer belonging to the relocation tree that doesn't have the full backref flag set. This is unexpected and should never happen (save for bugs or a potential bad memory). Reviewed-by:
-
Filipe Manana authored
We keep a "new_flags" variable only to keep track if we need to update the metadata extent's flags, and when we set BTRFS_BLOCK_FLAG_FULL_BACKREF in the variable, we do it in an inner scope. Then check in an outer scope if the variable is not 0 and if so call btrfs_set_disk_extent_flags(). The variable isn't used for anything else. This is somewhat confusing, so to make it more straightforward update the extent's flags where we are currently updating "new_flags" and remove the variable. Reviewed-by:
-
Filipe Manana authored
There are no callers of btrfs_lookup_extent_info() that pass a NULL value for the transaction handle argument, so there's no point in having special logic to deal with the NULL. The last caller that passed a NULL value was removed in commit 19b546d7 ("btrfs: relocation: Use btrfs_find_all_leafs to locate data extent parent tree leaves"). So remove the NULL handling from btrfs_lookup_extent_info(). Reviewed-by:
-
Filipe Manana authored
At btrfs_force_cow_block() we have several error paths that need to unlock the "cow" extent buffer, drop the reference on it and then return an error. This is a bit verbose so add a label where we perform these tasks and make the error paths jump to that label. Signed-off-by:
-
Filipe Manana authored
When freeing a tree block, at btrfs_free_tree_block(), if we fail to create a delayed reference we don't deal with the error and just do a BUG_ON(). The error most likely to happen is -ENOMEM, and we have a comment mentioning that only -ENOMEM can happen, but that is not true, because in case qgroups are enabled any error returned from btrfs_qgroup_trace_extent_post() (can be -EUCLEAN or anything returned from btrfs_search_slot() for example) can be propagated back to btrfs_free_tree_block(). So stop doing a BUG_ON() and return the error to the callers and make them abort the transaction to prevent leaking space. Syzbot was triggering this, likely due to memory allocation failure injection. Reported-by: syzbot+a306f914b4d01b3958fe@syzkaller.appspotmail.com Link: https://lore.kernel.org/linux-btrfs/000000000000fcba1e05e998263c@google.com/Reviewed-by:
-
Filipe Manana authored
It's pointless to pass a super block argument to btrfs_iget_locked() because we always pass a root and from it we can get the super block through: root->fs_info->sb So remove the super block argument. Reviewed-by:
-
Filipe Manana authored
It's pointless to pass a super block argument to btrfs_iget_path() because we always pass a root and from it we can get the super block through: root->fs_info->sb So remove the super block argument. Reviewed-by:
-
Filipe Manana authored
It's pointless to pass a super block argument to btrfs_iget() because we always pass a root and from it we can get the super block through: root->fs_info->sb So remove the super block argument. Reviewed-by:
-
Qu Wenruo authored
Since commit 2b2553f1 ("btrfs: stop setting PageError in the data I/O path") btrfs no longer utilizes subpage error bitmaps anymore, but the commit forgot to remove the error bitmap in btrfs_subpage_dump_bitmap(), resulting in possible meaningless result for the error bitmap. Fix it by just removing the error bitmap dumping. Signed-off-by:
-
