1. 27 10月, 2021 4 次提交
    • J
      btrfs: add a BTRFS_FS_ERROR helper · 84961539
      Josef Bacik 提交于
      We have a few flags that are inconsistently used to describe the fs in
      different states of failure.  As of 5963ffca ("btrfs: always abort
      the transaction if we abort a trans handle") we will always set
      BTRFS_FS_STATE_ERROR if we abort, so we don't have to check both ABORTED
      and ERROR to see if things have gone wrong.  Add a helper to check
      BTRFS_FS_STATE_ERROR and then convert all checkers of FS_STATE_ERROR to
      use the helper.
      
      The TRANS_ABORTED bit check was added in af722733 ("Btrfs: clean up
      resources during umount after trans is aborted") but is not actually
      specific.
      Reviewed-by: NAnand Jain <anand.jain@oracle.com>
      Reviewed-by: NNikolay Borisov <nborisov@suse.com>
      Signed-off-by: NJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      84961539
    • Q
      btrfs: subpage: add bitmap for PageChecked flag · e4f94347
      Qu Wenruo 提交于
      Although in btrfs we have very limited usage of PageChecked flag, it's
      still some page flag not yet subpage compatible.
      
      Fix it by introducing btrfs_subpage::checked_offset to do the convert.
      
      For most call sites, especially for free-space cache, COW fixup and
      btrfs_invalidatepage(), they all work in full page mode anyway.
      
      For other call sites, they work as subpage compatible mode.
      
      Some call sites need extra modification:
      
      - btrfs_drop_pages()
        Needs extra parameter to get the real range we need to clear checked
        flag.
      
        Also since btrfs_drop_pages() will accept pages beyond the dirtied
        range, update btrfs_subpage_clamp_range() to handle such case
        by setting @len to 0 if the page is beyond target range.
      
      - btrfs_invalidatepage()
        We need to call subpage helper before calling __btrfs_releasepage(),
        or it will trigger ASSERT() as page->private will be cleared.
      
      - btrfs_verify_data_csum()
        In theory we don't need the io_bio->csum check anymore, but it's
        won't hurt.  Just change the comment.
      Signed-off-by: NQu Wenruo <wqu@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      e4f94347
    • F
      btrfs: unexport setup_items_for_insert() · f0641656
      Filipe Manana 提交于
      Since setup_items_for_insert() is not used anymore outside of ctree.c,
      make it static and remove its prototype from ctree.h. This also requires
      to move the definition of setup_item_for_insert() from ctree.h to ctree.c
      and move down btrfs_duplicate_item() so that it's defined after
      setup_items_for_insert().
      
      Further, since setup_item_for_insert() is used outside ctree.c, rename it
      to btrfs_setup_item_for_insert().
      
      This patch is part of a small patchset that is comprised of the following
      patches:
      
        btrfs: loop only once over data sizes array when inserting an item batch
        btrfs: unexport setup_items_for_insert()
        btrfs: use single bulk copy operations when logging directories
      
      This is patch 2/3 and performance results, and the specific tests, are
      included in the changelog of patch 3/3.
      Signed-off-by: NFilipe Manana <fdmanana@suse.com>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      f0641656
    • F
      btrfs: loop only once over data sizes array when inserting an item batch · b7ef5f3a
      Filipe Manana 提交于
      When inserting a batch of items into a btree, we end up looping over the
      data sizes array 3 times:
      
      1) Once in the caller of btrfs_insert_empty_items(), when it populates the
         array with the data sizes for each item;
      
      2) Once at btrfs_insert_empty_items() to sum the elements of the data
         sizes array and compute the total data size;
      
      3) And then once again at setup_items_for_insert(), where we do exactly
         the same as what we do at btrfs_insert_empty_items(), to compute the
         total data size.
      
      That is not bad for small arrays, but when the arrays have hundreds of
      elements, the time spent on looping is not negligible. For example when
      doing batch inserts of delayed items for dir index items or when logging
      a directory, it's common to have 200 to 260 dir index items in a single
      batch when using a leaf size of 16K and using file names between 8 and 12
      characters. For a 64K leaf size, multiply that by 4. Taking into account
      that during directory logging or when flushing delayed dir index items we
      can have many of those large batches, the time spent on the looping adds
      up quickly.
      
      It's also more important to avoid it at setup_items_for_insert(), since
      we are holding a write lock on a leaf and, in some cases, on upper nodes
      of the btree, which causes us to block other tasks that want to access
      the leaf and nodes for longer than necessary.
      
      So change the code so that setup_items_for_insert() and
      btrfs_insert_empty_items() no longer compute the total data size, and
      instead rely on the caller to supply it. This makes us loop over the
      array only once, where we can both populate the data size array and
      compute the total data size, taking advantage of spatial and temporal
      locality. To make this more manageable, use a structure to contain
      all the relevant details for a batch of items (keys array, data sizes
      array, total data size, number of items), and use it as an argument
      for btrfs_insert_empty_items() and setup_items_for_insert().
      
      This patch is part of a small patchset that is comprised of the following
      patches:
      
        btrfs: loop only once over data sizes array when inserting an item batch
        btrfs: unexport setup_items_for_insert()
        btrfs: use single bulk copy operations when logging directories
      
      This is patch 1/3 and performance results, and the specific tests, are
      included in the changelog of patch 3/3.
      Signed-off-by: NFilipe Manana <fdmanana@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      b7ef5f3a
  2. 08 10月, 2021 2 次提交
    • J
      btrfs: fix abort logic in btrfs_replace_file_extents · 4afb912f
      Josef Bacik 提交于
      Error injection testing uncovered a case where we'd end up with a
      corrupt file system with a missing extent in the middle of a file.  This
      occurs because the if statement to decide if we should abort is wrong.
      
      The only way we would abort in this case is if we got a ret !=
      -EOPNOTSUPP and we called from the file clone code.  However the
      prealloc code uses this path too.  Instead we need to abort if there is
      an error, and the only error we _don't_ abort on is -EOPNOTSUPP and only
      if we came from the clone file code.
      
      CC: stable@vger.kernel.org # 5.10+
      Reviewed-by: NNikolay Borisov <nborisov@suse.com>
      Reviewed-by: NFilipe Manana <fdmanana@suse.com>
      Signed-off-by: NJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      4afb912f
    • J
      btrfs: update refs for any root except tree log roots · d175209b
      Josef Bacik 提交于
      I hit a stuck relocation on btrfs/061 during my overnight testing.  This
      turned out to be because we had left over extent entries in our extent
      root for a data reloc inode that no longer existed.  This happened
      because in btrfs_drop_extents() we only update refs if we have SHAREABLE
      set or we are the tree_root.  This regression was introduced by
      aeb935a4 ("btrfs: don't set SHAREABLE flag for data reloc tree")
      where we stopped setting SHAREABLE for the data reloc tree.
      
      The problem here is we actually do want to update extent references for
      data extents in the data reloc tree, in fact we only don't want to
      update extent references if the file extents are in the log tree.
      Update this check to only skip updating references in the case of the
      log tree.
      
      This is relatively rare, because you have to be running scrub at the
      same time, which is what btrfs/061 does.  The data reloc inode has its
      extents pre-allocated, and then we copy the extent into the
      pre-allocated chunks.  We theoretically should never be calling
      btrfs_drop_extents() on a data reloc inode.  The exception of course is
      with scrub, if our pre-allocated extent falls inside of the block group
      we are scrubbing, then the block group will be marked read only and we
      will be forced to cow that extent.  This means we will call
      btrfs_drop_extents() on that range when we COW that file extent.
      
      This isn't really problematic if we do this, the data reloc inode
      requires that our extent lengths match exactly with the extent we are
      copying, thankfully we validate the extent is correct with
      get_new_location(), so if we happen to COW only part of the extent we
      won't link it in when we do the relocation, so we are safe from any
      other shenanigans that arise because of this interaction with scrub.
      
      Fixes: aeb935a4 ("btrfs: don't set SHAREABLE flag for data reloc tree")
      CC: stable@vger.kernel.org # 5.8+
      Reviewed-by: NQu Wenruo <wqu@suse.com>
      Signed-off-by: NJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      d175209b
  3. 23 8月, 2021 3 次提交
    • B
      btrfs: initial fsverity support · 14605409
      Boris Burkov 提交于
      Add support for fsverity in btrfs. To support the generic interface in
      fs/verity, we add two new item types in the fs tree for inodes with
      verity enabled. One stores the per-file verity descriptor and btrfs
      verity item and the other stores the Merkle tree data itself.
      
      Verity checking is done in end_page_read just before a page is marked
      uptodate. This naturally handles a variety of edge cases like holes,
      preallocated extents, and inline extents. Some care needs to be taken to
      not try to verity pages past the end of the file, which are accessed by
      the generic buffered file reading code under some circumstances like
      reading to the end of the last page and trying to read again. Direct IO
      on a verity file falls back to buffered reads.
      
      Verity relies on PageChecked for the Merkle tree data itself to avoid
      re-walking up shared paths in the tree. For this reason, we need to
      cache the Merkle tree data. Since the file is immutable after verity is
      turned on, we can cache it at an index past EOF.
      
      Use the new inode ro_flags to store verity on the inode item, so that we
      can enable verity on a file, then rollback to an older kernel and still
      mount the file system and read the file. Since we can't safely write the
      file anymore without ruining the invariants of the Merkle tree, we mark
      a ro_compat flag on the file system when a file has verity enabled.
      Acked-by: NEric Biggers <ebiggers@google.com>
      Co-developed-by: NChris Mason <clm@fb.com>
      Signed-off-by: NChris Mason <clm@fb.com>
      Signed-off-by: NBoris Burkov <boris@bur.io>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      14605409
    • Q
      btrfs: subpage: fix a potential use-after-free in writeback helper · 7c11d0ae
      Qu Wenruo 提交于
      [BUG]
      There is a possible use-after-free bug when running generic/095.
      
       BUG: Unable to handle kernel data access on write at 0x6b6b6b6b6b6b725b
       Faulting instruction address: 0xc000000000283654
       c000000000283078 do_raw_spin_unlock+0x88/0x230
       c0000000012b1e14 _raw_spin_unlock_irqrestore+0x44/0x90
       c000000000a918dc btrfs_subpage_clear_writeback+0xac/0xe0
       c0000000009e0458 end_bio_extent_writepage+0x158/0x270
       c000000000b6fd14 bio_endio+0x254/0x270
       c0000000009fc0f0 btrfs_end_bio+0x1a0/0x200
       c000000000b6fd14 bio_endio+0x254/0x270
       c000000000b781fc blk_update_request+0x46c/0x670
       c000000000b8b394 blk_mq_end_request+0x34/0x1d0
       c000000000d82d1c lo_complete_rq+0x11c/0x140
       c000000000b880a4 blk_complete_reqs+0x84/0xb0
       c0000000012b2ca4 __do_softirq+0x334/0x680
       c0000000001dd878 irq_exit+0x148/0x1d0
       c000000000016f4c do_IRQ+0x20c/0x240
       c000000000009240 hardware_interrupt_common_virt+0x1b0/0x1c0
      
      [CAUSE]
      There is very small race window like the following in generic/095.
      
      	Thread 1		|		Thread 2
      --------------------------------+------------------------------------
        end_bio_extent_writepage()	| btrfs_releasepage()
        |- spin_lock_irqsave()	| |
        |- end_page_writeback()	| |
        |				| |- if (PageWriteback() ||...)
        |				| |- clear_page_extent_mapped()
        |				|    |- kfree(subpage);
        |- spin_unlock_irqrestore().
      
      The race can also happen between writeback and btrfs_invalidatepage(),
      although that would be much harder as btrfs_invalidatepage() has much
      more work to do before the clear_page_extent_mapped() call.
      
      [FIX]
      Here we "wait" for the subapge spinlock to be released before we detach
      subpage structure.
      So this patch will introduce a new function, wait_subpage_spinlock(), to
      do the "wait" by acquiring the spinlock and release it.
      
      Since the caller has ensured the page is not dirty nor writeback, and
      page is already locked, the only way to hold the subpage spinlock is
      from endio function.
      Thus we only need to acquire the spinlock to wait for any existing
      holder.
      Reported-by: NRitesh Harjani <riteshh@linux.ibm.com>
      Tested-by: NRitesh Harjani <riteshh@linux.ibm.com>
      Signed-off-by: NQu Wenruo <wqu@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      7c11d0ae
    • Q
      btrfs: subpage: fix race between prepare_pages() and btrfs_releasepage() · e0467866
      Qu Wenruo 提交于
      [BUG]
      When running generic/095, there is a high chance to crash with subpage
      data RW support:
      
       assertion failed: PagePrivate(page) && page->private
       ------------[ cut here ]------------
       kernel BUG at fs/btrfs/ctree.h:3403!
       Internal error: Oops - BUG: 0 [#1] SMP
       CPU: 1 PID: 3567 Comm: fio Tainted: 5.12.0-rc7-custom+ #17
       Hardware name: Khadas VIM3 (DT)
       Call trace:
        assertfail.constprop.0+0x28/0x2c [btrfs]
        btrfs_subpage_assert+0x80/0xa0 [btrfs]
        btrfs_subpage_set_uptodate+0x34/0xec [btrfs]
        btrfs_page_clamp_set_uptodate+0x74/0xa4 [btrfs]
        btrfs_dirty_pages+0x160/0x270 [btrfs]
        btrfs_buffered_write+0x444/0x630 [btrfs]
        btrfs_direct_write+0x1cc/0x2d0 [btrfs]
        btrfs_file_write_iter+0xc0/0x160 [btrfs]
        new_sync_write+0xe8/0x180
        vfs_write+0x1b4/0x210
        ksys_pwrite64+0x7c/0xc0
        __arm64_sys_pwrite64+0x24/0x30
        el0_svc_common.constprop.0+0x70/0x140
        do_el0_svc+0x28/0x90
        el0_svc+0x2c/0x54
        el0_sync_handler+0x1a8/0x1ac
        el0_sync+0x170/0x180
       Code: f0000160 913be042 913c4000 955444bc (d4210000)
       ---[ end trace 3fdd39f4cccedd68 ]---
      
      [CAUSE]
      Although prepare_pages() calls find_or_create_page(), which returns the
      page locked, but in later prepare_uptodate_page() calls, we may call
      btrfs_readpage() which will unlock the page before it returns.
      
      This leaves a window where btrfs_releasepage() can sneak in and release
      the page, clearing page->private and causing above ASSERT().
      
      [FIX]
      In prepare_uptodate_page(), we should not only check page->mapping, but
      also PagePrivate() to ensure we are still holding the correct page which
      has proper fs context setup.
      Reported-by: NRitesh Harjani <riteshh@linux.ibm.com>
      Tested-by: NRitesh Harjani <riteshh@linux.ibm.com>
      Reviewed-by: NFilipe Manana <fdmanana@suse.com>
      Signed-off-by: NQu Wenruo <wqu@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      e0467866
  4. 21 6月, 2021 4 次提交
    • N
      btrfs: eliminate insert label in add_falloc_range · 77d25534
      Nikolay Borisov 提交于
      By way of inverting the list_empty conditional the insert label can be
      eliminated, making the function's flow entirely linear.
      Signed-off-by: NNikolay Borisov <nborisov@suse.com>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      77d25534
    • Q
      btrfs: fix the filemap_range_has_page() call in btrfs_punch_hole_lock_range() · 0528476b
      Qu Wenruo 提交于
      [BUG]
      With current subpage RW support, the following script can hang the fs
      with 64K page size.
      
       # mkfs.btrfs -f -s 4k $dev
       # mount $dev -o nospace_cache $mnt
       # fsstress -w -n 50 -p 1 -s 1607749395 -d $mnt
      
      The kernel will do an infinite loop in btrfs_punch_hole_lock_range().
      
      [CAUSE]
      In btrfs_punch_hole_lock_range() we:
      
      - Truncate page cache range
      - Lock extent io tree
      - Wait any ordered extents in the range.
      
      We exit the loop until we meet all the following conditions:
      
      - No ordered extent in the lock range
      - No page is in the lock range
      
      The latter condition has a pitfall, it only works for sector size ==
      PAGE_SIZE case.
      
      While can't handle the following subpage case:
      
        0       32K     64K     96K     128K
        |       |///////||//////|       ||
      
      lockstart=32K
      lockend=96K - 1
      
      In this case, although the range crosses 2 pages,
      truncate_pagecache_range() will invalidate no page at all, but only zero
      the [32K, 96K) range of the two pages.
      
      Thus filemap_range_has_page(32K, 96K-1) will always return true, thus we
      will never meet the loop exit condition.
      
      [FIX]
      Fix the problem by doing page alignment for the lock range.
      
      Function filemap_range_has_page() has already handled lend < lstart
      case, we only need to round up @lockstart, and round_down @lockend for
      truncate_pagecache_range().
      
      This modification should not change any thing for sector size ==
      PAGE_SIZE case, as in that case our range is already page aligned.
      
      Tested-by: Ritesh Harjani <riteshh@linux.ibm.com> # [ppc64]
      Tested-by: Anand Jain <anand.jain@oracle.com> # [aarch64]
      Signed-off-by: NQu Wenruo <wqu@suse.com>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      0528476b
    • Q
      btrfs: make btrfs_dirty_pages() to be subpage compatible · f02a85d2
      Qu Wenruo 提交于
      Since the extent io tree operations in btrfs_dirty_pages() are already
      subpage compatible, we only need to make the page status update to use
      subpage helpers.
      
      Tested-by: Ritesh Harjani <riteshh@linux.ibm.com> # [ppc64]
      Tested-by: Anand Jain <anand.jain@oracle.com> # [aarch64]
      Reviewed-by: NJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: NQu Wenruo <wqu@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      f02a85d2
    • N
      btrfs: use list_last_entry in add_falloc_range · ec87b42f
      Nikolay Borisov 提交于
      Instead of calling list_entry with head->prev simply call
      list_last_entry which makes it obvious which member of the list is
      being referred. This allows to remove the extra 'prev' pointer.
      Signed-off-by: NNikolay Borisov <nborisov@suse.com>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      ec87b42f
  5. 10 6月, 2021 1 次提交
  6. 04 6月, 2021 1 次提交
  7. 29 4月, 2021 1 次提交
    • F
      btrfs: fix race leading to unpersisted data and metadata on fsync · 626e9f41
      Filipe Manana 提交于
      When doing a fast fsync on a file, there is a race which can result in the
      fsync returning success to user space without logging the inode and without
      durably persisting new data.
      
      The following example shows one possible scenario for this:
      
         $ mkfs.btrfs -f /dev/sdc
         $ mount /dev/sdc /mnt
      
         $ touch /mnt/bar
         $ xfs_io -f -c "pwrite -S 0xab 0 1M" -c "fsync" /mnt/baz
      
         # Now we have:
         # file bar == inode 257
         # file baz == inode 258
      
         $ mv /mnt/baz /mnt/foo
      
         # Now we have:
         # file bar == inode 257
         # file foo == inode 258
      
         $ xfs_io -c "pwrite -S 0xcd 0 1M" /mnt/foo
      
         # fsync bar before foo, it is important to trigger the race.
         $ xfs_io -c "fsync" /mnt/bar
         $ xfs_io -c "fsync" /mnt/foo
      
         # After this:
         # inode 257, file bar, is empty
         # inode 258, file foo, has 1M filled with 0xcd
      
         <power failure>
      
         # Replay the log:
         $ mount /dev/sdc /mnt
      
         # After this point file foo should have 1M filled with 0xcd and not 0xab
      
      The following steps explain how the race happens:
      
      1) Before the first fsync of inode 258, when it has the "baz" name, its
         ->logged_trans is 0, ->last_sub_trans is 0 and ->last_log_commit is -1.
         The inode also has the full sync flag set;
      
      2) After the first fsync, we set inode 258 ->logged_trans to 6, which is
         the generation of the current transaction, and set ->last_log_commit
         to 0, which is the current value of ->last_sub_trans (done at
         btrfs_log_inode()).
      
         The full sync flag is cleared from the inode during the fsync.
      
         The log sub transaction that was committed had an ID of 0 and when we
         synced the log, at btrfs_sync_log(), we incremented root->log_transid
         from 0 to 1;
      
      3) During the rename:
      
         We update inode 258, through btrfs_update_inode(), and that causes its
         ->last_sub_trans to be set to 1 (the current log transaction ID), and
         ->last_log_commit remains with a value of 0.
      
         After updating inode 258, because we have previously logged the inode
         in the previous fsync, we log again the inode through the call to
         btrfs_log_new_name(). This results in updating the inode's
         ->last_log_commit from 0 to 1 (the current value of its
         ->last_sub_trans).
      
         The ->last_sub_trans of inode 257 is updated to 1, which is the ID of
         the next log transaction;
      
      4) Then a buffered write against inode 258 is made. This leaves the value
         of ->last_sub_trans as 1 (the ID of the current log transaction, stored
         at root->log_transid);
      
      5) Then an fsync against inode 257 (or any other inode other than 258),
         happens. This results in committing the log transaction with ID 1,
         which results in updating root->last_log_commit to 1 and bumping
         root->log_transid from 1 to 2;
      
      6) Then an fsync against inode 258 starts. We flush delalloc and wait only
         for writeback to complete, since the full sync flag is not set in the
         inode's runtime flags - we do not wait for ordered extents to complete.
      
         Then, at btrfs_sync_file(), we call btrfs_inode_in_log() before the
         ordered extent completes. The call returns true:
      
           static inline bool btrfs_inode_in_log(...)
           {
               bool ret = false;
      
               spin_lock(&inode->lock);
               if (inode->logged_trans == generation &&
                   inode->last_sub_trans <= inode->last_log_commit &&
                   inode->last_sub_trans <= inode->root->last_log_commit)
                       ret = true;
               spin_unlock(&inode->lock);
               return ret;
           }
      
         generation has a value of 6 (fs_info->generation), ->logged_trans also
         has a value of 6 (set when we logged the inode during the first fsync
         and when logging it during the rename), ->last_sub_trans has a value
         of 1, set during the rename (step 3), ->last_log_commit also has a
         value of 1 (set in step 3) and root->last_log_commit has a value of 1,
         which was set in step 5 when fsyncing inode 257.
      
         As a consequence we don't log the inode, any new extents and do not
         sync the log, resulting in a data loss if a power failure happens
         after the fsync and before the current transaction commits.
         Also, because we do not log the inode, after a power failure the mtime
         and ctime of the inode do not match those we had before.
      
         When the ordered extent completes before we call btrfs_inode_in_log(),
         then the call returns false and we log the inode and sync the log,
         since at the end of ordered extent completion we update the inode and
         set ->last_sub_trans to 2 (the value of root->log_transid) and
         ->last_log_commit to 1.
      
      This problem is found after removing the check for the emptiness of the
      inode's list of modified extents in the recent commit 209ecbb8
      ("btrfs: remove stale comment and logic from btrfs_inode_in_log()"),
      added in the 5.13 merge window. However checking the emptiness of the
      list is not really the way to solve this problem, and was never intended
      to, because while that solves the problem for COW writes, the problem
      persists for NOCOW writes because in that case the list is always empty.
      
      In the case of NOCOW writes, even though we wait for the writeback to
      complete before returning from btrfs_sync_file(), we end up not logging
      the inode, which has a new mtime/ctime, and because we don't sync the log,
      we never issue disk barriers (send REQ_PREFLUSH to the device) since that
      only happens when we sync the log (when we write super blocks at
      btrfs_sync_log()). So effectively, for a NOCOW case, when we return from
      btrfs_sync_file() to user space, we are not guaranteeing that the data is
      durably persisted on disk.
      
      Also, while the example above uses a rename exchange to show how the
      problem happens, it is not the only way to trigger it. An alternative
      could be adding a new hard link to inode 258, since that also results
      in calling btrfs_log_new_name() and updating the inode in the log.
      An example reproducer using the addition of a hard link instead of a
      rename operation:
      
        $ mkfs.btrfs -f /dev/sdc
        $ mount /dev/sdc /mnt
      
        $ touch /mnt/bar
        $ xfs_io -f -c "pwrite -S 0xab 0 1M" -c "fsync" /mnt/foo
      
        $ ln /mnt/foo /mnt/foo_link
        $ xfs_io -c "pwrite -S 0xcd 0 1M" /mnt/foo
      
        $ xfs_io -c "fsync" /mnt/bar
        $ xfs_io -c "fsync" /mnt/foo
      
        <power failure>
      
        # Replay the log:
        $ mount /dev/sdc /mnt
      
        # After this point file foo often has 1M filled with 0xab and not 0xcd
      
      The reasons leading to the final fsync of file foo, inode 258, not
      persisting the new data are the same as for the previous example with
      a rename operation.
      
      So fix by never skipping logging and log syncing when there are still any
      ordered extents in flight. To avoid making the conditional if statement
      that checks if logging an inode is needed harder to read, place all the
      logic into an helper function with separate if statements to make it more
      manageable and easier to read.
      
      A test case for fstests will follow soon.
      
      For NOCOW writes, the problem existed before commit b5e6c3e1
      ("btrfs: always wait on ordered extents at fsync time"), introduced in
      kernel 4.19, then it went away with that commit since we started to always
      wait for ordered extent completion before logging.
      
      The problem came back again once the fast fsync path was changed again to
      avoid waiting for ordered extent completion, in commit 48778179
      ("btrfs: make fast fsyncs wait only for writeback"), added in kernel 5.10.
      
      However, for COW writes, the race only happens after the recent
      commit 209ecbb8 ("btrfs: remove stale comment and logic from
      btrfs_inode_in_log()"), introduced in the 5.13 merge window. For NOCOW
      writes, the bug existed before that commit. So tag 5.10+ as the release
      for stable backports.
      
      CC: stable@vger.kernel.org # 5.10+
      Signed-off-by: NFilipe Manana <fdmanana@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      626e9f41
  8. 19 4月, 2021 8 次提交
    • B
      btrfs: fix a potential hole punching failure · 3227788c
      BingJing Chang 提交于
      In commit d7781546 ("btrfs: Avoid trucating page or punching hole
      in a already existed hole."), existing holes can be skipped by calling
      find_first_non_hole() to adjust start and len. However, if the given len
      is invalid and large, when an EXTENT_MAP_HOLE extent is found, len will
      not be set to zero because (em->start + em->len) is less than
      (start + len). Then the ret will be 1 but len will not be set to 0.
      The propagated non-zero ret will result in fallocate failure.
      
      In the while-loop of btrfs_replace_file_extents(), len is not updated
      every time before it calls find_first_non_hole(). That is, after
      btrfs_drop_extents() successfully drops the last non-hole file extent,
      it may fail with ENOSPC when attempting to drop a file extent item
      representing a hole. The problem can happen. After it calls
      find_first_non_hole(), the cur_offset will be adjusted to be larger
      than or equal to end. However, since the len is not set to zero, the
      break-loop condition (ret && !len) will not be met. After it leaves the
      while-loop, fallocate will return 1, which is an unexpected return
      value.
      
      We're not able to construct a reproducible way to let
      btrfs_drop_extents() fail with ENOSPC after it drops the last non-hole
      file extent but with remaining holes left. However, it's quite easy to
      fix. We just need to update and check the len every time before we call
      find_first_non_hole(). To make the while loop more readable, we also
      pull the variable updates to the bottom of loop like this:
        while (cur_offset < end) {
      	  ...
      	  // update cur_offset & len
      	  // advance cur_offset & len in hole-punching case if needed
        }
      Reported-by: NRobbie Ko <robbieko@synology.com>
      Fixes: d7781546 ("btrfs: Avoid trucating page or punching hole in a already existed hole.")
      CC: stable@vger.kernel.org # 4.4+
      Reviewed-by: NRobbie Ko <robbieko@synology.com>
      Reviewed-by: NChung-Chiang Cheng <cccheng@synology.com>
      Reviewed-by: NFilipe Manana <fdmanana@suse.com>
      Signed-off-by: NBingJing Chang <bingjingc@synology.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      3227788c
    • F
      btrfs: update outdated comment at btrfs_replace_file_extents() · e2b84217
      Filipe Manana 提交于
      There is a comment at btrfs_replace_file_extents() that mentions that we
      set the full sync flag on an inode when cloning into a file with a size
      greater than or equals to 16MiB, through try_release_extent_mapping() when
      we truncate the page cache after replacing file extents during a clone
      operation.
      
      That is not true anymore since commit 5e548b32 ("btrfs: do not set
      the full sync flag on the inode during page release"), so update the
      comment to remove that part and rephrase it slightly to make it more
      clear why the full sync flag is set at btrfs_replace_file_extents().
      Signed-off-by: NFilipe Manana <fdmanana@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      e2b84217
    • F
      btrfs: fix race between marking inode needs to be logged and log syncing · bc0939fc
      Filipe Manana 提交于
      We have a race between marking that an inode needs to be logged, either
      at btrfs_set_inode_last_trans() or at btrfs_page_mkwrite(), and between
      btrfs_sync_log(). The following steps describe how the race happens.
      
      1) We are at transaction N;
      
      2) Inode I was previously fsynced in the current transaction so it has:
      
          inode->logged_trans set to N;
      
      3) The inode's root currently has:
      
         root->log_transid set to 1
         root->last_log_commit set to 0
      
         Which means only one log transaction was committed to far, log
         transaction 0. When a log tree is created we set ->log_transid and
         ->last_log_commit of its parent root to 0 (at btrfs_add_log_tree());
      
      4) One more range of pages is dirtied in inode I;
      
      5) Some task A starts an fsync against some other inode J (same root), and
         so it joins log transaction 1.
      
         Before task A calls btrfs_sync_log()...
      
      6) Task B starts an fsync against inode I, which currently has the full
         sync flag set, so it starts delalloc and waits for the ordered extent
         to complete before calling btrfs_inode_in_log() at btrfs_sync_file();
      
      7) During ordered extent completion we have btrfs_update_inode() called
         against inode I, which in turn calls btrfs_set_inode_last_trans(),
         which does the following:
      
           spin_lock(&inode->lock);
           inode->last_trans = trans->transaction->transid;
           inode->last_sub_trans = inode->root->log_transid;
           inode->last_log_commit = inode->root->last_log_commit;
           spin_unlock(&inode->lock);
      
         So ->last_trans is set to N and ->last_sub_trans set to 1.
         But before setting ->last_log_commit...
      
      8) Task A is at btrfs_sync_log():
      
         - it increments root->log_transid to 2
         - starts writeback for all log tree extent buffers
         - waits for the writeback to complete
         - writes the super blocks
         - updates root->last_log_commit to 1
      
         It's a lot of slow steps between updating root->log_transid and
         root->last_log_commit;
      
      9) The task doing the ordered extent completion, currently at
         btrfs_set_inode_last_trans(), then finally runs:
      
           inode->last_log_commit = inode->root->last_log_commit;
           spin_unlock(&inode->lock);
      
         Which results in inode->last_log_commit being set to 1.
         The ordered extent completes;
      
      10) Task B is resumed, and it calls btrfs_inode_in_log() which returns
          true because we have all the following conditions met:
      
          inode->logged_trans == N which matches fs_info->generation &&
          inode->last_subtrans (1) <= inode->last_log_commit (1) &&
          inode->last_subtrans (1) <= root->last_log_commit (1) &&
          list inode->extent_tree.modified_extents is empty
      
          And as a consequence we return without logging the inode, so the
          existing logged version of the inode does not point to the extent
          that was written after the previous fsync.
      
      It should be impossible in practice for one task be able to do so much
      progress in btrfs_sync_log() while another task is at
      btrfs_set_inode_last_trans() right after it reads root->log_transid and
      before it reads root->last_log_commit. Even if kernel preemption is enabled
      we know the task at btrfs_set_inode_last_trans() can not be preempted
      because it is holding the inode's spinlock.
      
      However there is another place where we do the same without holding the
      spinlock, which is in the memory mapped write path at:
      
        vm_fault_t btrfs_page_mkwrite(struct vm_fault *vmf)
        {
           (...)
           BTRFS_I(inode)->last_trans = fs_info->generation;
           BTRFS_I(inode)->last_sub_trans = BTRFS_I(inode)->root->log_transid;
           BTRFS_I(inode)->last_log_commit = BTRFS_I(inode)->root->last_log_commit;
           (...)
      
      So with preemption happening after setting ->last_sub_trans and before
      setting ->last_log_commit, it is less of a stretch to have another task
      do enough progress at btrfs_sync_log() such that the task doing the memory
      mapped write ends up with ->last_sub_trans and ->last_log_commit set to
      the same value. It is still a big stretch to get there, as the task doing
      btrfs_sync_log() has to start writeback, wait for its completion and write
      the super blocks.
      
      So fix this in two different ways:
      
      1) For btrfs_set_inode_last_trans(), simply set ->last_log_commit to the
         value of ->last_sub_trans minus 1;
      
      2) For btrfs_page_mkwrite() only set the inode's ->last_sub_trans, just
         like we do for buffered and direct writes at btrfs_file_write_iter(),
         which is all we need to make sure multiple writes and fsyncs to an
         inode in the same transaction never result in an fsync missing that
         the inode changed and needs to be logged. Turn this into a helper
         function and use it both at btrfs_page_mkwrite() and at
         btrfs_file_write_iter() - this also fixes the problem that at
         btrfs_page_mkwrite() we were setting those fields without the
         protection of the inode's spinlock.
      
      This is an extremely unlikely race to happen in practice.
      Signed-off-by: NFilipe Manana <fdmanana@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      bc0939fc
    • F
      btrfs: fix race between memory mapped writes and fsync · 885f46d8
      Filipe Manana 提交于
      When doing an fsync we flush all delalloc, lock the inode (VFS lock), flush
      any new delalloc that might have been created before taking the lock and
      then wait either for the ordered extents to complete or just for the
      writeback to complete (depending on whether the full sync flag is set or
      not). We then start logging the inode and assume that while we are doing it
      no one else is touching the inode's file extent items (or adding new ones).
      
      That is generally true because all operations that modify an inode acquire
      the inode's lock first, including buffered and direct IO writes. However
      there is one exception: memory mapped writes, which do not and can not
      acquire the inode's lock.
      
      This can cause two types of issues: ending up logging file extent items
      with overlapping ranges, which is detected by the tree checker and will
      result in aborting the transaction when starting writeback for a log
      tree's extent buffers, or a silent corruption where we log a version of
      the file that never existed.
      
      Scenario 1 - logging overlapping extents
      
      The following steps explain how we can end up with file extents items with
      overlapping ranges in a log tree due to a race between a fsync and memory
      mapped writes:
      
      1) Task A starts an fsync on inode X, which has the full sync runtime flag
         set. First it starts by flushing all delalloc for the inode;
      
      2) Task A then locks the inode and flushes any other delalloc that might
         have been created after the previous flush and waits for all ordered
         extents to complete;
      
      3) In the inode's root we have the following leaf:
      
         Leaf N, generation == current transaction id:
      
         ---------------------------------------------------------
         | (...)  [ file extent item, offset 640K, length 128K ] |
         ---------------------------------------------------------
      
         The last file extent item in leaf N covers the file range from 640K to
         768K;
      
      4) Task B does a memory mapped write for the page corresponding to the
         file range from 764K to 768K;
      
      5) Task A starts logging the inode. At copy_inode_items_to_log() it uses
         btrfs_search_forward() to search for leafs modified in the current
         transaction that contain items for the inode. It finds leaf N and copies
         all the inode items from that leaf into the log tree.
      
         Now the log tree has a copy of the last file extent item from leaf N.
      
         At the end of the while loop at copy_inode_items_to_log(), we have the
         minimum key set to:
      
         min_key.objectid = <inode X number>
         min_key.type = BTRFS_EXTENT_DATA_KEY
         min_key.offset = 640K
      
         Then we increment the key's offset by 1 so that the next call to
         btrfs_search_forward() leaves us at the first key greater than the key
         we just processed.
      
         But before btrfs_search_forward() is called again...
      
      6) Dellaloc for the page at offset 764K, dirtied by task B, is started.
         It can be started for several reasons:
      
           - The async reclaim task is attempting to satisfy metadata or data
             reservation requests, and it has reached a point where it decided
             to flush delalloc;
           - Due to memory pressure the VMM triggers writeback of dirty pages;
           - The system call sync_file_range(2) is called from user space.
      
      7) When the respective ordered extent completes, it trims the length of
         the existing file extent item for file offset 640K from 128K to 124K,
         and a new file extent item is added with a key offset of 764K and a
         length of 4K;
      
      8) Task A calls btrfs_search_forward(), which returns us a path pointing
         to the leaf (can be leaf N or some other) containing the new file extent
         item for file offset 764K.
      
         We end up copying this item to the log tree, which overlaps with the
         last copied file extent item, which covers the file range from 640K to
         768K.
      
         When writeback is triggered for log tree's extent buffers, the issue
         will be detected by the tree checker which will dump a trace and an
         error message on dmesg/syslog. If the writeback is triggered when
         syncing the log, which typically is, then we also end up aborting the
         current transaction.
      
      This is the same type of problem fixed in 0c713cba ("Btrfs: fix race
      between ranged fsync and writeback of adjacent ranges").
      
      Scenario 2 - logging a version of the file that never existed
      
      This scenario only happens when using the NO_HOLES feature and results in
      a silent corruption, in the sense that is not detectable by 'btrfs check'
      or the tree checker:
      
      1) We have an inode I with a size of 1M and two file extent items, one
         covering an extent with disk_bytenr == X for the file range [0, 512K)
         and another one covering another extent with disk_bytenr == Y for the
         file range [512K, 1M);
      
      2) A hole is punched for the file range [512K, 1M);
      
      3) Task A starts an fsync of inode I, which has the full sync runtime flag
         set. It starts by flushing all existing delalloc, locks the inode (VFS
         lock), starts any new delalloc that might have been created before
         taking the lock and waits for all ordered extents to complete;
      
      4) Some other task does a memory mapped write for the page corresponding to
         the file range [640K, 644K) for example;
      
      5) Task A then logs all items of the inode with the call to
         copy_inode_items_to_log();
      
      6) In the meanwhile delalloc for the range [640K, 644K) is started. It can
         be started for several reasons:
      
           - The async reclaim task is attempting to satisfy metadata or data
             reservation requests, and it has reached a point where it decided
             to flush delalloc;
           - Due to memory pressure the VMM triggers writeback of dirty pages;
           - The system call sync_file_range(2) is called from user space.
      
      7) The ordered extent for the range [640K, 644K) completes and a file
         extent item for that range is added to the subvolume tree, pointing
         to a 4K extent with a disk_bytenr == Z;
      
      8) Task A then calls btrfs_log_holes(), to scan for implicit holes in
         the subvolume tree. It finds two implicit holes:
      
         - one for the file range [512K, 640K)
         - one for the file range [644K, 1M)
      
         As a result we end up neither logging a hole for the range [640K, 644K)
         nor logging the file extent item with a disk_bytenr == Z.
         This means that if we have a power failure and replay the log tree we
         end up getting the following file extent layout:
      
         [ disk_bytenr X ]    [   hole   ]    [ disk_bytenr Y ]    [  hole  ]
         0             512K  512K      640K  640K           644K  644K     1M
      
         Which does not corresponding to any layout the file ever had before
         the power failure. The only two valid layouts would be:
      
         [ disk_bytenr X ]    [   hole   ]
         0             512K  512K        1M
      
         and
      
         [ disk_bytenr X ]    [   hole   ]    [ disk_bytenr Z ]    [  hole  ]
         0             512K  512K      640K  640K           644K  644K     1M
      
      This can be fixed by serializing memory mapped writes with fsync, and there
      are two ways to do it:
      
      1) Make a fsync lock the entire file range, from 0 to (u64)-1 / LLONG_MAX
         in the inode's io tree. This prevents the race but also blocks any reads
         during the duration of the fsync, which has a negative impact for many
         common workloads;
      
      2) Make an fsync write lock the i_mmap_lock semaphore in the inode. This
         semaphore was recently added by Josef's patch set:
      
         btrfs: add a i_mmap_lock to our inode
         btrfs: cleanup inode_lock/inode_unlock uses
         btrfs: exclude mmaps while doing remap
         btrfs: exclude mmap from happening during all fallocate operations
      
         and is used to solve races between memory mapped writes and
         clone/dedupe/fallocate. This also makes us have the same behaviour we
         have regarding other writes (buffered and direct IO) and fsync - block
         them while the inode logging is in progress.
      
      This change uses the second approach due to the performance impact of the
      first one.
      Signed-off-by: NFilipe Manana <fdmanana@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      885f46d8
    • J
      btrfs: exclude mmap from happening during all fallocate operations · 8d9b4a16
      Josef Bacik 提交于
      There's a small window where a deadlock can happen between fallocate and
      mmap.  This is described in detail by Filipe:
      
      """
      When doing a fallocate operation we lock the inode, flush delalloc within
      the target range, wait for any ordered extents to complete and then lock
      the file range. Before we lock the range and after we flush delalloc,
      there is a time window where another task can come in and do a memory
      mapped write for a page within the fallocate range.
      
      This means that after fallocate locks the range, there can be a dirty page
      in the range. More often than not, this does not cause any problem.
      The exception is when we are low on available metadata space, because an
      fallocate operation needs to start a transaction while holding the file
      range locked, either through btrfs_prealloc_file_range() or through the
      call to btrfs_fallocate_update_isize(). If that's the case, we can end up
      in a deadlock. The following list of steps explains how that happens:
      
      1) A fallocate operation starts, locks the inode, flushes delalloc in the
         range and waits for ordered extents in the range to complete;
      
      2) Before the fallocate task locks the file range, another task does a
         memory mapped write for a page in the fallocate target range. This is
         possible since memory mapped writes do not (and can not) lock the
         inode;
      
      3) The fallocate task locks the file range. At this point there is one
         dirty page in the range (due to the memory mapped write);
      
      4) When the fallocate task attempts to start a transaction, it blocks when
         attempting to reserve metadata space, since we are low on available
         metadata space. Before blocking (wait on its reservation ticket), it
         starts the async reclaim task (if not running already);
      
      5) The async reclaim task is not able to release space through any other
         means, so it decides to flush delalloc for inodes with dirty pages.
         It finds that the inode used in the fallocate operation has a dirty
         page and therefore queues a job (fs_info->flush_workers workqueue) to
         flush delalloc for that inode and waits on that job to complete;
      
      6) The flush job blocks when attempting to lock the file range because
         it is currently locked by the fallocate task;
      
      7) The fallocate task keeps waiting for its metadata reservation, waiting
         for a wakeup on its reservation ticket. The async reclaim task is
         waiting on the flush job, which in turn is waiting for locking the file
         range that is currently locked by the fallocate task. So unless some
         other task is able to release enough metadata space, for example an
         ordered extent for some other inode completes, we end up in a deadlock
         between all these tasks.
      
      When this happens stack traces like the following show up in dmesg/syslog:
      
       INFO: task kworker/u16:11:1810830 blocked for more than 120 seconds.
             Tainted: G    B   W         5.10.0-rc4-btrfs-next-73 #1
       "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
       task:kworker/u16:11  state:D stack:    0 pid:1810830 ppid:     2 flags:0x00004000
       Workqueue: btrfs-flush_delalloc btrfs_work_helper [btrfs]
       Call Trace:
        __schedule+0x5d1/0xcf0
        schedule+0x45/0xe0
        lock_extent_bits+0x1e6/0x2d0 [btrfs]
        ? finish_wait+0x90/0x90
        btrfs_invalidatepage+0x32c/0x390 [btrfs]
        ? __mod_memcg_state+0x8e/0x160
        __extent_writepage+0x2d4/0x400 [btrfs]
        extent_write_cache_pages+0x2b2/0x500 [btrfs]
        ? lock_release+0x20e/0x4c0
        ? trace_hardirqs_on+0x1b/0xf0
        extent_writepages+0x43/0x90 [btrfs]
        ? lock_acquire+0x1a3/0x490
        do_writepages+0x43/0xe0
        ? __filemap_fdatawrite_range+0xa4/0x100
        __filemap_fdatawrite_range+0xc5/0x100
        btrfs_run_delalloc_work+0x17/0x40 [btrfs]
        btrfs_work_helper+0xf1/0x600 [btrfs]
        process_one_work+0x24e/0x5e0
        worker_thread+0x50/0x3b0
        ? process_one_work+0x5e0/0x5e0
        kthread+0x153/0x170
        ? kthread_mod_delayed_work+0xc0/0xc0
        ret_from_fork+0x22/0x30
       INFO: task kworker/u16:1:2426217 blocked for more than 120 seconds.
             Tainted: G    B   W         5.10.0-rc4-btrfs-next-73 #1
       "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
       task:kworker/u16:1   state:D stack:    0 pid:2426217 ppid:     2 flags:0x00004000
       Workqueue: events_unbound btrfs_async_reclaim_metadata_space [btrfs]
       Call Trace:
        __schedule+0x5d1/0xcf0
        ? kvm_clock_read+0x14/0x30
        ? wait_for_completion+0x81/0x110
        schedule+0x45/0xe0
        schedule_timeout+0x30c/0x580
        ? _raw_spin_unlock_irqrestore+0x3c/0x60
        ? lock_acquire+0x1a3/0x490
        ? try_to_wake_up+0x7a/0xa20
        ? lock_release+0x20e/0x4c0
        ? lock_acquired+0x199/0x490
        ? wait_for_completion+0x81/0x110
        wait_for_completion+0xab/0x110
        start_delalloc_inodes+0x2af/0x390 [btrfs]
        btrfs_start_delalloc_roots+0x12d/0x250 [btrfs]
        flush_space+0x24f/0x660 [btrfs]
        btrfs_async_reclaim_metadata_space+0x1bb/0x480 [btrfs]
        process_one_work+0x24e/0x5e0
        worker_thread+0x20f/0x3b0
        ? process_one_work+0x5e0/0x5e0
        kthread+0x153/0x170
        ? kthread_mod_delayed_work+0xc0/0xc0
        ret_from_fork+0x22/0x30
      (...)
      several tasks waiting for the inode lock held by the fallocate task below
      (...)
       RIP: 0033:0x7f61efe73fff
       Code: Unable to access opcode bytes at RIP 0x7f61efe73fd5.
       RSP: 002b:00007ffc3371bbe8 EFLAGS: 00000202 ORIG_RAX: 000000000000013c
       RAX: ffffffffffffffda RBX: 00007ffc3371bea0 RCX: 00007f61efe73fff
       RDX: 00000000ffffff9c RSI: 0000560fbd5d90a0 RDI: 00000000ffffff9c
       RBP: 00007ffc3371beb0 R08: 0000000000000001 R09: 0000000000000003
       R10: 0000560fbd5d7ad0 R11: 0000000000000202 R12: 0000000000000001
       R13: 000000000000005e R14: 00007ffc3371bea0 R15: 00007ffc3371beb0
       task:fdm-stress        state:D stack:    0 pid:2508243 ppid:2508153 flags:0x00000000
       Call Trace:
        __schedule+0x5d1/0xcf0
        ? _raw_spin_unlock_irqrestore+0x3c/0x60
        schedule+0x45/0xe0
        __reserve_bytes+0x4a4/0xb10 [btrfs]
        ? finish_wait+0x90/0x90
        btrfs_reserve_metadata_bytes+0x29/0x190 [btrfs]
        btrfs_block_rsv_add+0x1f/0x50 [btrfs]
        start_transaction+0x2d1/0x760 [btrfs]
        btrfs_replace_file_extents+0x120/0x930 [btrfs]
        ? btrfs_fallocate+0xdcf/0x1260 [btrfs]
        btrfs_fallocate+0xdfb/0x1260 [btrfs]
        ? filename_lookup+0xf1/0x180
        vfs_fallocate+0x14f/0x440
        ioctl_preallocate+0x92/0xc0
        do_vfs_ioctl+0x66b/0x750
        ? __do_sys_newfstat+0x53/0x60
        __x64_sys_ioctl+0x62/0xb0
        do_syscall_64+0x33/0x80
        entry_SYSCALL_64_after_hwframe+0x44/0xa9
      """
      
      Fix this by disallowing mmaps from happening while we're doing any of
      the fallocate operations on this inode.
      Reviewed-by: NFilipe Manana <fdmanana@suse.com>
      Signed-off-by: NJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      8d9b4a16
    • J
      btrfs: use btrfs_inode_lock/btrfs_inode_unlock inode lock helpers · 64708539
      Josef Bacik 提交于
      A few places we intermix btrfs_inode_lock with a inode_unlock, and some
      places we just use inode_lock/inode_unlock instead of btrfs_inode_lock.
      
      None of these places are using this incorrectly, but as we adjust some
      of these callers it would be nice to keep everything consistent, so
      convert everybody to use btrfs_inode_lock/btrfs_inode_unlock.
      Reviewed-by: NFilipe Manana <fdmanana@suse.com>
      Signed-off-by: NJosef Bacik <josef@toxicpanda.com>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      64708539
    • N
      cca5de97
    • N
  9. 02 3月, 2021 1 次提交
  10. 25 2月, 2021 1 次提交
  11. 09 2月, 2021 5 次提交
    • N
      btrfs: zoned: use ZONE_APPEND write for zoned mode · d8e3fb10
      Naohiro Aota 提交于
      Enable zone append writing for zoned mode. When using zone append, a
      bio is issued to the start of a target zone and the device decides to
      place it inside the zone. Upon completion the device reports the actual
      written position back to the host.
      
      Three parts are necessary to enable zone append mode. First, modify the
      bio to use REQ_OP_ZONE_APPEND in btrfs_submit_bio_hook() and adjust the
      bi_sector to point the beginning of the zone.
      
      Second, record the returned physical address (and disk/partno) to the
      ordered extent in end_bio_extent_writepage() after the bio has been
      completed. We cannot resolve the physical address to the logical address
      because we can neither take locks nor allocate a buffer in this end_bio
      context. So, we need to record the physical address to resolve it later
      in btrfs_finish_ordered_io().
      
      And finally, rewrite the logical addresses of the extent mapping and
      checksum data according to the physical address using btrfs_rmap_block.
      If the returned address matches the originally allocated address, we can
      skip this rewriting process.
      Reviewed-by: NJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: NJohannes Thumshirn <johannes.thumshirn@wdc.com>
      Signed-off-by: NNaohiro Aota <naohiro.aota@wdc.com>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      d8e3fb10
    • Q
      btrfs: introduce btrfs_subpage for data inodes · 32443de3
      Qu Wenruo 提交于
      To support subpage sector size, data also need extra info to make sure
      which sectors in a page are uptodate/dirty/...
      
      This patch will make pages for data inodes get btrfs_subpage structure
      attached, and detached when the page is freed.
      
      This patch also slightly changes the timing when
      set_page_extent_mapped() is called to make sure:
      
      - We have page->mapping set
        page->mapping->host is used to grab btrfs_fs_info, thus we can only
        call this function after page is mapped to an inode.
      
        One call site attaches pages to inode manually, thus we have to modify
        the timing of set_page_extent_mapped() a bit.
      
      - As soon as possible, before other operations
        Since memory allocation can fail, we have to do extra error handling.
        Calling set_page_extent_mapped() as soon as possible can simply the
        error handling for several call sites.
      
      The idea is pretty much the same as iomap_page, but with more bitmaps
      for btrfs specific cases.
      
      Currently the plan is to switch iomap if iomap can provide sector
      aligned write back (only write back dirty sectors, but not the full
      page, data balance require this feature).
      
      So we will stick to btrfs specific bitmap for now.
      Signed-off-by: NQu Wenruo <wqu@suse.com>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      32443de3
    • F
      btrfs: make concurrent fsyncs wait less when waiting for a transaction commit · d0c2f4fa
      Filipe Manana 提交于
      Often an fsync needs to fallback to a transaction commit for several
      reasons (to ensure consistency after a power failure, a new block group
      was allocated or a temporary error such as ENOMEM or ENOSPC happened).
      
      In that case the log is marked as needing a full commit and any concurrent
      tasks attempting to log inodes or commit the log will also fallback to the
      transaction commit. When this happens they all wait for the task that first
      started the transaction commit to finish the transaction commit - however
      they wait until the full transaction commit happens, which is not needed,
      as they only need to wait for the superblocks to be persisted and not for
      unpinning all the extents pinned during the transaction's lifetime, which
      even for short lived transactions can be a few thousand and take some
      significant amount of time to complete - for dbench workloads I have
      observed up to 4~5 milliseconds of time spent unpinning extents in the
      worst cases, and the number of pinned extents was between 2 to 3 thousand.
      
      So allow fsync tasks to skip waiting for the unpinning of extents when
      they call btrfs_commit_transaction() and they were not the task that
      started the transaction commit (that one has to do it, the alternative
      would be to offload the transaction commit to another task so that it
      could avoid waiting for the extent unpinning or offload the extent
      unpinning to another task).
      
      This patch is part of a patchset comprised of the following patches:
      
        btrfs: remove unnecessary directory inode item update when deleting dir entry
        btrfs: stop setting nbytes when filling inode item for logging
        btrfs: avoid logging new ancestor inodes when logging new inode
        btrfs: skip logging directories already logged when logging all parents
        btrfs: skip logging inodes already logged when logging new entries
        btrfs: remove unnecessary check_parent_dirs_for_sync()
        btrfs: make concurrent fsyncs wait less when waiting for a transaction commit
      
      After applying the entire patchset, dbench shows improvements in respect
      to throughput and latency. The script used to measure it is the following:
      
        $ cat dbench-test.sh
        #!/bin/bash
      
        DEV=/dev/sdk
        MNT=/mnt/sdk
        MOUNT_OPTIONS="-o ssd"
        MKFS_OPTIONS="-m single -d single"
      
        echo "performance" | tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
      
        umount $DEV &> /dev/null
        mkfs.btrfs -f $MKFS_OPTIONS $DEV
        mount $MOUNT_OPTIONS $DEV $MNT
      
        dbench -D $MNT -t 300 64
      
        umount $MNT
      
      The test was run on a physical machine with 12 cores (Intel corei7), 64G
      of ram, using a NVMe device and a non-debug kernel configuration (Debian's
      default configuration).
      
      Before applying patchset, 32 clients:
      
       Operation      Count    AvgLat    MaxLat
       ----------------------------------------
       NTCreateX    9627107     0.153    61.938
       Close        7072076     0.001     3.175
       Rename        407633     1.222    44.439
       Unlink       1943895     0.658    44.440
       Deltree          256    17.339   110.891
       Mkdir            128     0.003     0.009
       Qpathinfo    8725406     0.064    17.850
       Qfileinfo    1529516     0.001     2.188
       Qfsinfo      1599884     0.002     1.457
       Sfileinfo     784200     0.005     3.562
       Find         3373513     0.411    30.312
       WriteX       4802132     0.053    29.054
       ReadX       15089959     0.002     5.801
       LockX          31344     0.002     0.425
       UnlockX        31344     0.001     0.173
       Flush         674724     5.952   341.830
      
      Throughput 1008.02 MB/sec  32 clients  32 procs  max_latency=341.833 ms
      
      After applying patchset, 32 clients:
      
      After patchset, with 32 clients:
      
       Operation      Count    AvgLat    MaxLat
       ----------------------------------------
       NTCreateX    9931568     0.111    25.597
       Close        7295730     0.001     2.171
       Rename        420549     0.982    49.714
       Unlink       2005366     0.497    39.015
       Deltree          256    11.149    89.242
       Mkdir            128     0.002     0.014
       Qpathinfo    9001863     0.049    20.761
       Qfileinfo    1577730     0.001     2.546
       Qfsinfo      1650508     0.002     3.531
       Sfileinfo     809031     0.005     5.846
       Find         3480259     0.309    23.977
       WriteX       4952505     0.043    41.283
       ReadX       15568127     0.002     5.476
       LockX          32338     0.002     0.978
       UnlockX        32338     0.001     2.032
       Flush         696017     7.485   228.835
      
      Throughput 1049.91 MB/sec  32 clients  32 procs  max_latency=228.847 ms
      
       --> +4.1% throughput, -39.6% max latency
      
      Before applying patchset, 64 clients:
      
       Operation      Count    AvgLat    MaxLat
       ----------------------------------------
       NTCreateX    8956748     0.342   108.312
       Close        6579660     0.001     3.823
       Rename        379209     2.396    81.897
       Unlink       1808625     1.108   131.148
       Deltree          256    25.632   172.176
       Mkdir            128     0.003     0.018
       Qpathinfo    8117615     0.131    55.916
       Qfileinfo    1423495     0.001     2.635
       Qfsinfo      1488496     0.002     5.412
       Sfileinfo     729472     0.007     8.643
       Find         3138598     0.855    78.321
       WriteX       4470783     0.102    79.442
       ReadX       14038139     0.002     7.578
       LockX          29158     0.002     0.844
       UnlockX        29158     0.001     0.567
       Flush         627746    14.168   506.151
      
      Throughput 924.738 MB/sec  64 clients  64 procs  max_latency=506.154 ms
      
      After applying patchset, 64 clients:
      
       Operation      Count    AvgLat    MaxLat
       ----------------------------------------
       NTCreateX    9069003     0.303    43.193
       Close        6662328     0.001     3.888
       Rename        383976     2.194    46.418
       Unlink       1831080     1.022    43.873
       Deltree          256    24.037   155.763
       Mkdir            128     0.002     0.005
       Qpathinfo    8219173     0.137    30.233
       Qfileinfo    1441203     0.001     3.204
       Qfsinfo      1507092     0.002     4.055
       Sfileinfo     738775     0.006     5.431
       Find         3177874     0.936    38.170
       WriteX       4526152     0.084    39.518
       ReadX       14213562     0.002    24.760
       LockX          29522     0.002     1.221
       UnlockX        29522     0.001     0.694
       Flush         635652    14.358   422.039
      
      Throughput 990.13 MB/sec  64 clients  64 procs  max_latency=422.043 ms
      
       --> +6.8% throughput, -18.1% max latency
      Reviewed-by: NJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: NFilipe Manana <fdmanana@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      d0c2f4fa
    • Q
      btrfs: update comment for btrfs_dirty_pages · c0fab480
      Qu Wenruo 提交于
      The original comment is from the initial merge, which has several
      problems:
      
      - No holes check any more
      - No inline decision is made
      
      Update the out-of-date comment with more correct one.
      Signed-off-by: NQu Wenruo <wqu@suse.com>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      c0fab480
    • N
      btrfs: cleanup local variables in btrfs_file_write_iter · 14971657
      Nikolay Borisov 提交于
      First replace all inode instances with a pointer to btrfs_inode. This
      removes multiple invocations of the BTRFS_I macro, subsequently remove
      2 local variables as they are called only once and simply refer to
      them directly.
      Signed-off-by: NNikolay Borisov <nborisov@suse.com>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      14971657
  12. 24 1月, 2021 1 次提交
  13. 10 12月, 2020 1 次提交
  14. 08 12月, 2020 7 次提交
    • N
    • N
    • N
    • N
    • N
    • N
    • F
      btrfs: update the number of bytes used by an inode atomically · 2766ff61
      Filipe Manana 提交于
      There are several occasions where we do not update the inode's number of
      used bytes atomically, resulting in a concurrent stat(2) syscall to report
      a value of used blocks that does not correspond to a valid value, that is,
      a value that does not match neither what we had before the operation nor
      what we get after the operation completes.
      
      In extreme cases it can result in stat(2) reporting zero used blocks, which
      can cause problems for some userspace tools where they can consider a file
      with a non-zero size and zero used blocks as completely sparse and skip
      reading data, as reported/discussed a long time ago in some threads like
      the following:
      
        https://lists.gnu.org/archive/html/bug-tar/2016-07/msg00001.html
      
      The cases where this can happen are the following:
      
      -> Case 1
      
      If we do a write (buffered or direct IO) against a file region for which
      there is already an allocated extent (or multiple extents), then we have a
      short time window where we can report a number of used blocks to stat(2)
      that does not take into account the file region being overwritten. This
      short time window happens when completing the ordered extent(s).
      
      This happens because when we drop the extents in the write range we
      decrement the inode's number of bytes and later on when we insert the new
      extent(s) we increment the number of bytes in the inode, resulting in a
      short time window where a stat(2) syscall can get an incorrect number of
      used blocks.
      
      If we do writes that overwrite an entire file, then we have a short time
      window where we report 0 used blocks to stat(2).
      
      Example reproducer:
      
        $ cat reproducer-1.sh
        #!/bin/bash
      
        MNT=/mnt/sdi
        DEV=/dev/sdi
      
        stat_loop()
        {
            trap "wait; exit" SIGTERM
            local filepath=$1
            local expected=$2
            local got
      
            while :; do
                got=$(stat -c %b $filepath)
                if [ $got -ne $expected ]; then
                   echo -n "ERROR: unexpected used blocks"
                   echo " (got: $got expected: $expected)"
                fi
            done
        }
      
        mkfs.btrfs -f $DEV > /dev/null
        # mkfs.xfs -f $DEV > /dev/null
        # mkfs.ext4 -F $DEV > /dev/null
        # mkfs.f2fs -f $DEV > /dev/null
        # mkfs.reiserfs -f $DEV > /dev/null
        mount $DEV $MNT
      
        xfs_io -f -s -c "pwrite -b 64K 0 64K" $MNT/foobar >/dev/null
        expected=$(stat -c %b $MNT/foobar)
      
        # Create a process to keep calling stat(2) on the file and see if the
        # reported number of blocks used (disk space used) changes, it should
        # not because we are not increasing the file size nor punching holes.
        stat_loop $MNT/foobar $expected &
        loop_pid=$!
      
        for ((i = 0; i < 50000; i++)); do
            xfs_io -s -c "pwrite -b 64K 0 64K" $MNT/foobar >/dev/null
        done
      
        kill $loop_pid &> /dev/null
        wait
      
        umount $DEV
      
        $ ./reproducer-1.sh
        ERROR: unexpected used blocks (got: 0 expected: 128)
        ERROR: unexpected used blocks (got: 0 expected: 128)
        (...)
      
      Note that since this is a short time window where the race can happen, the
      reproducer may not be able to always trigger the bug in one run, or it may
      trigger it multiple times.
      
      -> Case 2
      
      If we do a buffered write against a file region that does not have any
      allocated extents, like a hole or beyond EOF, then during ordered extent
      completion we have a short time window where a concurrent stat(2) syscall
      can report a number of used blocks that does not correspond to the value
      before or after the write operation, a value that is actually larger than
      the value after the write completes.
      
      This happens because once we start a buffered write into an unallocated
      file range we increment the inode's 'new_delalloc_bytes', to make sure
      any stat(2) call gets a correct used blocks value before delalloc is
      flushed and completes. However at ordered extent completion, after we
      inserted the new extent, we increment the inode's number of bytes used
      with the size of the new extent, and only later, when clearing the range
      in the inode's iotree, we decrement the inode's 'new_delalloc_bytes'
      counter with the size of the extent. So this results in a short time
      window where a concurrent stat(2) syscall can report a number of used
      blocks that accounts for the new extent twice.
      
      Example reproducer:
      
        $ cat reproducer-2.sh
        #!/bin/bash
      
        MNT=/mnt/sdi
        DEV=/dev/sdi
      
        stat_loop()
        {
            trap "wait; exit" SIGTERM
            local filepath=$1
            local expected=$2
            local got
      
            while :; do
                got=$(stat -c %b $filepath)
                if [ $got -ne $expected ]; then
                    echo -n "ERROR: unexpected used blocks"
                    echo " (got: $got expected: $expected)"
                fi
            done
        }
      
        mkfs.btrfs -f $DEV > /dev/null
        # mkfs.xfs -f $DEV > /dev/null
        # mkfs.ext4 -F $DEV > /dev/null
        # mkfs.f2fs -f $DEV > /dev/null
        # mkfs.reiserfs -f $DEV > /dev/null
        mount $DEV $MNT
      
        touch $MNT/foobar
        write_size=$((64 * 1024))
        for ((i = 0; i < 16384; i++)); do
           offset=$(($i * $write_size))
           xfs_io -c "pwrite -S 0xab $offset $write_size" $MNT/foobar >/dev/null
           blocks_used=$(stat -c %b $MNT/foobar)
      
           # Fsync the file to trigger writeback and keep calling stat(2) on it
           # to see if the number of blocks used changes.
           stat_loop $MNT/foobar $blocks_used &
           loop_pid=$!
           xfs_io -c "fsync" $MNT/foobar
      
           kill $loop_pid &> /dev/null
           wait $loop_pid
        done
      
        umount $DEV
      
        $ ./reproducer-2.sh
        ERROR: unexpected used blocks (got: 265472 expected: 265344)
        ERROR: unexpected used blocks (got: 284032 expected: 283904)
        (...)
      
      Note that since this is a short time window where the race can happen, the
      reproducer may not be able to always trigger the bug in one run, or it may
      trigger it multiple times.
      
      -> Case 3
      
      Another case where such problems happen is during other operations that
      replace extents in a file range with other extents. Those operations are
      extent cloning, deduplication and fallocate's zero range operation.
      
      The cause of the problem is similar to the first case. When we drop the
      extents from a range, we decrement the inode's number of bytes, and later
      on, after inserting the new extents we increment it. Since this is not
      done atomically, a concurrent stat(2) call can see and return a number of
      used blocks that is smaller than it should be, does not match the number
      of used blocks before or after the clone/deduplication/zero operation.
      
      Like for the first case, when doing a clone, deduplication or zero range
      operation against an entire file, we end up having a time window where we
      can report 0 used blocks to a stat(2) call.
      
      Example reproducer:
      
        $ cat reproducer-3.sh
        #!/bin/bash
      
        MNT=/mnt/sdi
        DEV=/dev/sdi
      
        mkfs.btrfs -f $DEV > /dev/null
        # mkfs.xfs -f -m reflink=1 $DEV > /dev/null
        mount $DEV $MNT
      
        extent_size=$((64 * 1024))
        num_extents=16384
        file_size=$(($extent_size * $num_extents))
      
        # File foo has many small extents.
        xfs_io -f -s -c "pwrite -S 0xab -b $extent_size 0 $file_size" $MNT/foo \
            > /dev/null
        # File bar has much less extents and has exactly the same data as foo.
        xfs_io -f -c "pwrite -S 0xab 0 $file_size" $MNT/bar > /dev/null
      
        expected=$(stat -c %b $MNT/foo)
      
        # Now deduplicate bar into foo. While the deduplication is in progres,
        # the number of used blocks/file size reported by stat should not change
        xfs_io -c "dedupe $MNT/bar 0 0 $file_size" $MNT/foo > /dev/null  &
        dedupe_pid=$!
        while [ -n "$(ps -p $dedupe_pid -o pid=)" ]; do
            used=$(stat -c %b $MNT/foo)
            if [ $used -ne $expected ]; then
                echo "Unexpected blocks used: $used (expected: $expected)"
            fi
        done
      
        umount $DEV
      
        $ ./reproducer-3.sh
        Unexpected blocks used: 2076800 (expected: 2097152)
        Unexpected blocks used: 2097024 (expected: 2097152)
        Unexpected blocks used: 2079872 (expected: 2097152)
        (...)
      
      Note that since this is a short time window where the race can happen, the
      reproducer may not be able to always trigger the bug in one run, or it may
      trigger it multiple times.
      
      So fix this by:
      
      1) Making btrfs_drop_extents() not decrement the VFS inode's number of
         bytes, and instead return the number of bytes;
      
      2) Making any code that drops extents and adds new extents update the
         inode's number of bytes atomically, while holding the btrfs inode's
         spinlock, which is also used by the stat(2) callback to get the inode's
         number of bytes;
      
      3) For ranges in the inode's iotree that are marked as 'delalloc new',
         corresponding to previously unallocated ranges, increment the inode's
         number of bytes when clearing the 'delalloc new' bit from the range,
         in the same critical section that decrements the inode's
         'new_delalloc_bytes' counter, delimited by the btrfs inode's spinlock.
      
      An alternative would be to have btrfs_getattr() wait for any IO (ordered
      extents in progress) and locking the whole range (0 to (u64)-1) while it
      it computes the number of blocks used. But that would mean blocking
      stat(2), which is a very used syscall and expected to be fast, waiting
      for writes, clone/dedupe, fallocate, page reads, fiemap, etc.
      
      CC: stable@vger.kernel.org # 5.4+
      Reviewed-by: NJosef Bacik <josef@toxicpanda.com>
      Signed-off-by: NFilipe Manana <fdmanana@suse.com>
      Reviewed-by: NDavid Sterba <dsterba@suse.com>
      Signed-off-by: NDavid Sterba <dsterba@suse.com>
      2766ff61