- 04 Oct, 2017 1 commit
-
-
Goffredo Baroncelli authored
Jean-Denis Girard noticed commit c821e7f3 "pass bytes to btrfs_bio_alloc" (https://patchwork.kernel.org/patch/9763081/) introduces a regression on 32 bit machines. When CONFIG_LBDAF is _not_ defined (CONFIG_LBDAF == Support for large (2TB+) block devices and files) sector_t is 32 bit on 32bit machines. In the function submit_extent_page, 'sector' (which is sector_t type) is multiplied by 512 to convert it from sectors to bytes, leading to an overflow when the disk is bigger than 4GB (!). I added a cast to u64 to avoid overflow. Fixes: c821e7f3 ("btrfs: pass bytes to btrfs_bio_alloc") CC: stable@vger.kernel.org # 4.13+ Signed-off-by:
Goffredo Baroncelli <kreijack@inwind.it> Tested-by:
Jean-Denis Girard <jd.girard@sysnux.pf> Reviewed-by:
David Sterba <dsterba@suse.com> Signed-off-by:
-
- 26 Sep, 2017 17 commits
-
-
Josef Bacik authored
Amir reported a bug discovered by his cleaned up version of my dm-log-writes xfstests where we were missing csums at certain replay points. This is because fsx was doing an msync(), which essentially fsync()'s a specific range of a file. We will log all modified extents, but only search for the checksums in the range we are being asked to sync. We cannot simply log the extents in the range we're being asked because we are logging the inode item as it is currently, which if it has had a i_size update before the msync means we will miss extents when replaying. We could possibly get around this by marking the inode with the transaction that extended the i_size to see if we have this case, but this would be racy and we'd have to lock the whole range of the inode to make sure we didn't have an ordered extent outside of our range that was in the middle of completing. Fix this simply by keeping track of the modified extents range and logging the csums for the entire range of extents that we are logging. This makes the xfstest pass. Reported-by:
Amir Goldstein <amir73il@gmail.com> Signed-off-by:
Josef Bacik <jbacik@fb.com> Signed-off-by:
-
Liu Bo authored
commit 4246a0b6 ("block: add a bi_error field to struct bio") changed the logic of how dio read endio reports errors. For single stripe dio read, %bio->bi_status reflects the error before verifying checksum, and now we're updating it when data block matches with its checksum, while in the mismatching case, %bio->bi_status is not updated to relfect that. When some blocks in a file have been corrupted on disk, reading such a file ends up with 1) checksum errors are reported in kernel log 2) read(2) returns successfully with some content being 0x01. In order to fix it, we need to report its checksum mismatch error to the upper layer (dio layer in this case) as well. Fixes: 4246a0b6 ("block: add a bi_error field to struct bio") Signed-off-by:
Liu Bo <bo.li.liu@oracle.com> Reported-by:
-
Sargun Dhillon authored
Previously, we were calling del_qgroup_item, and ignoring the return code resulting in a potential to have divergent in-memory state without an error. Perhaps, it makes sense to handle this error code, and put the filesystem into a read only, or similar state. This patch only adds reporting of the error if the error is fatal, (any error other than qgroup not found). Signed-off-by:
Sargun Dhillon <sargun@sargun.me> Reviewed-by:
Qu Wenruo <quwenruo.btrfs@gmx.com> Signed-off-by:
-
Liu Bo authored
Currently even if the underlying disk reports failure on IO, compressed read endio still gets to verify checksum and reports it as a checksum error. In fact, if some IO have failed during reading a compressed data extent , there's no way the checksum could match, therefore, we can skip that in order to return error quickly to the upper layer. Please note that we need to do this after recording the failed mirror index so that read-repair in the upper layer's endio can work properly. Signed-off-by:
Paul Jones <paul@pauljones.id.au> Reviewed-by:
-
Liu Bo authored
The kernel oops happens at kernel BUG at fs/btrfs/extent_io.c:2104! ... RIP: clean_io_failure+0x263/0x2a0 [btrfs] It's showing that read-repair code is using an improper mirror index. This is due to the fact that compression read's endio hasn't recorded the failed mirror index in %cb->orig_bio. With this, btrfs's read-repair can work properly on reading compressed data. Signed-off-by:
-
Liu Bo authored
This seems to be a leftover of commit cf8cddd3 ("btrfs: don't abuse REQ_OP_* flags for btrfs_map_block"). It should use btrfs_op() helper to provide one of 'enum btrfs_map_op' types. Fixes: cf8cddd3 ("btrfs: don't abuse REQ_OP_* flags for btrfs_map_block") Signed-off-by:
Satoru Takeuchi <satoru.takeuchi@gmail.com> Reviewed-by:
-
Liu Bo authored
It doesn't make sense to backup tree roots when doing fsync, since during fsync those tree roots have not been consistent on disk. Signed-off-by:
-
Misono, Tomohiro authored
Currently, "btrfs quota enable" would fail after "btrfs quota disable" on the first time with syslog output "qgroup_rescan_init failed with -22", but it would succeed on the second time. When "quota disable" is called, BTRFS_FS_QUOTA_DISABLING flag bit will be set in fs_info->flags in btrfs_quota_disable(), but it will not be droppd in btrfs_run_qgroups() (which is called in btrfs_commit_transaction()) because quota_root has already been freed. If "quota enable" is called after that, both BTRFS_FS_QUOTA_DISABLING and BTRFS_FS_QUOTA_ENABLED flag would be dropped in the btrfs_run_qgroups() since quota_root is not NULL. This leads to the failure of "quota enable" on the first time. BTRFS_FS_QUOTA_DISABLING flag is not used outside of "quota disable" context and is equivalent to whether quota_root is NULL or not. btrfs_run_qgroups() checks whether quota_root is NULL or not in the first place. So, let's remove BTRFS_FS_QUOTA_DISABLING flag. Signed-off-by:
Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> Reviewed-by:
-
Naohiro Aota authored
btrfs_cmp_data_prepare() (almost) always returns 0 i.e. ignoring errors from gather_extent_pages(). While the pages are freed by btrfs_cmp_data_free(), cmp->num_pages still has > 0. Then, btrfs_extent_same() try to access the already freed pages causing faults (or violates PageLocked assertion). This patch just return the error as is so that the caller stop the process. Signed-off-by:
Naohiro Aota <naohiro.aota@wdc.com> Fixes: f4414602 ("btrfs: fix deadlock with extent-same and readpage") Cc: <stable@vger.kernel.org> # 4.2 Reviewed-by:
-
satoru takeuchi authored
`btrfs sub set-default` succeeds to set an ID which isn't corresponding to any fs/file tree. If such the bad ID is set to a filesystem, we can't mount this filesystem without specifying `subvol` or `subvolid` mount options. Fixes: 6ef5ed0d ("Btrfs: add ioctl and incompat flag to set the default mount subvol") Cc: <stable@vger.kernel.org> Signed-off-by:
-
Tsutomu Itoh authored
ENOTSUPP should not be returned to the user program. (cf. include/linux/errno.h) Therefore, EOPNOTSUPP is used instead of ENOTSUPP. Signed-off-by:
Tsutomu Itoh <t-itoh@jp.fujitsu.com> Reviewed-by:
-
Naohiro Aota authored
__del_reloc_root should be called before freeing up reloc_root->node. If not, calling __del_reloc_root() dereference reloc_root->node, causing the system BUG. Fixes: 6bdf131f ("Btrfs: don't leak reloc root nodes on error") Cc: <stable@vger.kernel.org> # 4.9 Signed-off-by:
Nikolay Borisov <nborisov@suse.com> Reviewed-by:
-
Naohiro Aota authored
__endio_write_update_ordered() repeats the search until it reaches the end of the specified range. This works well with direct IO path, because before the function is called, it's ensured that there are ordered extents filling whole the range. It's not the case, however, when it's called from run_delalloc_range(): it is possible to have error in the midle of the loop in e.g. run_delalloc_nocow(), so that there exisits the range not covered by any ordered extents. By cleaning such "uncomplete" range, __endio_write_update_ordered() stucks at offset where there're no ordered extents. Since the ordered extents are created from head to tail, we can stop the search if there are no offset progress. Fixes: 52427260 ("btrfs: Handle delalloc error correctly to avoid ordered extent hang") Cc: <stable@vger.kernel.org> # 4.12 Signed-off-by:
-
Naohiro Aota authored
Commit 52427260 ("btrfs: Handle delalloc error correctly to avoid ordered extent hang") introduced btrfs_cleanup_ordered_extents() to cleanup submitted ordered extents. However, it does not clear the ordered bit (Private2) of corresponding pages. Thus, the following BUG occurs from free_pages_check_bad() (on btrfs/125 with nospace_cache). BUG: Bad page state in process btrfs pfn:3fa787 page:ffffdf2acfe9e1c0 count:0 mapcount:0 mapping: (null) index:0xd flags: 0x8000000000002008(uptodate|private_2) raw: 8000000000002008 0000000000000000 000000000000000d 00000000ffffffff raw: ffffdf2acf5c1b20 ffffb443802238b0 0000000000000000 0000000000000000 page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set bad because of flags: 0x2000(private_2) This patch clears the flag same as other places calling btrfs_dec_test_ordered_pending() for every page in the specified range. Fixes: 52427260 ("btrfs: Handle delalloc error correctly to avoid ordered extent hang") Cc: <stable@vger.kernel.org> # 4.12 Signed-off-by:
-
Omar Sandoval authored
fs_info->super_copy->{node,sector}size are little-endian, but the ioctl should return the values in native endianness. Use the cached values in btrfs_fs_info instead. Found with sparse. Fixes: 80a773fb ("btrfs: retrieve more info from FS_INFO ioctl") Signed-off-by:Omar Sandoval <osandov@fb.com> Reviewed-by:
-
Liu Bo authored
flush_epd_write_bio() sets bio->bi_opf by itself to honor REQ_SYNC, but it's not needed at all since bio->bi_opf has set up properly in both __extent_writepage() and write_one_eb(), and in the case of write_one_eb(), it also sets REQ_META, which we will lose in flush_epd_write_bio(). This remove this unnecessary bio->bi_opf setting. Signed-off-by:
-
Liu Bo authored
This updates btrfs to use the helper wbc_to_write_flags which has been applied in ext4/xfs/f2fs/block. Please note that, with this, btrfs's dirty pages written by a writeback job will carry the flag REQ_BACKGROUND, which is currently used by writeback-throttle to determine whether it should go to get a request or wait. Signed-off-by:
-
- 22 Aug, 2017 1 commit
-
-
David Sterba authored
The superblock is also metadata of the filesystem so the relevant IO should be tagged as such. We also tag it as high priority, as it's the last block committed for metadata from a given transaction. Any delays would effectively block the whole transaction, also blocking any other operation holding the device_list_mutex. Reviewed-by:
-
- 21 Aug, 2017 21 commits
-
-
Nikolay Borisov authored
Commit 38851cc1 ("Btrfs: implement unlocked dio write") implemented unlocked dio write, allowing multiple dio writers to write to non-overlapping, and non-eof-extending regions. In doing so it also introduced a broken memory barrier. It is broken due to 2 things: 1. Memory barriers _MUST_ always be paired, this is clearly not the case here 2. Checkpatch actually produces a warning if a memory barrier is introduced that doesn't have a comment explaining how it's being paired. Specifically for inode::i_dio_count that's wrapped inside inode_dio_begin, there is no explicit barrier semantics attached, so removing is fine as the atomic is used in common the waiter/wakeup pattern. Signed-off-by:
-
Nikolay Borisov authored
Currently this function is always called with the object id of the root key of the chunk_tree, which is always BTRFS_CHUNK_TREE_OBJECTID. So let's subsume it straight into the function itself. No functional change. Signed-off-by:
-
Nikolay Borisov authored
THe function is always called with chunk_objectid set to BTRFS_FIRST_CHUNK_TREE_OBJECTID. Let's collapse the parameter in the function itself. No functional changes. Signed-off-by:
-
Jeff Mahoney authored
btrfs_del_roots always uses the tree_root. Let's pass fs_info instead. Signed-off-by:
Jeff Mahoney <jeffm@suse.com> Reviewed-by:
-
Liu Bo authored
Every shared ref has a parent tree block, which can be get from btrfs_extent_inline_ref_offset(). And the tree block must be aligned to the nodesize, so we'd know this inline ref is not valid if this block's bytenr is not aligned to the nodesize, in which case, most likely the ref type has been misused. This adds the above mentioned check and also updates print_extent_item() called by btrfs_print_leaf() to point out the invalid ref while printing the tree structure. Signed-off-by:
-
Liu Bo authored
The BUG_ON() can be triggered when the caller is processing an invalid extent inline ref, e.g. a shared data ref is offered instead of an extent data ref, such that it tries to find a non-existent tree block and then btrfs_search_slot returns 1 for no such item. This replaces the BUG_ON() with a WARN() followed by calling btrfs_print_leaf() to show more details about what's going on and returning -EINVAL to upper callers. Signed-off-by:
-
Liu Bo authored
Now that we have a helper to report invalid value of extent inline ref type, we need to quit gracefully instead of throwing out a kernel panic. Signed-off-by:
-
Liu Bo authored
btrfs_print_leaf() is used in btrfs_get_extent_inline_ref_type, so here we really want to print the invalid value of ref type instead of causing a kernel panic. Signed-off-by:
-
Liu Bo authored
Now that btrfs_get_extent_inline_ref_type() can report if type is a valid one and all callers can gracefully deal with that, we don't need to crash here. Signed-off-by:
-
Liu Bo authored
Since we have a helper which can do sanity check, this converts all btrfs_extent_inline_ref_type to it. Signed-off-by:
-
Liu Bo authored
An invalid value of extent inline ref type may be read from a malicious image which may force btrfs to crash. This adds a helper which does sanity check for the ref type, so we can know if it's sane, return he type, otherwise return an error. Signed-off-by:
-
David Sterba authored
Minor simplification, merge calls to one. Signed-off-by: -
David Sterba authored
Use proper helpers for 64bit division. Signed-off-by: -
David Sterba authored
Use proper helpers for 64bit division and then cast to narrower type. Signed-off-by: -
David Sterba authored
flush_all_writes is an atomic but does not use the semantics at all, it's just on/off indicator, we can use bool. Signed-off-by: -
Ernesto A. Fernández authored
When changing a file's acl mask, btrfs_set_acl() will first set the group bits of i_mode to the value of the mask, and only then set the actual extended attribute representing the new acl. If the second part fails (due to lack of space, for example) and the file had no acl attribute to begin with, the system will from now on assume that the mask permission bits are actual group permission bits, potentially granting access to the wrong users. Prevent this by restoring the original mode bits if __btrfs_set_acl fails. Signed-off-by:
Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com> Reviewed-by:
-
Nikolay Borisov authored
BTRFS_FIRST_CHUNK_TREE_OBJECTIS id the only objectid being used in the chunk_tree. So remove a variable which is always set to that value and collapse its usage in callees which are passed this variable. No functional changes Signed-off-by:
-
Nikolay Borisov authored
btrfs_make_block_group is always called with chunk_objectid set to BTRFS_FIRST_CHUNK_TREE_OBJECTID. There's no reason why this behavior will change anytime soon, so let's remove the argument and decrease the cognitive load when reading the code path. No functional change Signed-off-by:
-
Matthias Kaehlcke authored
There is no need for the extra pair of parentheses, remove it. This fixes the following warning when building with clang: fs/btrfs/tree-log.c:3694:10: warning: equality comparison with extraneous parentheses [-Wparentheses-equality] if ((i == (nr - 1))) ~~^~~~~~~~~~~ Also remove the unnecessary parentheses around the substraction. Signed-off-by:Matthias Kaehlcke <mka@chromium.org> Signed-off-by:
-
Nikolay Borisov authored
btrfs_alloc_dev_extent currently unconditionally sets the uuid in the leaf block header the function is working with. This is unnecessary since this operation is peformed by the core btree handling code (splitting a node, allocating a new btree block etc). So let's remove it. Signed-off-by:
-
Hans van Kranenburg authored
This patch provides a band aid to improve the 'out of the box' behaviour of btrfs for disks that are detected as being an ssd. In a general purpose mixed workload scenario, the current ssd mode causes overallocation of available raw disk space for data, while leaving behind increasing amounts of unused fragmented free space. This situation leads to early ENOSPC problems which are harming user experience and adoption of btrfs as a general purpose filesystem. This patch modifies the data extent allocation behaviour of the ssd mode to make it behave identical to nossd mode. The metadata behaviour and additional ssd_spread option stay untouched so far. Recommendations for future development are to reconsider the current oversimplified nossd / ssd distinction and the broken detection mechanism based on the rotational attribute in sysfs and provide experienced users with a more flexible way to choose allocator behaviour for data and metadata, optimized for certain use cases, while keeping sane 'out of the box' default settings. The internals of the current btrfs code have more potential than what currently gets exposed to the user to choose from. The SSD story... In the first year of btrfs development, around early 2008, btrfs gained a mount option which enables specific functionality for filesystems on solid state devices. The first occurance of this functionality is in commit e18e4809, labeled "Add mount -o ssd, which includes optimizations for seek free storage". The effect on allocating free space for doing (data) writes is to 'cluster' writes together, writing them out in contiguous space, as opposed to a 'tetris' way of putting all separate writes into any free space fragment that fits (which is what the -o nossd behaviour does). A somewhat simplified explanation of what happens is that, when for example, the 'cluster' size is set to 2MiB, when we do some writes, the data allocator will search for a free space block that is 2MiB big, and put the writes in there. The ssd mode itself might allow a 2MiB cluster to be composed of multiple free space extents with some existing data in between, while the additional ssd_spread mount option kills off this option and requires fully free space. The idea behind this is (commit 536ac8ae): "The [...] clusters make it more likely a given IO will completely overwrite the ssd block, so it doesn't have to do an internal rwm cycle."; ssd block meaning nand erase block. So, effectively this means applying a "locality based algorithm" and trying to outsmart the actual ssd. Since then, various changes have been made to the involved code, but the basic idea is still present, and gets activated whenever the ssd mount option is active. This also happens by default, when the rotational flag as seen at /sys/block/<device>/queue/rotational is set to 0. However, there's a number of problems with this approach. First, what the optimization is trying to do is outsmart the ssd by assuming there is a relation between the physical address space of the block device as seen by btrfs and the actual physical storage of the ssd, and then adjusting data placement. However, since the introduction of the Flash Translation Layer (FTL) which is a part of the internal controller of an ssd, these attempts are futile. The use of good quality FTL in consumer ssd products might have been limited in 2008, but this situation has changed drastically soon after that time. Today, even the flash memory in your automatic cat feeding machine or your grandma's wheelchair has a full featured one. Second, the behaviour as described above results in the filesystem being filled up with badly fragmented free space extents because of relatively small pieces of space that are freed up by deletes, but not selected again as part of a 'cluster'. Since the algorithm prefers allocating a new chunk over going back to tetris mode, the end result is a filesystem in which all raw space is allocated, but which is composed of underutilized chunks with a 'shotgun blast' pattern of fragmented free space. Usually, the next problematic thing that happens is the filesystem wanting to allocate new space for metadata, which causes the filesystem to fail in spectacular ways. Third, the default mount options you get for an ssd ('ssd' mode enabled, 'discard' not enabled), in combination with spreading out writes over the full address space and ignoring freed up space leads to worst case behaviour in providing information to the ssd itself, since it will never learn that all the free space left behind is actually free. There are two ways to let an ssd know previously written data does not have to be preserved, which are sending explicit signals using discard or fstrim, or by simply overwriting the space with new data. The worst case behaviour is the btrfs ssd_spread mount option in combination with not having discard enabled. It has a side effect of minimizing the reuse of free space previously written in. Fourth, the rotational flag in /sys/ does not reliably indicate if the device is a locally attached ssd. For example, iSCSI or NBD displays as non-rotational, while a loop device on an ssd shows up as rotational. The combination of the second and third problem effectively means that despite all the good intentions, the btrfs ssd mode reliably causes the ssd hardware and the filesystem structures and performance to be choked to death. The clickbait version of the title of this story would have been "Btrfs ssd optimizations considered harmful for ssds". The current nossd 'tetris' mode (even still without discard) allows a pattern of overwriting much more previously used space, causing many more implicit discards to happen because of the overwrite information the ssd gets. The actual location in the physical address space, as seen from the point of view of btrfs is irrelevant, because the actual writes to the low level flash are reordered anyway thanks to the FTL. Changes made in the code 1. Make ssd mode data allocation identical to tetris mode, like nossd. 2. Adjust and clean up filesystem mount messages so that we can easily identify if a kernel has this patch applied or not, when providing support to end users. Also, make better use of the *_and_info helpers to only trigger messages on actual state changes. Backporting notes Notes for whoever wants to backport this patch to their 4.9 LTS kernel: * First apply commit 951e7966 "btrfs: drop the nossd flag when remounting with -o ssd", or fixup the differences manually. * The rest of the conflicts are because of the fs_info refactoring. So, for example, instead of using fs_info, it's root->fs_info in extent-tree.c Signed-off-by:Hans van Kranenburg <hans.van.kranenburg@mendix.com> Signed-off-by:
-
