- 29 Sep, 2022 18 commits
-
-
Filipe Manana authored
At free_extent_map(), it's pointless to have a WARN_ON() to check if the refcount of the extent map is zero. Such check is already done by the refcount_t module and refcount_dec_and_test(), which loudly complains if we try to decrement a reference count that is currently 0. The WARN_ON() dates back to the time when used a regular atomic_t type for the reference counter, before we switched to the refcount_t type. The main goal of the refcount_t type/module is precisely to catch such types of bugs and loudly complain if they happen. Signed-off-by:
Filipe Manana <fdmanana@suse.com> Reviewed-by:
David Sterba <dsterba@suse.com> Signed-off-by:
-
Filipe Manana authored
We have several places that need to drop all the extent maps in a given file range and then add a new extent map for that range. Currently they call btrfs_drop_extent_map_range() to delete all extent maps in the range and then keep trying to add the new extent map in a loop that keeps retrying while the insertion of the new extent map fails with -EEXIST. So instead of repeating this logic, add a helper to extent_map.c that does these steps and name it btrfs_replace_extent_map_range(). Also add a comment about why the retry loop is necessary. Signed-off-by:
-
Filipe Manana authored
Move the loop that removes all the extent maps from the inode's extent map tree during inode eviction out of inode.c and into extent_map.c, to btrfs_drop_extent_map_range(). Anything manipulating extent maps or the extent map tree should be in extent_map.c. Signed-off-by:
-
Filipe Manana authored
At evict_inode_truncate_pages(), instead of manually checking if rescheduling is needed, then unlock the extent map tree, reschedule and then write lock again the tree, use the helper cond_resched_rwlock_write() which does all that. Signed-off-by:
-
Filipe Manana authored
Instead of open coding the end offset calculation of an extent map, use the helper extent_map_end() and cache its result in a local variable, since it's used several times. Signed-off-by:
-
Filipe Manana authored
The function btrfs_drop_extent_cache() doesn't really belong at file.c because what it does is drop a range of extent maps for a file range. It directly allocates and manipulates extent maps, by dropping, splitting and replacing them in an extent map tree, so it should be located at extent_map.c, where all manipulations of an extent map tree and its extent maps are supposed to be done. So move it out of file.c and into extent_map.c. Additionally do the following changes: 1) Rename it into btrfs_drop_extent_map_range(), as this makes it more clear about what it does. The term "cache" is a bit confusing as it's not widely used, "extent maps" or "extent mapping" is much more common; 2) Change its 'skip_pinned' argument from int to bool; 3) Turn several of its local variables from int to bool, since they are used as booleans; 4) Move the declaration of some variables out of the function's main scope and into the scopes where they are used; 5) Remove pointless assignment of false to 'modified' early in the while loop, as later that variable is set and it's not used before that second assignment; 6) Remove checks for NULL before calling free_extent_map(). Signed-off-by:
-
Filipe Manana authored
When dropping extent maps for a range, through btrfs_drop_extent_cache(), if we find an extent map that starts before our target range and/or ends before the target range, and we are not able to allocate extent maps for splitting that extent map, then we don't fail and simply remove the entire extent map from the inode's extent map tree. This is generally fine, because in case anyone needs to access the extent map, it can just load it again later from the respective file extent item(s) in the subvolume btree. However, if that extent map is new and is in the list of modified extents, then a fast fsync will miss the parts of the extent that were outside our range (that needed to be split), therefore not logging them. Fix that by marking the inode for a full fsync. This issue was introduced after removing BUG_ON()s triggered when the split extent map allocations failed, done by commit 7014cdb4 ("Btrfs: btrfs_drop_extent_cache should never fail"), back in 2012, and the fast fsync path already existed but was very recent. Also, in the case where we could allocate extent maps for the split operations but then fail to add a split extent map to the tree, mark the inode for a full fsync as well. This is not supposed to ever fail, and we assert that, but in case assertions are disabled (CONFIG_BTRFS_ASSERT is not set), it's the correct thing to do to make sure a fast fsync will not miss a new extent. CC: stable@vger.kernel.org # 5.15+ Reviewed-by:
Anand Jain <anand.jain@oracle.com> Signed-off-by:
-
Jeff Layton authored
This function no longer exists, was removed in 3c427693 ("Btrfs: fix btrfs_write_inode vs delayed iput deadlock"). Signed-off-by:
Jeff Layton <jlayton@kernel.org> Reviewed-by:
-
Stefan Roesch authored
Enable nowait async buffered writes in btrfs_do_write_iter() and btrfs_file_open(). In this version encoded buffered writes have the optimization not enabled. Encoded writes are enabled by using an ioctl. io_uring currently does not support ioctls. This might be enabled in the future. Performance results: For fio the following results have been obtained with a queue depth of 1 and 4k block size (runtime 600 secs): sequential writes: without patch with patch libaio psync iops: 55k 134k 117K 148K bw: 221MB/s 538MB/s 469MB/s 592MB/s clat: 15286ns 82ns 994ns 6340ns For an io depth of 1, the new patch improves throughput by over two times (compared to the existing behavior, where buffered writes are processed by an io-worker process) and also the latency is considerably reduced. To achieve the same or better performance with the existing code an io depth of 4 is required. Increasing the iodepth further does not lead to improvements. The tests have been run like this: ./fio --name=seq-writers --ioengine=psync --iodepth=1 --rw=write \ --bs=4k --direct=0 --size=100000m --time_based --runtime=600 \ --numjobs=1 --filename=... ./fio --name=seq-writers --ioengine=io_uring --iodepth=1 --rw=write \ --bs=4k --direct=0 --size=100000m --time_based --runtime=600 \ --numjobs=1 --filename=... ./fio --name=seq-writers --ioengine=libaio --iodepth=1 --rw=write \ --bs=4k --direct=0 --size=100000m --time_based --runtime=600 \ --numjobs=1 --filename=... Testing: This patch has been tested with xfstests, fsx, fio. xfstests shows no new diffs compared to running without the patch series. Reviewed-by:Stefan Roesch <shr@fb.com> Reviewed-by:
-
Stefan Roesch authored
Adds nowait asserts to btree search functions which are not used by buffered IO and direct IO paths. Reviewed-by:
-
Stefan Roesch authored
We need to avoid unconditionally calling balance_dirty_pages_ratelimited as it could wait for some reason. Use balance_dirty_pages_ratelimited_flags with the BDP_ASYNC in case the buffered write is nowait, returning EAGAIN eventually. It also moves the function after the again label. This can cause the function to be called a bit later, but this should have no impact in the real world. Reviewed-by:
-
Stefan Roesch authored
We have everywhere setup for nowait, plumb NOWAIT through the write path. Reviewed-by:
Josef Bacik <josef@toxicpanda.com> Signed-off-by:
-
Stefan Roesch authored
Add the nowait parameter to lock_and_cleanup_extent_if_need(). If the nowait parameter is specified we try to lock the extent in nowait mode. Reviewed-by:
-
Stefan Roesch authored
Add nowait parameter to the prepare_pages function. In case nowait is specified for an async buffered write request, do a nowait allocation or return -EAGAIN. Reviewed-by:
-
Josef Bacik authored
Now all the helpers that btrfs_check_nocow_lock uses handle nowait, add a nowait flag to btrfs_check_nocow_lock so it can be used by the write path. Reviewed-by:
-
Josef Bacik authored
For IOCB_NOWAIT we're going to want to use try lock on the extent lock, and simply bail if there's an ordered extent in the range because the only choice there is to wait for the ordered extent to complete. Reviewed-by:
-
Josef Bacik authored
In order to accommodate NOWAIT IOCB's we need to be able to do NO_FLUSH data reservations, so plumb this through the delalloc reservation system. Reviewed-by:
-
Josef Bacik authored
If we have NOWAIT specified on our IOCB and we're writing into a PREALLOC or NOCOW extent then we need to be able to tell can_nocow_extent that we don't want to wait on any locks or metadata IO. Fix can_nocow_extent to allow for NOWAIT. Reviewed-by:
-
- 26 Sep, 2022 22 commits
-
-
Josef Bacik authored
For NOWAIT IOCBs we'll need a way to tell search to not wait on locks or anything. Accomplish this by adding a path->nowait flag that will use trylocks and skip reading of metadata, returning -EAGAIN in either of these cases. For now we only need this for reads, so only the read side is handled. Add an ASSERT() to catch anybody trying to use this for writes so they know they'll have to implement the write side. Reviewed-by:
-
Stefan Roesch authored
Export the function balance_dirty_pages_ratelimited_flags(). It is now also called from btrfs. Reviewed-by:
-
Qu Wenruo authored
[BUG] When one user did a wrong attempt to clear block group tree, which can not be done through mount option, by using "-o clear_cache,space_cache=v2", it will cause the following error on a fs with block-group-tree feature: BTRFS info (device dm-1): force clearing of disk cache BTRFS info (device dm-1): using free space tree BTRFS info (device dm-1): clearing free space tree BTRFS info (device dm-1): clearing compat-ro feature flag for FREE_SPACE_TREE (0x1) BTRFS info (device dm-1): clearing compat-ro feature flag for FREE_SPACE_TREE_VALID (0x2) BTRFS error (device dm-1): block-group-tree feature requires fres-space-tree and no-holes BTRFS error (device dm-1): super block corruption detected before writing it to disk BTRFS: error (device dm-1) in write_all_supers:4318: errno=-117 Filesystem corrupted (unexpected superblock corruption detected) BTRFS warning (device dm-1: state E): Skipping commit of aborted transaction. [CAUSE] Although the dependency for block-group-tree feature is just an artificial one (to reduce test matrix), we put the dependency check into btrfs_validate_super(). This is too strict, and during space cache clearing, we will have a window where free space tree is cleared, and we need to commit the super block. In that window, we had block group tree without v2 cache, and triggered the artificial dependency check. This is not necessary at all, especially for such a soft dependency. [FIX] Introduce a new helper, btrfs_check_features(), to do all the runtime limitation checks, including: - Unsupported incompat flags check - Unsupported compat RO flags check - Setting missing incompat flags - Artificial feature dependency checks Currently only block group tree will rely on this. - Subpage runtime check for v1 cache With this helper, we can move quite some checks from open_ctree()/btrfs_remount() into it, and just call it after btrfs_parse_options(). Now "-o clear_cache,space_cache=v2" will not trigger the above error anymore. Signed-off-by:
Qu Wenruo <wqu@suse.com> Reviewed-by:
-
Qu Wenruo authored
For function submit_extent_page() and alloc_new_bio(), we have an argument @end_io_func to indicate the end io function. But that function never change inside any call site of them, thus no need to pass the pointer around everywhere. There is a better match for the lifespan of all the call sites, as we have btrfs_bio_ctrl structure, thus we can put the endio function pointer there, and grab the pointer every time we allocate a new bio. Also add extra ASSERT()s to make sure every call site of submit_extent_page() and alloc_new_bio() has properly set the pointer inside btrfs_bio_ctrl. This removes one argument from the already long argument list of submit_extent_page(). Reviewed-by:
-
Qu Wenruo authored
Normally we put (page, pg_len, pg_offset) arguments together, just like what __bio_add_page() does. But in submit_extent_page(), what we got is, (page, disk_bytenr, pg_len, pg_offset), which sometimes can be confusing. Change the order to (disk_bytenr, page, pg_len, pg_offset) to make it to follow the common schema. Reviewed-by:
-
Qu Wenruo authored
Since commit 390ed29b ("btrfs: refactor submit_extent_page() to make bio and its flag tracing easier"), we are using bio_ctrl structure to replace some of arguments of submit_extent_page(). But unfortunately that commit didn't update the comment for submit_extent_page(), thus some arguments are stale like: - bio_ret - mirror_num Those are all contained in bio_ctrl now. - prev_bio_flags We no longer use this flag to determine if we can merge bios. Update the comment for submit_extent_page() to keep it up-to-date. Reviewed-by:
-
Josef Bacik authored
dev-replace.h just has function prototypes for device replace, however if you happen to include it in the wrong order you'll get compile errors because of different structures not being defined. Since these are just pointer args to functions we can declare them at the top in order to reduce the pain of using the header. Reviewed-by:
Johannes Thumshirn <johannes.thumshirn@wdc.com> Reviewed-by:
-
Josef Bacik authored
We always check the root of an inode as well as it's inode number to determine if it's a free space inode. This is problematic as the helper is in a header file where it doesn't have the fs_info definition. To avoid this and make the check a little cleaner simply add a flag to the runtime_flags to indicate that the inode is a free space inode, set that when we create the inode, and then change the helper to check for this flag. Reviewed-by:
-
Josef Bacik authored
This exists to insert the btree_inode in the super blocks inode hash table. Since it's only used for the btree inode move the code to where we use it in disk-io.c and remove the helper. Reviewed-by:
-
Josef Bacik authored
This is defined in btrfs_inode.h, and dereferences btrfs_root and btrfs_fs_info, both of which aren't defined in btrfs_inode.h. Additionally, in many places we already have root or fs_info, so this helper often makes the code harder to read. So delete the helper and simply open code it in the few places that we use it. Reviewed-by:
-
Josef Bacik authored
This is defined in ordered-data.h, but is only used in file-item.c. Move this to file-item.c as it doesn't need to be global. Reviewed-by:
-
Josef Bacik authored
This is purely cosmetic, to make it straightforward to copy and paste the definition and helpers from ctree.h into fs.h. These are helpers that act directly on the fs_info, and were scattered throughout ctree.h. Move them directly below the fs_info definition to make it easier to move them later. This includes the exclop prototypes, which shares an enum that's used in struct btrfs_fs_info as well. Reviewed-by:
-
Josef Bacik authored
This helper is only used in inode.c, move it locally to that file instead of defining it in ctree.h. Reviewed-by:
-
Josef Bacik authored
In order to make it more straightforward to move the fs_info struct and it's related structures, move the struct declarations to the top of ctree.h. This will make it easier to clean up after the fact. Reviewed-by:
-
Josef Bacik authored
This isn't a great spot for this, but one of the swapfile helper functions is in volumes.c, so move the struct to volumes.h. In the future when we have better separation of code there will be a more natural spot for this. Reviewed-by:
-
Josef Bacik authored
This is defined in volumes.c, move the prototype into volumes.h. Reviewed-by:
-
Josef Bacik authored
The code for this helper is in space-info.c, move the prototype to space-info.h. Reviewed-by:
-
Josef Bacik authored
This is actually embedded in struct btrfs_block_group, so move this definition to block-group.h, and then open-code the init of the tree where we init the rest of the block group instead of using a helper. Reviewed-by:
-
Josef Bacik authored
This is a block group related definition, move it into block-group.h. Reviewed-by:
-
Christoph Hellwig authored
There is a separate I/O failure tree to track the fail reads, so remove the extra EXTENT_DAMAGED bit in the I/O tree as it's set but never used. Reviewed-by:
Christoph Hellwig <hch@lst.de> Reviewed-by:
-
Josef Bacik authored
We're only initializing extent_io_tree's with a private data if we're a normal inode, so we don't need this extra check. Signed-off-by:
-
Josef Bacik authored
We only use this for normal inodes, so don't set it if we're not a normal inode. Signed-off-by:
-
