1. 28 Jun, 2018 2 commits
    • Chris Mason's avatar
      Btrfs: fix regression in btrfs_page_mkwrite() from vm_fault_t conversion · 717beb96
      Chris Mason authored
      The vm_fault_t conversion commit introduced a ret2 variable for tracking
      the integer return values from internal btrfs functions.  It was
      sometimes returning VM_FAULT_LOCKED for pages that were actually invalid
      and had been removed from the radix.  Something like this:
      
          ret2 = btrfs_delalloc_reserve_space() // returns zero on success
      
          lock_page(page)
          if (page->mapping != inode->i_mapping)
      	goto out_unlock;
      
      ...
      
      out_unlock:
          if (!ret2) {
      	    ...
      	    return VM_FAULT_LOCKED;
          }
      
      This ends up triggering this WARNING in btrfs_destroy_inode()
          WARN_ON(BTRFS_I(inode)->block_rsv.size);
      
      xfstests generic/095 was able to reliably reproduce the errors.
      
      Since out_unlock: is only used for errors, this fix moves it below the
      if (!ret2) check we use to return VM_FAULT_LOCKED for success.
      
      Fixes: a528a241 (btrfs: change return type of btrfs_page_mkwrite to vm_fault_t)
      Signed-off-by: default avatarChris Mason <clm@fb.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      717beb96
    • Qu Wenruo's avatar
      btrfs: quota: Set rescan progress to (u64)-1 if we hit last leaf · 6f7de19e
      Qu Wenruo authored
      Commit ff3d27a0 ("btrfs: qgroup: Finish rescan when hit the last leaf
      of extent tree") added a new exit for rescan finish.
      
      However after finishing quota rescan, we set
      fs_info->qgroup_rescan_progress to (u64)-1 before we exit through the
      original exit path.
      While we missed that assignment of (u64)-1 in the new exit path.
      
      The end result is, the quota status item doesn't have the same value.
      (-1 vs the last bytenr + 1)
      Although it doesn't affect quota accounting, it's still better to keep
      the original behavior.
      Reported-by: default avatarMisono Tomohiro <misono.tomohiro@jp.fujitsu.com>
      Fixes: ff3d27a0 ("btrfs: qgroup: Finish rescan when hit the last leaf of extent tree")
      Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
      Reviewed-by: default avatarMisono Tomohiro <misono.tomohiro@jp.fujitsu.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      6f7de19e
  2. 22 Jun, 2018 1 commit
    • Filipe Manana's avatar
      Btrfs: fix return value on rename exchange failure · c5b4a50b
      Filipe Manana authored
      If we failed during a rename exchange operation after starting/joining a
      transaction, we would end up replacing the return value, stored in the
      local 'ret' variable, with the return value from btrfs_end_transaction().
      So this could end up returning 0 (success) to user space despite the
      operation having failed and aborted the transaction, because if there are
      multiple tasks having a reference on the transaction at the time
      btrfs_end_transaction() is called by the rename exchange, that function
      returns 0 (otherwise it returns -EIO and not the original error value).
      So fix this by not overwriting the return value on error after getting
      a transaction handle.
      
      Fixes: cdd1fedf ("btrfs: add support for RENAME_EXCHANGE and RENAME_WHITEOUT")
      CC: stable@vger.kernel.org # 4.9+
      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>
      c5b4a50b
  3. 21 Jun, 2018 2 commits
    • Lu Fengqi's avatar
      btrfs: fix invalid-free in btrfs_extent_same · 22883ddc
      Lu Fengqi authored
      If this condition ((BTRFS_I(src)->flags & BTRFS_INODE_NODATASUM) !=
      		   (BTRFS_I(dst)->flags & BTRFS_INODE_NODATASUM))
      is hit, we will go to free the uninitialized cmp.src_pages and
      cmp.dst_pages.
      
      Fixes: 67b07bd4 ("Btrfs: reuse cmp workspace in EXTENT_SAME ioctl")
      Signed-off-by: default avatarLu Fengqi <lufq.fnst@cn.fujitsu.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      22883ddc
    • Filipe Manana's avatar
      Btrfs: fix physical offset reported by fiemap for inline extents · f0986318
      Filipe Manana authored
      Commit 9d311e11 ("Btrfs: fiemap: pass correct bytenr when
      fm_extent_count is zero") introduced a regression where we no longer
      report 0 as the physical offset for inline extents (and other extents
      with a special block_start value). This is because it always sets the
      variable used to report the physical offset ("disko") as em->block_start
      plus some offset, and em->block_start has the value 18446744073709551614
      ((u64) -2) for inline extents.
      
      This made the btrfs test 004 (from fstests) often fail, for example, for
      a file with an inline extent we have the following items in the subvolume
      tree:
      
          item 101 key (418 INODE_ITEM 0) itemoff 11029 itemsize 160
                 generation 25 transid 38 size 1525 nbytes 1525
                 block group 0 mode 100666 links 1 uid 0 gid 0 rdev 0
                 sequence 0 flags 0x2(none)
                 atime 1529342058.461891730 (2018-06-18 18:14:18)
                 ctime 1529342058.461891730 (2018-06-18 18:14:18)
                 mtime 1529342058.461891730 (2018-06-18 18:14:18)
                 otime 1529342055.869892885 (2018-06-18 18:14:15)
          item 102 key (418 INODE_REF 264) itemoff 11016 itemsize 13
                 index 25 namelen 3 name: fc7
          item 103 key (418 EXTENT_DATA 0) itemoff 9470 itemsize 1546
                 generation 38 type 0 (inline)
                 inline extent data size 1525 ram_bytes 1525 compression 0 (none)
      
      Then when test 004 invoked fiemap against the file it got a non-zero
      physical offset:
      
       $ filefrag -v /mnt/p0/d4/d7/fc7
       Filesystem type is: 9123683e
       File size of /mnt/p0/d4/d7/fc7 is 1525 (1 block of 4096 bytes)
        ext:     logical_offset:        physical_offset: length:   expected: flags:
          0:        0..    4095: 18446744073709551614..      4093:   4096:             last,not_aligned,inline,eof
       /mnt/p0/d4/d7/fc7: 1 extent found
      
      This resulted in the test failing like this:
      
      btrfs/004 49s ... [failed, exit status 1]- output mismatch (see /home/fdmanana/git/hub/xfstests/results//btrfs/004.out.bad)
          --- tests/btrfs/004.out	2016-08-23 10:17:35.027012095 +0100
          +++ /home/fdmanana/git/hub/xfstests/results//btrfs/004.out.bad	2018-06-18 18:15:02.385872155 +0100
          @@ -1,3 +1,10 @@
           QA output created by 004
           *** test backref walking
          -*** done
          +./tests/btrfs/004: line 227: [: 7.55578637259143e+22: integer expression expected
          +ERROR: 7.55578637259143e+22 is not a valid numeric value.
          +unexpected output from
          +	/home/fdmanana/git/hub/btrfs-progs/btrfs inspect-internal logical-resolve -s 65536 -P 7.55578637259143e+22 /home/fdmanana/btrfs-tests/scratch_1
          ...
          (Run 'diff -u tests/btrfs/004.out /home/fdmanana/git/hub/xfstests/results//btrfs/004.out.bad'  to see the entire diff)
      Ran: btrfs/004
      
      The large number in scientific notation reported as an invalid numeric
      value is the result from the filter passed to perl which multiplies the
      physical offset by the block size reported by fiemap.
      
      So fix this by ensuring the physical offset is always set to 0 when we
      are processing an extent with a special block_start value.
      
      Fixes: 9d311e11 ("Btrfs: fiemap: pass correct bytenr when fm_extent_count is zero")
      Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      f0986318
  4. 11 Jun, 2018 1 commit
    • Qu Wenruo's avatar
      btrfs: scrub: Don't use inode pages for device replace · ac0b4145
      Qu Wenruo authored
      [BUG]
      Btrfs can create compressed extent without checksum (even though it
      shouldn't), and if we then try to replace device containing such extent,
      the result device will contain all the uncompressed data instead of the
      compressed one.
      
      Test case already submitted to fstests:
      https://patchwork.kernel.org/patch/10442353/
      
      [CAUSE]
      When handling compressed extent without checksum, device replace will
      goe into copy_nocow_pages() function.
      
      In that function, btrfs will get all inodes referring to this data
      extents and then use find_or_create_page() to get pages direct from that
      inode.
      
      The problem here is, pages directly from inode are always uncompressed.
      And for compressed data extent, they mismatch with on-disk data.
      Thus this leads to corrupted compressed data extent written to replace
      device.
      
      [FIX]
      In this attempt, we could just remove the "optimization" branch, and let
      unified scrub_pages() to handle it.
      
      Although scrub_pages() won't bother reusing page cache, it will be a
      little slower, but it does the correct csum checking and won't cause
      such data corruption caused by "optimization".
      
      Note about the fix: this is the minimal fix that can be backported to
      older stable trees without conflicts. The whole callchain from
      copy_nocow_pages() can be deleted, and will be in followup patches.
      
      Fixes: ff023aac ("Btrfs: add code to scrub to copy read data to another disk")
      CC: stable@vger.kernel.org # 4.4+
      Reported-by: default avatarJames Harvey <jamespharvey20@gmail.com>
      Reviewed-by: default avatarJames Harvey <jamespharvey20@gmail.com>
      Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
      [ remove code removal, add note why ]
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      ac0b4145
  5. 07 Jun, 2018 2 commits
    • Souptick Joarder's avatar
      btrfs: change return type of btrfs_page_mkwrite to vm_fault_t · a528a241
      Souptick Joarder authored
      Use the new return type vm_fault_t for fault handler. For now, this is
      just documenting that the function returns a VM_FAULT value rather than
      an errno. Once all instances are converted, vm_fault_t will become a
      distinct type.
      
      Reference commit 1c8f4220 ("mm: change return type to vm_fault_t")
      
      vmf_error() is the newly introduced inline function in 4.17-rc6.
      Signed-off-by: default avatarSouptick Joarder <jrdr.linux@gmail.com>
      Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      a528a241
    • Robbie Ko's avatar
      Btrfs: fiemap: pass correct bytenr when fm_extent_count is zero · 9d311e11
      Robbie Ko authored
      [BUG]
      fm_mapped_extents is not correct when fm_extent_count is 0
      Like:
         # mount /dev/vdb5 /mnt/btrfs
         # dd if=/dev/zero bs=16K count=4 oflag=dsync of=/mnt/btrfs/file
         # xfs_io -c "fiemap -v" /mnt/btrfs/file
         /mnt/btrfs/file:
         EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
           0: [0..127]:        25088..25215       128   0x1
      
      When user space wants to get the number of file extents,
      set fm_extent_count to 0 to run fiemap and then read fm_mapped_extents.
      
      In the above example, fiemap will return with fm_mapped_extents set to 4,
      but it should be 1 since there's only one entry in the output.
      
      [REASON]
      The problem seems to be that disko is only set if
      fieinfo->fi_extents_max is set. And this member is initialized, in the
      generic ioctl_fiemap function, to the value of used-passed
      fm_extent_count. So when the user passes 0 then fi_extent_max is also
      set to zero and this causes btrfs to not initialize disko at all.
      Eventually this leads emit_fiemap_extent being called with a bogus
      'phys' argument preventing proper fiemap entries merging.
      
      [FIX]
      Move the disko initialization earlier in extent_fiemap making it
      independent of user-passed arguments, allowing emit_fiemap_extent to
      properly handle consecutive extent entries.
      Signed-off-by: default avatarRobbie Ko <robbieko@synology.com>
      Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      9d311e11
  6. 05 Jun, 2018 1 commit
  7. 31 May, 2018 3 commits
  8. 30 May, 2018 24 commits
  9. 29 May, 2018 4 commits