[PATCH 0/4] misc compression tracing related patches

[PATCH 0/4] misc compression tracing related patches

[Anand Jain]

Anand Jain (4):
  btrfs: remove unused BTRFS_COMPRESS_LAST
  btrfs: convert enum btrfs_compression_type to define
  btrfs: decode compress type for tracing
  btrfs: add compression trace points

 fs/btrfs/compression.c          | 11 +++++++
 fs/btrfs/compression.h          |  8 ------
 fs/btrfs/super.c                |  2 +-
 include/trace/events/btrfs.h    | 64 +++++++++++++++++++++++++++++++++++------
 include/uapi/linux/btrfs_tree.h |  5 ++++
 5 files changed, 72 insertions(+), 18 deletions(-)

-- 
2.13.1

[PATCH 1/4] btrfs: remove unused BTRFS_COMPRESS_LAST

[Anand Jain]

We aren't using this define, so removing it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/compression.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 87f6d3332163..e749df6dd39a 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -100,7 +100,6 @@ enum btrfs_compression_type {
 	BTRFS_COMPRESS_ZLIB  = 1,
 	BTRFS_COMPRESS_LZO   = 2,
 	BTRFS_COMPRESS_TYPES = 2,
-	BTRFS_COMPRESS_LAST  = 3,
 };
 
 struct btrfs_compress_op {
-- 
2.13.1

[PATCH 2/4] btrfs: convert enum btrfs_compression_type to define

[Anand Jain]

There isn't a huge list to manage the types, which can be managed
with defines. It helps to easily print the types in tracing as well.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/compression.h          | 7 -------
 fs/btrfs/super.c                | 2 +-
 include/uapi/linux/btrfs_tree.h | 5 +++++
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index e749df6dd39a..f28a501e7828 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -95,13 +95,6 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start,
 blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 				 int mirror_num, unsigned long bio_flags);
 
-enum btrfs_compression_type {
-	BTRFS_COMPRESS_NONE  = 0,
-	BTRFS_COMPRESS_ZLIB  = 1,
-	BTRFS_COMPRESS_LZO   = 2,
-	BTRFS_COMPRESS_TYPES = 2,
-};
-
 struct btrfs_compress_op {
 	struct list_head *(*alloc_workspace)(void);
 
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 12540b6104b5..b711357352f8 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -404,7 +404,7 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
 	int ret = 0;
 	char *compress_type;
 	bool compress_force = false;
-	enum btrfs_compression_type saved_compress_type;
+	unsigned int saved_compress_type;
 	bool saved_compress_force;
 	int no_compress = 0;
 
diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
index 10689e1fdf11..7a1fec2d10ab 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -733,6 +733,11 @@ struct btrfs_balance_item {
 #define BTRFS_FILE_EXTENT_REG 1
 #define BTRFS_FILE_EXTENT_PREALLOC 2
 
+#define BTRFS_COMPRESS_NONE	0
+#define BTRFS_COMPRESS_TYPES	2
+#define BTRFS_COMPRESS_ZLIB	1
+#define BTRFS_COMPRESS_LZO	2
+
 struct btrfs_file_extent_item {
 	/*
 	 * transaction id that created this extent
-- 
2.13.1

[PATCH 3/4] btrfs: decode compress type for tracing

[Anand Jain]

So with this now we see the compression type in string.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 include/trace/events/btrfs.h | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index c4e4b9427b81..d412c49f5a6a 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -190,6 +190,12 @@ DEFINE_EVENT(btrfs__inode, btrfs_inode_evict,
 		{ (1 << EXTENT_FLAG_FILLING),	 	"FILLING" 	},\
 		{ (1 << EXTENT_FLAG_FS_MAPPING),	"FS_MAPPING"	})
 
+#define show_compress_type(type)		 \
+	__print_symbolic(type,			 \
+		{ BTRFS_COMPRESS_NONE, 	"none" },\
+		{ BTRFS_COMPRESS_ZLIB, 	"zlib" },\
+		{ BTRFS_COMPRESS_LZO, 	"lzo" })
+
 TRACE_EVENT_CONDITION(btrfs_get_extent,
 
 	TP_PROTO(struct btrfs_root *root, struct btrfs_inode *inode,
@@ -228,7 +234,7 @@ TRACE_EVENT_CONDITION(btrfs_get_extent,
 	TP_printk_btrfs("root=%llu(%s) ino=%llu start=%llu len=%llu "
 		  "orig_start=%llu block_start=%llu(%s) "
 		  "block_len=%llu flags=%s refs=%u "
-		  "compress_type=%u",
+		  "compress_type=%s",
 		  show_root_type(__entry->root_objectid),
 		  (unsigned long long)__entry->ino,
 		  (unsigned long long)__entry->start,
@@ -236,8 +242,8 @@ TRACE_EVENT_CONDITION(btrfs_get_extent,
 		  (unsigned long long)__entry->orig_start,
 		  show_map_type(__entry->block_start),
 		  (unsigned long long)__entry->block_len,
-		  show_map_flags(__entry->flags),
-		  __entry->refs, __entry->compress_type)
+		  show_map_flags(__entry->flags), __entry->refs,
+		  show_compress_type(__entry->compress_type))
 );
 
 /* file extent item */
@@ -285,14 +291,14 @@ DECLARE_EVENT_CLASS(btrfs__file_extent_item_regular,
 		"file extent range=[%llu %llu] "
 		"(num_bytes=%llu ram_bytes=%llu disk_bytenr=%llu "
 		"disk_num_bytes=%llu extent_offset=%llu type=%s "
-		"compression=%u",
+		"compression=%s",
 		show_root_type(__entry->root_obj), __entry->ino,
 		__entry->isize,
 		__entry->disk_isize, __entry->extent_start,
 		__entry->extent_end, __entry->num_bytes, __entry->ram_bytes,
 		__entry->disk_bytenr, __entry->disk_num_bytes,
 		__entry->extent_offset, show_fi_type(__entry->extent_type),
-		__entry->compression)
+		show_compress_type(__entry->compression))
 );
 
 DECLARE_EVENT_CLASS(
@@ -329,11 +335,11 @@ DECLARE_EVENT_CLASS(
 	TP_printk_btrfs(
 		"root=%llu(%s) inode=%llu size=%llu disk_isize=%llu "
 		"file extent range=[%llu %llu] "
-		"extent_type=%s compression=%u",
+		"extent_type=%s compression=%s",
 		show_root_type(__entry->root_obj), __entry->ino, __entry->isize,
 		__entry->disk_isize, __entry->extent_start,
 		__entry->extent_end, show_fi_type(__entry->extent_type),
-		__entry->compression)
+		show_compress_type(__entry->compression))
 );
 
 DEFINE_EVENT(
@@ -424,7 +430,7 @@ DECLARE_EVENT_CLASS(btrfs__ordered_extent,
 	TP_printk_btrfs("root=%llu(%s) ino=%llu file_offset=%llu "
 		  "start=%llu len=%llu disk_len=%llu "
 		  "truncated_len=%llu "
-		  "bytes_left=%llu flags=%s compress_type=%d "
+		  "bytes_left=%llu flags=%s compress_type=%s "
 		  "refs=%d",
 		  show_root_type(__entry->root_objectid),
 		  (unsigned long long)__entry->ino,
@@ -435,7 +441,8 @@ DECLARE_EVENT_CLASS(btrfs__ordered_extent,
 		  (unsigned long long)__entry->truncated_len,
 		  (unsigned long long)__entry->bytes_left,
 		  show_ordered_flags(__entry->flags),
-		  __entry->compress_type, __entry->refs)
+		  show_compress_type(__entry->compress_type),
+		  __entry->refs)
 );
 
 DEFINE_EVENT(btrfs__ordered_extent, btrfs_ordered_extent_add,
-- 
2.13.1

[PATCH 4/4 v3] btrfs: add compression trace points

[Anand Jain]

From: Anand Jain <Anand.Jain@oracle.com>

This patch adds compression and decompression trace points for the
purpose of debugging.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
v3:
 . Rename to a simple names, without worrying about being
   compatible with the future naming.
 . The type was not working fixed it.
v2:
 . Use better naming.
   (If transform is not good enough I have run out of ideas, pls suggest).
 . To be applied on top of
   git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
   (tested without namelen check patch set)
 fs/btrfs/compression.c       | 11 +++++++++++
 include/trace/events/btrfs.h | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index d2ef9ac2a630..4a652f67ee87 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -895,6 +895,10 @@ int btrfs_compress_pages(int type, struct address_space *mapping,
 						      start, pages,
 						      out_pages,
 						      total_in, total_out);
+
+	trace_btrfs_compress(1, 1, mapping->host, type, *total_in,
+						*total_out, start, ret);
+
 	free_workspace(type, workspace);
 	return ret;
 }
@@ -921,6 +925,10 @@ static int btrfs_decompress_bio(struct compressed_bio *cb)
 
 	workspace = find_workspace(type);
 	ret = btrfs_compress_op[type - 1]->decompress_bio(workspace, cb);
+
+	trace_btrfs_compress(0, 0, cb->inode, type,
+				cb->compressed_len, cb->len, cb->start, ret);
+
 	free_workspace(type, workspace);
 
 	return ret;
@@ -943,6 +951,9 @@ int btrfs_decompress(int type, unsigned char *data_in, struct page *dest_page,
 						  dest_page, start_byte,
 						  srclen, destlen);
 
+	trace_btrfs_compress(0, 1, dest_page->mapping->host,
+				type, srclen, destlen, start_byte, ret);
+
 	free_workspace(type, workspace);
 	return ret;
 }
diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index d412c49f5a6a..db33d6649d12 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -1629,6 +1629,45 @@ TRACE_EVENT(qgroup_meta_reserve,
 		show_root_type(__entry->refroot), __entry->diff)
 );
 
+TRACE_EVENT(btrfs_compress,
+
+	TP_PROTO(int compress, int page, struct inode *inode,
+			unsigned int type,
+			unsigned long len_before, unsigned long len_after,
+			unsigned long start, int ret),
+
+	TP_ARGS(compress, page, inode, type, len_before,
+					len_after, start, ret),
+
+	TP_STRUCT__entry_btrfs(
+		__field(int,				compress)
+		__field(int,				page)
+		__field(ino_t,				i_ino)
+		__field(unsigned int,			type)
+		__field(unsigned long,			len_before)
+		__field(unsigned long,			len_after)
+		__field(unsigned long,			start)
+		__field(int,				ret)
+	),
+
+	TP_fast_assign_btrfs(btrfs_sb(inode->i_sb),
+		__entry->compress	= compress;
+		__entry->page		= page;
+		__entry->i_ino		= inode->i_ino;
+		__entry->type		= type;
+		__entry->len_before	= len_before;
+		__entry->len_after	= len_after;
+		__entry->start		= start;
+		__entry->ret		= ret;
+	),
+
+	TP_printk_btrfs("%s %s ino=%lu type=%s len_before=%lu len_after=%lu start=%lu ret=%d",
+		__entry->compress ? "compress":"uncompress",
+		__entry->page ? "page":"bio", __entry->i_ino,
+		show_compress_type(__entry->type),
+		__entry->len_before, __entry->len_after, __entry->start,
+		__entry->ret)
+);
 #endif /* _TRACE_BTRFS_H */
 
 /* This part must be outside protection */
-- 
2.13.1

Re: [PATCH 4/4 v3] btrfs: add compression trace points

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3407 bytes --]

Hi Anand,

[auto build test WARNING on tip/perf/core]
[also build test WARNING on v4.13-rc5]
[cannot apply to btrfs/next next-20170815]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Anand-Jain/misc-compression-tracing-related-patches/20170816-043401
config: s390-allmodconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=s390 

All warnings (new ones prefixed by >>):

   In file included from include/trace/define_trace.h:95:0,
                    from include/trace/events/btrfs.h:1674,
                    from fs/btrfs/super.c:65:
   include/trace/events/btrfs.h: In function 'trace_raw_output_btrfs_compress':
>> include/trace/events/btrfs.h:91:12: warning: format '%lu' expects argument of type 'long unsigned int', but argument 6 has type 'ino_t {aka unsigned int}' [-Wformat=]
     TP_printk("%pU: " fmt, __entry->fsid, args)
               ^
   include/trace/trace_events.h:359:22: note: in definition of macro 'DECLARE_EVENT_CLASS'
     trace_seq_printf(s, print);     \
                         ^~~~~
   include/trace/trace_events.h:78:9: note: in expansion of macro 'PARAMS'
            PARAMS(print));         \
            ^~~~~~
>> include/trace/events/btrfs.h:1632:1: note: in expansion of macro 'TRACE_EVENT'
    TRACE_EVENT(btrfs_compress,
    ^~~~~~~~~~~
>> include/trace/events/btrfs.h:91:2: note: in expansion of macro 'TP_printk'
     TP_printk("%pU: " fmt, __entry->fsid, args)
     ^~~~~~~~~
>> include/trace/events/btrfs.h:1664:2: note: in expansion of macro 'TP_printk_btrfs'
     TP_printk_btrfs("%s %s ino=%lu type=%s len_before=%lu len_after=%lu start=%lu ret=%d",
     ^~~~~~~~~~~~~~~

vim +91 include/trace/events/btrfs.h

bc074524 Jeff Mahoney 2016-06-09  78  
bc074524 Jeff Mahoney 2016-06-09  79  #define TP_fast_assign_fsid(fs_info)					\
bc074524 Jeff Mahoney 2016-06-09  80  	memcpy(__entry->fsid, fs_info->fsid, BTRFS_UUID_SIZE)
bc074524 Jeff Mahoney 2016-06-09  81  
bc074524 Jeff Mahoney 2016-06-09  82  #define TP_STRUCT__entry_btrfs(args...)					\
bc074524 Jeff Mahoney 2016-06-09  83  	TP_STRUCT__entry(						\
bc074524 Jeff Mahoney 2016-06-09  84  		TP_STRUCT__entry_fsid					\
bc074524 Jeff Mahoney 2016-06-09  85  		args)
bc074524 Jeff Mahoney 2016-06-09  86  #define TP_fast_assign_btrfs(fs_info, args...)				\
bc074524 Jeff Mahoney 2016-06-09  87  	TP_fast_assign(							\
bc074524 Jeff Mahoney 2016-06-09  88  		TP_fast_assign_fsid(fs_info);				\
bc074524 Jeff Mahoney 2016-06-09  89  		args)
bc074524 Jeff Mahoney 2016-06-09  90  #define TP_printk_btrfs(fmt, args...) \
bc074524 Jeff Mahoney 2016-06-09 @91  	TP_printk("%pU: " fmt, __entry->fsid, args)
8c2a3ca2 Josef Bacik  2012-01-10  92  

:::::: The code at line 91 was first introduced by commit
:::::: bc074524e123ded281cde25ebc5661910f9679e3 btrfs: prefix fsid to all trace events

:::::: TO: Jeff Mahoney <jeffm@suse.com>
:::::: CC: David Sterba <dsterba@suse.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47231 bytes --]

Re: [PATCH 1/4] btrfs: remove unused BTRFS_COMPRESS_LAST

[David Sterba]

On Sun, Aug 13, 2017 at 12:02:41PM +0800, Anand Jain wrote:
> We aren't using this define, so removing it.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Reviewed-by: David Sterba <dsterba@suse.com>

Re: [PATCH 2/4] btrfs: convert enum btrfs_compression_type to define

[David Sterba]

On Sun, Aug 13, 2017 at 12:02:42PM +0800, Anand Jain wrote:
> There isn't a huge list to manage the types, which can be managed
> with defines. It helps to easily print the types in tracing as well.

We use enums in a lot of places, I'd rather keep it as it is.

Re: [PATCH 3/4] btrfs: decode compress type for tracing

[David Sterba]

On Sun, Aug 13, 2017 at 12:02:43PM +0800, Anand Jain wrote:
> So with this now we see the compression type in string.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Reviewed-by: David Sterba <dsterba@suse.com>

Re: [PATCH 4/4 v3] btrfs: add compression trace points

[David Sterba]

On Sun, Aug 13, 2017 at 12:02:44PM +0800, Anand Jain wrote:
> From: Anand Jain <Anand.Jain@oracle.com>
> 
> This patch adds compression and decompression trace points for the
> purpose of debugging.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> v3:
>  . Rename to a simple names, without worrying about being
>    compatible with the future naming.
>  . The type was not working fixed it.
> v2:
>  . Use better naming.
>    (If transform is not good enough I have run out of ideas, pls suggest).
>  . To be applied on top of
>    git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
>    (tested without namelen check patch set)
>  fs/btrfs/compression.c       | 11 +++++++++++
>  include/trace/events/btrfs.h | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index d2ef9ac2a630..4a652f67ee87 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -895,6 +895,10 @@ int btrfs_compress_pages(int type, struct address_space *mapping,
>  						      start, pages,
>  						      out_pages,
>  						      total_in, total_out);
> +
> +	trace_btrfs_compress(1, 1, mapping->host, type, *total_in,
> +						*total_out, start, ret);
> +
>  	free_workspace(type, workspace);
>  	return ret;
>  }
> @@ -921,6 +925,10 @@ static int btrfs_decompress_bio(struct compressed_bio *cb)
>  
>  	workspace = find_workspace(type);
>  	ret = btrfs_compress_op[type - 1]->decompress_bio(workspace, cb);
> +
> +	trace_btrfs_compress(0, 0, cb->inode, type,
> +				cb->compressed_len, cb->len, cb->start, ret);
> +
>  	free_workspace(type, workspace);
>  
>  	return ret;
> @@ -943,6 +951,9 @@ int btrfs_decompress(int type, unsigned char *data_in, struct page *dest_page,
>  						  dest_page, start_byte,
>  						  srclen, destlen);
>  
> +	trace_btrfs_compress(0, 1, dest_page->mapping->host,
> +				type, srclen, destlen, start_byte, ret);
> +
>  	free_workspace(type, workspace);
>  	return ret;
>  }
> diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
> index d412c49f5a6a..db33d6649d12 100644
> --- a/include/trace/events/btrfs.h
> +++ b/include/trace/events/btrfs.h
> @@ -1629,6 +1629,45 @@ TRACE_EVENT(qgroup_meta_reserve,
>  		show_root_type(__entry->refroot), __entry->diff)
>  );
>  
> +TRACE_EVENT(btrfs_compress,
> +
> +	TP_PROTO(int compress, int page, struct inode *inode,
> +			unsigned int type,
> +			unsigned long len_before, unsigned long len_after,
> +			unsigned long start, int ret),
> +
> +	TP_ARGS(compress, page, inode, type, len_before,
> +					len_after, start, ret),
> +
> +	TP_STRUCT__entry_btrfs(
> +		__field(int,				compress)
> +		__field(int,				page)
> +		__field(ino_t,				i_ino)

u64 for the inode number

> +		__field(unsigned int,			type)
> +		__field(unsigned long,			len_before)
> +		__field(unsigned long,			len_after)
> +		__field(unsigned long,			start)

and u64 here

> +		__field(int,				ret)
> +	),
> +
> +	TP_fast_assign_btrfs(btrfs_sb(inode->i_sb),
> +		__entry->compress	= compress;
> +		__entry->page		= page;
> +		__entry->i_ino		= inode->i_ino;
> +		__entry->type		= type;
> +		__entry->len_before	= len_before;
> +		__entry->len_after	= len_after;
> +		__entry->start		= start;
> +		__entry->ret		= ret;
> +	),
> +
> +	TP_printk_btrfs("%s %s ino=%lu type=%s len_before=%lu len_after=%lu start=%lu ret=%d",

The format looks good, although I'm not sure we need to make the
distinction between page and bio compression. This also needs the extra
argument for the tracepoint.

> +		__entry->compress ? "compress":"uncompress",

decompress

> +		__entry->page ? "page":"bio", __entry->i_ino,

add spaces around :

> +		show_compress_type(__entry->type),
> +		__entry->len_before, __entry->len_after, __entry->start,
> +		__entry->ret)
> +);
>  #endif /* _TRACE_BTRFS_H */
>  
>  /* This part must be outside protection */

Re: [PATCH 2/4] btrfs: convert enum btrfs_compression_type to define

[Anand Jain]

On 08/16/2017 09:59 PM, David Sterba wrote:
> On Sun, Aug 13, 2017 at 12:02:42PM +0800, Anand Jain wrote:
>> There isn't a huge list to manage the types, which can be managed
>> with defines. It helps to easily print the types in tracing as well.
> 
> We use enums in a lot of places, I'd rather keep it as it is.

  This patch converts all of them, and it was at only one place.
  I hope I didn't miss any. Further the next patch 3/4 needs it
  to be define instead of enums, handling enums in the tracing
  isn't as easy as define.

Thanks, Anand

[PATCH 4/4 v4] btrfs: add compression trace points

[Anand Jain]

From: Anand Jain <Anand.Jain@oracle.com>

This patch adds compression and decompression trace points for the
purpose of debugging.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
v4:
 Accepts David's review comments
 . changes from unsigned long to u64.
 . format changes
v3:
 . Rename to a simple names, without worrying about being
   compatible with the future naming.
 . The type was not working fixed it.
v2:
 . Use better naming.
   (If transform is not good enough I have run out of ideas, pls suggest).
 . To be applied on top of
   git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
   (tested without namelen check patch set)

 fs/btrfs/compression.c       | 11 +++++++++++
 include/trace/events/btrfs.h | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index d2ef9ac2a630..4a652f67ee87 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -895,6 +895,10 @@ int btrfs_compress_pages(int type, struct address_space *mapping,
 						      start, pages,
 						      out_pages,
 						      total_in, total_out);
+
+	trace_btrfs_compress(1, 1, mapping->host, type, *total_in,
+						*total_out, start, ret);
+
 	free_workspace(type, workspace);
 	return ret;
 }
@@ -921,6 +925,10 @@ static int btrfs_decompress_bio(struct compressed_bio *cb)
 
 	workspace = find_workspace(type);
 	ret = btrfs_compress_op[type - 1]->decompress_bio(workspace, cb);
+
+	trace_btrfs_compress(0, 0, cb->inode, type,
+				cb->compressed_len, cb->len, cb->start, ret);
+
 	free_workspace(type, workspace);
 
 	return ret;
@@ -943,6 +951,9 @@ int btrfs_decompress(int type, unsigned char *data_in, struct page *dest_page,
 						  dest_page, start_byte,
 						  srclen, destlen);
 
+	trace_btrfs_compress(0, 1, dest_page->mapping->host,
+				type, srclen, destlen, start_byte, ret);
+
 	free_workspace(type, workspace);
 	return ret;
 }
diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index d412c49f5a6a..d0c0bd4fe3c2 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -1629,6 +1629,42 @@ TRACE_EVENT(qgroup_meta_reserve,
 		show_root_type(__entry->refroot), __entry->diff)
 );
 
+TRACE_EVENT(btrfs_compress,
+
+	TP_PROTO(int compress, int page, struct inode *inode, unsigned int type,
+			u64 len_before, u64 len_after, u64 start, int ret),
+
+	TP_ARGS(compress, page, inode, type, len_before, len_after, start, ret),
+
+	TP_STRUCT__entry_btrfs(
+		__field(int,		compress)
+		__field(int,		page)
+		__field(u64,		i_ino)
+		__field(unsigned int,	type)
+		__field(u64,		len_before)
+		__field(u64,		len_after)
+		__field(u64,		start)
+		__field(int,		ret)
+	),
+
+	TP_fast_assign_btrfs(btrfs_sb(inode->i_sb),
+		__entry->compress	= compress;
+		__entry->page		= page;
+		__entry->i_ino		= inode->i_ino;
+		__entry->type		= type;
+		__entry->len_before	= len_before;
+		__entry->len_after	= len_after;
+		__entry->start		= start;
+		__entry->ret		= ret;
+	),
+
+	TP_printk_btrfs("%s %s ino=%llu type=%s len_before=%llu len_after=%llu "\
+		"start=%llu ret=%d",
+		__entry->compress ? "compress" : "decompress",
+		__entry->page ? "page" : "bio", __entry->i_ino,
+		show_compress_type(__entry->type), __entry->len_before,
+		__entry->len_after, __entry->start, __entry->ret)
+);
 #endif /* _TRACE_BTRFS_H */
 
 /* This part must be outside protection */
-- 
2.7.0

Re: [PATCH 2/4] btrfs: convert enum btrfs_compression_type to define

[David Sterba]

On Thu, Aug 17, 2017 at 04:33:41AM +0800, Anand Jain wrote:
> 
> 
> On 08/16/2017 09:59 PM, David Sterba wrote:
> > On Sun, Aug 13, 2017 at 12:02:42PM +0800, Anand Jain wrote:
> >> There isn't a huge list to manage the types, which can be managed
> >> with defines. It helps to easily print the types in tracing as well.
> > 
> > We use enums in a lot of places, I'd rather keep it as it is.
> 
>   This patch converts all of them, and it was at only one place.

Yeah, but I mean the enum vs define style of constant definition.

>   I hope I didn't miss any. Further the next patch 3/4 needs it
>   to be define instead of enums, handling enums in the tracing
>   isn't as easy as define.

Interesting, in what way are defines better use in tracepoints? I see
eg. show_flush_state using enum btrfs_flush_state in btrfs_flush_space,
the same pattern applies to patch 3/4.

Re: [PATCH 2/4] btrfs: convert enum btrfs_compression_type to define

[Anand Jain]

On 08/17/2017 07:57 PM, David Sterba wrote:
> On Thu, Aug 17, 2017 at 04:33:41AM +0800, Anand Jain wrote:
>>
>>
>> On 08/16/2017 09:59 PM, David Sterba wrote:
>>> On Sun, Aug 13, 2017 at 12:02:42PM +0800, Anand Jain wrote:
>>>> There isn't a huge list to manage the types, which can be managed
>>>> with defines. It helps to easily print the types in tracing as well.
>>>
>>> We use enums in a lot of places, I'd rather keep it as it is.
>>
>>    This patch converts all of them, and it was at only one place.
> 
> Yeah, but I mean the enum vs define style of constant definition.
> 
>>    I hope I didn't miss any. Further the next patch 3/4 needs it
>>    to be define instead of enums, handling enums in the tracing
>>    isn't as easy as define.
> 
> Interesting, in what way are defines better use in tracepoints? I see
> eg. show_flush_state using enum btrfs_flush_state in btrfs_flush_space,
> the same pattern applies to patch 3/4.

  As of now TRACE_DEFINE_ENUM() for show_flush_state is missing (patch 
sent) which is needed for the state= symbol in the user space as shown 
below,

perf record -e 'btrfs:btrfs_flush_space' -a fill_and_bal /btrfs/sv1 
10000 && perf script

::
kworker/u2:4  4220 [000] 19032.858184: btrfs:btrfs_flush_space: (nil)U: 
state=1() flags=4(METADATA) num_bytes=173015040 orig_bytes=173015040 ret=0

(%p does not work in user space, ignore it for now).

sysfs trace is fine.
cat ./trace_pipe
---
kworker/u2:3-4219  [000] .... 18952.145677: btrfs_flush_space: 
f0918b8d-88a6-4e9f-8ca6-02e2fc290380: state=1(FLUSH_DELAYED_ITEMS_NR) 
flags=4(METADATA) num_bytes
---

  So for BTRFS_COMPRESS.. its either TRADE_DEFINE_ENUM() and enum or 
just #define. I am ok either ways.

Thanks, Anand

Re: [PATCH 2/4] btrfs: convert enum btrfs_compression_type to define

[David Sterba]

On Wed, Aug 30, 2017 at 10:38:21PM +0800, Anand Jain wrote:
> On 08/17/2017 07:57 PM, David Sterba wrote:
> > On Thu, Aug 17, 2017 at 04:33:41AM +0800, Anand Jain wrote:
> >>
> >>
> >> On 08/16/2017 09:59 PM, David Sterba wrote:
> >>> On Sun, Aug 13, 2017 at 12:02:42PM +0800, Anand Jain wrote:
> >>>> There isn't a huge list to manage the types, which can be managed
> >>>> with defines. It helps to easily print the types in tracing as well.
> >>>
> >>> We use enums in a lot of places, I'd rather keep it as it is.
> >>
> >>    This patch converts all of them, and it was at only one place.
> > 
> > Yeah, but I mean the enum vs define style of constant definition.
> > 
> >>    I hope I didn't miss any. Further the next patch 3/4 needs it
> >>    to be define instead of enums, handling enums in the tracing
> >>    isn't as easy as define.
> > 
> > Interesting, in what way are defines better use in tracepoints? I see
> > eg. show_flush_state using enum btrfs_flush_state in btrfs_flush_space,
> > the same pattern applies to patch 3/4.
> 
>   As of now TRACE_DEFINE_ENUM() for show_flush_state is missing (patch 
> sent) which is needed for the state= symbol in the user space as shown 
> below,
> 
> perf record -e 'btrfs:btrfs_flush_space' -a fill_and_bal /btrfs/sv1 
> 10000 && perf script
> 
> ::
> kworker/u2:4  4220 [000] 19032.858184: btrfs:btrfs_flush_space: (nil)U: 
> state=1() flags=4(METADATA) num_bytes=173015040 orig_bytes=173015040 ret=0
> 
> (%p does not work in user space, ignore it for now).
> 
> sysfs trace is fine.
> cat ./trace_pipe
> ---
> kworker/u2:3-4219  [000] .... 18952.145677: btrfs_flush_space: 
> f0918b8d-88a6-4e9f-8ca6-02e2fc290380: state=1(FLUSH_DELAYED_ITEMS_NR) 
> flags=4(METADATA) num_bytes
> ---
> 
>   So for BTRFS_COMPRESS.. its either TRADE_DEFINE_ENUM() and enum or 
> just #define. I am ok either ways.

Thanks for the examples, I've updated wiki so we don't forget about it.
The enums look more convenient, as we'll just copy the names to the
defining macro, unlike a set of defnies that would need
__print_symbolic.

Looking at current use of __print_symbolic, the printed strings are
shortened versions of the macros that have a prefix, this may be
better suited for the trace output.