Re: [PATCH 2/2] Btrfs: fix fiemap extent SHARED flag error with range clone.

2018-03-07 Thread Qu Wenruo 


On 2018年03月07日 20:17, Nikolay Borisov wrote:
> 
> 
> On  7.03.2018 14:14, Qu Wenruo wrote:
>>
>>
> 
> 
> 

 SHARED flag is determined after extent map merge, so here we can't rely
 on em here.
>>>
>>> Shouldn't extent maps correspond to 1:1 disk-state. I.e. they are just
>>> the memory cache of the extent state. So if we merge them, shouldn't we
>>> also merge the on-disk extents as well ?
>>
>> Not 1:1.
>>
>> In memory one is merged maybe to save memory.
>>
>> But on-disk file extents has size limit.
>> For compressed one it's 128K and 128M for uncompressed one.
> 
> Fair enough, however 4 extents, 16k each should warrant merging on-disk
> as well, no ?

dd oflag=dsync
That ensures we won't merge on-disk extents.

Thanks,
Qu

> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 2/2] Btrfs: fix fiemap extent SHARED flag error with range clone.

2018-03-07 Thread Nikolay Borisov 


On  7.03.2018 14:14, Qu Wenruo wrote:
> 
> 



>>>
>>> SHARED flag is determined after extent map merge, so here we can't rely
>>> on em here.
>>
>> Shouldn't extent maps correspond to 1:1 disk-state. I.e. they are just
>> the memory cache of the extent state. So if we merge them, shouldn't we
>> also merge the on-disk extents as well ?
> 
> Not 1:1.
> 
> In memory one is merged maybe to save memory.
> 
> But on-disk file extents has size limit.
> For compressed one it's 128K and 128M for uncompressed one.

Fair enough, however 4 extents, 16k each should warrant merging on-disk
as well, no ?


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] Btrfs: fix fiemap extent SHARED flag error with range clone.

2018-03-07 Thread Qu Wenruo 


On 2018年03月07日 19:27, Nikolay Borisov wrote:
> 
> 
> On  7.03.2018 13:18, Qu Wenruo wrote:
>>
>>
>> On 2018年03月07日 19:01, robbieko wrote:
>>> Qu Wenruo 於 2018-03-07 18:42 寫到:
 On 2018年03月07日 18:33, Qu Wenruo wrote:
>
>
> On 2018年03月07日 16:20, robbieko wrote:
>> From: Robbie Ko 
>>
>> [BUG]
>> Range clone can cause fiemap to return error result.
>> Like:
>>  # mount /dev/vdb5 /mnt/btrfs
>>  # dd if=ev/zero bsK count=2 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..63]: 4241424..4241487    64   0x1
>>
>>  # cloner -s $((16*1024)) /mnt/btrfs/file /mnt/btrfs/file_clone
>>  # xfs_io -c "fiemap -v" /mnt/btrfs/file
>>  /mnt/btrfs/file:
>>  EXT: FILE-OFFSET  BLOCK-RANGE  TOTAL FLAGS
>>    0: [0..63]: 4241424..4241487    64   0x1
>>  If we clone second file extent, we will get error FLAGS,
>>  SHARED bit is not set.
>>
>> [REASON]
>> Btrfs only checks if the first extent in extent map is shared,
>> but extent may merge.
>>
>> [FIX]
>> Here we will check each extent with extent map range,
>> if one of them is shared, extent map is shared.
>>
>> [PATCH RESULT]
>>  # xfs_io -c "fiemap -v" /mnt/btrfs/file
>>  /mnt/btrfs/file:
>>  EXT: FILE-OFFSET  BLOCK-RANGE  TOTAL FLAGS
>>    0: [0..63]: 4241424..4241487    64 0x2001
>>
>> Signed-off-by: Robbie Ko 
>> ---
>>  fs/btrfs/extent_io.c | 146
>> +--
>>  1 file changed, 131 insertions(+), 15 deletions(-)
>>
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index 066b6df..5c6dca9 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -4394,8 +4394,8 @@ static int emit_fiemap_extent(struct
>> fiemap_extent_info *fieinfo,
>>   */
>>  if (cache->offset + cache->len  =offset &&
>>  cache->phys + cache->len =phys  &&
>> -    (cache->flags & ~FIEMAP_EXTENT_LAST) => -    (flags 
>> & ~FIEMAP_EXTENT_LAST)) {
>> +    (cache->flags & ~(FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_SHARED)) 
>> => +    (flags & 
>> ~(FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_SHARED))) {
>>  cache->len +=en;
>>  cache->flags |=lags;
>>  goto try_submit_last;
>> @@ -4450,6 +4450,134 @@ static int emit_last_fiemap_cache(struct
>> btrfs_fs_info *fs_info,
>>  return ret;
>>  }
>>
>> +/*
>> + * Helper to check the file range is shared.
>> + *
>> + * Fiemap extent will be combined with many extents, so we need to
>> examine
>> + * each extent, and if shared, the results are shared.
>> + *
>> + * Return: 0 if file range is not shared, 1 if it is shared, < 0 on
>> error.
>> + */
>> +static int extent_map_check_shared(struct inode *inode, u64 start,
>> u64 end)
>> +{
>> +    struct btrfs_fs_info *fs_info =trfs_sb(inode->i_sb);
>> +    struct btrfs_root *root =TRFS_I(inode)->root;
>> +    int ret =;
>> +    struct extent_buffer *leaf;
>> +    struct btrfs_path *path;
>> +    struct btrfs_file_extent_item *fi;
>> +    struct btrfs_key found_key;
>> +    int check_prev =;
>> +    int extent_type;
>> +    int shared =;
>> +    u64 cur_offset;
>> +    u64 extent_end;
>> +    u64 ino =trfs_ino(BTRFS_I(inode));
>> +    u64 disk_bytenr;
>> +
>> +    path =trfs_alloc_path();
>> +    if (!path) {
>> +    return -ENOMEM;
>> +    }
>> +
>> +    cur_offset =tart;
>> +    while (1) {
>> +    ret =trfs_lookup_file_extent(NULL, root, path, ino,
>> +   cur_offset, 0);
>> +    if (ret < 0)
>> +    goto error;
>> +    if (ret > 0 && path->slots[0] > 0 && check_prev) {
>> +    leaf =ath->nodes[0];
>> +    btrfs_item_key_to_cpu(leaf, &found_key,
>> +  path->slots[0] - 1);
>> +    if (found_key.objectid =ino &&
>> +    found_key.type =BTRFS_EXTENT_DATA_KEY)
>> +    path->slots[0]--;
>> +    }
>> +    check_prev =;
>> +next_slot:
>> +    leaf =ath->nodes[0];
>> +    if (path->slots[0] >=trfs_header_nritems(leaf)) {
>> +    ret =trfs_next_leaf(root, path);
>> +    if (ret < 0)
>> +    goto error;
>> +    if (ret > 0)
>> +    break;
>> +    leaf =ath->nodes[0];
>> +    }
>> +
>> +    disk_bytenr =;
>> +    btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
>> +
>> +    if (found_key.objectid > ino)
>> +    break;
>> + 

Re: [PATCH 2/2] Btrfs: fix fiemap extent SHARED flag error with range clone.

2018-03-07 Thread Nikolay Borisov 


On  7.03.2018 13:18, Qu Wenruo wrote:
> 
> 
> On 2018年03月07日 19:01, robbieko wrote:
>> Qu Wenruo 於 2018-03-07 18:42 寫到:
>>> On 2018年03月07日 18:33, Qu Wenruo wrote:


 On 2018年03月07日 16:20, robbieko wrote:
> From: Robbie Ko 
>
> [BUG]
> Range clone can cause fiemap to return error result.
> Like:
>  # mount /dev/vdb5 /mnt/btrfs
>  # dd if=/dev/zero bs=16K count=2 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..63]: 4241424..4241487    64   0x1
>
>  # cloner -s $((16*1024)) /mnt/btrfs/file /mnt/btrfs/file_clone
>  # xfs_io -c "fiemap -v" /mnt/btrfs/file
>  /mnt/btrfs/file:
>  EXT: FILE-OFFSET  BLOCK-RANGE  TOTAL FLAGS
>    0: [0..63]: 4241424..4241487    64   0x1
>  If we clone second file extent, we will get error FLAGS,
>  SHARED bit is not set.
>
> [REASON]
> Btrfs only checks if the first extent in extent map is shared,
> but extent may merge.
>
> [FIX]
> Here we will check each extent with extent map range,
> if one of them is shared, extent map is shared.
>
> [PATCH RESULT]
>  # xfs_io -c "fiemap -v" /mnt/btrfs/file
>  /mnt/btrfs/file:
>  EXT: FILE-OFFSET  BLOCK-RANGE  TOTAL FLAGS
>    0: [0..63]: 4241424..4241487    64 0x2001
>
> Signed-off-by: Robbie Ko 
> ---
>  fs/btrfs/extent_io.c | 146
> +--
>  1 file changed, 131 insertions(+), 15 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 066b6df..5c6dca9 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4394,8 +4394,8 @@ static int emit_fiemap_extent(struct
> fiemap_extent_info *fieinfo,
>   */
>  if (cache->offset + cache->len  == offset &&
>  cache->phys + cache->len == phys  &&
> -    (cache->flags & ~FIEMAP_EXTENT_LAST) ==
> -    (flags & ~FIEMAP_EXTENT_LAST)) {
> +    (cache->flags & ~(FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_SHARED)) ==
> +    (flags & ~(FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_SHARED))) {
>  cache->len += len;
>  cache->flags |= flags;
>  goto try_submit_last;
> @@ -4450,6 +4450,134 @@ static int emit_last_fiemap_cache(struct
> btrfs_fs_info *fs_info,
>  return ret;
>  }
>
> +/*
> + * Helper to check the file range is shared.
> + *
> + * Fiemap extent will be combined with many extents, so we need to
> examine
> + * each extent, and if shared, the results are shared.
> + *
> + * Return: 0 if file range is not shared, 1 if it is shared, < 0 on
> error.
> + */
> +static int extent_map_check_shared(struct inode *inode, u64 start,
> u64 end)
> +{
> +    struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> +    struct btrfs_root *root = BTRFS_I(inode)->root;
> +    int ret = 0;
> +    struct extent_buffer *leaf;
> +    struct btrfs_path *path;
> +    struct btrfs_file_extent_item *fi;
> +    struct btrfs_key found_key;
> +    int check_prev = 1;
> +    int extent_type;
> +    int shared = 0;
> +    u64 cur_offset;
> +    u64 extent_end;
> +    u64 ino = btrfs_ino(BTRFS_I(inode));
> +    u64 disk_bytenr;
> +
> +    path = btrfs_alloc_path();
> +    if (!path) {
> +    return -ENOMEM;
> +    }
> +
> +    cur_offset = start;
> +    while (1) {
> +    ret = btrfs_lookup_file_extent(NULL, root, path, ino,
> +   cur_offset, 0);
> +    if (ret < 0)
> +    goto error;
> +    if (ret > 0 && path->slots[0] > 0 && check_prev) {
> +    leaf = path->nodes[0];
> +    btrfs_item_key_to_cpu(leaf, &found_key,
> +  path->slots[0] - 1);
> +    if (found_key.objectid == ino &&
> +    found_key.type == BTRFS_EXTENT_DATA_KEY)
> +    path->slots[0]--;
> +    }
> +    check_prev = 0;
> +next_slot:
> +    leaf = path->nodes[0];
> +    if (path->slots[0] >= btrfs_header_nritems(leaf)) {
> +    ret = btrfs_next_leaf(root, path);
> +    if (ret < 0)
> +    goto error;
> +    if (ret > 0)
> +    break;
> +    leaf = path->nodes[0];
> +    }
> +
> +    disk_bytenr = 0;
> +    btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
> +
> +    if (found_key.objectid > ino)
> +    break;
> +    if (WARN_ON_ONCE(found_key.objectid < ino) ||
> +    found_key.type < BTRFS_EXTENT_DATA_KEY) {
> +    path->slots[0]++;

Re: [PATCH 2/2] Btrfs: fix fiemap extent SHARED flag error with range clone.

2018-03-07 Thread Qu Wenruo 


On 2018年03月07日 19:01, robbieko wrote:
> Qu Wenruo 於 2018-03-07 18:42 寫到:
>> On 2018年03月07日 18:33, Qu Wenruo wrote:
>>>
>>>
>>> On 2018年03月07日 16:20, robbieko wrote:
 From: Robbie Ko 

 [BUG]
 Range clone can cause fiemap to return error result.
 Like:
  # mount /dev/vdb5 /mnt/btrfs
  # dd if=/dev/zero bs=16K count=2 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..63]: 4241424..4241487    64   0x1

  # cloner -s $((16*1024)) /mnt/btrfs/file /mnt/btrfs/file_clone
  # xfs_io -c "fiemap -v" /mnt/btrfs/file
  /mnt/btrfs/file:
  EXT: FILE-OFFSET  BLOCK-RANGE  TOTAL FLAGS
    0: [0..63]: 4241424..4241487    64   0x1
  If we clone second file extent, we will get error FLAGS,
  SHARED bit is not set.

 [REASON]
 Btrfs only checks if the first extent in extent map is shared,
 but extent may merge.

 [FIX]
 Here we will check each extent with extent map range,
 if one of them is shared, extent map is shared.

 [PATCH RESULT]
  # xfs_io -c "fiemap -v" /mnt/btrfs/file
  /mnt/btrfs/file:
  EXT: FILE-OFFSET  BLOCK-RANGE  TOTAL FLAGS
    0: [0..63]: 4241424..4241487    64 0x2001

 Signed-off-by: Robbie Ko 
 ---
  fs/btrfs/extent_io.c | 146
 +--
  1 file changed, 131 insertions(+), 15 deletions(-)

 diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
 index 066b6df..5c6dca9 100644
 --- a/fs/btrfs/extent_io.c
 +++ b/fs/btrfs/extent_io.c
 @@ -4394,8 +4394,8 @@ static int emit_fiemap_extent(struct
 fiemap_extent_info *fieinfo,
   */
  if (cache->offset + cache->len  == offset &&
  cache->phys + cache->len == phys  &&
 -    (cache->flags & ~FIEMAP_EXTENT_LAST) ==
 -    (flags & ~FIEMAP_EXTENT_LAST)) {
 +    (cache->flags & ~(FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_SHARED)) ==
 +    (flags & ~(FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_SHARED))) {
  cache->len += len;
  cache->flags |= flags;
  goto try_submit_last;
 @@ -4450,6 +4450,134 @@ static int emit_last_fiemap_cache(struct
 btrfs_fs_info *fs_info,
  return ret;
  }

 +/*
 + * Helper to check the file range is shared.
 + *
 + * Fiemap extent will be combined with many extents, so we need to
 examine
 + * each extent, and if shared, the results are shared.
 + *
 + * Return: 0 if file range is not shared, 1 if it is shared, < 0 on
 error.
 + */
 +static int extent_map_check_shared(struct inode *inode, u64 start,
 u64 end)
 +{
 +    struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 +    struct btrfs_root *root = BTRFS_I(inode)->root;
 +    int ret = 0;
 +    struct extent_buffer *leaf;
 +    struct btrfs_path *path;
 +    struct btrfs_file_extent_item *fi;
 +    struct btrfs_key found_key;
 +    int check_prev = 1;
 +    int extent_type;
 +    int shared = 0;
 +    u64 cur_offset;
 +    u64 extent_end;
 +    u64 ino = btrfs_ino(BTRFS_I(inode));
 +    u64 disk_bytenr;
 +
 +    path = btrfs_alloc_path();
 +    if (!path) {
 +    return -ENOMEM;
 +    }
 +
 +    cur_offset = start;
 +    while (1) {
 +    ret = btrfs_lookup_file_extent(NULL, root, path, ino,
 +   cur_offset, 0);
 +    if (ret < 0)
 +    goto error;
 +    if (ret > 0 && path->slots[0] > 0 && check_prev) {
 +    leaf = path->nodes[0];
 +    btrfs_item_key_to_cpu(leaf, &found_key,
 +  path->slots[0] - 1);
 +    if (found_key.objectid == ino &&
 +    found_key.type == BTRFS_EXTENT_DATA_KEY)
 +    path->slots[0]--;
 +    }
 +    check_prev = 0;
 +next_slot:
 +    leaf = path->nodes[0];
 +    if (path->slots[0] >= btrfs_header_nritems(leaf)) {
 +    ret = btrfs_next_leaf(root, path);
 +    if (ret < 0)
 +    goto error;
 +    if (ret > 0)
 +    break;
 +    leaf = path->nodes[0];
 +    }
 +
 +    disk_bytenr = 0;
 +    btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
 +
 +    if (found_key.objectid > ino)
 +    break;
 +    if (WARN_ON_ONCE(found_key.objectid < ino) ||
 +    found_key.type < BTRFS_EXTENT_DATA_KEY) {
 +    path->slots[0]++;
 +    goto next_slot;
 +    }
 +    if (found_key.type > BTRFS_EXTENT_DATA_KEY ||
 +    found_key.offset > end)
 +    break;
 +

Re: [PATCH 2/2] Btrfs: fix fiemap extent SHARED flag error with range clone.

2018-03-07 Thread robbieko 

Qu Wenruo 於 2018-03-07 18:42 寫到:

On 2018年03月07日 18:33, Qu Wenruo wrote:



On 2018年03月07日 16:20, robbieko wrote:

From: Robbie Ko 

[BUG]
Range clone can cause fiemap to return error result.
Like:
 # mount /dev/vdb5 /mnt/btrfs
 # dd if=/dev/zero bs=16K count=2 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..63]: 4241424..424148764   0x1

 # cloner -s $((16*1024)) /mnt/btrfs/file /mnt/btrfs/file_clone
 # xfs_io -c "fiemap -v" /mnt/btrfs/file
 /mnt/btrfs/file:
 EXT: FILE-OFFSET  BLOCK-RANGE  TOTAL FLAGS
   0: [0..63]: 4241424..424148764   0x1
 If we clone second file extent, we will get error FLAGS,
 SHARED bit is not set.

[REASON]
Btrfs only checks if the first extent in extent map is shared,
but extent may merge.

[FIX]
Here we will check each extent with extent map range,
if one of them is shared, extent map is shared.

[PATCH RESULT]
 # xfs_io -c "fiemap -v" /mnt/btrfs/file
 /mnt/btrfs/file:
 EXT: FILE-OFFSET  BLOCK-RANGE  TOTAL FLAGS
   0: [0..63]: 4241424..424148764 0x2001

Signed-off-by: Robbie Ko 
---
 fs/btrfs/extent_io.c | 146 
+--

 1 file changed, 131 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 066b6df..5c6dca9 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4394,8 +4394,8 @@ static int emit_fiemap_extent(struct 
fiemap_extent_info *fieinfo,

 */
if (cache->offset + cache->len  == offset &&
cache->phys + cache->len == phys  &&
-   (cache->flags & ~FIEMAP_EXTENT_LAST) ==
-   (flags & ~FIEMAP_EXTENT_LAST)) {
+   (cache->flags & ~(FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_SHARED)) ==
+   (flags & ~(FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_SHARED))) {
cache->len += len;
cache->flags |= flags;
goto try_submit_last;
@@ -4450,6 +4450,134 @@ static int emit_last_fiemap_cache(struct 
btrfs_fs_info *fs_info,

return ret;
 }

+/*
+ * Helper to check the file range is shared.
+ *
+ * Fiemap extent will be combined with many extents, so we need to 
examine

+ * each extent, and if shared, the results are shared.
+ *
+ * Return: 0 if file range is not shared, 1 if it is shared, < 0 on 
error.

+ */
+static int extent_map_check_shared(struct inode *inode, u64 start, 
u64 end)

+{
+   struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
+   struct btrfs_root *root = BTRFS_I(inode)->root;
+   int ret = 0;
+   struct extent_buffer *leaf;
+   struct btrfs_path *path;
+   struct btrfs_file_extent_item *fi;
+   struct btrfs_key found_key;
+   int check_prev = 1;
+   int extent_type;
+   int shared = 0;
+   u64 cur_offset;
+   u64 extent_end;
+   u64 ino = btrfs_ino(BTRFS_I(inode));
+   u64 disk_bytenr;
+
+   path = btrfs_alloc_path();
+   if (!path) {
+   return -ENOMEM;
+   }
+
+   cur_offset = start;
+   while (1) {
+   ret = btrfs_lookup_file_extent(NULL, root, path, ino,
+  cur_offset, 0);
+   if (ret < 0)
+   goto error;
+   if (ret > 0 && path->slots[0] > 0 && check_prev) {
+   leaf = path->nodes[0];
+   btrfs_item_key_to_cpu(leaf, &found_key,
+ path->slots[0] - 1);
+   if (found_key.objectid == ino &&
+   found_key.type == BTRFS_EXTENT_DATA_KEY)
+   path->slots[0]--;
+   }
+   check_prev = 0;
+next_slot:
+   leaf = path->nodes[0];
+   if (path->slots[0] >= btrfs_header_nritems(leaf)) {
+   ret = btrfs_next_leaf(root, path);
+   if (ret < 0)
+   goto error;
+   if (ret > 0)
+   break;
+   leaf = path->nodes[0];
+   }
+
+   disk_bytenr = 0;
+   btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
+
+   if (found_key.objectid > ino)
+   break;
+   if (WARN_ON_ONCE(found_key.objectid < ino) ||
+   found_key.type < BTRFS_EXTENT_DATA_KEY) {
+   path->slots[0]++;
+   goto next_slot;
+   }
+   if (found_key.type > BTRFS_EXTENT_DATA_KEY ||
+   found_key.offset > end)
+   break;
+
+   fi = btrfs_item_ptr(leaf, path->slots[0],
+   struct btrfs_file_extent_item);
+   extent_type = btrfs_file_extent_type(leaf, fi);
+
+   if (extent_type == BTRFS_

Re: [PATCH 2/2] Btrfs: fix fiemap extent SHARED flag error with range clone.

2018-03-07 Thread Qu Wenruo 


On 2018年03月07日 18:33, Qu Wenruo wrote:
> 
> 
> On 2018年03月07日 16:20, robbieko wrote:
>> From: Robbie Ko 
>>
>> [BUG]
>> Range clone can cause fiemap to return error result.
>> Like:
>>  # mount /dev/vdb5 /mnt/btrfs
>>  # dd if=/dev/zero bs=16K count=2 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..63]: 4241424..424148764   0x1
>>
>>  # cloner -s $((16*1024)) /mnt/btrfs/file /mnt/btrfs/file_clone
>>  # xfs_io -c "fiemap -v" /mnt/btrfs/file
>>  /mnt/btrfs/file:
>>  EXT: FILE-OFFSET  BLOCK-RANGE  TOTAL FLAGS
>>0: [0..63]: 4241424..424148764   0x1
>>  If we clone second file extent, we will get error FLAGS,
>>  SHARED bit is not set.
>>
>> [REASON]
>> Btrfs only checks if the first extent in extent map is shared,
>> but extent may merge.
>>
>> [FIX]
>> Here we will check each extent with extent map range,
>> if one of them is shared, extent map is shared.
>>
>> [PATCH RESULT]
>>  # xfs_io -c "fiemap -v" /mnt/btrfs/file
>>  /mnt/btrfs/file:
>>  EXT: FILE-OFFSET  BLOCK-RANGE  TOTAL FLAGS
>>0: [0..63]: 4241424..424148764 0x2001
>>
>> Signed-off-by: Robbie Ko 
>> ---
>>  fs/btrfs/extent_io.c | 146 
>> +--
>>  1 file changed, 131 insertions(+), 15 deletions(-)
>>
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index 066b6df..5c6dca9 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -4394,8 +4394,8 @@ static int emit_fiemap_extent(struct 
>> fiemap_extent_info *fieinfo,
>>   */
>>  if (cache->offset + cache->len  == offset &&
>>  cache->phys + cache->len == phys  &&
>> -(cache->flags & ~FIEMAP_EXTENT_LAST) ==
>> -(flags & ~FIEMAP_EXTENT_LAST)) {
>> +(cache->flags & ~(FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_SHARED)) ==
>> +(flags & ~(FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_SHARED))) {
>>  cache->len += len;
>>  cache->flags |= flags;
>>  goto try_submit_last;
>> @@ -4450,6 +4450,134 @@ static int emit_last_fiemap_cache(struct 
>> btrfs_fs_info *fs_info,
>>  return ret;
>>  }
>>
>> +/*
>> + * Helper to check the file range is shared.
>> + *
>> + * Fiemap extent will be combined with many extents, so we need to examine
>> + * each extent, and if shared, the results are shared.
>> + *
>> + * Return: 0 if file range is not shared, 1 if it is shared, < 0 on error.
>> + */
>> +static int extent_map_check_shared(struct inode *inode, u64 start, u64 end)
>> +{
>> +struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>> +struct btrfs_root *root = BTRFS_I(inode)->root;
>> +int ret = 0;
>> +struct extent_buffer *leaf;
>> +struct btrfs_path *path;
>> +struct btrfs_file_extent_item *fi;
>> +struct btrfs_key found_key;
>> +int check_prev = 1;
>> +int extent_type;
>> +int shared = 0;
>> +u64 cur_offset;
>> +u64 extent_end;
>> +u64 ino = btrfs_ino(BTRFS_I(inode));
>> +u64 disk_bytenr;
>> +
>> +path = btrfs_alloc_path();
>> +if (!path) {
>> +return -ENOMEM;
>> +}
>> +
>> +cur_offset = start;
>> +while (1) {
>> +ret = btrfs_lookup_file_extent(NULL, root, path, ino,
>> +   cur_offset, 0);
>> +if (ret < 0)
>> +goto error;
>> +if (ret > 0 && path->slots[0] > 0 && check_prev) {
>> +leaf = path->nodes[0];
>> +btrfs_item_key_to_cpu(leaf, &found_key,
>> +  path->slots[0] - 1);
>> +if (found_key.objectid == ino &&
>> +found_key.type == BTRFS_EXTENT_DATA_KEY)
>> +path->slots[0]--;
>> +}
>> +check_prev = 0;
>> +next_slot:
>> +leaf = path->nodes[0];
>> +if (path->slots[0] >= btrfs_header_nritems(leaf)) {
>> +ret = btrfs_next_leaf(root, path);
>> +if (ret < 0)
>> +goto error;
>> +if (ret > 0)
>> +break;
>> +leaf = path->nodes[0];
>> +}
>> +
>> +disk_bytenr = 0;
>> +btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
>> +
>> +if (found_key.objectid > ino)
>> +break;
>> +if (WARN_ON_ONCE(found_key.objectid < ino) ||
>> +found_key.type < BTRFS_EXTENT_DATA_KEY) {
>> +path->slots[0]++;
>> +goto next_slot;
>> +}
>> +if (found_key.type > BTRFS_EXTENT_DATA_KEY ||
>> +found_key.offset > end)
>> +break;
>> +
>> +fi = btrfs_item_ptr(leaf, path->slots[0],
>> +

Re: [PATCH 2/2] Btrfs: fix fiemap extent SHARED flag error with range clone.

2018-03-07 Thread Qu Wenruo 


On 2018年03月07日 16:20, robbieko wrote:
> From: Robbie Ko 
> 
> [BUG]
> Range clone can cause fiemap to return error result.
> Like:
>  # mount /dev/vdb5 /mnt/btrfs
>  # dd if=/dev/zero bs=16K count=2 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..63]: 4241424..424148764   0x1
> 
>  # cloner -s $((16*1024)) /mnt/btrfs/file /mnt/btrfs/file_clone
>  # xfs_io -c "fiemap -v" /mnt/btrfs/file
>  /mnt/btrfs/file:
>  EXT: FILE-OFFSET  BLOCK-RANGE  TOTAL FLAGS
>0: [0..63]: 4241424..424148764   0x1
>  If we clone second file extent, we will get error FLAGS,
>  SHARED bit is not set.
> 
> [REASON]
> Btrfs only checks if the first extent in extent map is shared,
> but extent may merge.
> 
> [FIX]
> Here we will check each extent with extent map range,
> if one of them is shared, extent map is shared.
> 
> [PATCH RESULT]
>  # xfs_io -c "fiemap -v" /mnt/btrfs/file
>  /mnt/btrfs/file:
>  EXT: FILE-OFFSET  BLOCK-RANGE  TOTAL FLAGS
>0: [0..63]: 4241424..424148764 0x2001
> 
> Signed-off-by: Robbie Ko 
> ---
>  fs/btrfs/extent_io.c | 146 
> +--
>  1 file changed, 131 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 066b6df..5c6dca9 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4394,8 +4394,8 @@ static int emit_fiemap_extent(struct fiemap_extent_info 
> *fieinfo,
>*/
>   if (cache->offset + cache->len  == offset &&
>   cache->phys + cache->len == phys  &&
> - (cache->flags & ~FIEMAP_EXTENT_LAST) ==
> - (flags & ~FIEMAP_EXTENT_LAST)) {
> + (cache->flags & ~(FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_SHARED)) ==
> + (flags & ~(FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_SHARED))) {
>   cache->len += len;
>   cache->flags |= flags;
>   goto try_submit_last;
> @@ -4450,6 +4450,134 @@ static int emit_last_fiemap_cache(struct 
> btrfs_fs_info *fs_info,
>   return ret;
>  }
> 
> +/*
> + * Helper to check the file range is shared.
> + *
> + * Fiemap extent will be combined with many extents, so we need to examine
> + * each extent, and if shared, the results are shared.
> + *
> + * Return: 0 if file range is not shared, 1 if it is shared, < 0 on error.
> + */
> +static int extent_map_check_shared(struct inode *inode, u64 start, u64 end)
> +{
> + struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> + struct btrfs_root *root = BTRFS_I(inode)->root;
> + int ret = 0;
> + struct extent_buffer *leaf;
> + struct btrfs_path *path;
> + struct btrfs_file_extent_item *fi;
> + struct btrfs_key found_key;
> + int check_prev = 1;
> + int extent_type;
> + int shared = 0;
> + u64 cur_offset;
> + u64 extent_end;
> + u64 ino = btrfs_ino(BTRFS_I(inode));
> + u64 disk_bytenr;
> +
> + path = btrfs_alloc_path();
> + if (!path) {
> + return -ENOMEM;
> + }
> +
> + cur_offset = start;
> + while (1) {
> + ret = btrfs_lookup_file_extent(NULL, root, path, ino,
> +cur_offset, 0);
> + if (ret < 0)
> + goto error;
> + if (ret > 0 && path->slots[0] > 0 && check_prev) {
> + leaf = path->nodes[0];
> + btrfs_item_key_to_cpu(leaf, &found_key,
> +   path->slots[0] - 1);
> + if (found_key.objectid == ino &&
> + found_key.type == BTRFS_EXTENT_DATA_KEY)
> + path->slots[0]--;
> + }
> + check_prev = 0;
> +next_slot:
> + leaf = path->nodes[0];
> + if (path->slots[0] >= btrfs_header_nritems(leaf)) {
> + ret = btrfs_next_leaf(root, path);
> + if (ret < 0)
> + goto error;
> + if (ret > 0)
> + break;
> + leaf = path->nodes[0];
> + }
> +
> + disk_bytenr = 0;
> + btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
> +
> + if (found_key.objectid > ino)
> + break;
> + if (WARN_ON_ONCE(found_key.objectid < ino) ||
> + found_key.type < BTRFS_EXTENT_DATA_KEY) {
> + path->slots[0]++;
> + goto next_slot;
> + }
> + if (found_key.type > BTRFS_EXTENT_DATA_KEY ||
> + found_key.offset > end)
> + break;
> +
> + fi = btrfs_item_ptr(leaf, path->slots[0],
> + struct btrfs_file_extent_item);
> + extent_type = btrfs_file_extent_type(leaf, f