Skip to content
  1. Jun 28, 2018
    • Filipe Manana's avatar
      Btrfs: fix mount failure when qgroup rescan is in progress · e4e7ede7
      Filipe Manana authored 
      If a power failure happens while the qgroup rescan kthread is running,
the next mount operation will always fail. This is because of a recent
regression that makes qgroup_rescan_init() incorrectly return -EINVAL
when we are mounting the filesystem (through btrfs_read_qgroup_config()).
This causes the -EINVAL error to be returned regardless of any qgroup
flags being set instead of returning the error only when neither of
the flags BTRFS_QGROUP_STATUS_FLAG_RESCAN nor BTRFS_QGROUP_STATUS_FLAG_ON
are set.

A test case for fstests follows up soon.

Fixes: 9593bf49

 ("btrfs: qgroup: show more meaningful qgroup_rescan_init error message")
Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
Reviewed-by: default avatarQu Wenruo <wqu@suse.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
      e4e7ede7
    • 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
  3. Jun 22, 2018
    • 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
    • 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
  5. Jun 11, 2018
    • 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
  7. Jun 07, 2018
    • 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
  9. Jun 05, 2018
  11. May 31, 2018
  13. May 30, 2018