1. 14 Mar, 2022 40 commits
    • Qu Wenruo's avatar
      btrfs: verify the tranisd of the to-be-written dirty extent buffer · 3777369f
      Qu Wenruo authored
      [BUG]
      There is a bug report that a bitflip in the transid part of an extent
      buffer makes btrfs to reject certain tree blocks:
      
        BTRFS error (device dm-0): parent transid verify failed on 1382301696 wanted 262166 found 22
      
      [CAUSE]
      Note the failed transid check, hex(262166) = 0x40016, while
      hex(22) = 0x16.
      
      It's an obvious bitflip.
      
      Furthermore, the reporter also confirmed the bitflip is from the
      hardware, so it's a real hardware caused bitflip, and such problem can
      not be detected by the existing tree-checker framework.
      
      As tree-checker can only verify the content inside one tree block, while
      generation of a tree block can only be verified against its parent.
      
      So such problem remain undetected.
      
      [FIX]
      Although tree-checker can not verify it at write-time, we still have a
      quick (but not the most accurate) way to catch such obvious corruption.
      
      Function csum_one_extent_buffer() is called before we submit metadata
      write.
      
      Thus it means, all the extent buffer passed in should be dirty tree
      blocks, and should be newer than last committed transaction.
      
      Using that we can catch the above bitflip.
      
      Although it's not a perfect solution, as if the corrupted generation is
      higher than the correct value, we have no way to catch it at all.
      Reported-by: default avatarChristoph Anton Mitterer <calestyo@scientia.org>
      Link: https://lore.kernel.org/linux-btrfs/2dfcbc130c55cc6fd067b93752e90bd2b079baca.camel@scientia.org/
      CC: stable@vger.kernel.org # 5.15+
      Signed-off-by: default avatarQu Wenruo <wqu@sus,ree.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      3777369f
    • Qu Wenruo's avatar
      btrfs: unify the error handling of btrfs_read_buffer() · 9a4ffa1b
      Qu Wenruo authored
      There is one oddball error handling of btrfs_read_buffer():
      
      	ret = btrfs_read_buffer(tmp, gen, parent_level - 1, &first_key);
      	if (!ret) {
      		*eb_ret = tmp;
      		return 0;
      	}
      	free_extent_buffer(tmp);
      	btrfs_release_path(p);
      	return -EIO;
      
      While all other call sites check the error first.  Unify the behavior.
      Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      9a4ffa1b
    • Qu Wenruo's avatar
      btrfs: unify the error handling pattern for read_tree_block() · 4eb150d6
      Qu Wenruo authored
      We had an error handling pattern for read_tree_block() like this:
      
      	eb = read_tree_block();
      	if (IS_ERR(eb)) {
      		/*
      		 * Handling error here
      		 * Normally ended up with return or goto out.
      		 */
      	} else if (!extent_buffer_uptodate(eb)) {
      		/*
      		 * Different error handling here
      		 * Normally also ended up with return or goto out;
      		 */
      	}
      
      This is fine, but if we want to add extra check for each
      read_tree_block(), the existing if-else-if is not that expandable and
      will take reader some seconds to figure out there is no extra branch.
      
      Here we change it to a more common way, without the extra else:
      
      	eb = read_tree_block();
      	if (IS_ERR(eb)) {
      		/*
      		 * Handling error here
      		 */
      		return eb or goto out;
      	}
      	if (!extent_buffer_uptodate(eb)) {
      		/*
      		 * Different error handling here
      		 */
      		return eb or goto out;
      	}
      
      This also removes some oddball call sites which uses some creative way
      to check error.
      Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      4eb150d6
    • Josef Bacik's avatar
      btrfs: factor out do_free_extent_accounting helper · 8f8aa4c7
      Josef Bacik authored
      __btrfs_free_extent() does all of the hard work of updating the extent
      ref items, and then at the end if we dropped the extent completely it
      does the cleanup accounting work.  We're going to only want to do that
      work for metadata with extent tree v2, so extract this bit into its own
      helper.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      8f8aa4c7
    • Josef Bacik's avatar
      btrfs: remove last_ref from the extent freeing code · 5b2a54bb
      Josef Bacik authored
      This is a remnant of the work I did for qgroups a long time ago to only
      run for a block when we had dropped the last ref.  We haven't done that
      for years, but the code remains.  Drop this remnant.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      5b2a54bb
    • Josef Bacik's avatar
      btrfs: add a alloc_reserved_extent helper · 34666705
      Josef Bacik authored
      We duplicate this logic for both data and metadata, at this point we've
      already done our type specific extent root operations, this is just
      doing the accounting and removing the space from the free space tree.
      Extract this common logic out into a helper.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      34666705
    • Josef Bacik's avatar
      btrfs: remove BUG_ON(ret) in alloc_reserved_tree_block · b3c958a3
      Josef Bacik authored
      Switch this to an ASSERT() and return the error in the normal case.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      b3c958a3
    • Filipe Manana's avatar
      btrfs: add and use helper for unlinking inode during log replay · 313ab753
      Filipe Manana authored
      During log replay there is this pattern of running delayed items after
      every inode unlink. To avoid repeating this several times, move the
      logic into an helper function and use it instead of calling
      btrfs_unlink_inode() followed by btrfs_run_delayed_items().
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      313ab753
    • Niels Dossche's avatar
      btrfs: extend locking to all space_info members accesses · 06bae876
      Niels Dossche authored
      bytes_pinned is always accessed under space_info->lock, except in
      btrfs_preempt_reclaim_metadata_space, however the other members are
      accessed under that lock. The reserved member of the rsv's are also
      partially accessed under a lock and partially not. Move all these
      accesses into the same lock to ensure consistency.
      
      This could potentially race and lead to a flush instead of a commit but
      it's not a big problem as it's only for preemptive flush.
      
      CC: stable@vger.kernel.org # 5.15+
      Reviewed-by: default avatarJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarNiels Dossche <niels.dossche@ugent.be>
      Signed-off-by: default avatarNiels Dossche <dossche.niels@gmail.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      06bae876
    • Naohiro Aota's avatar
      btrfs: zoned: mark relocation as writing · ca5e4ea0
      Naohiro Aota authored
      There is a hung_task issue with running generic/068 on an SMR
      device. The hang occurs while a process is trying to thaw the
      filesystem. The process is trying to take sb->s_umount to thaw the
      FS. The lock is held by fsstress, which calls btrfs_sync_fs() and is
      waiting for an ordered extent to finish. However, as the FS is frozen,
      the ordered extents never finish.
      
      Having an ordered extent while the FS is frozen is the root cause of
      the hang. The ordered extent is initiated from btrfs_relocate_chunk()
      which is called from btrfs_reclaim_bgs_work().
      
      This commit adds sb_*_write() around btrfs_relocate_chunk() call
      site. For the usual "btrfs balance" command, we already call it with
      mnt_want_file() in btrfs_ioctl_balance().
      
      Fixes: 18bb8bbf ("btrfs: zoned: automatically reclaim zones")
      CC: stable@vger.kernel.org # 5.13+
      Link: https://github.com/naota/linux/issues/56Reviewed-by: default avatarJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Signed-off-by: default avatarNaohiro Aota <naohiro.aota@wdc.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      ca5e4ea0
    • Josef Bacik's avatar
      fs: allow cross-vfsmount reflink/dedupe · 9f5710bb
      Josef Bacik authored
      Currently we disallow reflink and dedupe if the two files aren't on the
      same vfsmount.  However we really only need to disallow it if they're
      not on the same super block.  It is very common for btrfs to have a main
      subvolume that is mounted and then different subvolumes mounted at
      different locations.  It's allowed to reflink between these volumes, but
      the vfsmount check disallows this.  Instead fix dedupe to check for the
      same superblock, and simply remove the vfsmount check for reflink as it
      already does the superblock check.
      Reviewed-by: default avatarDarrick J. Wong <djwong@kernel.org>
      Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      9f5710bb
    • Josef Bacik's avatar
      btrfs: remove the cross file system checks from remap · ae460f05
      Josef Bacik authored
      The sb check is already done in do_clone_file_range, and the mnt check
      (which will hopefully go away in a subsequent patch) is done in
      ioctl_file_clone().  Remove the check in our code and put an ASSERT() to
      make sure it doesn't change underneath us.
      Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      ae460f05
    • Josef Bacik's avatar
      btrfs: pass btrfs_fs_info to btrfs_recover_relocation · 7eefae6b
      Josef Bacik authored
      We don't need a root here, we just need the btrfs_fs_info, we can just
      get the specific roots we need from fs_info.
      Reviewed-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      7eefae6b
    • Josef Bacik's avatar
      btrfs: pass btrfs_fs_info for deleting snapshots and cleaner · 33c44184
      Josef Bacik authored
      We're passing a root around here, but we only really need the fs_info,
      so fix up btrfs_clean_one_deleted_snapshot() to take an fs_info instead,
      and then fix up all the callers appropriately.
      Reviewed-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      33c44184
    • Sweet Tea Dorminy's avatar
      btrfs: add filesystems state details to error messages · c067da87
      Sweet Tea Dorminy authored
      When a filesystem goes read-only due to an error, multiple errors tend
      to be reported, some of which are knock-on failures. Logging fs_states,
      in btrfs_handle_fs_error() and btrfs_printk() helps distinguish the
      first error from subsequent messages which may only exist due to an
      error state.
      
      Under the new format, most initial errors will look like:
      `BTRFS: error (device loop0) in ...`
      while subsequent errors will begin with:
      `error (device loop0: state E) in ...`
      
      An initial transaction abort error will look like
      `error (device loop0: state A) in ...`
      and subsequent messages will contain
      `(device loop0: state EA) in ...`
      
      In addition to the error states we can also print other states that are
      temporary, like remounting, device replace, or indicate a global state
      that may affect functionality.
      
      Now implemented:
      
      E - filesystem error detected
      A - transaction aborted
      L - log tree errors
      
      M - remounting in progress
      R - device replace in progress
      C - data checksums not verified (mounted with ignoredatacsums)
      Signed-off-by: default avatarSweet Tea Dorminy <sweettea-kernel@dorminy.me>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      c067da87
    • Filipe Manana's avatar
      btrfs: deal with unexpected extent type during reflinking · b2d9f2dc
      Filipe Manana authored
      Smatch complains about a possible dereference of a pointer that was not
      initialized:
      
          CC [M]  fs/btrfs/reflink.o
          CHECK   fs/btrfs/reflink.c
        fs/btrfs/reflink.c:533 btrfs_clone() error: potentially dereferencing uninitialized 'trans'.
      
      This is because we are not dealing with the case where the type of a file
      extent has an unexpected value (not regular, not prealloc and not inline),
      in which case the transaction handle pointer is not initialized.
      
      Such unexpected type should be impossible, except in case of some memory
      corruption caused either by bad hardware or some software bug causing
      something like a buffer overrun.
      
      So ASSERT that if the extent type is neither regular nor prealloc, then
      it must be inline. Bail out with -EUCLEAN and a warning in case it is
      not. This silences smatch.
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      b2d9f2dc
    • Filipe Manana's avatar
      btrfs: fix unexpected error path when reflinking an inline extent · 1f4613cd
      Filipe Manana authored
      When reflinking an inline extent, we assert that its file offset is 0 and
      that its uncompressed length is not greater than the sector size. We then
      return an error if one of those conditions is not satisfied. However we
      use a return statement, which results in returning from btrfs_clone()
      without freeing the path and buffer that were allocated before, as well as
      not clearing the flag BTRFS_INODE_NO_DELALLOC_FLUSH for the destination
      inode.
      
      Fix that by jumping to the 'out' label instead, and also add a WARN_ON()
      for each condition so that in case assertions are disabled, we get to
      known which of the unexpected conditions triggered the error.
      
      Fixes: a61e1e0d ("Btrfs: simplify inline extent handling when doing reflinks")
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      1f4613cd
    • Filipe Manana's avatar
      btrfs: reset last_reflink_trans after fsyncing inode · 23e3337f
      Filipe Manana authored
      When an inode has a last_reflink_trans matching the current transaction,
      we have to take special care when logging its checksums in order to
      avoid getting checksum items with overlapping ranges in a log tree,
      which could result in missing checksums after log replay (more on that
      in the changelogs of commit 40e046ac ("Btrfs: fix missing data
      checksums after replaying a log tree") and commit e289f03e ("btrfs:
      fix corrupt log due to concurrent fsync of inodes with shared extents")).
      We also need to make sure a full fsync will copy all old file extent
      items it finds in modified leaves, because they might have been copied
      from some other inode.
      
      However once we fsync an inode, we don't need to keep paying the price of
      that extra special care in future fsyncs done in the same transaction,
      unless the inode is used for another reflink operation or the full sync
      flag is set on it (truncate, failure to allocate extent maps for holes,
      and other exceptional and infrequent cases).
      
      So after we fsync an inode reset its last_unlink_trans to zero. In case
      another reflink happens, we continue to update the last_reflink_trans of
      the inode, just as before. Also set last_reflink_trans to the generation
      of the last transaction that modified the inode whenever we need to set
      the full sync flag on the inode, just like when we need to load an inode
      from disk after eviction.
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      23e3337f
    • Filipe Manana's avatar
      btrfs: voluntarily relinquish cpu when doing a full fsync · 96acb375
      Filipe Manana authored
      Doing a full fsync may require processing many leaves of metadata, which
      can take some time and result in a task monopolizing a cpu for too long.
      So add a cond_resched() after processing a leaf when doing a full fsync,
      while not holding any locks on any tree (a subvolume or a log tree).
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      96acb375
    • Filipe Manana's avatar
      btrfs: hold on to less memory when logging checksums during full fsync · 5b7ce5e2
      Filipe Manana authored
      When doing a full fsync, at copy_items(), we iterate over all extents and
      then collect their checksums into a list. After copying all the extents to
      the log tree, we then log all the previously collected checksums.
      
      Before the previous patch in the series (subject "btrfs: stop copying old
      file extents when doing a full fsync"), we had to do it this way, because
      while we were iterating over the items in the leaf of the subvolume tree,
      we were holding a write lock on a leaf of the log tree, so logging the
      checksums for an extent right after we collected them could result in a
      deadlock, in case the checksum items ended up in the same leaf.
      
      However after the previous patch in the series we now do a first iteration
      over all the items in the leaf of the subvolume tree before locking a path
      in the log tree, so we can now log the checksums right after we have
      obtained them. This avoids holding in memory all checksums for all extents
      in the leaf while copying items from the source leaf to the log tree. The
      amount of memory used to hold all checksums of the extents in a leaf can
      be significant. For example if a leaf has 200 file extent items referring
      to 1M extents, using the default crc32c checksums, would result in using
      over 200K of memory (not accounting for the extra overhead of struct
      btrfs_ordered_sum), with smaller or less extents it would be less, but
      it could be much more with more extents per leaf and/or much larger
      extents.
      
      So change copy_items() to log the checksums for an extent after looking
      them up, and then free their memory, as they are no longer necessary.
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      5b7ce5e2
    • Filipe Manana's avatar
      btrfs: stop copying old file extents when doing a full fsync · 7f30c072
      Filipe Manana authored
      When logging an inode in full sync mode, we go over every leaf that was
      modified in the current transaction and has items associated to our inode,
      and then copy all those items into the log tree. This includes copying
      file extent items that were created and added to the inode in past
      transactions, which is useless and only makes use more leaf space in the
      log tree.
      
      It's common to have a file with many file extent items spanning many
      leaves where only a few file extent items are new and need to be logged,
      and in such case we log all the file extent items we find in the modified
      leaves.
      
      So change the full sync behaviour to skip over file extent items that are
      not needed. Those are the ones that match the following criteria:
      
      1) Have a generation older than the current transaction and the inode
         was not a target of a reflink operation, as that can copy file extent
         items from a past generation from some other inode into our inode, so
         we have to log them;
      
      2) Start at an offset within i_size - we must log anything at or beyond
         i_size, otherwise we would lose prealloc extents after log replay.
      
      The following script exercises a scenario where this happens, and it's
      somehow close enough to what happened often on a SQL Server workload which
      I had to debug sometime ago to fix an issue where a pattern of writes to
      prealloc extents and fsync resulted in fsync failing with -EIO (that was
      commit ea7036de ("btrfs: fix fsync failure and transaction abort
      after writes to prealloc extents")). In that particular case, we had large
      files that had random writes and were often truncated, which made the
      next fsync be a full sync.
      
        $ cat test.sh
        #!/bin/bash
      
        DEV=/dev/sdi
        MNT=/mnt/sdi
      
        MKFS_OPTIONS="-O no-holes -R free-space-tree"
        MOUNT_OPTIONS="-o ssd"
      
        FILE_SIZE=$((1 * 1024 * 1024 * 1024)) # 1G
        # FILE_SIZE=$((2 * 1024 * 1024 * 1024)) # 2G
        # FILE_SIZE=$((512 * 1024 * 1024)) # 512M
      
        mkfs.btrfs -f $MKFS_OPTIONS $DEV
        mount $MOUNT_OPTIONS $DEV $MNT
      
        # Create a file with many extents. Use direct IO to make it faster
        # to create the file - using buffered IO we would have to fsync
        # after each write (terribly slow).
        echo "Creating file with $((FILE_SIZE / 4096)) extents of 4K each..."
        xfs_io -f -d -c "pwrite -b 4K 0 $FILE_SIZE" $MNT/foobar
      
        # Commit the transaction, so every extent after this is from an
        # old generation.
        sync
      
        # Now rewrite only a few extents, which are all far spread apart from
        # each other (e.g. 1G / 32M = 32 extents).
        # After this only a few extents have a new generation, while all other
        # ones have an old generation.
        echo "Rewriting $((FILE_SIZE / (32 * 1024 * 1024))) extents..."
        for ((i = 0; i < $FILE_SIZE; i += $((32 * 1024 * 1024)))); do
            xfs_io -c "pwrite $i 4K" $MNT/foobar >/dev/null
        done
      
        # Fsync, the inode logged in full sync mode since it was never fsynced
        # before.
        echo "Fsyncing file..."
        xfs_io -c "fsync" $MNT/foobar
      
        umount $MNT
      
      And the following bpftrace program was running when executing the test
      script:
      
        $ cat bpf-script.sh
        #!/usr/bin/bpftrace
      
        k:btrfs_log_inode
        {
            @start_log_inode[tid] = nsecs;
        }
      
        kr:btrfs_log_inode
        /@start_log_inode[tid]/
        {
            @log_inode_dur[tid] = (nsecs - @start_log_inode[tid]) / 1000;
            delete(@start_log_inode[tid]);
        }
      
        k:btrfs_sync_log
        {
            @start_sync_log[tid] = nsecs;
        }
      
        kr:btrfs_sync_log
        /@start_sync_log[tid]/
        {
            $sync_log_dur = (nsecs - @start_sync_log[tid]) / 1000;
            printf("btrfs_log_inode() took %llu us\n", @log_inode_dur[tid]);
            printf("btrfs_sync_log()  took %llu us\n", $sync_log_dur);
            delete(@start_sync_log[tid]);
            delete(@log_inode_dur[tid]);
            exit();
        }
      
      With 512M test file, before this patch:
      
        btrfs_log_inode() took 15218 us
        btrfs_sync_log()  took 1328 us
      
        Log tree has 17 leaves and 1 node, its total size is 294912 bytes.
      
      With 512M test file, after this patch:
      
        btrfs_log_inode() took 14760 us
        btrfs_sync_log()  took 588 us
      
        Log tree has a single leaf, its total size is 16K.
      
      With 1G test file, before this patch:
      
        btrfs_log_inode() took 27301 us
        btrfs_sync_log()  took 1767 us
      
        Log tree has 33 leaves and 1 node, its total size is 557056 bytes.
      
      With 1G test file, after this patch:
      
        btrfs_log_inode() took 26166 us
        btrfs_sync_log()  took 593 us
      
        Log tree has a single leaf, its total size is 16K
      
      With 2G test file, before this patch:
      
        btrfs_log_inode() took 50892 us
        btrfs_sync_log()  took 3127 us
      
        Log tree has 65 leaves and 1 node, its total size is 1081344 bytes.
      
      With 2G test file, after this patch:
      
        btrfs_log_inode() took 50126 us
        btrfs_sync_log()  took 586 us
      
        Log tree has a single leaf, its total size is 16K.
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      7f30c072
    • Josef Bacik's avatar
      btrfs: do not clean up repair bio if submit fails · 8cbc3001
      Josef Bacik authored
      The submit helper will always run bio_endio() on the bio if it fails to
      submit, so cleaning up the bio just leads to a variety of use-after-free
      and NULL pointer dereference bugs because we race with the endio
      function that is cleaning up the bio.  Instead just return BLK_STS_OK as
      the repair function has to continue to process the rest of the pages,
      and the endio for the repair bio will do the appropriate cleanup for the
      page that it was given.
      Reviewed-by: default avatarBoris Burkov <boris@bur.io>
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      8cbc3001
    • Josef Bacik's avatar
      btrfs: do not try to repair bio that has no mirror set · 510671d2
      Josef Bacik authored
      If we fail to submit a bio for whatever reason, we may not have setup a
      mirror_num for that bio.  This means we shouldn't try to do the repair
      workflow, if we do we'll hit an BUG_ON(!failrec->this_mirror) in
      clean_io_failure.  Instead simply skip the repair workflow if we have no
      mirror set, and add an assert to btrfs_check_repairable() to make it
      easier to catch what is happening in the future.
      Reviewed-by: default avatarBoris Burkov <boris@bur.io>
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      510671d2
    • Josef Bacik's avatar
      btrfs: do not double complete bio on errors during compressed reads · f9f15de8
      Josef Bacik authored
      I hit some weird panics while fixing up the error handling from
      btrfs_lookup_bio_sums().  Turns out the compression path will complete
      the bio we use if we set up any of the compression bios and then return
      an error, and then btrfs_submit_data_bio() will also call bio_endio() on
      the bio.
      
      Fix this by making btrfs_submit_compressed_read() responsible for
      calling bio_endio() on the bio if there are any errors.  Currently it
      was only doing it if we created the compression bios, otherwise it was
      depending on btrfs_submit_data_bio() to do the right thing.  This
      creates the above problem, so fix up btrfs_submit_compressed_read() to
      always call bio_endio() in case of an error, and then simply return from
      btrfs_submit_data_bio() if we had to call
      btrfs_submit_compressed_read().
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      f9f15de8
    • Josef Bacik's avatar
      btrfs: track compressed bio errors as blk_status_t · 606f82e7
      Josef Bacik authored
      Right now we just have a binary "errors" flag, so any error we get on
      the compressed bio's gets translated to EIO.  This isn't necessarily a
      bad thing, but if we get an ENOMEM it may be nice to know that's what
      happened instead of an EIO.  Track our errors as a blk_status_t, and do
      the appropriate setting of the errors accordingly.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      606f82e7
    • Josef Bacik's avatar
      btrfs: remove the bio argument from finish_compressed_bio_read · e14bfdb5
      Josef Bacik authored
      This bio is usually one of the compressed bio's, and we don't actually
      need it in this function, so remove the argument and stop passing it
      around.
      Reviewed-by: default avatarJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      e14bfdb5
    • Josef Bacik's avatar
      btrfs: check correct bio in finish_compressed_bio_read · b0bbc8a3
      Josef Bacik authored
      Commit c09abff8 ("btrfs: cloned bios must not be iterated by
      bio_for_each_segment_all") added ASSERT()'s to make sure we weren't
      calling bio_for_each_segment_all() on a RAID5/6 bio.  However it was
      checking the bio that the compression code passed in, not the
      cb->orig_bio that we actually iterate over, so adjust this ASSERT() to
      check the correct bio.
      Reviewed-by: default avatarBoris Burkov <boris@bur.io>
      Reviewed-by: default avatarJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      b0bbc8a3
    • Josef Bacik's avatar
      btrfs: handle csum lookup errors properly on reads · 1784b7d5
      Josef Bacik authored
      Currently any error we get while trying to lookup csums during reads
      shows up as a missing csum, and then on the read completion side we
      print an error saying there was a csum mismatch and we increase the
      device corruption count.
      
      However we could have gotten an EIO from the lookup.  We could also be
      inside of a memory constrained container and gotten a ENOMEM while
      trying to do the read.  In either case we don't want to make this look
      like a file system corruption problem, we want to make it look like the
      actual error it is.  Capture any negative value, convert it to the
      appropriate blk_status_t, free the csum array if we have one and bail.
      
      Note: a possible improvement would be to make the relocation code look
      up the owning inode and see if it's marked as NODATASUM and set
      EXTENT_NODATASUM there, that way if there's corruption and there isn't a
      checksum when we want it we can fail here rather than later.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      1784b7d5
    • Josef Bacik's avatar
      btrfs: make search_csum_tree return 0 if we get -EFBIG · 03ddb19d
      Josef Bacik authored
      We can either fail to find a csum entry at all and return -ENOENT, or we
      can find a range that is close, but return -EFBIG.  In essence these
      both mean the same thing when we are doing a lookup for a csum in an
      existing range, we didn't find a csum.  We want to treat both of these
      errors the same way, complain loudly that there wasn't a csum.  This
      currently happens anyway because we do
      
      	count = search_csum_tree();
      	if (count <= 0) {
      		// reloc and error handling
      	}
      
      However it forces us to incorrectly treat EIO or ENOMEM errors as on
      disk corruption.  Fix this by returning 0 if we get either -ENOENT or
      -EFBIG from btrfs_lookup_csum() so we can do proper error handling.
      Reviewed-by: default avatarBoris Burkov <boris@bur.io>
      Reviewed-by: default avatarJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      03ddb19d
    • Omar Sandoval's avatar
      btrfs: add BTRFS_IOC_ENCODED_WRITE · 7c0c7269
      Omar Sandoval authored
      The implementation resembles direct I/O: we have to flush any ordered
      extents, invalidate the page cache, and do the io tree/delalloc/extent
      map/ordered extent dance. From there, we can reuse the compression code
      with a minor modification to distinguish the write from writeback. This
      also creates inline extents when possible.
      Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      7c0c7269
    • Omar Sandoval's avatar
      btrfs: add BTRFS_IOC_ENCODED_READ ioctl · 1881fba8
      Omar Sandoval authored
      There are 4 main cases:
      
      1. Inline extents: we copy the data straight out of the extent buffer.
      2. Hole/preallocated extents: we fill in zeroes.
      3. Regular, uncompressed extents: we read the sectors we need directly
         from disk.
      4. Regular, compressed extents: we read the entire compressed extent
         from disk and indicate what subset of the decompressed extent is in
         the file.
      
      This initial implementation simplifies a few things that can be improved
      in the future:
      
      - Cases 1, 3, and 4 allocate temporary memory to read into before
        copying out to userspace.
      - We don't do read repair, because it turns out that read repair is
        currently broken for compressed data.
      - We hold the inode lock during the operation.
      
      Note that we don't need to hold the mmap lock. We may race with
      btrfs_page_mkwrite() and read the old data from before the page was
      dirtied:
      
      btrfs_page_mkwrite         btrfs_encoded_read
      ---------------------------------------------------
      (enter)                    (enter)
                                 btrfs_wait_ordered_range
      lock_extent_bits
      btrfs_page_set_dirty
      unlock_extent_cached
      (exit)
                                 lock_extent_bits
                                 read extent (dirty page hasn't been flushed,
                                              so this is the old data)
                                 unlock_extent_cached
                                 (exit)
      
      we read the old data from before the page was dirtied. But, that's true
      even if we were to hold the mmap lock:
      
      btrfs_page_mkwrite               btrfs_encoded_read
      -------------------------------------------------------------------
      (enter)                          (enter)
                                       btrfs_inode_lock(BTRFS_ILOCK_MMAP)
      down_read(i_mmap_lock) (blocked)
                                       btrfs_wait_ordered_range
                                       lock_extent_bits
      				 read extent (page hasn't been dirtied,
                                                    so this is the old data)
                                       unlock_extent_cached
                                       btrfs_inode_unlock(BTRFS_ILOCK_MMAP)
      down_read(i_mmap_lock) returns
      lock_extent_bits
      btrfs_page_set_dirty
      unlock_extent_cached
      
      In other words, this is inherently racy, so it's fine that we return the
      old data in this tiny window.
      Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      1881fba8
    • Omar Sandoval's avatar
      btrfs: add definitions and documentation for encoded I/O ioctls · dcb77a9a
      Omar Sandoval authored
      In order to allow sending and receiving compressed data without
      decompressing it, we need an interface to write pre-compressed data
      directly to the filesystem and the matching interface to read compressed
      data without decompressing it. This adds the definitions for ioctls to
      do that and detailed explanations of how to use them.
      Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
      Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      dcb77a9a
    • Omar Sandoval's avatar
      btrfs: optionally extend i_size in cow_file_range_inline() · d9496e8a
      Omar Sandoval authored
      Currently, an inline extent is always created after i_size is extended
      from btrfs_dirty_pages(). However, for encoded writes, we only want to
      update i_size after we successfully created the inline extent. Add an
      update_i_size parameter to cow_file_range_inline() and
      insert_inline_extent() and pass in the size of the extent rather than
      determining it from i_size.
      Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      [ reformat comment ]
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      d9496e8a
    • Omar Sandoval's avatar
      btrfs: clean up cow_file_range_inline() · 8dd9872d
      Omar Sandoval authored
      The start parameter to cow_file_range_inline() (and
      insert_inline_extent()) is always 0, so get rid of it and simplify the
      logic in those two functions. Pass btrfs_inode to insert_inline_extent()
      and remove the redundant root parameter. Also document the requirements
      for creating an inline extent. No functional change.
      Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      8dd9872d
    • Omar Sandoval's avatar
      btrfs: support different disk extent size for delalloc · 28c9b1e7
      Omar Sandoval authored
      Currently, we always reserve the same extent size in the file and extent
      size on disk for delalloc because the former is the worst case for the
      latter. For BTRFS_IOC_ENCODED_WRITE writes, we know the exact size of
      the extent on disk, which may be less than or greater than (for
      bookends) the size in the file. Add a disk_num_bytes parameter to
      btrfs_delalloc_reserve_metadata() so that we can reserve the correct
      amount of csum bytes. No functional change.
      Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
      Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      28c9b1e7
    • Omar Sandoval's avatar
      btrfs: add ram_bytes and offset to btrfs_ordered_extent · cb36a9bb
      Omar Sandoval authored
      Currently, we only create ordered extents when ram_bytes == num_bytes
      and offset == 0. However, BTRFS_IOC_ENCODED_WRITE writes may create
      extents which only refer to a subset of the full unencoded extent, so we
      need to plumb these fields through the ordered extent infrastructure and
      pass them down to insert_reserved_file_extent().
      
      Since we're changing the btrfs_add_ordered_extent* signature, let's get
      rid of the trivial wrappers and add a kernel-doc.
      Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
      Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      cb36a9bb
    • Omar Sandoval's avatar
      btrfs: don't advance offset for compressed bios in btrfs_csum_one_bio() · e331f6b1
      Omar Sandoval authored
      btrfs_csum_one_bio() loops over each filesystem block in the bio while
      keeping a cursor of its current logical position in the file in order to
      look up the ordered extent to add the checksums to. However, this
      doesn't make much sense for compressed extents, as a sector on disk does
      not correspond to a sector of decompressed file data. It happens to work
      because:
      
      1) the compressed bio always covers one ordered extent
      2) the size of the bio is always less than the size of the ordered
         extent
      
      However, the second point will not always be true for encoded writes.
      
      Let's add a boolean parameter to btrfs_csum_one_bio() to indicate that
      it can assume that the bio only covers one ordered extent. Since we're
      already changing the signature, let's get rid of the contig parameter
      and make it implied by the offset parameter, similar to the change we
      recently made to btrfs_lookup_bio_sums(). Additionally, let's rename
      nr_sectors to blockcount to make it clear that it's the number of
      filesystem blocks, not the number of 512-byte sectors.
      Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
      Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      e331f6b1
    • Omar Sandoval's avatar
      fs: export variant of generic_write_checks without iov_iter · f6f7a25a
      Omar Sandoval authored
      Encoded I/O in Btrfs needs to check a write with a given logical size
      without an iov_iter that matches that size (because the iov_iter we have
      is for the compressed data). So, factor out the parts of
      generic_write_check() that don't need an iov_iter into a new
      generic_write_checks_count() function and export that.
      Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
      Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      f6f7a25a
    • Omar Sandoval's avatar
      fs: export rw_verify_area() · 87112933
      Omar Sandoval authored
      I'm adding btrfs ioctls to read and write compressed data, and rather
      than duplicating the checks in rw_verify_area(), let's just export it.
      Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      87112933
    • Sidong Yang's avatar
      btrfs: qgroup: remove outdated TODO comments · 457b0a3d
      Sidong Yang authored
      These comments are old, outdated and not very specific. It seems that it
      doesn't help to inspire anybody to work on that.  So we remove them.
      Reviewed-by: default avatarQu Wenruo <wqu@suse.com>
      Signed-off-by: default avatarSidong Yang <realwakka@gmail.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      457b0a3d