On Wed, Jun 20, 2018 at 3:55 AM, robbieko <robbieko@xxxxxxxxxxxx> wrote:
> fdmanana@xxxxxxxxxx 於 2018-06-19 19:31 寫到:
>
>> From: Filipe Manana <fdmanana@xxxxxxxx>
>>
>> Commit 9d311e11fc1f ("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. 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 inline extent.
>>
>> Fixes: 9d311e11fc1f ("Btrfs: fiemap: pass correct bytenr when
>> fm_extent_count is zero")
>> Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
>> ---
>> fs/btrfs/extent_io.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index 8e4a7cdbc9f5..978327d98fc5 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -4559,6 +4559,7 @@ int extent_fiemap(struct inode *inode, struct
>> fiemap_extent_info *fieinfo,
>> end = 1;
>> flags |= FIEMAP_EXTENT_LAST;
>> } else if (em->block_start == EXTENT_MAP_INLINE) {
>> + disko = 0;
>> flags |= (FIEMAP_EXTENT_DATA_INLINE |
>> FIEMAP_EXTENT_NOT_ALIGNED);
>> } else if (em->block_start == EXTENT_MAP_DELALLOC) {
>
>
>
> EXTENT_MAP_DELALLOC should have the same problem.
>
> em->block_start has some special values. The following values should not be
> considered disko
> #define EXTENT_MAP_LAST_BYTE((u64)-4)
> #define EXTENT_MAP_HOLE((u64)-3)
> #define EXTENT_MAP_INLINE((u64)-2)
> #define EXTENT_MAP_DELALLOC((u64)-1)
>
> Is the following change more suitable?
> if (em->block_start >= EXTENT_MAP_LAST_BYTE)
> disko = 0;
> else
> disko = em->block_start + offset_in_extent;
Yes, I was thinking about it yesterday's evening regarding at least
holes/delalloc and leaving it for today's morning after leaving some
tests running during the evening.
>
> Thanks.
> Robbie Ko
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
