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

2017-09-07 Thread 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 
> 1 && 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2017-08-30 Thread 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 
1 && 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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2017-08-17 Thread 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2017-08-16 Thread 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

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

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

2017-08-16 Thread 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2017-08-12 Thread 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 
---
 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_NONE0
+#define BTRFS_COMPRESS_TYPES   2
+#define BTRFS_COMPRESS_ZLIB1
+#define BTRFS_COMPRESS_LZO 2
+
 struct btrfs_file_extent_item {
/*
 * transaction id that created this extent
-- 
2.13.1

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