All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] f2fs: change fiemap way in printing compression chunk
@ 2021-07-22 21:19 ` Daeho Jeong
  0 siblings, 0 replies; 8+ messages in thread
From: Daeho Jeong @ 2021-07-22 21:19 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong

From: Daeho Jeong <daehojeong@google.com>

When we print out a discontinuous compression chunk, it shows like a
continuous chunk now. To show it more correctly, I've changed the way of
printing fiemap info like below. Plus, eliminated NEW_ADDR(-1) in fiemap
info, since it is not in fiemap user api manual.

   Logical          Physical         Length           Flags
0: 0000000000000000 0000000fdf692000 0000000000004000 1008
1: 0000000000004000 0000000fdf693000 0000000000004000 1008
2: 0000000000008000 0000000fdf694000 0000000000004000 1008
3: 000000000000c000 0000000fdf695000 0000000000004000 1008
4: 0000000000010000 0000000fdf696000 000000000000c000 1000
5: 000000000001c000 0000000f8c60d000 0000000000010000 1000
6: 000000000002c000 0000000f8c61d000 0000000000004000 1008
7: 0000000000030000 0000000f8c620000 0000000000004000 1008
8: 0000000000034000 0000000f8c623000 0000000000001000 1008
9: 0000000000035000 0000000fc7af4000 0000000000003000 1008

Flags
0x1000 => FIEMAP_EXTENT_MERGED
0x0008 => FIEMAP_EXTENT_ENCODED

Signed-off-by: Daeho Jeong <daehojeong@google.com>

---
v2: changed the print format
---
 fs/f2fs/data.c | 76 ++++++++++++++++++++++++++++----------------------
 1 file changed, 42 insertions(+), 34 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 3a01a1b50104..058dc751e3a6 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1843,8 +1843,9 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	u64 logical = 0, phys = 0, size = 0;
 	u32 flags = 0;
 	int ret = 0;
-	bool compr_cluster = false;
+	bool compr_cluster = false, compr_appended;
 	unsigned int cluster_size = F2FS_I(inode)->i_cluster_size;
+	unsigned int count_in_cluster;
 	loff_t maxbytes;
 
 	if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
@@ -1892,8 +1893,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	map.m_next_pgofs = &next_pgofs;
 	map.m_seg_type = NO_CHECK_TYPE;
 
-	if (compr_cluster)
-		map.m_len = cluster_size - 1;
+	if (compr_cluster) {
+		map.m_lblk += 1;
+		map.m_len = cluster_size - count_in_cluster;
+	}
 
 	ret = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_FIEMAP);
 	if (ret)
@@ -1903,11 +1906,23 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	if (!(map.m_flags & F2FS_MAP_FLAGS)) {
 		start_blk = next_pgofs;
 
-		if (blks_to_bytes(inode, start_blk) < blks_to_bytes(inode,
+		if (blks_to_bytes(inode, start_blk) >= blks_to_bytes(inode,
 						max_inode_blocks(inode)))
+			flags |= FIEMAP_EXTENT_LAST;
+		else if (!compr_cluster)
 			goto prep_next;
+	}
+
+	compr_appended = false;
+	/* In a case of compressed cluster, append this to the last extent */
+	if (compr_cluster && ((map.m_flags & F2FS_MAP_UNWRITTEN) ||
+			!(map.m_flags & F2FS_MAP_FLAGS))) {
+		unsigned int appended_blks = cluster_size - count_in_cluster + 1;
 
-		flags |= FIEMAP_EXTENT_LAST;
+		size += blks_to_bytes(inode, appended_blks);
+		if (map.m_flags & F2FS_MAP_UNWRITTEN)
+			start_blk += appended_blks;
+		compr_appended = true;
 	}
 
 	if (size) {
@@ -1926,38 +1941,31 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	if (start_blk > last_blk)
 		goto out;
 
-	if (compr_cluster) {
-		compr_cluster = false;
-
-
-		logical = blks_to_bytes(inode, start_blk - 1);
-		phys = blks_to_bytes(inode, map.m_pblk);
-		size = blks_to_bytes(inode, cluster_size);
-
-		flags |= FIEMAP_EXTENT_ENCODED;
-
-		start_blk += cluster_size - 1;
-
-		if (start_blk > last_blk)
-			goto out;
-
-		goto prep_next;
-	}
-
 	if (map.m_pblk == COMPRESS_ADDR) {
 		compr_cluster = true;
-		start_blk++;
-		goto prep_next;
-	}
-
-	logical = blks_to_bytes(inode, start_blk);
-	phys = blks_to_bytes(inode, map.m_pblk);
-	size = blks_to_bytes(inode, map.m_len);
-	flags = 0;
-	if (map.m_flags & F2FS_MAP_UNWRITTEN)
-		flags = FIEMAP_EXTENT_UNWRITTEN;
+		count_in_cluster = 1;
+	} else if (compr_appended) {
+		compr_cluster = false;
+	} else {
+		logical = blks_to_bytes(inode, start_blk);
+		phys = __is_valid_data_blkaddr(map.m_pblk) ?
+			blks_to_bytes(inode, map.m_pblk) : 0;
+		size = blks_to_bytes(inode, map.m_len);
+		flags = 0;
+
+		if (compr_cluster) {
+			flags = FIEMAP_EXTENT_ENCODED;
+			count_in_cluster += map.m_len;
+			if (count_in_cluster == cluster_size) {
+				compr_cluster = false;
+				size += blks_to_bytes(inode, 1);
+			}
+		} else if (map.m_flags & F2FS_MAP_UNWRITTEN) {
+			flags = FIEMAP_EXTENT_UNWRITTEN;
+		}
 
-	start_blk += bytes_to_blks(inode, size);
+		start_blk += bytes_to_blks(inode, size);
+	}
 
 prep_next:
 	cond_resched();
-- 
2.32.0.432.gabb21c7263-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [f2fs-dev] [PATCH v2] f2fs: change fiemap way in printing compression chunk
@ 2021-07-22 21:19 ` Daeho Jeong
  0 siblings, 0 replies; 8+ messages in thread
From: Daeho Jeong @ 2021-07-22 21:19 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong

From: Daeho Jeong <daehojeong@google.com>

When we print out a discontinuous compression chunk, it shows like a
continuous chunk now. To show it more correctly, I've changed the way of
printing fiemap info like below. Plus, eliminated NEW_ADDR(-1) in fiemap
info, since it is not in fiemap user api manual.

   Logical          Physical         Length           Flags
0: 0000000000000000 0000000fdf692000 0000000000004000 1008
1: 0000000000004000 0000000fdf693000 0000000000004000 1008
2: 0000000000008000 0000000fdf694000 0000000000004000 1008
3: 000000000000c000 0000000fdf695000 0000000000004000 1008
4: 0000000000010000 0000000fdf696000 000000000000c000 1000
5: 000000000001c000 0000000f8c60d000 0000000000010000 1000
6: 000000000002c000 0000000f8c61d000 0000000000004000 1008
7: 0000000000030000 0000000f8c620000 0000000000004000 1008
8: 0000000000034000 0000000f8c623000 0000000000001000 1008
9: 0000000000035000 0000000fc7af4000 0000000000003000 1008

Flags
0x1000 => FIEMAP_EXTENT_MERGED
0x0008 => FIEMAP_EXTENT_ENCODED

Signed-off-by: Daeho Jeong <daehojeong@google.com>

---
v2: changed the print format
---
 fs/f2fs/data.c | 76 ++++++++++++++++++++++++++++----------------------
 1 file changed, 42 insertions(+), 34 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 3a01a1b50104..058dc751e3a6 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1843,8 +1843,9 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	u64 logical = 0, phys = 0, size = 0;
 	u32 flags = 0;
 	int ret = 0;
-	bool compr_cluster = false;
+	bool compr_cluster = false, compr_appended;
 	unsigned int cluster_size = F2FS_I(inode)->i_cluster_size;
+	unsigned int count_in_cluster;
 	loff_t maxbytes;
 
 	if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
@@ -1892,8 +1893,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	map.m_next_pgofs = &next_pgofs;
 	map.m_seg_type = NO_CHECK_TYPE;
 
-	if (compr_cluster)
-		map.m_len = cluster_size - 1;
+	if (compr_cluster) {
+		map.m_lblk += 1;
+		map.m_len = cluster_size - count_in_cluster;
+	}
 
 	ret = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_FIEMAP);
 	if (ret)
@@ -1903,11 +1906,23 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	if (!(map.m_flags & F2FS_MAP_FLAGS)) {
 		start_blk = next_pgofs;
 
-		if (blks_to_bytes(inode, start_blk) < blks_to_bytes(inode,
+		if (blks_to_bytes(inode, start_blk) >= blks_to_bytes(inode,
 						max_inode_blocks(inode)))
+			flags |= FIEMAP_EXTENT_LAST;
+		else if (!compr_cluster)
 			goto prep_next;
+	}
+
+	compr_appended = false;
+	/* In a case of compressed cluster, append this to the last extent */
+	if (compr_cluster && ((map.m_flags & F2FS_MAP_UNWRITTEN) ||
+			!(map.m_flags & F2FS_MAP_FLAGS))) {
+		unsigned int appended_blks = cluster_size - count_in_cluster + 1;
 
-		flags |= FIEMAP_EXTENT_LAST;
+		size += blks_to_bytes(inode, appended_blks);
+		if (map.m_flags & F2FS_MAP_UNWRITTEN)
+			start_blk += appended_blks;
+		compr_appended = true;
 	}
 
 	if (size) {
@@ -1926,38 +1941,31 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	if (start_blk > last_blk)
 		goto out;
 
-	if (compr_cluster) {
-		compr_cluster = false;
-
-
-		logical = blks_to_bytes(inode, start_blk - 1);
-		phys = blks_to_bytes(inode, map.m_pblk);
-		size = blks_to_bytes(inode, cluster_size);
-
-		flags |= FIEMAP_EXTENT_ENCODED;
-
-		start_blk += cluster_size - 1;
-
-		if (start_blk > last_blk)
-			goto out;
-
-		goto prep_next;
-	}
-
 	if (map.m_pblk == COMPRESS_ADDR) {
 		compr_cluster = true;
-		start_blk++;
-		goto prep_next;
-	}
-
-	logical = blks_to_bytes(inode, start_blk);
-	phys = blks_to_bytes(inode, map.m_pblk);
-	size = blks_to_bytes(inode, map.m_len);
-	flags = 0;
-	if (map.m_flags & F2FS_MAP_UNWRITTEN)
-		flags = FIEMAP_EXTENT_UNWRITTEN;
+		count_in_cluster = 1;
+	} else if (compr_appended) {
+		compr_cluster = false;
+	} else {
+		logical = blks_to_bytes(inode, start_blk);
+		phys = __is_valid_data_blkaddr(map.m_pblk) ?
+			blks_to_bytes(inode, map.m_pblk) : 0;
+		size = blks_to_bytes(inode, map.m_len);
+		flags = 0;
+
+		if (compr_cluster) {
+			flags = FIEMAP_EXTENT_ENCODED;
+			count_in_cluster += map.m_len;
+			if (count_in_cluster == cluster_size) {
+				compr_cluster = false;
+				size += blks_to_bytes(inode, 1);
+			}
+		} else if (map.m_flags & F2FS_MAP_UNWRITTEN) {
+			flags = FIEMAP_EXTENT_UNWRITTEN;
+		}
 
-	start_blk += bytes_to_blks(inode, size);
+		start_blk += bytes_to_blks(inode, size);
+	}
 
 prep_next:
 	cond_resched();
-- 
2.32.0.432.gabb21c7263-goog



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change fiemap way in printing compression chunk
  2021-07-22 21:19 ` [f2fs-dev] " Daeho Jeong
@ 2021-07-23  0:20   ` Chao Yu
  -1 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2021-07-23  0:20 UTC (permalink / raw)
  To: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong

Hi Daeho,

On 2021/7/23 5:19, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
> 
> When we print out a discontinuous compression chunk, it shows like a
> continuous chunk now. To show it more correctly, I've changed the way of
> printing fiemap info like below. Plus, eliminated NEW_ADDR(-1) in fiemap
> info, since it is not in fiemap user api manual.
> 
>     Logical          Physical         Length           Flags
> 0: 0000000000000000 0000000fdf692000 0000000000004000 1008
> 1: 0000000000004000 0000000fdf693000 0000000000004000 1008
> 2: 0000000000008000 0000000fdf694000 0000000000004000 1008
> 3: 000000000000c000 0000000fdf695000 0000000000004000 1008
> 4: 0000000000010000 0000000fdf696000 000000000000c000 1000
> 5: 000000000001c000 0000000f8c60d000 0000000000010000 1000
> 6: 000000000002c000 0000000f8c61d000 0000000000004000 1008
> 7: 0000000000030000 0000000f8c620000 0000000000004000 1008
> 8: 0000000000034000 0000000f8c623000 0000000000001000 1008
> 9: 0000000000035000 0000000fc7af4000 0000000000003000 1008

I wrote a file some like this:

i_addr[0x9] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0xa]                 		[0x   72800 : 468992]
i_addr[0xb] reserved flag   		[0xffffffff : 4294967295]
i_addr[0xc] reserved flag   		[0xffffffff : 4294967295]
i_addr[0xd] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0xe]                 		[0x   72801 : 468993]
i_addr[0xf] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x10] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x11]                 		[0x   72832 : 469042]
i_addr[0x12]                 		[0x   72802 : 468994]
i_addr[0x13]                 		[0x   72833 : 469043]
i_addr[0x14]                 		[0x   72834 : 469044]
i_addr[0x15] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0x16]                 		[0x   72803 : 468995]
i_addr[0x17] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x18] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x19]                 		[0x   72835 : 469045]
i_addr[0x1a]                 		[0x   72804 : 468996]
i_addr[0x1b]                 		[0x   72836 : 469046]
i_addr[0x1c]                 		[0x   72837 : 469047]
i_addr[0x1d] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0x1e]                 		[0x   72805 : 468997]
i_addr[0x1f] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x20] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x21] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0x22]                 		[0x   72806 : 468998]
i_addr[0x23] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x24] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x25] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0x26]                 		[0x   72807 : 468999]
i_addr[0x27] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x28] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x29] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0x2a]                 		[0x   72808 : 469000]
i_addr[0x2b] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x2c] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x2d] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0x2e]                 		[0x   72809 : 469001]
i_addr[0x2f] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x30] reserved flag   		[0xffffffff : 4294967295]
i_nid[0]                      		[0x       0 : 0]
i_nid[1]                      		[0x       0 : 0]
i_nid[2]                      		[0x       0 : 0]
i_nid[3]                      		[0x       0 : 0]
i_nid[4]                      		[0x       0 : 0]

xfs_io file -c "fiemap -v" shows:

before:
  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
    0: [0..31]:         3751936..3751967    32 0x1008
    1: [32..63]:        3751944..3751975    32 0x1008
    2: [64..71]:        3752336..3752343     8 0x1000
    3: [72..79]:        3751952..3751959     8 0x1000
    4: [80..95]:        3752344..3752359    16 0x1000
    5: [96..127]:       3751960..3751991    32 0x1008
    6: [128..135]:      3752360..3752367     8 0x1000
    7: [136..143]:      3751968..3751975     8 0x1000
    8: [144..159]:      3752368..3752383    16 0x1000
    9: [160..191]:      3751976..3752007    32 0x1008
   10: [192..223]:      3751984..3752015    32 0x1008
   11: [224..255]:      3751992..3752023    32 0x1008
   12: [256..287]:      3752000..3752031    32 0x1008
   13: [288..319]:      3752008..3752039    32 0x1009

after:
    0: [0..31]:         3751936..3751967    32 0x1008
    1: [32..63]:        3751944..3751975    32 0x1008
    2: [64..71]:        3752336..3752343     8 0x1000
    3: [72..79]:        3751952..3751959     8 0x1000
    4: [80..95]:        3752344..3752359    16 0x1000
    5: [96..127]:       3751960..3751991    32 0x1008
    6: [128..135]:      3752360..3752367     8 0x1000
    7: [136..143]:      3751968..3751975     8 0x1000
    8: [144..159]:      3752368..3752383    16 0x1000
    9: [160..191]:      3751976..3752007    32 0x1008
   10: [192..223]:      3751984..3752015    32 0x1008
   11: [224..255]:      3751992..3752023    32 0x1008
   12: [256..287]:      3752000..3752031    32 0x1008
   13: [288..319]:      3752008..3752039    32 0x1008

I don't see any obvious difference, except w/ current patch, last
FIEMAP_EXTENT_LAST is missing.

Sorry, I didn't get the point of this patch, could you please explain
more details for that problem this patch tries to fix and show the
difference before/after the patch in commit message?

Thanks,

> 
> Flags
> 0x1000 => FIEMAP_EXTENT_MERGED
> 0x0008 => FIEMAP_EXTENT_ENCODED
> 
> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> 
> ---
> v2: changed the print format
> ---
>   fs/f2fs/data.c | 76 ++++++++++++++++++++++++++++----------------------
>   1 file changed, 42 insertions(+), 34 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 3a01a1b50104..058dc751e3a6 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1843,8 +1843,9 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>   	u64 logical = 0, phys = 0, size = 0;
>   	u32 flags = 0;
>   	int ret = 0;
> -	bool compr_cluster = false;
> +	bool compr_cluster = false, compr_appended;
>   	unsigned int cluster_size = F2FS_I(inode)->i_cluster_size;
> +	unsigned int count_in_cluster;
>   	loff_t maxbytes;
>   
>   	if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
> @@ -1892,8 +1893,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>   	map.m_next_pgofs = &next_pgofs;
>   	map.m_seg_type = NO_CHECK_TYPE;
>   
> -	if (compr_cluster)
> -		map.m_len = cluster_size - 1;
> +	if (compr_cluster) {
> +		map.m_lblk += 1;
> +		map.m_len = cluster_size - count_in_cluster;
> +	}
>   
>   	ret = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_FIEMAP);
>   	if (ret)
> @@ -1903,11 +1906,23 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>   	if (!(map.m_flags & F2FS_MAP_FLAGS)) {
>   		start_blk = next_pgofs;
>   
> -		if (blks_to_bytes(inode, start_blk) < blks_to_bytes(inode,
> +		if (blks_to_bytes(inode, start_blk) >= blks_to_bytes(inode,
>   						max_inode_blocks(inode)))
> +			flags |= FIEMAP_EXTENT_LAST;
> +		else if (!compr_cluster)
>   			goto prep_next;
> +	}
> +
> +	compr_appended = false;
> +	/* In a case of compressed cluster, append this to the last extent */
> +	if (compr_cluster && ((map.m_flags & F2FS_MAP_UNWRITTEN) ||
> +			!(map.m_flags & F2FS_MAP_FLAGS))) {
> +		unsigned int appended_blks = cluster_size - count_in_cluster + 1;
>   
> -		flags |= FIEMAP_EXTENT_LAST;
> +		size += blks_to_bytes(inode, appended_blks);
> +		if (map.m_flags & F2FS_MAP_UNWRITTEN)
> +			start_blk += appended_blks;
> +		compr_appended = true;
>   	}
>   
>   	if (size) {
> @@ -1926,38 +1941,31 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>   	if (start_blk > last_blk)
>   		goto out;
>   
> -	if (compr_cluster) {
> -		compr_cluster = false;
> -
> -
> -		logical = blks_to_bytes(inode, start_blk - 1);
> -		phys = blks_to_bytes(inode, map.m_pblk);
> -		size = blks_to_bytes(inode, cluster_size);
> -
> -		flags |= FIEMAP_EXTENT_ENCODED;
> -
> -		start_blk += cluster_size - 1;
> -
> -		if (start_blk > last_blk)
> -			goto out;
> -
> -		goto prep_next;
> -	}
> -
>   	if (map.m_pblk == COMPRESS_ADDR) {
>   		compr_cluster = true;
> -		start_blk++;
> -		goto prep_next;
> -	}
> -
> -	logical = blks_to_bytes(inode, start_blk);
> -	phys = blks_to_bytes(inode, map.m_pblk);
> -	size = blks_to_bytes(inode, map.m_len);
> -	flags = 0;
> -	if (map.m_flags & F2FS_MAP_UNWRITTEN)
> -		flags = FIEMAP_EXTENT_UNWRITTEN;
> +		count_in_cluster = 1;
> +	} else if (compr_appended) {
> +		compr_cluster = false;
> +	} else {
> +		logical = blks_to_bytes(inode, start_blk);
> +		phys = __is_valid_data_blkaddr(map.m_pblk) ?
> +			blks_to_bytes(inode, map.m_pblk) : 0;
> +		size = blks_to_bytes(inode, map.m_len);
> +		flags = 0;
> +
> +		if (compr_cluster) {
> +			flags = FIEMAP_EXTENT_ENCODED;
> +			count_in_cluster += map.m_len;
> +			if (count_in_cluster == cluster_size) {
> +				compr_cluster = false;
> +				size += blks_to_bytes(inode, 1);
> +			}
> +		} else if (map.m_flags & F2FS_MAP_UNWRITTEN) {
> +			flags = FIEMAP_EXTENT_UNWRITTEN;
> +		}
>   
> -	start_blk += bytes_to_blks(inode, size);
> +		start_blk += bytes_to_blks(inode, size);
> +	}
>   
>   prep_next:
>   	cond_resched();
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change fiemap way in printing compression chunk
@ 2021-07-23  0:20   ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2021-07-23  0:20 UTC (permalink / raw)
  To: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong

Hi Daeho,

On 2021/7/23 5:19, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
> 
> When we print out a discontinuous compression chunk, it shows like a
> continuous chunk now. To show it more correctly, I've changed the way of
> printing fiemap info like below. Plus, eliminated NEW_ADDR(-1) in fiemap
> info, since it is not in fiemap user api manual.
> 
>     Logical          Physical         Length           Flags
> 0: 0000000000000000 0000000fdf692000 0000000000004000 1008
> 1: 0000000000004000 0000000fdf693000 0000000000004000 1008
> 2: 0000000000008000 0000000fdf694000 0000000000004000 1008
> 3: 000000000000c000 0000000fdf695000 0000000000004000 1008
> 4: 0000000000010000 0000000fdf696000 000000000000c000 1000
> 5: 000000000001c000 0000000f8c60d000 0000000000010000 1000
> 6: 000000000002c000 0000000f8c61d000 0000000000004000 1008
> 7: 0000000000030000 0000000f8c620000 0000000000004000 1008
> 8: 0000000000034000 0000000f8c623000 0000000000001000 1008
> 9: 0000000000035000 0000000fc7af4000 0000000000003000 1008

I wrote a file some like this:

i_addr[0x9] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0xa]                 		[0x   72800 : 468992]
i_addr[0xb] reserved flag   		[0xffffffff : 4294967295]
i_addr[0xc] reserved flag   		[0xffffffff : 4294967295]
i_addr[0xd] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0xe]                 		[0x   72801 : 468993]
i_addr[0xf] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x10] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x11]                 		[0x   72832 : 469042]
i_addr[0x12]                 		[0x   72802 : 468994]
i_addr[0x13]                 		[0x   72833 : 469043]
i_addr[0x14]                 		[0x   72834 : 469044]
i_addr[0x15] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0x16]                 		[0x   72803 : 468995]
i_addr[0x17] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x18] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x19]                 		[0x   72835 : 469045]
i_addr[0x1a]                 		[0x   72804 : 468996]
i_addr[0x1b]                 		[0x   72836 : 469046]
i_addr[0x1c]                 		[0x   72837 : 469047]
i_addr[0x1d] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0x1e]                 		[0x   72805 : 468997]
i_addr[0x1f] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x20] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x21] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0x22]                 		[0x   72806 : 468998]
i_addr[0x23] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x24] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x25] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0x26]                 		[0x   72807 : 468999]
i_addr[0x27] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x28] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x29] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0x2a]                 		[0x   72808 : 469000]
i_addr[0x2b] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x2c] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x2d] cluster flag    		[0xfffffffe : 4294967294]
i_addr[0x2e]                 		[0x   72809 : 469001]
i_addr[0x2f] reserved flag   		[0xffffffff : 4294967295]
i_addr[0x30] reserved flag   		[0xffffffff : 4294967295]
i_nid[0]                      		[0x       0 : 0]
i_nid[1]                      		[0x       0 : 0]
i_nid[2]                      		[0x       0 : 0]
i_nid[3]                      		[0x       0 : 0]
i_nid[4]                      		[0x       0 : 0]

xfs_io file -c "fiemap -v" shows:

before:
  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
    0: [0..31]:         3751936..3751967    32 0x1008
    1: [32..63]:        3751944..3751975    32 0x1008
    2: [64..71]:        3752336..3752343     8 0x1000
    3: [72..79]:        3751952..3751959     8 0x1000
    4: [80..95]:        3752344..3752359    16 0x1000
    5: [96..127]:       3751960..3751991    32 0x1008
    6: [128..135]:      3752360..3752367     8 0x1000
    7: [136..143]:      3751968..3751975     8 0x1000
    8: [144..159]:      3752368..3752383    16 0x1000
    9: [160..191]:      3751976..3752007    32 0x1008
   10: [192..223]:      3751984..3752015    32 0x1008
   11: [224..255]:      3751992..3752023    32 0x1008
   12: [256..287]:      3752000..3752031    32 0x1008
   13: [288..319]:      3752008..3752039    32 0x1009

after:
    0: [0..31]:         3751936..3751967    32 0x1008
    1: [32..63]:        3751944..3751975    32 0x1008
    2: [64..71]:        3752336..3752343     8 0x1000
    3: [72..79]:        3751952..3751959     8 0x1000
    4: [80..95]:        3752344..3752359    16 0x1000
    5: [96..127]:       3751960..3751991    32 0x1008
    6: [128..135]:      3752360..3752367     8 0x1000
    7: [136..143]:      3751968..3751975     8 0x1000
    8: [144..159]:      3752368..3752383    16 0x1000
    9: [160..191]:      3751976..3752007    32 0x1008
   10: [192..223]:      3751984..3752015    32 0x1008
   11: [224..255]:      3751992..3752023    32 0x1008
   12: [256..287]:      3752000..3752031    32 0x1008
   13: [288..319]:      3752008..3752039    32 0x1008

I don't see any obvious difference, except w/ current patch, last
FIEMAP_EXTENT_LAST is missing.

Sorry, I didn't get the point of this patch, could you please explain
more details for that problem this patch tries to fix and show the
difference before/after the patch in commit message?

Thanks,

> 
> Flags
> 0x1000 => FIEMAP_EXTENT_MERGED
> 0x0008 => FIEMAP_EXTENT_ENCODED
> 
> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> 
> ---
> v2: changed the print format
> ---
>   fs/f2fs/data.c | 76 ++++++++++++++++++++++++++++----------------------
>   1 file changed, 42 insertions(+), 34 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 3a01a1b50104..058dc751e3a6 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1843,8 +1843,9 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>   	u64 logical = 0, phys = 0, size = 0;
>   	u32 flags = 0;
>   	int ret = 0;
> -	bool compr_cluster = false;
> +	bool compr_cluster = false, compr_appended;
>   	unsigned int cluster_size = F2FS_I(inode)->i_cluster_size;
> +	unsigned int count_in_cluster;
>   	loff_t maxbytes;
>   
>   	if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
> @@ -1892,8 +1893,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>   	map.m_next_pgofs = &next_pgofs;
>   	map.m_seg_type = NO_CHECK_TYPE;
>   
> -	if (compr_cluster)
> -		map.m_len = cluster_size - 1;
> +	if (compr_cluster) {
> +		map.m_lblk += 1;
> +		map.m_len = cluster_size - count_in_cluster;
> +	}
>   
>   	ret = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_FIEMAP);
>   	if (ret)
> @@ -1903,11 +1906,23 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>   	if (!(map.m_flags & F2FS_MAP_FLAGS)) {
>   		start_blk = next_pgofs;
>   
> -		if (blks_to_bytes(inode, start_blk) < blks_to_bytes(inode,
> +		if (blks_to_bytes(inode, start_blk) >= blks_to_bytes(inode,
>   						max_inode_blocks(inode)))
> +			flags |= FIEMAP_EXTENT_LAST;
> +		else if (!compr_cluster)
>   			goto prep_next;
> +	}
> +
> +	compr_appended = false;
> +	/* In a case of compressed cluster, append this to the last extent */
> +	if (compr_cluster && ((map.m_flags & F2FS_MAP_UNWRITTEN) ||
> +			!(map.m_flags & F2FS_MAP_FLAGS))) {
> +		unsigned int appended_blks = cluster_size - count_in_cluster + 1;
>   
> -		flags |= FIEMAP_EXTENT_LAST;
> +		size += blks_to_bytes(inode, appended_blks);
> +		if (map.m_flags & F2FS_MAP_UNWRITTEN)
> +			start_blk += appended_blks;
> +		compr_appended = true;
>   	}
>   
>   	if (size) {
> @@ -1926,38 +1941,31 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>   	if (start_blk > last_blk)
>   		goto out;
>   
> -	if (compr_cluster) {
> -		compr_cluster = false;
> -
> -
> -		logical = blks_to_bytes(inode, start_blk - 1);
> -		phys = blks_to_bytes(inode, map.m_pblk);
> -		size = blks_to_bytes(inode, cluster_size);
> -
> -		flags |= FIEMAP_EXTENT_ENCODED;
> -
> -		start_blk += cluster_size - 1;
> -
> -		if (start_blk > last_blk)
> -			goto out;
> -
> -		goto prep_next;
> -	}
> -
>   	if (map.m_pblk == COMPRESS_ADDR) {
>   		compr_cluster = true;
> -		start_blk++;
> -		goto prep_next;
> -	}
> -
> -	logical = blks_to_bytes(inode, start_blk);
> -	phys = blks_to_bytes(inode, map.m_pblk);
> -	size = blks_to_bytes(inode, map.m_len);
> -	flags = 0;
> -	if (map.m_flags & F2FS_MAP_UNWRITTEN)
> -		flags = FIEMAP_EXTENT_UNWRITTEN;
> +		count_in_cluster = 1;
> +	} else if (compr_appended) {
> +		compr_cluster = false;
> +	} else {
> +		logical = blks_to_bytes(inode, start_blk);
> +		phys = __is_valid_data_blkaddr(map.m_pblk) ?
> +			blks_to_bytes(inode, map.m_pblk) : 0;
> +		size = blks_to_bytes(inode, map.m_len);
> +		flags = 0;
> +
> +		if (compr_cluster) {
> +			flags = FIEMAP_EXTENT_ENCODED;
> +			count_in_cluster += map.m_len;
> +			if (count_in_cluster == cluster_size) {
> +				compr_cluster = false;
> +				size += blks_to_bytes(inode, 1);
> +			}
> +		} else if (map.m_flags & F2FS_MAP_UNWRITTEN) {
> +			flags = FIEMAP_EXTENT_UNWRITTEN;
> +		}
>   
> -	start_blk += bytes_to_blks(inode, size);
> +		start_blk += bytes_to_blks(inode, size);
> +	}
>   
>   prep_next:
>   	cond_resched();
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change fiemap way in printing compression chunk
  2021-07-23  0:20   ` Chao Yu
@ 2021-07-23  0:49     ` Daeho Jeong
  -1 siblings, 0 replies; 8+ messages in thread
From: Daeho Jeong @ 2021-07-23  0:49 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong

On Thu, Jul 22, 2021 at 5:20 PM Chao Yu <chao@kernel.org> wrote:
>
> Hi Daeho,
>
> On 2021/7/23 5:19, Daeho Jeong wrote:
> > From: Daeho Jeong <daehojeong@google.com>
> >
> > When we print out a discontinuous compression chunk, it shows like a
> > continuous chunk now. To show it more correctly, I've changed the way of
> > printing fiemap info like below. Plus, eliminated NEW_ADDR(-1) in fiemap
> > info, since it is not in fiemap user api manual.
> >
> >     Logical          Physical         Length           Flags
> > 0: 0000000000000000 0000000fdf692000 0000000000004000 1008
> > 1: 0000000000004000 0000000fdf693000 0000000000004000 1008
> > 2: 0000000000008000 0000000fdf694000 0000000000004000 1008
> > 3: 000000000000c000 0000000fdf695000 0000000000004000 1008
> > 4: 0000000000010000 0000000fdf696000 000000000000c000 1000
> > 5: 000000000001c000 0000000f8c60d000 0000000000010000 1000
> > 6: 000000000002c000 0000000f8c61d000 0000000000004000 1008
> > 7: 0000000000030000 0000000f8c620000 0000000000004000 1008
> > 8: 0000000000034000 0000000f8c623000 0000000000001000 1008
> > 9: 0000000000035000 0000000fc7af4000 0000000000003000 1008
>
> I wrote a file some like this:
>
> i_addr[0x9] cluster flag                [0xfffffffe : 4294967294]
> i_addr[0xa]                             [0x   72800 : 468992]
> i_addr[0xb] reserved flag               [0xffffffff : 4294967295]
> i_addr[0xc] reserved flag               [0xffffffff : 4294967295]
> i_addr[0xd] cluster flag                [0xfffffffe : 4294967294]
> i_addr[0xe]                             [0x   72801 : 468993]
> i_addr[0xf] reserved flag               [0xffffffff : 4294967295]
> i_addr[0x10] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x11]                            [0x   72832 : 469042]
> i_addr[0x12]                            [0x   72802 : 468994]
> i_addr[0x13]                            [0x   72833 : 469043]
> i_addr[0x14]                            [0x   72834 : 469044]
> i_addr[0x15] cluster flag               [0xfffffffe : 4294967294]
> i_addr[0x16]                            [0x   72803 : 468995]
> i_addr[0x17] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x18] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x19]                            [0x   72835 : 469045]
> i_addr[0x1a]                            [0x   72804 : 468996]
> i_addr[0x1b]                            [0x   72836 : 469046]
> i_addr[0x1c]                            [0x   72837 : 469047]
> i_addr[0x1d] cluster flag               [0xfffffffe : 4294967294]
> i_addr[0x1e]                            [0x   72805 : 468997]
> i_addr[0x1f] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x20] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x21] cluster flag               [0xfffffffe : 4294967294]
> i_addr[0x22]                            [0x   72806 : 468998]
> i_addr[0x23] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x24] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x25] cluster flag               [0xfffffffe : 4294967294]
> i_addr[0x26]                            [0x   72807 : 468999]
> i_addr[0x27] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x28] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x29] cluster flag               [0xfffffffe : 4294967294]
> i_addr[0x2a]                            [0x   72808 : 469000]
> i_addr[0x2b] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x2c] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x2d] cluster flag               [0xfffffffe : 4294967294]
> i_addr[0x2e]                            [0x   72809 : 469001]
> i_addr[0x2f] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x30] reserved flag              [0xffffffff : 4294967295]
> i_nid[0]                                [0x       0 : 0]
> i_nid[1]                                [0x       0 : 0]
> i_nid[2]                                [0x       0 : 0]
> i_nid[3]                                [0x       0 : 0]
> i_nid[4]                                [0x       0 : 0]
>
> xfs_io file -c "fiemap -v" shows:
>
> before:
>   EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
>     0: [0..31]:         3751936..3751967    32 0x1008
>     1: [32..63]:        3751944..3751975    32 0x1008
>     2: [64..71]:        3752336..3752343     8 0x1000
>     3: [72..79]:        3751952..3751959     8 0x1000
>     4: [80..95]:        3752344..3752359    16 0x1000
>     5: [96..127]:       3751960..3751991    32 0x1008
>     6: [128..135]:      3752360..3752367     8 0x1000
>     7: [136..143]:      3751968..3751975     8 0x1000
>     8: [144..159]:      3752368..3752383    16 0x1000
>     9: [160..191]:      3751976..3752007    32 0x1008
>    10: [192..223]:      3751984..3752015    32 0x1008
>    11: [224..255]:      3751992..3752023    32 0x1008
>    12: [256..287]:      3752000..3752031    32 0x1008
>    13: [288..319]:      3752008..3752039    32 0x1009
>
> after:
>     0: [0..31]:         3751936..3751967    32 0x1008
>     1: [32..63]:        3751944..3751975    32 0x1008
>     2: [64..71]:        3752336..3752343     8 0x1000
>     3: [72..79]:        3751952..3751959     8 0x1000
>     4: [80..95]:        3752344..3752359    16 0x1000
>     5: [96..127]:       3751960..3751991    32 0x1008
>     6: [128..135]:      3752360..3752367     8 0x1000
>     7: [136..143]:      3751968..3751975     8 0x1000
>     8: [144..159]:      3752368..3752383    16 0x1000
>     9: [160..191]:      3751976..3752007    32 0x1008
>    10: [192..223]:      3751984..3752015    32 0x1008
>    11: [224..255]:      3751992..3752023    32 0x1008
>    12: [256..287]:      3752000..3752031    32 0x1008
>    13: [288..319]:      3752008..3752039    32 0x1008
>
> I don't see any obvious difference, except w/ current patch, last
> FIEMAP_EXTENT_LAST is missing.
>
> Sorry, I didn't get the point of this patch, could you please explain
> more details for that problem this patch tries to fix and show the
> difference before/after the patch in commit message?
>
> Thanks,
>
> >
> > Flags
> > 0x1000 => FIEMAP_EXTENT_MERGED
> > 0x0008 => FIEMAP_EXTENT_ENCODED
> >
> > Signed-off-by: Daeho Jeong <daehojeong@google.com>
> >
> > ---
> > v2: changed the print format
> > ---
> >   fs/f2fs/data.c | 76 ++++++++++++++++++++++++++++----------------------
> >   1 file changed, 42 insertions(+), 34 deletions(-)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 3a01a1b50104..058dc751e3a6 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1843,8 +1843,9 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> >       u64 logical = 0, phys = 0, size = 0;
> >       u32 flags = 0;
> >       int ret = 0;
> > -     bool compr_cluster = false;
> > +     bool compr_cluster = false, compr_appended;
> >       unsigned int cluster_size = F2FS_I(inode)->i_cluster_size;
> > +     unsigned int count_in_cluster;
> >       loff_t maxbytes;
> >
> >       if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
> > @@ -1892,8 +1893,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> >       map.m_next_pgofs = &next_pgofs;
> >       map.m_seg_type = NO_CHECK_TYPE;
> >
> > -     if (compr_cluster)
> > -             map.m_len = cluster_size - 1;
> > +     if (compr_cluster) {
> > +             map.m_lblk += 1;
> > +             map.m_len = cluster_size - count_in_cluster;
> > +     }
> >
> >       ret = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_FIEMAP);
> >       if (ret)
> > @@ -1903,11 +1906,23 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> >       if (!(map.m_flags & F2FS_MAP_FLAGS)) {
> >               start_blk = next_pgofs;
> >
> > -             if (blks_to_bytes(inode, start_blk) < blks_to_bytes(inode,
> > +             if (blks_to_bytes(inode, start_blk) >= blks_to_bytes(inode,
> >                                               max_inode_blocks(inode)))
> > +                     flags |= FIEMAP_EXTENT_LAST;
> > +             else if (!compr_cluster)
> >                       goto prep_next;
> > +     }
> > +
> > +     compr_appended = false;
> > +     /* In a case of compressed cluster, append this to the last extent */
> > +     if (compr_cluster && ((map.m_flags & F2FS_MAP_UNWRITTEN) ||
> > +                     !(map.m_flags & F2FS_MAP_FLAGS))) {
> > +             unsigned int appended_blks = cluster_size - count_in_cluster + 1;
> >
> > -             flags |= FIEMAP_EXTENT_LAST;
> > +             size += blks_to_bytes(inode, appended_blks);
> > +             if (map.m_flags & F2FS_MAP_UNWRITTEN)
> > +                     start_blk += appended_blks;
> > +             compr_appended = true;
> >       }
> >
> >       if (size) {
> > @@ -1926,38 +1941,31 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> >       if (start_blk > last_blk)
> >               goto out;
> >
> > -     if (compr_cluster) {
> > -             compr_cluster = false;
> > -
> > -
> > -             logical = blks_to_bytes(inode, start_blk - 1);
> > -             phys = blks_to_bytes(inode, map.m_pblk);
> > -             size = blks_to_bytes(inode, cluster_size);
> > -
> > -             flags |= FIEMAP_EXTENT_ENCODED;
> > -
> > -             start_blk += cluster_size - 1;
> > -
> > -             if (start_blk > last_blk)
> > -                     goto out;
> > -
> > -             goto prep_next;
> > -     }
> > -
> >       if (map.m_pblk == COMPRESS_ADDR) {
> >               compr_cluster = true;
> > -             start_blk++;
> > -             goto prep_next;
> > -     }
> > -
> > -     logical = blks_to_bytes(inode, start_blk);
> > -     phys = blks_to_bytes(inode, map.m_pblk);
> > -     size = blks_to_bytes(inode, map.m_len);
> > -     flags = 0;
> > -     if (map.m_flags & F2FS_MAP_UNWRITTEN)
> > -             flags = FIEMAP_EXTENT_UNWRITTEN;
> > +             count_in_cluster = 1;
> > +     } else if (compr_appended) {
> > +             compr_cluster = false;
> > +     } else {
> > +             logical = blks_to_bytes(inode, start_blk);
> > +             phys = __is_valid_data_blkaddr(map.m_pblk) ?
> > +                     blks_to_bytes(inode, map.m_pblk) : 0;
> > +             size = blks_to_bytes(inode, map.m_len);
> > +             flags = 0;
> > +
> > +             if (compr_cluster) {
> > +                     flags = FIEMAP_EXTENT_ENCODED;
> > +                     count_in_cluster += map.m_len;
> > +                     if (count_in_cluster == cluster_size) {
> > +                             compr_cluster = false;
> > +                             size += blks_to_bytes(inode, 1);
> > +                     }
> > +             } else if (map.m_flags & F2FS_MAP_UNWRITTEN) {
> > +                     flags = FIEMAP_EXTENT_UNWRITTEN;
> > +             }
> >
> > -     start_blk += bytes_to_blks(inode, size);
> > +             start_blk += bytes_to_blks(inode, size);
> > +     }
> >
> >   prep_next:
> >       cond_resched();
> >

Oh, I am sorry for too concrete exlpanation.
I am trying to fix this issue. Let's assume 4 block cluster, which has
been compressed with 3 blocks whose address is 0x1000, 0x5000 and
0x9000.
This cluster is discontinous cluster. In this condition, the previous
version just returns one extent starting from 0x1000 and we are
missing the information related to 0x5000 and 0x9000.
The current version will return 3 extents like below.
Logical Physical Len
0x0      0x1000.  0x1000
0x1.     0x5000.  0x1000
0x2.     0x9000.  0x2000

Thank you for letting me know a bug. I will fix it.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change fiemap way in printing compression chunk
@ 2021-07-23  0:49     ` Daeho Jeong
  0 siblings, 0 replies; 8+ messages in thread
From: Daeho Jeong @ 2021-07-23  0:49 UTC (permalink / raw)
  To: Chao Yu; +Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel

On Thu, Jul 22, 2021 at 5:20 PM Chao Yu <chao@kernel.org> wrote:
>
> Hi Daeho,
>
> On 2021/7/23 5:19, Daeho Jeong wrote:
> > From: Daeho Jeong <daehojeong@google.com>
> >
> > When we print out a discontinuous compression chunk, it shows like a
> > continuous chunk now. To show it more correctly, I've changed the way of
> > printing fiemap info like below. Plus, eliminated NEW_ADDR(-1) in fiemap
> > info, since it is not in fiemap user api manual.
> >
> >     Logical          Physical         Length           Flags
> > 0: 0000000000000000 0000000fdf692000 0000000000004000 1008
> > 1: 0000000000004000 0000000fdf693000 0000000000004000 1008
> > 2: 0000000000008000 0000000fdf694000 0000000000004000 1008
> > 3: 000000000000c000 0000000fdf695000 0000000000004000 1008
> > 4: 0000000000010000 0000000fdf696000 000000000000c000 1000
> > 5: 000000000001c000 0000000f8c60d000 0000000000010000 1000
> > 6: 000000000002c000 0000000f8c61d000 0000000000004000 1008
> > 7: 0000000000030000 0000000f8c620000 0000000000004000 1008
> > 8: 0000000000034000 0000000f8c623000 0000000000001000 1008
> > 9: 0000000000035000 0000000fc7af4000 0000000000003000 1008
>
> I wrote a file some like this:
>
> i_addr[0x9] cluster flag                [0xfffffffe : 4294967294]
> i_addr[0xa]                             [0x   72800 : 468992]
> i_addr[0xb] reserved flag               [0xffffffff : 4294967295]
> i_addr[0xc] reserved flag               [0xffffffff : 4294967295]
> i_addr[0xd] cluster flag                [0xfffffffe : 4294967294]
> i_addr[0xe]                             [0x   72801 : 468993]
> i_addr[0xf] reserved flag               [0xffffffff : 4294967295]
> i_addr[0x10] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x11]                            [0x   72832 : 469042]
> i_addr[0x12]                            [0x   72802 : 468994]
> i_addr[0x13]                            [0x   72833 : 469043]
> i_addr[0x14]                            [0x   72834 : 469044]
> i_addr[0x15] cluster flag               [0xfffffffe : 4294967294]
> i_addr[0x16]                            [0x   72803 : 468995]
> i_addr[0x17] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x18] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x19]                            [0x   72835 : 469045]
> i_addr[0x1a]                            [0x   72804 : 468996]
> i_addr[0x1b]                            [0x   72836 : 469046]
> i_addr[0x1c]                            [0x   72837 : 469047]
> i_addr[0x1d] cluster flag               [0xfffffffe : 4294967294]
> i_addr[0x1e]                            [0x   72805 : 468997]
> i_addr[0x1f] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x20] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x21] cluster flag               [0xfffffffe : 4294967294]
> i_addr[0x22]                            [0x   72806 : 468998]
> i_addr[0x23] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x24] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x25] cluster flag               [0xfffffffe : 4294967294]
> i_addr[0x26]                            [0x   72807 : 468999]
> i_addr[0x27] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x28] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x29] cluster flag               [0xfffffffe : 4294967294]
> i_addr[0x2a]                            [0x   72808 : 469000]
> i_addr[0x2b] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x2c] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x2d] cluster flag               [0xfffffffe : 4294967294]
> i_addr[0x2e]                            [0x   72809 : 469001]
> i_addr[0x2f] reserved flag              [0xffffffff : 4294967295]
> i_addr[0x30] reserved flag              [0xffffffff : 4294967295]
> i_nid[0]                                [0x       0 : 0]
> i_nid[1]                                [0x       0 : 0]
> i_nid[2]                                [0x       0 : 0]
> i_nid[3]                                [0x       0 : 0]
> i_nid[4]                                [0x       0 : 0]
>
> xfs_io file -c "fiemap -v" shows:
>
> before:
>   EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
>     0: [0..31]:         3751936..3751967    32 0x1008
>     1: [32..63]:        3751944..3751975    32 0x1008
>     2: [64..71]:        3752336..3752343     8 0x1000
>     3: [72..79]:        3751952..3751959     8 0x1000
>     4: [80..95]:        3752344..3752359    16 0x1000
>     5: [96..127]:       3751960..3751991    32 0x1008
>     6: [128..135]:      3752360..3752367     8 0x1000
>     7: [136..143]:      3751968..3751975     8 0x1000
>     8: [144..159]:      3752368..3752383    16 0x1000
>     9: [160..191]:      3751976..3752007    32 0x1008
>    10: [192..223]:      3751984..3752015    32 0x1008
>    11: [224..255]:      3751992..3752023    32 0x1008
>    12: [256..287]:      3752000..3752031    32 0x1008
>    13: [288..319]:      3752008..3752039    32 0x1009
>
> after:
>     0: [0..31]:         3751936..3751967    32 0x1008
>     1: [32..63]:        3751944..3751975    32 0x1008
>     2: [64..71]:        3752336..3752343     8 0x1000
>     3: [72..79]:        3751952..3751959     8 0x1000
>     4: [80..95]:        3752344..3752359    16 0x1000
>     5: [96..127]:       3751960..3751991    32 0x1008
>     6: [128..135]:      3752360..3752367     8 0x1000
>     7: [136..143]:      3751968..3751975     8 0x1000
>     8: [144..159]:      3752368..3752383    16 0x1000
>     9: [160..191]:      3751976..3752007    32 0x1008
>    10: [192..223]:      3751984..3752015    32 0x1008
>    11: [224..255]:      3751992..3752023    32 0x1008
>    12: [256..287]:      3752000..3752031    32 0x1008
>    13: [288..319]:      3752008..3752039    32 0x1008
>
> I don't see any obvious difference, except w/ current patch, last
> FIEMAP_EXTENT_LAST is missing.
>
> Sorry, I didn't get the point of this patch, could you please explain
> more details for that problem this patch tries to fix and show the
> difference before/after the patch in commit message?
>
> Thanks,
>
> >
> > Flags
> > 0x1000 => FIEMAP_EXTENT_MERGED
> > 0x0008 => FIEMAP_EXTENT_ENCODED
> >
> > Signed-off-by: Daeho Jeong <daehojeong@google.com>
> >
> > ---
> > v2: changed the print format
> > ---
> >   fs/f2fs/data.c | 76 ++++++++++++++++++++++++++++----------------------
> >   1 file changed, 42 insertions(+), 34 deletions(-)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 3a01a1b50104..058dc751e3a6 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1843,8 +1843,9 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> >       u64 logical = 0, phys = 0, size = 0;
> >       u32 flags = 0;
> >       int ret = 0;
> > -     bool compr_cluster = false;
> > +     bool compr_cluster = false, compr_appended;
> >       unsigned int cluster_size = F2FS_I(inode)->i_cluster_size;
> > +     unsigned int count_in_cluster;
> >       loff_t maxbytes;
> >
> >       if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
> > @@ -1892,8 +1893,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> >       map.m_next_pgofs = &next_pgofs;
> >       map.m_seg_type = NO_CHECK_TYPE;
> >
> > -     if (compr_cluster)
> > -             map.m_len = cluster_size - 1;
> > +     if (compr_cluster) {
> > +             map.m_lblk += 1;
> > +             map.m_len = cluster_size - count_in_cluster;
> > +     }
> >
> >       ret = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_FIEMAP);
> >       if (ret)
> > @@ -1903,11 +1906,23 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> >       if (!(map.m_flags & F2FS_MAP_FLAGS)) {
> >               start_blk = next_pgofs;
> >
> > -             if (blks_to_bytes(inode, start_blk) < blks_to_bytes(inode,
> > +             if (blks_to_bytes(inode, start_blk) >= blks_to_bytes(inode,
> >                                               max_inode_blocks(inode)))
> > +                     flags |= FIEMAP_EXTENT_LAST;
> > +             else if (!compr_cluster)
> >                       goto prep_next;
> > +     }
> > +
> > +     compr_appended = false;
> > +     /* In a case of compressed cluster, append this to the last extent */
> > +     if (compr_cluster && ((map.m_flags & F2FS_MAP_UNWRITTEN) ||
> > +                     !(map.m_flags & F2FS_MAP_FLAGS))) {
> > +             unsigned int appended_blks = cluster_size - count_in_cluster + 1;
> >
> > -             flags |= FIEMAP_EXTENT_LAST;
> > +             size += blks_to_bytes(inode, appended_blks);
> > +             if (map.m_flags & F2FS_MAP_UNWRITTEN)
> > +                     start_blk += appended_blks;
> > +             compr_appended = true;
> >       }
> >
> >       if (size) {
> > @@ -1926,38 +1941,31 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> >       if (start_blk > last_blk)
> >               goto out;
> >
> > -     if (compr_cluster) {
> > -             compr_cluster = false;
> > -
> > -
> > -             logical = blks_to_bytes(inode, start_blk - 1);
> > -             phys = blks_to_bytes(inode, map.m_pblk);
> > -             size = blks_to_bytes(inode, cluster_size);
> > -
> > -             flags |= FIEMAP_EXTENT_ENCODED;
> > -
> > -             start_blk += cluster_size - 1;
> > -
> > -             if (start_blk > last_blk)
> > -                     goto out;
> > -
> > -             goto prep_next;
> > -     }
> > -
> >       if (map.m_pblk == COMPRESS_ADDR) {
> >               compr_cluster = true;
> > -             start_blk++;
> > -             goto prep_next;
> > -     }
> > -
> > -     logical = blks_to_bytes(inode, start_blk);
> > -     phys = blks_to_bytes(inode, map.m_pblk);
> > -     size = blks_to_bytes(inode, map.m_len);
> > -     flags = 0;
> > -     if (map.m_flags & F2FS_MAP_UNWRITTEN)
> > -             flags = FIEMAP_EXTENT_UNWRITTEN;
> > +             count_in_cluster = 1;
> > +     } else if (compr_appended) {
> > +             compr_cluster = false;
> > +     } else {
> > +             logical = blks_to_bytes(inode, start_blk);
> > +             phys = __is_valid_data_blkaddr(map.m_pblk) ?
> > +                     blks_to_bytes(inode, map.m_pblk) : 0;
> > +             size = blks_to_bytes(inode, map.m_len);
> > +             flags = 0;
> > +
> > +             if (compr_cluster) {
> > +                     flags = FIEMAP_EXTENT_ENCODED;
> > +                     count_in_cluster += map.m_len;
> > +                     if (count_in_cluster == cluster_size) {
> > +                             compr_cluster = false;
> > +                             size += blks_to_bytes(inode, 1);
> > +                     }
> > +             } else if (map.m_flags & F2FS_MAP_UNWRITTEN) {
> > +                     flags = FIEMAP_EXTENT_UNWRITTEN;
> > +             }
> >
> > -     start_blk += bytes_to_blks(inode, size);
> > +             start_blk += bytes_to_blks(inode, size);
> > +     }
> >
> >   prep_next:
> >       cond_resched();
> >

Oh, I am sorry for too concrete exlpanation.
I am trying to fix this issue. Let's assume 4 block cluster, which has
been compressed with 3 blocks whose address is 0x1000, 0x5000 and
0x9000.
This cluster is discontinous cluster. In this condition, the previous
version just returns one extent starting from 0x1000 and we are
missing the information related to 0x5000 and 0x9000.
The current version will return 3 extents like below.
Logical Physical Len
0x0      0x1000.  0x1000
0x1.     0x5000.  0x1000
0x2.     0x9000.  0x2000

Thank you for letting me know a bug. I will fix it.


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change fiemap way in printing compression chunk
  2021-07-23  0:49     ` Daeho Jeong
@ 2021-07-23  1:34       ` Chao Yu
  -1 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2021-07-23  1:34 UTC (permalink / raw)
  To: Daeho Jeong; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong

On 2021/7/23 8:49, Daeho Jeong wrote:
> On Thu, Jul 22, 2021 at 5:20 PM Chao Yu <chao@kernel.org> wrote:
>>
>> Hi Daeho,
>>
>> On 2021/7/23 5:19, Daeho Jeong wrote:
>>> From: Daeho Jeong <daehojeong@google.com>
>>>
>>> When we print out a discontinuous compression chunk, it shows like a
>>> continuous chunk now. To show it more correctly, I've changed the way of
>>> printing fiemap info like below. Plus, eliminated NEW_ADDR(-1) in fiemap
>>> info, since it is not in fiemap user api manual.
>>>
>>>      Logical          Physical         Length           Flags
>>> 0: 0000000000000000 0000000fdf692000 0000000000004000 1008
>>> 1: 0000000000004000 0000000fdf693000 0000000000004000 1008
>>> 2: 0000000000008000 0000000fdf694000 0000000000004000 1008
>>> 3: 000000000000c000 0000000fdf695000 0000000000004000 1008
>>> 4: 0000000000010000 0000000fdf696000 000000000000c000 1000
>>> 5: 000000000001c000 0000000f8c60d000 0000000000010000 1000
>>> 6: 000000000002c000 0000000f8c61d000 0000000000004000 1008
>>> 7: 0000000000030000 0000000f8c620000 0000000000004000 1008
>>> 8: 0000000000034000 0000000f8c623000 0000000000001000 1008
>>> 9: 0000000000035000 0000000fc7af4000 0000000000003000 1008
>>
>> I wrote a file some like this:
>>
>> i_addr[0x9] cluster flag                [0xfffffffe : 4294967294]
>> i_addr[0xa]                             [0x   72800 : 468992]
>> i_addr[0xb] reserved flag               [0xffffffff : 4294967295]
>> i_addr[0xc] reserved flag               [0xffffffff : 4294967295]
>> i_addr[0xd] cluster flag                [0xfffffffe : 4294967294]
>> i_addr[0xe]                             [0x   72801 : 468993]
>> i_addr[0xf] reserved flag               [0xffffffff : 4294967295]
>> i_addr[0x10] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x11]                            [0x   72832 : 469042]
>> i_addr[0x12]                            [0x   72802 : 468994]
>> i_addr[0x13]                            [0x   72833 : 469043]
>> i_addr[0x14]                            [0x   72834 : 469044]
>> i_addr[0x15] cluster flag               [0xfffffffe : 4294967294]
>> i_addr[0x16]                            [0x   72803 : 468995]
>> i_addr[0x17] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x18] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x19]                            [0x   72835 : 469045]
>> i_addr[0x1a]                            [0x   72804 : 468996]
>> i_addr[0x1b]                            [0x   72836 : 469046]
>> i_addr[0x1c]                            [0x   72837 : 469047]
>> i_addr[0x1d] cluster flag               [0xfffffffe : 4294967294]
>> i_addr[0x1e]                            [0x   72805 : 468997]
>> i_addr[0x1f] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x20] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x21] cluster flag               [0xfffffffe : 4294967294]
>> i_addr[0x22]                            [0x   72806 : 468998]
>> i_addr[0x23] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x24] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x25] cluster flag               [0xfffffffe : 4294967294]
>> i_addr[0x26]                            [0x   72807 : 468999]
>> i_addr[0x27] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x28] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x29] cluster flag               [0xfffffffe : 4294967294]
>> i_addr[0x2a]                            [0x   72808 : 469000]
>> i_addr[0x2b] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x2c] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x2d] cluster flag               [0xfffffffe : 4294967294]
>> i_addr[0x2e]                            [0x   72809 : 469001]
>> i_addr[0x2f] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x30] reserved flag              [0xffffffff : 4294967295]
>> i_nid[0]                                [0x       0 : 0]
>> i_nid[1]                                [0x       0 : 0]
>> i_nid[2]                                [0x       0 : 0]
>> i_nid[3]                                [0x       0 : 0]
>> i_nid[4]                                [0x       0 : 0]
>>
>> xfs_io file -c "fiemap -v" shows:
>>
>> before:
>>    EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
>>      0: [0..31]:         3751936..3751967    32 0x1008
>>      1: [32..63]:        3751944..3751975    32 0x1008
>>      2: [64..71]:        3752336..3752343     8 0x1000
>>      3: [72..79]:        3751952..3751959     8 0x1000
>>      4: [80..95]:        3752344..3752359    16 0x1000
>>      5: [96..127]:       3751960..3751991    32 0x1008
>>      6: [128..135]:      3752360..3752367     8 0x1000
>>      7: [136..143]:      3751968..3751975     8 0x1000
>>      8: [144..159]:      3752368..3752383    16 0x1000
>>      9: [160..191]:      3751976..3752007    32 0x1008
>>     10: [192..223]:      3751984..3752015    32 0x1008
>>     11: [224..255]:      3751992..3752023    32 0x1008
>>     12: [256..287]:      3752000..3752031    32 0x1008
>>     13: [288..319]:      3752008..3752039    32 0x1009
>>
>> after:
>>      0: [0..31]:         3751936..3751967    32 0x1008
>>      1: [32..63]:        3751944..3751975    32 0x1008
>>      2: [64..71]:        3752336..3752343     8 0x1000
>>      3: [72..79]:        3751952..3751959     8 0x1000
>>      4: [80..95]:        3752344..3752359    16 0x1000
>>      5: [96..127]:       3751960..3751991    32 0x1008
>>      6: [128..135]:      3752360..3752367     8 0x1000
>>      7: [136..143]:      3751968..3751975     8 0x1000
>>      8: [144..159]:      3752368..3752383    16 0x1000
>>      9: [160..191]:      3751976..3752007    32 0x1008
>>     10: [192..223]:      3751984..3752015    32 0x1008
>>     11: [224..255]:      3751992..3752023    32 0x1008
>>     12: [256..287]:      3752000..3752031    32 0x1008
>>     13: [288..319]:      3752008..3752039    32 0x1008
>>
>> I don't see any obvious difference, except w/ current patch, last
>> FIEMAP_EXTENT_LAST is missing.
>>
>> Sorry, I didn't get the point of this patch, could you please explain
>> more details for that problem this patch tries to fix and show the
>> difference before/after the patch in commit message?
>>
>> Thanks,
>>
>>>
>>> Flags
>>> 0x1000 => FIEMAP_EXTENT_MERGED
>>> 0x0008 => FIEMAP_EXTENT_ENCODED
>>>
>>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
>>>
>>> ---
>>> v2: changed the print format
>>> ---
>>>    fs/f2fs/data.c | 76 ++++++++++++++++++++++++++++----------------------
>>>    1 file changed, 42 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 3a01a1b50104..058dc751e3a6 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1843,8 +1843,9 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>        u64 logical = 0, phys = 0, size = 0;
>>>        u32 flags = 0;
>>>        int ret = 0;
>>> -     bool compr_cluster = false;
>>> +     bool compr_cluster = false, compr_appended;
>>>        unsigned int cluster_size = F2FS_I(inode)->i_cluster_size;
>>> +     unsigned int count_in_cluster;
>>>        loff_t maxbytes;
>>>
>>>        if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
>>> @@ -1892,8 +1893,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>        map.m_next_pgofs = &next_pgofs;
>>>        map.m_seg_type = NO_CHECK_TYPE;
>>>
>>> -     if (compr_cluster)
>>> -             map.m_len = cluster_size - 1;
>>> +     if (compr_cluster) {
>>> +             map.m_lblk += 1;
>>> +             map.m_len = cluster_size - count_in_cluster;
>>> +     }
>>>
>>>        ret = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_FIEMAP);
>>>        if (ret)
>>> @@ -1903,11 +1906,23 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>        if (!(map.m_flags & F2FS_MAP_FLAGS)) {
>>>                start_blk = next_pgofs;
>>>
>>> -             if (blks_to_bytes(inode, start_blk) < blks_to_bytes(inode,
>>> +             if (blks_to_bytes(inode, start_blk) >= blks_to_bytes(inode,
>>>                                                max_inode_blocks(inode)))
>>> +                     flags |= FIEMAP_EXTENT_LAST;
>>> +             else if (!compr_cluster)
>>>                        goto prep_next;
>>> +     }
>>> +
>>> +     compr_appended = false;
>>> +     /* In a case of compressed cluster, append this to the last extent */
>>> +     if (compr_cluster && ((map.m_flags & F2FS_MAP_UNWRITTEN) ||
>>> +                     !(map.m_flags & F2FS_MAP_FLAGS))) {
>>> +             unsigned int appended_blks = cluster_size - count_in_cluster + 1;
>>>
>>> -             flags |= FIEMAP_EXTENT_LAST;
>>> +             size += blks_to_bytes(inode, appended_blks);
>>> +             if (map.m_flags & F2FS_MAP_UNWRITTEN)
>>> +                     start_blk += appended_blks;
>>> +             compr_appended = true;
>>>        }
>>>
>>>        if (size) {
>>> @@ -1926,38 +1941,31 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>        if (start_blk > last_blk)
>>>                goto out;
>>>
>>> -     if (compr_cluster) {
>>> -             compr_cluster = false;
>>> -
>>> -
>>> -             logical = blks_to_bytes(inode, start_blk - 1);
>>> -             phys = blks_to_bytes(inode, map.m_pblk);
>>> -             size = blks_to_bytes(inode, cluster_size);
>>> -
>>> -             flags |= FIEMAP_EXTENT_ENCODED;
>>> -
>>> -             start_blk += cluster_size - 1;
>>> -
>>> -             if (start_blk > last_blk)
>>> -                     goto out;
>>> -
>>> -             goto prep_next;
>>> -     }
>>> -
>>>        if (map.m_pblk == COMPRESS_ADDR) {
>>>                compr_cluster = true;
>>> -             start_blk++;
>>> -             goto prep_next;
>>> -     }
>>> -
>>> -     logical = blks_to_bytes(inode, start_blk);
>>> -     phys = blks_to_bytes(inode, map.m_pblk);
>>> -     size = blks_to_bytes(inode, map.m_len);
>>> -     flags = 0;
>>> -     if (map.m_flags & F2FS_MAP_UNWRITTEN)
>>> -             flags = FIEMAP_EXTENT_UNWRITTEN;
>>> +             count_in_cluster = 1;
>>> +     } else if (compr_appended) {
>>> +             compr_cluster = false;
>>> +     } else {
>>> +             logical = blks_to_bytes(inode, start_blk);
>>> +             phys = __is_valid_data_blkaddr(map.m_pblk) ?
>>> +                     blks_to_bytes(inode, map.m_pblk) : 0;
>>> +             size = blks_to_bytes(inode, map.m_len);
>>> +             flags = 0;
>>> +
>>> +             if (compr_cluster) {
>>> +                     flags = FIEMAP_EXTENT_ENCODED;
>>> +                     count_in_cluster += map.m_len;
>>> +                     if (count_in_cluster == cluster_size) {
>>> +                             compr_cluster = false;
>>> +                             size += blks_to_bytes(inode, 1);
>>> +                     }
>>> +             } else if (map.m_flags & F2FS_MAP_UNWRITTEN) {
>>> +                     flags = FIEMAP_EXTENT_UNWRITTEN;
>>> +             }
>>>
>>> -     start_blk += bytes_to_blks(inode, size);
>>> +             start_blk += bytes_to_blks(inode, size);
>>> +     }
>>>
>>>    prep_next:
>>>        cond_resched();
>>>
> 
> Oh, I am sorry for too concrete exlpanation.
> I am trying to fix this issue. Let's assume 4 block cluster, which has
> been compressed with 3 blocks whose address is 0x1000, 0x5000 and
> 0x9000.
> This cluster is discontinous cluster. In this condition, the previous
> version just returns one extent starting from 0x1000 and we are
> missing the information related to 0x5000 and 0x9000.
> The current version will return 3 extents like below.
> Logical Physical Len
> 0x0      0x1000.  0x1000
> 0x1.     0x5000.  0x1000
> 0x2.     0x9000.  0x2000

Ah, thanks for detailed explanation, I misread your commit message
previously... :(

> 
> Thank you for letting me know a bug. I will fix it.

So, let me review the patch after the fix. :)

Thanks,

> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change fiemap way in printing compression chunk
@ 2021-07-23  1:34       ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2021-07-23  1:34 UTC (permalink / raw)
  To: Daeho Jeong; +Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel

On 2021/7/23 8:49, Daeho Jeong wrote:
> On Thu, Jul 22, 2021 at 5:20 PM Chao Yu <chao@kernel.org> wrote:
>>
>> Hi Daeho,
>>
>> On 2021/7/23 5:19, Daeho Jeong wrote:
>>> From: Daeho Jeong <daehojeong@google.com>
>>>
>>> When we print out a discontinuous compression chunk, it shows like a
>>> continuous chunk now. To show it more correctly, I've changed the way of
>>> printing fiemap info like below. Plus, eliminated NEW_ADDR(-1) in fiemap
>>> info, since it is not in fiemap user api manual.
>>>
>>>      Logical          Physical         Length           Flags
>>> 0: 0000000000000000 0000000fdf692000 0000000000004000 1008
>>> 1: 0000000000004000 0000000fdf693000 0000000000004000 1008
>>> 2: 0000000000008000 0000000fdf694000 0000000000004000 1008
>>> 3: 000000000000c000 0000000fdf695000 0000000000004000 1008
>>> 4: 0000000000010000 0000000fdf696000 000000000000c000 1000
>>> 5: 000000000001c000 0000000f8c60d000 0000000000010000 1000
>>> 6: 000000000002c000 0000000f8c61d000 0000000000004000 1008
>>> 7: 0000000000030000 0000000f8c620000 0000000000004000 1008
>>> 8: 0000000000034000 0000000f8c623000 0000000000001000 1008
>>> 9: 0000000000035000 0000000fc7af4000 0000000000003000 1008
>>
>> I wrote a file some like this:
>>
>> i_addr[0x9] cluster flag                [0xfffffffe : 4294967294]
>> i_addr[0xa]                             [0x   72800 : 468992]
>> i_addr[0xb] reserved flag               [0xffffffff : 4294967295]
>> i_addr[0xc] reserved flag               [0xffffffff : 4294967295]
>> i_addr[0xd] cluster flag                [0xfffffffe : 4294967294]
>> i_addr[0xe]                             [0x   72801 : 468993]
>> i_addr[0xf] reserved flag               [0xffffffff : 4294967295]
>> i_addr[0x10] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x11]                            [0x   72832 : 469042]
>> i_addr[0x12]                            [0x   72802 : 468994]
>> i_addr[0x13]                            [0x   72833 : 469043]
>> i_addr[0x14]                            [0x   72834 : 469044]
>> i_addr[0x15] cluster flag               [0xfffffffe : 4294967294]
>> i_addr[0x16]                            [0x   72803 : 468995]
>> i_addr[0x17] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x18] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x19]                            [0x   72835 : 469045]
>> i_addr[0x1a]                            [0x   72804 : 468996]
>> i_addr[0x1b]                            [0x   72836 : 469046]
>> i_addr[0x1c]                            [0x   72837 : 469047]
>> i_addr[0x1d] cluster flag               [0xfffffffe : 4294967294]
>> i_addr[0x1e]                            [0x   72805 : 468997]
>> i_addr[0x1f] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x20] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x21] cluster flag               [0xfffffffe : 4294967294]
>> i_addr[0x22]                            [0x   72806 : 468998]
>> i_addr[0x23] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x24] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x25] cluster flag               [0xfffffffe : 4294967294]
>> i_addr[0x26]                            [0x   72807 : 468999]
>> i_addr[0x27] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x28] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x29] cluster flag               [0xfffffffe : 4294967294]
>> i_addr[0x2a]                            [0x   72808 : 469000]
>> i_addr[0x2b] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x2c] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x2d] cluster flag               [0xfffffffe : 4294967294]
>> i_addr[0x2e]                            [0x   72809 : 469001]
>> i_addr[0x2f] reserved flag              [0xffffffff : 4294967295]
>> i_addr[0x30] reserved flag              [0xffffffff : 4294967295]
>> i_nid[0]                                [0x       0 : 0]
>> i_nid[1]                                [0x       0 : 0]
>> i_nid[2]                                [0x       0 : 0]
>> i_nid[3]                                [0x       0 : 0]
>> i_nid[4]                                [0x       0 : 0]
>>
>> xfs_io file -c "fiemap -v" shows:
>>
>> before:
>>    EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
>>      0: [0..31]:         3751936..3751967    32 0x1008
>>      1: [32..63]:        3751944..3751975    32 0x1008
>>      2: [64..71]:        3752336..3752343     8 0x1000
>>      3: [72..79]:        3751952..3751959     8 0x1000
>>      4: [80..95]:        3752344..3752359    16 0x1000
>>      5: [96..127]:       3751960..3751991    32 0x1008
>>      6: [128..135]:      3752360..3752367     8 0x1000
>>      7: [136..143]:      3751968..3751975     8 0x1000
>>      8: [144..159]:      3752368..3752383    16 0x1000
>>      9: [160..191]:      3751976..3752007    32 0x1008
>>     10: [192..223]:      3751984..3752015    32 0x1008
>>     11: [224..255]:      3751992..3752023    32 0x1008
>>     12: [256..287]:      3752000..3752031    32 0x1008
>>     13: [288..319]:      3752008..3752039    32 0x1009
>>
>> after:
>>      0: [0..31]:         3751936..3751967    32 0x1008
>>      1: [32..63]:        3751944..3751975    32 0x1008
>>      2: [64..71]:        3752336..3752343     8 0x1000
>>      3: [72..79]:        3751952..3751959     8 0x1000
>>      4: [80..95]:        3752344..3752359    16 0x1000
>>      5: [96..127]:       3751960..3751991    32 0x1008
>>      6: [128..135]:      3752360..3752367     8 0x1000
>>      7: [136..143]:      3751968..3751975     8 0x1000
>>      8: [144..159]:      3752368..3752383    16 0x1000
>>      9: [160..191]:      3751976..3752007    32 0x1008
>>     10: [192..223]:      3751984..3752015    32 0x1008
>>     11: [224..255]:      3751992..3752023    32 0x1008
>>     12: [256..287]:      3752000..3752031    32 0x1008
>>     13: [288..319]:      3752008..3752039    32 0x1008
>>
>> I don't see any obvious difference, except w/ current patch, last
>> FIEMAP_EXTENT_LAST is missing.
>>
>> Sorry, I didn't get the point of this patch, could you please explain
>> more details for that problem this patch tries to fix and show the
>> difference before/after the patch in commit message?
>>
>> Thanks,
>>
>>>
>>> Flags
>>> 0x1000 => FIEMAP_EXTENT_MERGED
>>> 0x0008 => FIEMAP_EXTENT_ENCODED
>>>
>>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
>>>
>>> ---
>>> v2: changed the print format
>>> ---
>>>    fs/f2fs/data.c | 76 ++++++++++++++++++++++++++++----------------------
>>>    1 file changed, 42 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 3a01a1b50104..058dc751e3a6 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1843,8 +1843,9 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>        u64 logical = 0, phys = 0, size = 0;
>>>        u32 flags = 0;
>>>        int ret = 0;
>>> -     bool compr_cluster = false;
>>> +     bool compr_cluster = false, compr_appended;
>>>        unsigned int cluster_size = F2FS_I(inode)->i_cluster_size;
>>> +     unsigned int count_in_cluster;
>>>        loff_t maxbytes;
>>>
>>>        if (fieinfo->fi_flags & FIEMAP_FLAG_CACHE) {
>>> @@ -1892,8 +1893,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>        map.m_next_pgofs = &next_pgofs;
>>>        map.m_seg_type = NO_CHECK_TYPE;
>>>
>>> -     if (compr_cluster)
>>> -             map.m_len = cluster_size - 1;
>>> +     if (compr_cluster) {
>>> +             map.m_lblk += 1;
>>> +             map.m_len = cluster_size - count_in_cluster;
>>> +     }
>>>
>>>        ret = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_FIEMAP);
>>>        if (ret)
>>> @@ -1903,11 +1906,23 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>        if (!(map.m_flags & F2FS_MAP_FLAGS)) {
>>>                start_blk = next_pgofs;
>>>
>>> -             if (blks_to_bytes(inode, start_blk) < blks_to_bytes(inode,
>>> +             if (blks_to_bytes(inode, start_blk) >= blks_to_bytes(inode,
>>>                                                max_inode_blocks(inode)))
>>> +                     flags |= FIEMAP_EXTENT_LAST;
>>> +             else if (!compr_cluster)
>>>                        goto prep_next;
>>> +     }
>>> +
>>> +     compr_appended = false;
>>> +     /* In a case of compressed cluster, append this to the last extent */
>>> +     if (compr_cluster && ((map.m_flags & F2FS_MAP_UNWRITTEN) ||
>>> +                     !(map.m_flags & F2FS_MAP_FLAGS))) {
>>> +             unsigned int appended_blks = cluster_size - count_in_cluster + 1;
>>>
>>> -             flags |= FIEMAP_EXTENT_LAST;
>>> +             size += blks_to_bytes(inode, appended_blks);
>>> +             if (map.m_flags & F2FS_MAP_UNWRITTEN)
>>> +                     start_blk += appended_blks;
>>> +             compr_appended = true;
>>>        }
>>>
>>>        if (size) {
>>> @@ -1926,38 +1941,31 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>        if (start_blk > last_blk)
>>>                goto out;
>>>
>>> -     if (compr_cluster) {
>>> -             compr_cluster = false;
>>> -
>>> -
>>> -             logical = blks_to_bytes(inode, start_blk - 1);
>>> -             phys = blks_to_bytes(inode, map.m_pblk);
>>> -             size = blks_to_bytes(inode, cluster_size);
>>> -
>>> -             flags |= FIEMAP_EXTENT_ENCODED;
>>> -
>>> -             start_blk += cluster_size - 1;
>>> -
>>> -             if (start_blk > last_blk)
>>> -                     goto out;
>>> -
>>> -             goto prep_next;
>>> -     }
>>> -
>>>        if (map.m_pblk == COMPRESS_ADDR) {
>>>                compr_cluster = true;
>>> -             start_blk++;
>>> -             goto prep_next;
>>> -     }
>>> -
>>> -     logical = blks_to_bytes(inode, start_blk);
>>> -     phys = blks_to_bytes(inode, map.m_pblk);
>>> -     size = blks_to_bytes(inode, map.m_len);
>>> -     flags = 0;
>>> -     if (map.m_flags & F2FS_MAP_UNWRITTEN)
>>> -             flags = FIEMAP_EXTENT_UNWRITTEN;
>>> +             count_in_cluster = 1;
>>> +     } else if (compr_appended) {
>>> +             compr_cluster = false;
>>> +     } else {
>>> +             logical = blks_to_bytes(inode, start_blk);
>>> +             phys = __is_valid_data_blkaddr(map.m_pblk) ?
>>> +                     blks_to_bytes(inode, map.m_pblk) : 0;
>>> +             size = blks_to_bytes(inode, map.m_len);
>>> +             flags = 0;
>>> +
>>> +             if (compr_cluster) {
>>> +                     flags = FIEMAP_EXTENT_ENCODED;
>>> +                     count_in_cluster += map.m_len;
>>> +                     if (count_in_cluster == cluster_size) {
>>> +                             compr_cluster = false;
>>> +                             size += blks_to_bytes(inode, 1);
>>> +                     }
>>> +             } else if (map.m_flags & F2FS_MAP_UNWRITTEN) {
>>> +                     flags = FIEMAP_EXTENT_UNWRITTEN;
>>> +             }
>>>
>>> -     start_blk += bytes_to_blks(inode, size);
>>> +             start_blk += bytes_to_blks(inode, size);
>>> +     }
>>>
>>>    prep_next:
>>>        cond_resched();
>>>
> 
> Oh, I am sorry for too concrete exlpanation.
> I am trying to fix this issue. Let's assume 4 block cluster, which has
> been compressed with 3 blocks whose address is 0x1000, 0x5000 and
> 0x9000.
> This cluster is discontinous cluster. In this condition, the previous
> version just returns one extent starting from 0x1000 and we are
> missing the information related to 0x5000 and 0x9000.
> The current version will return 3 extents like below.
> Logical Physical Len
> 0x0      0x1000.  0x1000
> 0x1.     0x5000.  0x1000
> 0x2.     0x9000.  0x2000

Ah, thanks for detailed explanation, I misread your commit message
previously... :(

> 
> Thank you for letting me know a bug. I will fix it.

So, let me review the patch after the fix. :)

Thanks,

> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-07-23  1:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-22 21:19 [PATCH v2] f2fs: change fiemap way in printing compression chunk Daeho Jeong
2021-07-22 21:19 ` [f2fs-dev] " Daeho Jeong
2021-07-23  0:20 ` Chao Yu
2021-07-23  0:20   ` Chao Yu
2021-07-23  0:49   ` Daeho Jeong
2021-07-23  0:49     ` Daeho Jeong
2021-07-23  1:34     ` Chao Yu
2021-07-23  1:34       ` Chao Yu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.