- 25 May, 2020 40 commits
-
-
Omar Sandoval authored
Currently, direct I/O has its own versions of bio_readpage_error() and btrfs_check_repairable() (dio_read_error() and btrfs_check_dio_repairable(), respectively). The main difference is that the direct I/O version doesn't do read validation. The rework of direct I/O repair makes it possible to do validation, so we can get rid of btrfs_check_dio_repairable() and combine bio_readpage_error() and dio_read_error() into a new helper, btrfs_submit_read_repair(). Reviewed-by:
Josef Bacik <josef@toxicpanda.com> Signed-off-by:
Omar Sandoval <osandov@fb.com> Signed-off-by:
David Sterba <dsterba@suse.com>
-
Omar Sandoval authored
This was originally added in commit 8b110e39 ("Btrfs: implement repair function when direct read fails") to avoid a deadlock. In that commit, the direct I/O read endio executes on the endio_workers workqueue, submits a repair bio, and waits for it to complete. The repair bio endio must execute on a different workqueue, otherwise it could block on the endio_workers workqueue becoming available, which won't happen because the original endio is blocked on the repair bio. As of the previous commit, the original endio doesn't wait for the repair bio, so this separate workqueue is unnecessary. Reviewed-by:
-
Omar Sandoval authored
Direct I/O read repair was originally implemented in commit 8b110e39 ("Btrfs: implement repair function when direct read fails"). This implementation is unnecessarily complicated. There is major code duplication between __btrfs_subio_endio_read() (checks checksums and handles I/O errors for files with checksums), __btrfs_correct_data_nocsum() (handles I/O errors for files without checksums), btrfs_retry_endio() (checks checksums and handles I/O errors for retries of files with checksums), and btrfs_retry_endio_nocsum() (handles I/O errors for retries of files without checksum). If it sounds like these should be one function, that's because they should. Additionally, these functions are very hard to follow due to their excessive use of goto. This commit replaces the original implementation. After the previous commit getting rid of orig_bio, we can reuse the same endio callback for repair I/O and the original I/O, we just need to track the file offset and original iterator in the repair bio. We can also unify the handling of files with and without checksums and simplify the control flow. We also no longer have to wait for each repair I/O to complete one by one. Reviewed-by:
Nikolay Borisov <nborisov@suse.com> Signed-off-by:
-
Omar Sandoval authored
In the worst case, there are _4_ layers of bios in the Btrfs direct I/O path: 1. The bio created by the generic direct I/O code (dio_bio). 2. A clone of dio_bio we create in btrfs_submit_direct() to represent the entire direct I/O range (orig_bio). 3. A partial clone of orig_bio limited to the size of a RAID stripe that we create in btrfs_submit_direct_hook(). 4. Clones of each of those split bios for each RAID stripe that we create in btrfs_map_bio(). As of the previous commit, the second layer (orig_bio) is no longer needed for anything: we can split dio_bio instead, and complete dio_bio directly when all of the cloned bios complete. This lets us clean up a bunch of cruft, including dip->subio_endio and dip->errors (we can use dio_bio->bi_status instead). It also enables the next big cleanup of direct I/O read repair. Reviewed-by:
-
Omar Sandoval authored
The next commit will get rid of btrfs_dio_private->orig_bio. The only thing we really need it for is containing all of the checksums, but we can easily put the checksum array in btrfs_dio_private and have the submitted bios reference the array. We can also look the checksums up while we're setting up instead of the current awkward logic that looks them up for orig_bio when the first split bio is submitted. (Interestingly, btrfs_dio_private did contain the checksums before commit 23ea8e5a ("Btrfs: load checksum data once when submitting a direct read io"), but it didn't look them up up front.) Reviewed-by:
Johannes Thumshirn <johannes.thumshirn@wdc.com> Signed-off-by:
-
Omar Sandoval authored
This is really a reference count now, so convert it to refcount_t and rename it to refs. Reviewed-by:
-
Omar Sandoval authored
We haven't used this since commit 9be3395b ("Btrfs: use a btrfs bioset instead of abusing bio internals"). Reviewed-by:
-
Omar Sandoval authored
Since its introduction in commit 2fe6303e ("Btrfs: split bio_readpage_error into several functions"), btrfs_check_repairable() has only been used from extent_io.c where it is defined. Reviewed-by:
-
Omar Sandoval authored
__readpage_endio_check() is also used from the direct I/O read code, so give it a more descriptive name. Reviewed-by:
-
Omar Sandoval authored
Fix a couple of issues in the btrfs_lookup_bio_sums documentation: * The bio doesn't need to be a btrfs_io_bio if dst was provided. Move the declaration in the code to make that clear, too. * dst must be large enough to hold nblocks * csum_size, not just csum_size. Reviewed-by:
-
Omar Sandoval authored
The purpose of the validation step is to distinguish between good and bad sectors in a failed multi-sector read. If a multi-sector read succeeded but some of those sectors had checksum errors, we don't need to validate anything; we know the sectors with bad checksums need to be repaired. Reviewed-by:
-
Omar Sandoval authored
Read repair does two things: it finds a good copy of data to return to the reader, and it corrects the bad copy on disk. If a read of multiple sectors has an I/O error, repair does an extra "validation" step that issues a separate read for each sector. This allows us to find the exact failing sectors and only rewrite those. This heuristic is implemented in bio_readpage_error()/btrfs_check_repairable() as: failed_bio_pages = failed_bio->bi_iter.bi_size >> PAGE_SHIFT; if (failed_bio_pages > 1) do validation However, at this point, bi_iter may have already been advanced. This means that we'll skip the validation step and rewrite the entire failed read. Fix it by getting the actual size from the biovec (which we can do because this is only called for non-cloned bios, although that will change in a later commit). Fixes: 8a2ee44a ("btrfs: look at bi_size for repair decisions") Reviewed-by:
-
Omar Sandoval authored
In btrfs_submit_direct(), if we fail to allocate the btrfs_dio_private, we complete the ordered extent range. However, we don't mark that the range doesn't need to be cleaned up from btrfs_direct_IO() until later. Therefore, if we fail to allocate the btrfs_dio_private, we complete the ordered extent range twice. We could fix this by updating unsubmitted_oe_range earlier, but it's cleaner to reorganize the code so that creating the btrfs_dio_private and submitting the bios are separate, and once the btrfs_dio_private is created, cleanup always happens through the btrfs_dio_private. The logic around unsubmitted_oe_range_end and unsubmitted_oe_range_start is really subtle. We have the following: 1. btrfs_direct_IO sets those two to the same value. 2. When we call __blockdev_direct_IO unless btrfs_get_blocks_direct->btrfs_get_blocks_direct_write is called to modify unsubmitted_oe_range_start so that start < end. Cleanup won't happen. 3. We come into btrfs_submit_direct - if it dip allocation fails we'd return with oe_range_end now modified so cleanup will happen. 4. If we manage to allocate the dip we reset the unsubmitted range members to be equal so that cleanup happens from btrfs_endio_direct_write. This 4-step logic is not really obvious, especially given it's scattered across 3 functions. Fixes: f28a4928 ("Btrfs: fix leaking of ordered extents after direct IO write error") Reviewed-by: -
Omar Sandoval authored
In btrfs_submit_direct_hook(), if a direct I/O write doesn't span a RAID stripe or chunk, we submit orig_bio without cloning it. In this case, we don't increment pending_bios. Then, if btrfs_submit_dio_bio() fails, we decrement pending_bios to -1, and we never complete orig_bio. Fix it by initializing pending_bios to 1 instead of incrementing later. Fixing this exposes another bug: we put orig_bio prematurely and then put it again from end_io. Fix it by not putting orig_bio. After this change, pending_bios is really more of a reference count, but I'll leave that cleanup separate to keep the fix small. Fixes: e65e1535 ("btrfs: fix panic caused by direct IO") CC: stable@vger.kernel.org # 4.4+ Reviewed-by:
-
Omar Sandoval authored
An upcoming Btrfs fix needs to know the original size of a non-cloned bios. Rather than accessing the bvec table directly, let's add a bio_for_each_bvec_all() accessor. Reviewed-by:
-
Filipe Manana authored
At clean_pinned_extents(), whether we end up returning success or failure, we pretty much have to do the same things: 1) unlock unused_bg_unpin_mutex 2) decrement reference count on the previous transaction We also call btrfs_dec_block_group_ro() in case of failure, but that is better done in its caller, btrfs_delete_unused_bgs(), since its the caller that calls inc_block_group_ro(), so it should be responsible for the decrement operation, as it is in case any of the other functions it calls fail. So move the call to btrfs_dec_block_group_ro() from clean_pinned_extents() into btrfs_delete_unused_bgs() and unify the error and success return paths for clean_pinned_extents(), reducing duplicated code and making it simpler. Signed-off-by:
Filipe Manana <fdmanana@suse.com> Signed-off-by:
-
Qu Wenruo authored
All callers pass the eb::level so we can get read it directly inside the btrfs_bin_search and key_search. This is inspired by the work of Marek in U-boot. CC: Marek Behun <marek.behun@nic.cz> Reviewed-by:
Qu Wenruo <wqu@suse.com> Reviewed-by:
-
Nikolay Borisov authored
Instead of returning both the page and the super block structure, make btrfs_read_disk_super just return a pointer to struct btrfs_disk_super. As a result the function signature is simplified. Also, read_cache_page_gfp can never return NULL so check its return value only for IS_ERR. Reviewed-by:
-
Nikolay Borisov authored
The function always works on a local copy of the reloc root list, which cannot be modified outside of it so using list_for_each_entry is fine. Additionally the macro handles empty lists so drop list_empty checks of callers. No semantic changes. Reviewed-by:
-
David Sterba authored
Deleting a subvolume on a full filesystem leads to ENOSPC followed by a forced read-only. This is not a transaction abort and the filesystem is otherwise ok, so the error should be just propagated to the callers. This is caused by unnecessary call to btrfs_handle_fs_error for all errors, except EAGAIN. This does not make sense as the standard transaction abort mechanism is in btrfs_drop_snapshot so all relevant failures are handled. Originally in commit cb1b69f4 ("Btrfs: forced readonly when btrfs_drop_snapshot() fails") there was no return value at all, so the btrfs_std_error made some sense but once the error handling and propagation has been implemented we don't need it anymore. Signed-off-by:
-
Filipe Manana authored
The reclaim_size counter of a space_info object is unsigned. So its value can never be negative, it's pointless to have an assertion that checks its value is >= 0, therefore remove it. Reviewed-by:
-
Zheng Wei authored
Remove the duplicate definition of 'inode_item_err' in the file tree-checker.c that got there by accident in c23c77b0 ("btrfs: tree-checker: Refactor inode key check into seperate function"). Reviewed-by:
Zheng Wei <wei.zheng@vivo.com> Reviewed-by:
-
Josef Bacik authored
Nikolay noticed a bunch of test failures with my global rsv steal patches. At first he thought they were introduced by them, but they've been failing for a while with 64k nodes. The problem is with 64k nodes we have a global reserve that calculates out to 13MiB on a freshly made file system, which only has 8MiB of metadata space. Because of changes I previously made we no longer account for the global reserve in the overcommit logic, which means we correctly allow overcommit to happen even though we are already overcommitted. However in some corner cases, for example btrfs/170, we will allocate the entire file system up with data chunks before we have enough space pressure to allocate a metadata chunk. Then once the fs is full we ENOSPC out because we cannot overcommit and the global reserve is taking up all of the available space. The most ideal way to deal with this is to change our space reservation stuff to take into account the height of the tree's that we're modifying, so that our global reserve calculation does not end up so obscenely large. However that is a huge undertaking. Instead fix this by forcing a chunk allocation if the global reserve is larger than the total metadata space. This gives us essentially the same behavior that happened before, we get a chunk allocated and these tests can pass. This is meant to be a stop-gap measure until we can tackle the "tree height only" project. Fixes: 0096420a ("btrfs: do not account global reserve in can_overcommit") CC: stable@vger.kernel.org # 5.4+ Reviewed-by:
-
Josef Bacik authored
With normal tickets we could have a large reservation at the front of the list that is unable to be satisfied, but a smaller ticket later on that can be satisfied. The way we handle this is to run btrfs_try_granting_tickets() in maybe_fail_all_tickets(). However no such protection exists for priority tickets. Fix this by handling it in handle_reserve_ticket(). If we've returned after attempting to flush space in a priority related way, we'll still be on the priority list and need to be removed. We rely on the flushing to free up space and wake the ticket, but if there is not enough space to reclaim _but_ there's enough space in the space_info to handle subsequent reservations then we would have gotten an ENOSPC erroneously. Address this by catching where we are still on the list, meaning we were a priority ticket, and removing ourselves and then running btrfs_try_granting_tickets(). This will handle this particular corner case. Reviewed-by:
-
Josef Bacik authored
In debugging a generic/320 failure on ppc64, Nikolay noticed that sometimes we'd ENOSPC out with plenty of space to reclaim if we had committed the transaction. He further discovered that this was because there was a priority ticket that was small enough to fit in the free space currently in the space_info. Consider the following scenario. There is no more space to reclaim in the fs without committing the transaction. Assume there's 1MiB of space free in the space info, but there are pending normal tickets with 2MiB reservations. Now a priority ticket comes in with a .5MiB reservation. Because we have normal tickets pending we add ourselves to the priority list, despite the fact that we could satisfy this reservation. The flushing machinery now gets to the point where it wants to commit the transaction, but because there's a .5MiB ticket on the priority list and we have 1MiB of free space we assume the ticket will be granted soon, so we bail without committing the transaction. Meanwhile the priority flushing does not commit the transaction, and eventually fails with an ENOSPC. Then all other tickets are failed with ENOSPC because we were never able to actually commit the transaction. The fix for this is we should have simply granted the priority flusher his reservation, because there was space to make the reservation. Priority flushers by definition take priority, so they are allowed to make their reservations before any previous normal tickets. By not adding this priority ticket to the list the normal flushing mechanisms will then commit the transaction and everything will continue normally. We still need to serialize ourselves with other priority tickets, so if there are any tickets on the priority list then we need to add ourselves to that list in order to maintain the serialization between priority tickets. Reviewed-by:
-
Josef Bacik authored
On ppc64le with 64k page size (respectively 64k block size) generic/320 was failing and debug output showed we were getting a premature ENOSPC with a bunch of space in btrfs_fs_info::trans_block_rsv. This meant there were still open transaction handles holding space, yet the flusher didn't commit the transaction because it deemed the freed space won't be enough to satisfy the current reserve ticket. Fix this by accounting for space in trans_block_rsv when deciding whether the current transaction should be committed or not. Reviewed-by:
-
Josef Bacik authored
We previously had a limit of stealing 50% of the global reserve for unlink. This was from a time when the global reserve was used for the delayed refs as well. However now those reservations are kept separate, so the global reserve can be depleted much more to allow us to make progress for space restoring operations like unlink. Change the minimum amount of space required to be left in the global reserve to 10%. Reviewed-by:
-
Josef Bacik authored
For unlink transactions and block group removal btrfs_start_transaction_fallback_global_rsv will first try to start an ordinary transaction and if it fails it will fall back to reserving the required amount by stealing from the global reserve. This is problematic because of all the same reasons we had with previous iterations of the ENOSPC handling, thundering herd. We get a bunch of failures all at once, everybody tries to allocate from the global reserve, some win and some lose, we get an ENSOPC. Fix this behavior by introducing BTRFS_RESERVE_FLUSH_ALL_STEAL. It's used to mark unlink reservation. To fix this we need to integrate this logic into the normal ENOSPC infrastructure. We still go through all of the normal flushing work, and at the moment we begin to fail all the tickets we try to satisfy any tickets that are allowed to steal by stealing from the global reserve. If this works we start the flushing system over again just like we would with a normal ticket satisfaction. This serializes our global reserve stealing, so we don't have the thundering herd problem. Reviewed-by:
-
Qu Wenruo authored
For relocation tree detection, relocation backref cache uses btrfs_should_ignore_reloc_root() which uses relocation-specific checks like checking the DEAD_RELOC_ROOT bit. However for general purpose backref cache, we can rely on that check, as it's possible that relocation is also running. For generic purposed backref cache, we detect reloc root by SHARED_BLOCK_REF item. Only reloc root node has its parent bytenr pointing back to itself. And in that case, backref cache will mark the reloc root node useless, dropping any child orphan nodes. So only call btrfs_should_ignore_reloc_root() if the backref cache is for relocation. Signed-off-by:
-
Qu Wenruo authored
The error cleanup will be extracted as a new function, btrfs_backref_error_cleanup(), and moved to backref.c and exported for later usage. Signed-off-by:
-
Qu Wenruo authored
This the the 2nd major part of generic backref cache. Move it to backref.c so we can reuse it. Signed-off-by:
-
Qu Wenruo authored
This function is the major part of backref cache build process, move it to backref.c so we can reuse it later. Signed-off-by:
-
Qu Wenruo authored
The backref code is going to be moved to backref.c, and read_fs_root() is just a simple wrapper, open-code it to prepare to the incoming code move. Signed-off-by:
-
Qu Wenruo authored
This function is mostly single purpose to relocation backref cache, but since we're moving the main part of backref cache to backref.c, we need to export such function. And to avoid confusion, rename the function to btrfs_should_ignore_reloc_root() make the name a little more clear. Signed-off-by:
-
Qu Wenruo authored
Also change the parameter, since all callers can easily grab an fs_info, there is no need for all the pointer chasing. Signed-off-by:
-
Qu Wenruo authored
Since we're releasing all existing nodes/edges, other than cleanup the mess after error, "release" is a more proper naming here. Signed-off-by:
-
Qu Wenruo authored
Also add comment explaining the cleanup progress, to differ it from btrfs_backref_drop_node(). Signed-off-by:
-
Qu Wenruo authored
With extra comment for drop_backref_node() as it has some similarity with remove_backref_node(), thus we need extra comment explaining the difference. Signed-off-by:
-
Qu Wenruo authored
Signed-off-by:
-
Qu Wenruo authored
Signed-off-by:
-
