1. 14 Mar, 2022 40 commits
    • Johannes Thumshirn's avatar
      btrfs: stop checking for NULL return from btrfs_get_extent_fiemap() · 6b5b7a41
      Johannes Thumshirn authored
      In get_extent_skip_holes() we're checking the return of
      btrfs_get_extent_fiemap() for an error pointer or NULL, but
      btrfs_get_extent_fiemap() will never return NULL, only error pointers or
      a valid extent_map.
      
      The other caller of btrfs_get_extent_fiemap(), find_desired_extent(),
      correctly only checks for error-pointers.
      Reviewed-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      6b5b7a41
    • Pankaj Raghav's avatar
      btrfs: zoned: remove redundant assignment in btrfs_check_zoned_mode · f716fa47
      Pankaj Raghav authored
      Remove the redundant assignment to zone_info variable in
      btrfs_check_zoned_mode function.
      Reviewed-by: default avatarJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Signed-off-by: default avatarPankaj Raghav <p.raghav@samsung.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      f716fa47
    • David Sterba's avatar
      btrfs: replace BUILD_BUG_ON by static_assert · a55e65b8
      David Sterba authored
      The static_assert introduced in 6bab69c6 ("build_bug.h: add wrapper
      for _Static_assert") has been supported by compilers for a long time
      (gcc 4.6, clang 3.0) and can be used in header files. We don't need to
      put BUILD_BUG_ON to random functions but rather keep it next to the
      definition.
      
      The exception here is the UAPI header btrfs_tree.h that could be
      potentially included by userspace code and the static assert is not
      defined (nor used in any other header).
      Reviewed-by: default avatarJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      a55e65b8
    • Johannes Thumshirn's avatar
      btrfs: zoned: allow DUP on meta-data block groups · 265f7237
      Johannes Thumshirn authored
      Allow creating or reading block-groups on a zoned device with DUP as a
      meta-data profile.
      
      This works because we're using the zoned_meta_io_lock and REQ_OP_WRITE
      operations for meta-data on zoned btrfs, so all writes to meta-data zones
      are aligned to the zone's write-pointer.
      
      Upon loading of the block-group, it is ensured both zones do have the same
      zone capacity and write-pointer offsets, so no extra machinery is needed
      to keep the write-pointers in sync for the meta-data zones. If this
      prerequisite is not met, loading of the block-group is refused.
      Signed-off-by: default avatarJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      265f7237
    • Johannes Thumshirn's avatar
      btrfs: zoned: prepare for allowing DUP on zoned · dbfcc18f
      Johannes Thumshirn authored
      Allow for a block-group to be placed on more than one physical zone.
      
      This is a preparation for allowing DUP profiles for meta-data on a zoned
      file-system.
      Signed-off-by: default avatarJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      dbfcc18f
    • Johannes Thumshirn's avatar
      btrfs: zoned: make zone finishing multi stripe capable · 4dcbb8ab
      Johannes Thumshirn authored
      Currently finishing of a zone only works if the block group isn't
      spanning more than one zone.
      
      This limitation is purely artificial and can be easily expanded to block
      groups being places across multiple zones.
      
      This is a preparation for allowing DUP and later more complex block-group
      profiles on zoned btrfs.
      Signed-off-by: default avatarJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      4dcbb8ab
    • Johannes Thumshirn's avatar
      btrfs: zoned: make zone activation multi stripe capable · f9a912a3
      Johannes Thumshirn authored
      Currently activation of a zone only works if the block group isn't
      spanning more than one zone.
      
      This limitation is purely artificial and can be easily expanded to block
      groups being places across multiple zones.
      
      This is a preparation for allowing DUP and later more complex block-group
      profiles on zoned btrfs.
      Signed-off-by: default avatarJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      f9a912a3
    • Josef Bacik's avatar
      btrfs: add support for multiple global roots · f7238e50
      Josef Bacik authored
      With extent tree v2 you will be able to create multiple csum, extent,
      and free space trees.  They will be used based on the block group, which
      will now use the block_group_item->chunk_objectid to point to the set of
      global roots that it will use.  When allocating new block groups we'll
      simply mod the gigabyte offset of the block group against the number of
      global roots we have and that will be the block groups global id.
      
      >From there we can take the bytenr that we're modifying in the respective
      tree, look up the block group and get that block groups corresponding
      global root id.  From there we can get to the appropriate global root
      for that bytenr.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      f7238e50
    • Josef Bacik's avatar
      btrfs: add code to support the block group root · 9c54e80d
      Josef Bacik authored
      This code adds the on disk structures for the block group root, which
      will hold the block group items for extent tree v2.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      9c54e80d
    • Josef Bacik's avatar
      btrfs: abstract out loading the tree root · bd676446
      Josef Bacik authored
      We're going to be adding more roots that need to be loaded from the
      super block, so abstract out the code to read the tree_root from the
      super block, and use this helper for the chunk root as well.  This will
      make it simpler to load the new trees in the future.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      bd676446
    • Josef Bacik's avatar
      btrfs: tree-checker: don't fail on empty extent roots for extent tree v2 · c2fa821c
      Josef Bacik authored
      For extent tree v2 we can definitely have empty extent roots, so skip
      this particular check if we have that set.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      c2fa821c
    • Josef Bacik's avatar
      btrfs: disable space cache related mount options for extent tree v2 · 63cd070d
      Josef Bacik authored
      We cannot fall back on the slow caching for extent tree v2, which means
      we can't just arbitrarily clear the free space trees at mount time.
      Furthermore we can't do v1 space cache with extent tree v2.  Simply
      ignore these mount options for extent tree v2 as they aren't relevant.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      63cd070d
    • Josef Bacik's avatar
      btrfs: disable snapshot creation/deletion for extent tree v2 · 813febdb
      Josef Bacik authored
      When we stop tracking metadata blocks all of snapshotting will break, so
      disable it until I add the snapshot root and drop tree support.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      813febdb
    • Josef Bacik's avatar
      btrfs: disable scrub for extent-tree-v2 · da32c6d5
      Josef Bacik authored
      Scrub depends on extent references for every block, and with extent tree
      v2 we won't have that, so disable scrub until we can add back the proper
      code to handle extent-tree-v2.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      da32c6d5
    • Josef Bacik's avatar
      btrfs: disable qgroups in extent tree v2 · ef3eccc1
      Josef Bacik authored
      Backref lookups are going to be drastically different with extent tree
      v2, disable qgroups until we do the work to add this support for extent
      tree v2.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      ef3eccc1
    • Josef Bacik's avatar
      btrfs: disable device manipulation ioctl's EXTENT_TREE_V2 · 914a519b
      Josef Bacik authored
      Device add, remove, and replace all require balance, which doesn't work
      right now on extent tree v2, so disable these for now.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      914a519b
    • Josef Bacik's avatar
      btrfs: disable balance for extent tree v2 for now · 4b349253
      Josef Bacik authored
      With global root id's it makes it problematic to do backref lookups for
      balance.  This isn't hard to deal with, but future changes are going to
      make it impossible to lookup backrefs on any COWonly roots, so go ahead
      and disable balance for now on extent tree v2 until we can add balance
      support back in future patches.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      4b349253
    • Josef Bacik's avatar
      btrfs: add definition for EXTENT_TREE_V2 · 2c7d2a23
      Josef Bacik authored
      This adds the initial definition of the EXTENT_TREE_V2 incompat feature
      flag.  This also hides the support behind CONFIG_BTRFS_DEBUG.
      
      THIS IS A IN DEVELOPMENT FORMAT CHANGE, DO NOT USE UNLESS YOU ARE A
      DEVELOPER OR A TESTER.
      
      The format is in flux and will be added in stages, any fs will need to
      be re-made between updates to the format.
      Signed-off-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      2c7d2a23
    • Filipe Manana's avatar
      btrfs: use single variable to track return value at btrfs_log_inode() · 65faced5
      Filipe Manana authored
      At btrfs_log_inode(), we have two variables to track errors and the
      return value of the function, named 'ret' and 'err'. In some places we
      use 'ret' and if gets a non-zero value we assign its value to 'err'
      and then jump to the 'out' label, while in other places we use 'err'
      directly without 'ret' as an intermediary. This is inconsistent, error
      prone and not necessary. So change that to use only the 'ret' variable,
      making this consistent with most functions in btrfs.
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      65faced5
    • Filipe Manana's avatar
      btrfs: avoid inode logging during rename and link when possible · 0f8ce498
      Filipe Manana authored
      During a rename or link operation, we need to determine if an inode was
      previously logged or not, and if it was, do some update to the logged
      inode. We used to rely exclusively on the logged_trans field of struct
      btrfs_inode to determine that, but that was not reliable because the
      value of that field is not persisted in the inode item, so it's lost
      when an inode is evicted and loaded back again. That led to several
      issues in the past, such as not persisting deletions (such as the case
      fixed by commit 803f0f64 ("Btrfs: fix fsync not persisting dentry
      deletions due to inode evictions")), or resulting in losing a file
      after an inode eviction followed by a rename (commit ecc64fab
      ("btrfs: fix lost inode on log replay after mix of fsync, rename and
      inode eviction")), besides other issues.
      
      So the inode_logged() helper was introduced and used to determine if an
      inode was possibly logged before in the current transaction, with the
      caveat that it could return false positives, in the sense that even if an
      inode was not logged before in the current transaction, it could still
      return true, but never to return false in case the inode was logged.
      >From a functional point of view that is fine, but from a performance
      perspective it can introduce significant latencies to rename and link
      operations, as they will end up doing inode logging even when it is not
      necessary.
      
      Recently on a 5.15 kernel, an openSUSE Tumbleweed user reported package
      installations and upgrades, with the zypper tool, were often taking a
      long time to complete. With strace it could be observed that zypper was
      spending about 99% of its time on rename operations, and then with
      further analysis we checked that directory logging was happening too
      frequently. Taking into account that installation/upgrade of some of the
      packages needed a few thousand file renames, the slowdown was very
      noticeable for the user.
      
      The issue was caused indirectly due to an excessive number of inode
      evictions on a 5.15 kernel, about 100x more compared to a 5.13, 5.14 or
      a 5.16-rc8 kernel. While triggering the inode evictions if something
      outside btrfs' control, btrfs could still behave better by eliminating
      the false positives from the inode_logged() helper.
      
      So change inode_logged() to actually eliminate such false positives caused
      by inode eviction and when an inode was never logged since the filesystem
      was mounted, as both cases relate to when the logged_trans field of struct
      btrfs_inode has a value of zero. When it can not determine if the inode
      was logged based only on the logged_trans value, lookup for the existence
      of the inode item in the log tree - if it's there then we known the inode
      was logged, if it's not there then it can not have been logged in the
      current transaction. Once we determine if the inode was logged, update
      the logged_trans value to avoid future calls to have to search in the log
      tree again.
      
      Alternatively, we could start storing logged_trans in the on disk inode
      item structure (struct btrfs_inode_item) in the unused space it still has,
      but that would be a bit odd because:
      
      1) We only care about logged_trans since the filesystem was mounted, we
         don't care about its value from a previous mount. Having it persisted
         in the inode item structure would not make the best use of the precious
         unused space;
      
      2) In order to get logged_trans persisted before inode eviction, we would
         have to update the delayed inode when we finish logging the inode and
         update its logged_trans in struct btrfs_inode, which makes it a bit
         cumbersome since we need to check if the delayed inode exists, if not
         create it and populate it and deal with any errors (-ENOMEM mostly).
      
      This change is part of a patchset comprised of the following patches:
      
        1/5 btrfs: add helper to delete a dir entry from a log tree
        2/5 btrfs: pass the dentry to btrfs_log_new_name() instead of the inode
        3/5 btrfs: avoid logging all directory changes during renames
        4/5 btrfs: stop doing unnecessary log updates during a rename
        5/5 btrfs: avoid inode logging during rename and link when possible
      
      The following test script mimics part of what the zypper tool does during
      package installations/upgrades. It does not triggers inode evictions, but
      it's similar because it triggers false positives from the inode_logged()
      helper, because the inodes have a logged_trans of 0, there's a log tree
      due to a fsync of an unrelated file and the directory inode has its
      last_trans field set to the current transaction:
      
        $ cat test.sh
      
        #!/bin/bash
      
        DEV=/dev/nvme0n1
        MNT=/mnt/nvme0n1
      
        NUM_FILES=10000
      
        mkfs.btrfs -f $DEV
        mount $DEV $MNT
      
        mkdir $MNT/testdir
      
        for ((i = 1; i <= $NUM_FILES; i++)); do
            echo -n > $MNT/testdir/file_$i
        done
      
        sync
      
        # Now do some change to an unrelated file and fsync it.
        # This is just to create a log tree to make sure that inode_logged()
        # does not return false when called against "testdir".
        xfs_io -f -c "pwrite 0 4K" -c "fsync" $MNT/foo
      
        # Do some change to testdir. This is to make sure inode_logged()
        # will return true when called against "testdir", because its
        # logged_trans is 0, it was changed in the current transaction
        # and there's a log tree.
        echo -n > $MNT/testdir/file_$((NUM_FILES + 1))
      
        echo "Renaming $NUM_FILES files..."
        start=$(date +%s%N)
        for ((i = 1; i <= $NUM_FILES; i++)); do
            mv $MNT/testdir/file_$i $MNT/testdir/file_$i-RPMDELETE
        done
        end=$(date +%s%N)
      
        dur=$(( (end - start) / 1000000 ))
        echo "Renames took $dur milliseconds"
      
        umount $MNT
      
      Testing this change on a box using a non-debug kernel (Debian's default
      kernel config) gave the following results:
      
      NUM_FILES=10000, before patchset:                   27837 ms
      NUM_FILES=10000, after patches 1/5 to 4/5 applied:   9236 ms (-66.8%)
      NUM_FILES=10000, after whole patchset applied:       8902 ms (-68.0%)
      
      NUM_FILES=5000, before patchset:                     9127 ms
      NUM_FILES=5000, after patches 1/5 to 4/5 applied:    4640 ms (-49.2%)
      NUM_FILES=5000, after whole patchset applied:        4441 ms (-51.3%)
      
      NUM_FILES=2000, before patchset:                     2528 ms
      NUM_FILES=2000, after patches 1/5 to 4/5 applied:    1983 ms (-21.6%)
      NUM_FILES=2000, after whole patchset applied:        1747 ms (-30.9%)
      
      NUM_FILES=1000, before patchset:                     1085 ms
      NUM_FILES=1000, after patches 1/5 to 4/5 applied:     893 ms (-17.7%)
      NUM_FILES=1000, after whole patchset applied:         867 ms (-20.1%)
      
      Running dbench on the same physical machine with the following script:
      
        $ cat run-dbench.sh
        #!/bin/bash
      
        NUM_JOBS=$(nproc --all)
      
        DEV=/dev/nvme0n1
        MNT=/mnt/nvme0n1
        MOUNT_OPTIONS="-o ssd"
        MKFS_OPTIONS="-O no-holes -R free-space-tree"
      
        echo "performance" | \
            tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
      
        mkfs.btrfs -f $MKFS_OPTIONS $DEV
        mount $MOUNT_OPTIONS $DEV $MNT
      
        dbench -D $MNT -t 120 $NUM_JOBS
      
        umount $MNT
      
      Before patchset:
      
       Operation      Count    AvgLat    MaxLat
       ----------------------------------------
       NTCreateX    3761352     0.032   143.843
       Close        2762770     0.002     2.273
       Rename        159304     0.291    67.037
       Unlink        759784     0.207   143.998
       Deltree           72     4.028    15.977
       Mkdir             36     0.003     0.006
       Qpathinfo    3409780     0.013     9.678
       Qfileinfo     596772     0.001     0.878
       Qfsinfo       625189     0.003     1.245
       Sfileinfo     306443     0.006     1.840
       Find         13181061     0.063    19.798
       WriteX       1871137     0.021     8.532
       ReadX        5897325     0.003     3.567
       LockX          12252     0.003     0.258
       UnlockX        12252     0.002     0.100
       Flush         263666     3.327   155.632
      
      Throughput 980.047 MB/sec  12 clients  12 procs  max_latency=155.636 ms
      
      After whole patchset applied:
      
       Operation      Count    AvgLat    MaxLat
       ----------------------------------------
       NTCreateX    4195584     0.033   107.742
       Close        3081932     0.002     1.935
       Rename        177641     0.218    14.905
       Unlink        847333     0.166   107.822
       Deltree          118     5.315    15.247
       Mkdir             59     0.004     0.048
       Qpathinfo    3802612     0.014    10.302
       Qfileinfo     666748     0.001     1.034
       Qfsinfo       697329     0.003     0.944
       Sfileinfo     341712     0.006     2.099
       Find         1470365     0.065     9.359
       WriteX       2093921     0.021     8.087
       ReadX        6576234     0.003     3.407
       LockX          13660     0.003     0.308
       UnlockX        13660     0.002     0.114
       Flush         294090     2.906   115.539
      
      Throughput 1093.11 MB/sec  12 clients  12 procs  max_latency=115.544 ms
      
      +11.5% throughput    -25.8% max latency   rename max latency -77.8%
      
      Link: https://bugzilla.opensuse.org/show_bug.cgi?id=1193549Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      0f8ce498
    • Filipe Manana's avatar
      btrfs: stop doing unnecessary log updates during a rename · 259c4b96
      Filipe Manana authored
      During a rename, we call __btrfs_unlink_inode(), which will call
      btrfs_del_inode_ref_in_log() and btrfs_del_dir_entries_in_log(), in order
      to remove an inode reference and a directory entry from the log. These
      are necessary when __btrfs_unlink_inode() is called from the unlink path,
      but not necessary when it's called from a rename context, because:
      
      1) For the btrfs_del_inode_ref_in_log() call, it's pointless to delete the
         inode reference related to the old name, because later in the rename
         path we call btrfs_log_new_name(), which will drop all inode references
         from the log and copy all inode references from the subvolume tree to
         the log tree. So we are doing one unnecessary btree operation which
         adds additional latency and lock contention in case there are other
         tasks accessing the log tree;
      
      2) For the btrfs_del_dir_entries_in_log() call, we are now doing the
         equivalent at btrfs_log_new_name() since the previous patch in the
         series, that has the subject "btrfs: avoid logging all directory
         changes during renames". In fact, having __btrfs_unlink_inode() call
         this function not only adds additional latency and lock contention due
         to the extra btree operation, but also can make btrfs_log_new_name()
         unnecessarily log a range item to track the deletion of the old name,
         since it has no way to known that the directory entry related to the
         old name was previously logged and already deleted by
         __btrfs_unlink_inode() through its call to
         btrfs_del_dir_entries_in_log().
      
      So skip those calls at __btrfs_unlink_inode() when we are doing a rename.
      Skipping them also allows us now to reduce the duration of time we are
      pinning a log transaction during renames, which is always beneficial as
      it's not delaying so much other tasks trying to sync the log tree, in
      particular we end up not holding the log transaction pinned while adding
      the new name (adding inode ref, directory entry, etc).
      
      This change is part of a patchset comprised of the following patches:
      
        1/5 btrfs: add helper to delete a dir entry from a log tree
        2/5 btrfs: pass the dentry to btrfs_log_new_name() instead of the inode
        3/5 btrfs: avoid logging all directory changes during renames
        4/5 btrfs: stop doing unnecessary log updates during a rename
        5/5 btrfs: avoid inode logging during rename and link when possible
      
      Just like the previous patch in the series, "btrfs: avoid logging all
      directory changes during renames", the following script mimics part of
      what a package installation/upgrade with zypper does, which is basically
      renaming a lot of files, in some directory under /usr, to a name with a
      suffix of "-RPMDELETE":
      
        $ cat test.sh
        #!/bin/bash
      
        DEV=/dev/nvme0n1
        MNT=/mnt/nvme0n1
      
        NUM_FILES=10000
      
        mkfs.btrfs -f $DEV
        mount $DEV $MNT
      
        mkdir $MNT/testdir
      
        for ((i = 1; i <= $NUM_FILES; i++)); do
            echo -n > $MNT/testdir/file_$i
        done
      
        sync
      
        # Do some change to testdir and fsync it.
        echo -n > $MNT/testdir/file_$((NUM_FILES + 1))
        xfs_io -c "fsync" $MNT/testdir
      
        echo "Renaming $NUM_FILES files..."
        start=$(date +%s%N)
        for ((i = 1; i <= $NUM_FILES; i++)); do
            mv $MNT/testdir/file_$i $MNT/testdir/file_$i-RPMDELETE
        done
        end=$(date +%s%N)
      
        dur=$(( (end - start) / 1000000 ))
        echo "Renames took $dur milliseconds"
      
        umount $MNT
      
      Testing this change on box a using a non-debug kernel (Debian's default
      kernel config) gave the following results:
      
      NUM_FILES=10000, before patchset:                   27399 ms
      NUM_FILES=10000, after patches 1/5 to 3/5 applied:   9093 ms (-66.8%)
      NUM_FILES=10000, after patches 1/5 to 4/5 applied:   9016 ms (-67.1%)
      
      NUM_FILES=5000, before patchset:                     9241 ms
      NUM_FILES=5000, after patches 1/5 to 3/5 applied:    4642 ms (-49.8%)
      NUM_FILES=5000, after patches 1/5 to 4/5 applied:    4553 ms (-50.7%)
      
      NUM_FILES=2000, before patchset:                     2550 ms
      NUM_FILES=2000, after patches 1/5 to 3/5 applied:    1788 ms (-29.9%)
      NUM_FILES=2000, after patches 1/5 to 4/5 applied:    1767 ms (-30.7%)
      
      NUM_FILES=1000, before patchset:                     1088 ms
      NUM_FILES=1000, after patches 1/5 to 3/5 applied:     905 ms (-16.9%)
      NUM_FILES=1000, after patches 1/5 to 4/5 applied:     883 ms (-18.8%)
      
      The next patch in the series (5/5), also contains dbench results after
      applying to whole patchset.
      
      Link: https://bugzilla.opensuse.org/show_bug.cgi?id=1193549Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      259c4b96
    • Filipe Manana's avatar
      btrfs: avoid logging all directory changes during renames · 88d2beec
      Filipe Manana authored
      When doing a rename of a file, if the file or its old parent directory
      were logged before, we log the new name of the file and then make sure
      we log the old parent directory, to ensure that after a log replay the
      old name of the file is deleted and the new name added.
      
      The logging of the old parent directory can take some time, because it
      will scan all leaves modified in the current transaction, check which
      directory entries were already logged, copy the ones that were not
      logged before, etc. In this rename context all we need to do is make
      sure that the old name of the file is deleted on log replay, so instead
      of triggering a directory log operation, we can just delete the old
      directory entry from the log if it's there, or in case it isn't there,
      just log a range item to signal log replay that the old name must be
      deleted. So change btrfs_log_new_name() to do that.
      
      This scenario is actually not uncommon to trigger, and recently on a
      5.15 kernel, an openSUSE Tumbleweed user reported package installations
      and upgrades, with the zypper tool, were often taking a long time to
      complete, much more than usual. With strace it could be observed that
      zypper was spending over 99% of its time on rename operations, and then
      with further analysis we checked that directory logging was happening
      too frequently and causing high latencies for the rename operations.
      Taking into account that installation/upgrade of some of these packages
      needed about a few thousand file renames, the slowdown was very noticeable
      for the user.
      
      The issue was caused indirectly due to an excessive number of inode
      evictions on a 5.15 kernel, about 100x more compared to a 5.13, 5.14
      or a 5.16-rc8 kernel. After an inode eviction we can't tell for sure,
      in an efficient way, if an inode was previously logged in the current
      transaction, so we are pessimistic and assume it was, because in case
      it was we need to update the logged inode. More details on that in one
      of the patches in the same series (subject "btrfs: avoid inode logging
      during rename and link when possible"). Either way, in case the parent
      directory was logged before, we currently do more work then necessary
      during a rename, and this change minimizes that amount of work.
      
      The following script mimics part of what a package installation/upgrade
      with zypper does, which is basically renaming a lot of files, in some
      directory under /usr, to a name with a suffix of "-RPMDELETE":
      
        $ cat test.sh
        #!/bin/bash
      
        DEV=/dev/nvme0n1
        MNT=/mnt/nvme0n1
      
        NUM_FILES=10000
      
        mkfs.btrfs -f $DEV
        mount $DEV $MNT
      
        mkdir $MNT/testdir
      
        for ((i = 1; i <= $NUM_FILES; i++)); do
            echo -n > $MNT/testdir/file_$i
        done
      
        sync
      
        # Do some change to testdir and fsync it.
        echo -n > $MNT/testdir/file_$((NUM_FILES + 1))
        xfs_io -c "fsync" $MNT/testdir
      
        echo "Renaming $NUM_FILES files..."
        start=$(date +%s%N)
        for ((i = 1; i <= $NUM_FILES; i++)); do
            mv $MNT/testdir/file_$i $MNT/testdir/file_$i-RPMDELETE
        done
        end=$(date +%s%N)
      
        dur=$(( (end - start) / 1000000 ))
        echo "Renames took $dur milliseconds"
      
        umount $MNT
      
      Testing this change on box using a non-debug kernel (Debian's default
      kernel config) gave the following results:
      
      NUM_FILES=10000, before this patch: 27399 ms
      NUM_FILES=10000, after this patch:   9093 ms (-66.8%)
      
      NUM_FILES=5000, before this patch:   9241 ms
      NUM_FILES=5000, after this patch:    4642 ms (-49.8%)
      
      NUM_FILES=2000, before this patch:   2550 ms
      NUM_FILES=2000, after this patch:    1788 ms (-29.9%)
      
      NUM_FILES=1000, before this patch:   1088 ms
      NUM_FILES=1000, after this patch:     905 ms (-16.9%)
      
      Link: https://bugzilla.opensuse.org/show_bug.cgi?id=1193549Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      88d2beec
    • Filipe Manana's avatar
      btrfs: pass the dentry to btrfs_log_new_name() instead of the inode · d5f5bd54
      Filipe Manana authored
      In the next patch in the series, there will be the need to access the old
      name, and its length, of an inode when logging the inode during a rename.
      So instead of passing the inode to btrfs_log_new_name() pass the dentry,
      because from the dentry we can get the inode, the name and its length.
      
      This will avoid passing 3 new parameters to btrfs_log_new_name() in the
      next patch - the name, its length and an index number. This way we end
      up passing only 1 new parameter, the index number.
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      d5f5bd54
    • Filipe Manana's avatar
      btrfs: add helper to delete a dir entry from a log tree · 839061fe
      Filipe Manana authored
      Move the code that finds and deletes a logged dir entry out of
      btrfs_del_dir_entries_in_log() into a helper function. This new helper
      function will be used by another patch in the same series, and serves
      to avoid having duplicated logic.
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      839061fe
    • Minghao Chi's avatar
      btrfs: send: remove redundant ret variable in fs_path_copy · 0292ecf1
      Minghao Chi authored
      Return value from fs_path_add_path() directly instead of taking this in
      another redundant variable.
      Reported-by: default avatarZeal Robot <zealci@zte.com.cn>
      Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
      Signed-off-by: default avatarMinghao Chi <chi.minghao@zte.com.cn>
      Signed-off-by: default avatarCGEL ZTE <cgel.zte@gmail.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      0292ecf1
    • Nikolay Borisov's avatar
      btrfs: move QUOTA_ENABLED check to rescan_should_stop from btrfs_qgroup_rescan_worker · db5df254
      Nikolay Borisov authored
      Instead of having 2 places that short circuit the qgroup leaf scan have
      everything in the qgroup_rescan_leaf function. In addition to that, also
      ensure that the inconsistent qgroup flag is set when rescan_should_stop
      returns true. This both retains the old behavior when -EINTR was set in
      the body of the loop and at the same time also extends this behavior
      when scanning is interrupted due to remount or unmount operations.
      Signed-off-by: default avatarNikolay Borisov <nborisov@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      db5df254
    • Jiapeng Chong's avatar
      btrfs: scrub: remove redundant initialization of increment · 5c07c53f
      Jiapeng Chong authored
      increment is being initialized to map->stripe_len but this is never
      read as increment is overwritten later on. Remove the redundant
      initialization.
      
      Cleans up the following clang-analyzer warning:
      
      fs/btrfs/scrub.c:3193:6: warning: Value stored to 'increment' during its
      initialization is never read [clang-analyzer-deadcode.DeadStores].
      Reported-by: default avatarAbaci Robot <abaci@linux.alibaba.com>
      Signed-off-by: default avatarJiapeng Chong <jiapeng.chong@linux.alibaba.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      5c07c53f
    • Jiapeng Chong's avatar
      btrfs: zoned: remove redundant initialization of to_add · c4bf1909
      Jiapeng Chong authored
      to_add is being initialized to len but this is never read as to_add is
      overwritten later on. Remove the redundant initialization.
      
      Cleans up the following clang-analyzer warning:
      
      fs/btrfs/extent-tree.c:2769:8: warning: Value stored to 'to_add' during
      its initialization is never read [clang-analyzer-deadcode.DeadStores].
      Reported-by: default avatarAbaci Robot <abaci@linux.alibaba.com>
      Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
      Signed-off-by: default avatarJiapeng Chong <jiapeng.chong@linux.alibaba.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      c4bf1909
    • Anand Jain's avatar
      btrfs: cleanup temporary variables when finding rotational device status · 823f8e5c
      Anand Jain authored
      The pointer to struct request_queue is used only to get device type
      rotating or the non-rotating. So use it directly.
      Signed-off-by: default avatarAnand Jain <anand.jain@oracle.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      823f8e5c
    • Anand Jain's avatar
      btrfs: use dev_t to match device in device_matched · 330a5bf4
      Anand Jain authored
      Commit "btrfs: add device major-minor info in the struct btrfs_device"
      saved the device major-minor number in the struct btrfs_device upon
      discovering it.
      
      So no need to lookup_bdev() again just match, which means
      device_matched() can go away.
      Signed-off-by: default avatarAnand Jain <anand.jain@oracle.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      330a5bf4
    • Anand Jain's avatar
      btrfs: add device major-minor info in the struct btrfs_device · 4889bc05
      Anand Jain authored
      Internally it is common to use the major-minor number to identify a
      device and, at a few locations in btrfs, we use the major-minor number
      to match the device.
      
      So when we identify a new btrfs device through device add or device
      replace or device-scan/ready save the device's major-minor (dev_t) in the
      struct btrfs_device so that we don't have to call lookup_bdev() again.
      Signed-off-by: default avatarAnand Jain <anand.jain@oracle.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      4889bc05
    • Anand Jain's avatar
      btrfs: match stale devices by dev_t · 16cab91a
      Anand Jain authored
      After the commit "btrfs: harden identification of the stale device", we
      don't have to match the device path anymore. Instead, we match the dev_t.
      So pass in the dev_t instead of the device path, in the call chain
      btrfs_forget_devices()->btrfs_free_stale_devices().
      Signed-off-by: default avatarAnand Jain <anand.jain@oracle.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      16cab91a
    • Anand Jain's avatar
      btrfs: harden identification of a stale device · 770c79fb
      Anand Jain authored
      Identifying and removing the stale device from the fs_uuids list is done
      by btrfs_free_stale_devices().  btrfs_free_stale_devices() in turn
      depends on device_path_matched() to check if the device appears in more
      than one btrfs_device structure.
      
      The matching of the device happens by its path, the device path. However,
      when device mapper is in use, the dm device paths are nothing but a link
      to the actual block device, which leads to the device_path_matched()
      failing to match.
      
      Fix this by matching the dev_t as provided by lookup_bdev() instead of
      plain string compare of the device paths.
      Reported-by: default avatarJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: default avatarAnand Jain <anand.jain@oracle.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      770c79fb
    • Anand Jain's avatar
      btrfs: simplify fs_devices member access in btrfs_init_dev_replace_tgtdev · bef16b52
      Anand Jain authored
      In btrfs_init_dev_replace_tgtdev() we dereference fs_info to get
      fs_devices many times, instead save a point to the fs_devices.
      Signed-off-by: default avatarAnand Jain <anand.jain@oracle.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      bef16b52
    • Sahil Kang's avatar
      btrfs: reuse existing inode from btrfs_ioctl · 9ad12305
      Sahil Kang authored
      btrfs_ioctl extracts inode from file so we can pass that into the
      callbacks.
      Signed-off-by: default avatarSahil Kang <sahil.kang@asilaycomputing.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      9ad12305
    • Nikolay Borisov's avatar
      btrfs: move missing device handling in a dedicate function · ff37c89f
      Nikolay Borisov authored
      This simplifies the code flow in read_one_chunk and makes error handling
      when handling missing devices a bit simpler by reducing it to a single
      check if something went wrong. No functional changes.
      Reviewed-by: default avatarSu Yue <l@damenly.su>
      Signed-off-by: default avatarNikolay Borisov <nborisov@suse.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      ff37c89f
    • Filipe Manana's avatar
      btrfs: stop trying to log subdirectories created in past transactions · de6bc7f5
      Filipe Manana authored
      When logging a directory we are trying to log subdirectories that were
      changed in the current transaction and created in a past transaction.
      This type of behaviour was introduced by commit 2f2ff0ee ("Btrfs:
      fix metadata inconsistencies after directory fsync"), to fix some metadata
      inconsistencies that in the meanwhile no longer need this behaviour due to
      numerous other changes that happened throughout the years.
      
      This behaviour, besides not needed anymore, it's also undesirable because:
      
      1) It's not reliable because it's only triggered for the directories
         of dentries (dir items) that happen to be present on a leaf that
         was changed in the current transaction. If a dentry that points to
         a directory resides on a leaf that was not changed in the current
         transaction, then it's not logged, as at log_dir_items() and
         log_new_dir_dentries() we use btrfs_search_forward();
      
      2) It's not required by posix or any standard, it's undefined territory.
         The only way to guarantee a subdirectory is logged, it to explicitly
         fsync it;
      
      Making the behaviour guaranteed would require scanning all directory
      items, check which point to a directory, and then fsync each subdirectory
      which was modified in the current transaction. This could be very
      expensive for large directories with many subdirectories and/or large
      subdirectories.
      
      So remove that obsolete logic.
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      de6bc7f5
    • Filipe Manana's avatar
      btrfs: stop copying old dir items when logging a directory · 732d591a
      Filipe Manana authored
      When logging a directory, we go over every leaf of the subvolume tree that
      was changed in the current transaction and copy all its dir index keys to
      the log tree.
      
      That includes copying dir index keys created in past transactions. This is
      done mostly for simplicity, as after logging the keys we log an item that
      specifies the start and end ranges of the keys we logged. That item is
      then used during log replay to figure out which keys need to be deleted -
      every key in that range that we find in the subvolume tree and is not in
      the log tree, needs to be deleted.
      
      Now that we log only dir index keys, and not dir item keys anymore, when
      we remove dentries from a directory (due to unlink and rename operations),
      we can get entire leaves that we changed only for deleting old dir index
      keys, or that have few dir index keys that are new - this is due to the
      fact that the offset for new index keys comes from a monotonically
      increasing counter.
      
      We can avoid logging dir index keys from past transactions, and in order
      to track the deletions, only log range items (BTRFS_DIR_LOG_INDEX_KEY key
      type) when we find gaps between consecutive index keys. This massively
      reduces the amount of logged metadata when we have deleted directory
      entries, even if it's a small percentage of the total number of entries.
      The reduction comes from both less items that are logged and instead of
      logging many dir index items (struct btrfs_dir_item), which have a size
      of 30 bytes plus a file name, we typically log just a few range items
      (struct btrfs_dir_log_item), which take only 8 bytes each.
      
      Even if no entries were deleted from a directory and only new entries
      were added, we typically still get a reduction on the amount of logged
      metadata, because it's very likely the first leaf that got the new
      dir index entries also has several old dir index entries.
      
      So change the logging logic to not log dir index keys created in past
      transactions and log a range item for every gap it finds between each
      pair of consecutive index keys, to ensure deletions are tracked and
      replayed on log replay.
      
      This patch is part of a patchset comprised of the following patches:
      
       1/4 btrfs: don't log unnecessary boundary keys when logging directory
       2/4 btrfs: put initial index value of a directory in a constant
       3/4 btrfs: stop copying old dir items when logging a directory
       4/4 btrfs: stop trying to log subdirectories created in past transactions
      
      The following test was run on a branch without this patchset and on a
      branch with the first three patches applied:
      
        $ cat test.sh
        #!/bin/bash
      
        DEV=/dev/nvme0n1
        MNT=/mnt/nvme0n1
      
        NUM_FILES=1000000
        NUM_FILE_DELETES=10000
      
        MKFS_OPTIONS="-O no-holes -R free-space-tree"
        MOUNT_OPTIONS="-o ssd"
      
        mkfs.btrfs -f $MKFS_OPTIONS $DEV
        mount $MOUNT_OPTIONS $DEV $MNT
      
        mkdir $MNT/testdir
        for ((i = 1; i <= $NUM_FILES; i++)); do
            echo -n > $MNT/testdir/file_$i
        done
      
        sync
      
        del_inc=$(( $NUM_FILES / $NUM_FILE_DELETES ))
        for ((i = 1; i <= $NUM_FILES; i += $del_inc)); do
            rm -f $MNT/testdir/file_$i
        done
      
        start=$(date +%s%N)
        xfs_io -c "fsync" $MNT/testdir
        end=$(date +%s%N)
      
        dur=$(( (end - start) / 1000000 ))
        echo "dir fsync took $dur ms after deleting $NUM_FILE_DELETES files"
        echo
      
        umount $MNT
      
      The test was run on a non-debug kernel (Debian's default kernel config),
      and the results were the following for various values of NUM_FILES and
      NUM_FILE_DELETES:
      
      ** before, NUM_FILES = 1 000 000, NUM_FILE_DELETES = 10 000 **
      
      dir fsync took 585 ms after deleting 10000 files
      
      ** after, NUM_FILES = 1 000 000, NUM_FILE_DELETES = 10 000 **
      
      dir fsync took 34 ms after deleting 10000 files   (-94.2%)
      
      ** before, NUM_FILES = 100 000, NUM_FILE_DELETES = 1 000 **
      
      dir fsync took 50 ms after deleting 1000 files
      
      ** after, NUM_FILES = 100 000, NUM_FILE_DELETES = 1 000 **
      
      dir fsync took 7 ms after deleting 1000 files    (-86.0%)
      
      ** before, NUM_FILES = 10 000, NUM_FILE_DELETES = 100 **
      
      dir fsync took 9 ms after deleting 100 files
      
      ** after, NUM_FILES = 10 000, NUM_FILE_DELETES = 100 **
      
      dir fsync took 5 ms after deleting 100 files     (-44.4%)
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      732d591a
    • Filipe Manana's avatar
      btrfs: put initial index value of a directory in a constant · 528ee697
      Filipe Manana authored
      At btrfs_set_inode_index_count() we refer twice to the number 2 as the
      initial index value for a directory (when it's empty), with a proper
      comment explaining the reason for that value. In the next patch I'll
      have to use that magic value in the directory logging code, so put
      the value in a #define at btrfs_inode.h, to avoid hardcoding the
      magic value again at tree-log.c.
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      528ee697
    • Filipe Manana's avatar
      btrfs: don't log unnecessary boundary keys when logging directory · a450a4af
      Filipe Manana authored
      Before we start to log dir index keys from a leaf, we check if there is a
      previous index key, which normally is at the end of a leaf that was not
      changed in the current transaction. Then we log that key and set the start
      of logged range (item of type BTRFS_DIR_LOG_INDEX_KEY) to the offset of
      that key. This is to ensure that if there were deleted index keys between
      that key and the first key we are going to log, those deletions are
      replayed in case we need to replay to the log after a power failure.
      However we really don't need to log that previous key, we can just set the
      start of the logged range to that key's offset plus 1. This achieves the
      same and avoids logging one dir index key.
      
      The same logic is performed when we finish logging the index keys of a
      leaf and we find that the next leaf has index keys and was not changed in
      the current transaction. We are logging the first key of that next leaf
      and use its offset as the end of range we log. This is just to ensure that
      if there were deleted index keys between the last index key we logged and
      the first key of that next leaf, those index keys are deleted if we end
      up replaying the log. However that is not necessary, we can avoid logging
      that first index key of the next leaf and instead set the end of the
      logged range to match the offset of that index key minus 1.
      
      So avoid logging those index keys at the boundaries and adjust the start
      and end offsets of the logged ranges as described above.
      
      This patch is part of a patchset comprised of the following patches:
      
        1/4 btrfs: don't log unnecessary boundary keys when logging directory
        2/4 btrfs: put initial index value of a directory in a constant
        3/4 btrfs: stop copying old dir items when logging a directory
        4/4 btrfs: stop trying to log subdirectories created in past transactions
      
      Performance test results are listed in the changelog of patch 3/4.
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      a450a4af