Linux-BTRFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/3] btrfs: remove unnecessary hash_init()
@ 2019-10-05  5:17 Chengguang Xu
  2019-10-05  5:17 ` [PATCH 2/3] btrfs: code cleanup for compression type Chengguang Xu
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Chengguang Xu @ 2019-10-05  5:17 UTC (permalink / raw)
  To: clm, josef, dsterba; +Cc: linux-btrfs, Chengguang Xu

hash_init() is not necessary in btrfs_props_init(),
so remove it.

Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
---
 fs/btrfs/props.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index 1e664e0b59b8..68508db3dc65 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -437,8 +437,6 @@ void __init btrfs_props_init(void)
 {
 	int i;
 
-	hash_init(prop_handlers_ht);
-
 	for (i = 0; i < ARRAY_SIZE(prop_handlers); i++) {
 		struct prop_handler *p = &prop_handlers[i];
 		u64 h = btrfs_name_hash(p->xattr_name, strlen(p->xattr_name));
-- 
2.21.0




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

* [PATCH 2/3] btrfs: code cleanup for compression type
  2019-10-05  5:17 [PATCH 1/3] btrfs: remove unnecessary hash_init() Chengguang Xu
@ 2019-10-05  5:17 ` Chengguang Xu
  2019-10-06 23:28   ` David Sterba
  2019-10-05  5:17 ` [PATCH 3/3] btrfs: using enum to replace macro Chengguang Xu
  2019-10-07 15:44 ` [PATCH 1/3] btrfs: remove unnecessary hash_init() David Sterba
  2 siblings, 1 reply; 8+ messages in thread
From: Chengguang Xu @ 2019-10-05  5:17 UTC (permalink / raw)
  To: clm, josef, dsterba; +Cc: linux-btrfs, Chengguang Xu

Let BTRFS_COMPRESS_TYPES represents the total number
of cmpressoin types and fix related calling places.
It will be more safe when adding new compression type
in the future.

Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
---
 fs/btrfs/compression.c  |  2 ++
 fs/btrfs/compression.h  | 12 ++++++------
 fs/btrfs/ioctl.c        |  2 +-
 fs/btrfs/tree-checker.c |  4 ++--
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index d70c46407420..93deaf0cc2b8 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -39,6 +39,8 @@ const char* btrfs_compress_type2str(enum btrfs_compression_type type)
 	case BTRFS_COMPRESS_ZSTD:
 	case BTRFS_COMPRESS_NONE:
 		return btrfs_compress_types[type];
+	default:
+		break;
 	}
 
 	return NULL;
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index dd392278ab3f..091ff3f986e5 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -101,11 +101,11 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 unsigned int btrfs_compress_str2level(unsigned int type, const char *str);
 
 enum btrfs_compression_type {
-	BTRFS_COMPRESS_NONE  = 0,
-	BTRFS_COMPRESS_ZLIB  = 1,
-	BTRFS_COMPRESS_LZO   = 2,
-	BTRFS_COMPRESS_ZSTD  = 3,
-	BTRFS_COMPRESS_TYPES = 3,
+	BTRFS_COMPRESS_NONE,
+	BTRFS_COMPRESS_ZLIB,
+	BTRFS_COMPRESS_LZO,
+	BTRFS_COMPRESS_ZSTD,
+	BTRFS_COMPRESS_TYPES
 };
 
 struct workspace_manager {
@@ -163,7 +163,7 @@ struct btrfs_compress_op {
 };
 
 /* The heuristic workspaces are managed via the 0th workspace manager */
-#define BTRFS_NR_WORKSPACE_MANAGERS	(BTRFS_COMPRESS_TYPES + 1)
+#define BTRFS_NR_WORKSPACE_MANAGERS	BTRFS_COMPRESS_TYPES
 
 extern const struct btrfs_compress_op btrfs_heuristic_compress;
 extern const struct btrfs_compress_op btrfs_zlib_compress;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index de730e56d3f5..8c7196ed7ae0 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1411,7 +1411,7 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
 		return -EINVAL;
 
 	if (do_compress) {
-		if (range->compress_type > BTRFS_COMPRESS_TYPES)
+		if (range->compress_type >= BTRFS_COMPRESS_TYPES)
 			return -EINVAL;
 		if (range->compress_type)
 			compress_type = range->compress_type;
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index f28f9725cef1..2d91c34bbf63 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -168,11 +168,11 @@ static int check_extent_data_item(struct extent_buffer *leaf,
 	 * Support for new compression/encryption must introduce incompat flag,
 	 * and must be caught in open_ctree().
 	 */
-	if (btrfs_file_extent_compression(leaf, fi) > BTRFS_COMPRESS_TYPES) {
+	if (btrfs_file_extent_compression(leaf, fi) >= BTRFS_COMPRESS_TYPES) {
 		file_extent_err(leaf, slot,
 	"invalid compression for file extent, have %u expect range [0, %u]",
 			btrfs_file_extent_compression(leaf, fi),
-			BTRFS_COMPRESS_TYPES);
+			BTRFS_COMPRESS_TYPES - 1);
 		return -EUCLEAN;
 	}
 	if (btrfs_file_extent_encryption(leaf, fi)) {
-- 
2.21.0




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

* [PATCH 3/3] btrfs: using enum to replace macro
  2019-10-05  5:17 [PATCH 1/3] btrfs: remove unnecessary hash_init() Chengguang Xu
  2019-10-05  5:17 ` [PATCH 2/3] btrfs: code cleanup for compression type Chengguang Xu
@ 2019-10-05  5:17 ` Chengguang Xu
  2019-10-06 23:29   ` David Sterba
  2019-10-07 15:44 ` [PATCH 1/3] btrfs: remove unnecessary hash_init() David Sterba
  2 siblings, 1 reply; 8+ messages in thread
From: Chengguang Xu @ 2019-10-05  5:17 UTC (permalink / raw)
  To: clm, josef, dsterba; +Cc: linux-btrfs, Chengguang Xu

using enum to replace macro definition for extent
types.

Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
---
 fs/btrfs/tree-checker.c         |  4 ++--
 include/uapi/linux/btrfs_tree.h | 10 ++++++----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 2d91c34bbf63..9b0c5fdbe04e 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -156,11 +156,11 @@ static int check_extent_data_item(struct extent_buffer *leaf,
 
 	fi = btrfs_item_ptr(leaf, slot, struct btrfs_file_extent_item);
 
-	if (btrfs_file_extent_type(leaf, fi) > BTRFS_FILE_EXTENT_TYPES) {
+	if (btrfs_file_extent_type(leaf, fi) >= BTRFS_FILE_EXTENT_TYPES) {
 		file_extent_err(leaf, slot,
 		"invalid type for file extent, have %u expect range [0, %u]",
 			btrfs_file_extent_type(leaf, fi),
-			BTRFS_FILE_EXTENT_TYPES);
+			BTRFS_FILE_EXTENT_TYPES - 1);
 		return -EUCLEAN;
 	}
 
diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
index b65c7ee75bc7..34bd09ffc71d 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -737,10 +737,12 @@ struct btrfs_balance_item {
 	__le64 unused[4];
 } __attribute__ ((__packed__));
 
-#define BTRFS_FILE_EXTENT_INLINE 0
-#define BTRFS_FILE_EXTENT_REG 1
-#define BTRFS_FILE_EXTENT_PREALLOC 2
-#define BTRFS_FILE_EXTENT_TYPES	2
+enum {
+	BTRFS_FILE_EXTENT_INLINE,
+	BTRFS_FILE_EXTENT_REG,
+	BTRFS_FILE_EXTENT_PREALLOC,
+	BTRFS_FILE_EXTENT_TYPES
+};
 
 struct btrfs_file_extent_item {
 	/*
-- 
2.21.0




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

* Re: [PATCH 2/3] btrfs: code cleanup for compression type
  2019-10-05  5:17 ` [PATCH 2/3] btrfs: code cleanup for compression type Chengguang Xu
@ 2019-10-06 23:28   ` David Sterba
  2019-10-07 13:47     ` Chengguang Xu
  0 siblings, 1 reply; 8+ messages in thread
From: David Sterba @ 2019-10-06 23:28 UTC (permalink / raw)
  To: Chengguang Xu; +Cc: clm, josef, dsterba, linux-btrfs

On Sat, Oct 05, 2019 at 01:17:35PM +0800, Chengguang Xu wrote:
> Let BTRFS_COMPRESS_TYPES represents the total number
> of cmpressoin types and fix related calling places.
> It will be more safe when adding new compression type
> in the future.

I think we're not going to add a new type anytime soon, zstd provides
the choice between fast and good ratio. This itself is not an objection
to your patch but is not IMO the true reason for the changes.

Can you please describe the motivation behind the patches? Eg. if it's a
general cleanup or if there are other changes planned on top.

> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
> ---
>  fs/btrfs/compression.c  |  2 ++
>  fs/btrfs/compression.h  | 12 ++++++------
>  fs/btrfs/ioctl.c        |  2 +-
>  fs/btrfs/tree-checker.c |  4 ++--
>  4 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index d70c46407420..93deaf0cc2b8 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -39,6 +39,8 @@ const char* btrfs_compress_type2str(enum btrfs_compression_type type)
>  	case BTRFS_COMPRESS_ZSTD:
>  	case BTRFS_COMPRESS_NONE:
>  		return btrfs_compress_types[type];
> +	default:
> +		break;
>  	}
>  
>  	return NULL;
> diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
> index dd392278ab3f..091ff3f986e5 100644
> --- a/fs/btrfs/compression.h
> +++ b/fs/btrfs/compression.h
> @@ -101,11 +101,11 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
>  unsigned int btrfs_compress_str2level(unsigned int type, const char *str);
>  
>  enum btrfs_compression_type {
> -	BTRFS_COMPRESS_NONE  = 0,
> -	BTRFS_COMPRESS_ZLIB  = 1,
> -	BTRFS_COMPRESS_LZO   = 2,
> -	BTRFS_COMPRESS_ZSTD  = 3,
> -	BTRFS_COMPRESS_TYPES = 3,
> +	BTRFS_COMPRESS_NONE,
> +	BTRFS_COMPRESS_ZLIB,
> +	BTRFS_COMPRESS_LZO,
> +	BTRFS_COMPRESS_ZSTD,
> +	BTRFS_COMPRESS_TYPES

Please note that the on-disk format values should be expressed by the
values, even if it's the same as the automatic enum assignments.

Regarding change of the BTRFS_COMPRESS_TYPES value, I vaguely remember
we had patches for that but I don't recall why it was not changed. The
progs have an extra BTRFS_COMPRESS_LAST (== 4) that would be used the
same way as you do in this patch.

BTRFS_COMPRESS_* is not in the public API so changing the value should
be safe, but needs double checking.

>  };
>  
>  struct workspace_manager {
> @@ -163,7 +163,7 @@ struct btrfs_compress_op {
>  };
>  
>  /* The heuristic workspaces are managed via the 0th workspace manager */
> -#define BTRFS_NR_WORKSPACE_MANAGERS	(BTRFS_COMPRESS_TYPES + 1)
> +#define BTRFS_NR_WORKSPACE_MANAGERS	BTRFS_COMPRESS_TYPES
>  
>  extern const struct btrfs_compress_op btrfs_heuristic_compress;
>  extern const struct btrfs_compress_op btrfs_zlib_compress;
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index de730e56d3f5..8c7196ed7ae0 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1411,7 +1411,7 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
>  		return -EINVAL;
>  
>  	if (do_compress) {
> -		if (range->compress_type > BTRFS_COMPRESS_TYPES)
> +		if (range->compress_type >= BTRFS_COMPRESS_TYPES)
>  			return -EINVAL;
>  		if (range->compress_type)
>  			compress_type = range->compress_type;
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index f28f9725cef1..2d91c34bbf63 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -168,11 +168,11 @@ static int check_extent_data_item(struct extent_buffer *leaf,
>  	 * Support for new compression/encryption must introduce incompat flag,
>  	 * and must be caught in open_ctree().
>  	 */
> -	if (btrfs_file_extent_compression(leaf, fi) > BTRFS_COMPRESS_TYPES) {
> +	if (btrfs_file_extent_compression(leaf, fi) >= BTRFS_COMPRESS_TYPES) {
>  		file_extent_err(leaf, slot,
>  	"invalid compression for file extent, have %u expect range [0, %u]",
>  			btrfs_file_extent_compression(leaf, fi),
> -			BTRFS_COMPRESS_TYPES);
> +			BTRFS_COMPRESS_TYPES - 1);
>  		return -EUCLEAN;
>  	}
>  	if (btrfs_file_extent_encryption(leaf, fi)) {
> -- 
> 2.21.0
> 
> 
> 

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

* Re: [PATCH 3/3] btrfs: using enum to replace macro
  2019-10-05  5:17 ` [PATCH 3/3] btrfs: using enum to replace macro Chengguang Xu
@ 2019-10-06 23:29   ` David Sterba
  0 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2019-10-06 23:29 UTC (permalink / raw)
  To: Chengguang Xu; +Cc: clm, josef, dsterba, linux-btrfs

On Sat, Oct 05, 2019 at 01:17:36PM +0800, Chengguang Xu wrote:
> using enum to replace macro definition for extent
> types.
> 
> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
> ---
>  fs/btrfs/tree-checker.c         |  4 ++--
>  include/uapi/linux/btrfs_tree.h | 10 ++++++----
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 2d91c34bbf63..9b0c5fdbe04e 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -156,11 +156,11 @@ static int check_extent_data_item(struct extent_buffer *leaf,
>  
>  	fi = btrfs_item_ptr(leaf, slot, struct btrfs_file_extent_item);
>  
> -	if (btrfs_file_extent_type(leaf, fi) > BTRFS_FILE_EXTENT_TYPES) {
> +	if (btrfs_file_extent_type(leaf, fi) >= BTRFS_FILE_EXTENT_TYPES) {
>  		file_extent_err(leaf, slot,
>  		"invalid type for file extent, have %u expect range [0, %u]",
>  			btrfs_file_extent_type(leaf, fi),
> -			BTRFS_FILE_EXTENT_TYPES);
> +			BTRFS_FILE_EXTENT_TYPES - 1);
>  		return -EUCLEAN;
>  	}
>  
> diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
> index b65c7ee75bc7..34bd09ffc71d 100644
> --- a/include/uapi/linux/btrfs_tree.h
> +++ b/include/uapi/linux/btrfs_tree.h
> @@ -737,10 +737,12 @@ struct btrfs_balance_item {
>  	__le64 unused[4];
>  } __attribute__ ((__packed__));
>  
> -#define BTRFS_FILE_EXTENT_INLINE 0
> -#define BTRFS_FILE_EXTENT_REG 1
> -#define BTRFS_FILE_EXTENT_PREALLOC 2
> -#define BTRFS_FILE_EXTENT_TYPES	2
> +enum {
> +	BTRFS_FILE_EXTENT_INLINE,
> +	BTRFS_FILE_EXTENT_REG,
> +	BTRFS_FILE_EXTENT_PREALLOC,
> +	BTRFS_FILE_EXTENT_TYPES
> +};

As stated before, using enums is fine and for on-disk structure,s the
explicit value should be specified as well.

>  struct btrfs_file_extent_item {
>  	/*
> -- 
> 2.21.0
> 
> 
> 

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

* Re: [PATCH 2/3] btrfs: code cleanup for compression type
  2019-10-06 23:28   ` David Sterba
@ 2019-10-07 13:47     ` Chengguang Xu
  0 siblings, 0 replies; 8+ messages in thread
From: Chengguang Xu @ 2019-10-07 13:47 UTC (permalink / raw)
  To: dsterba; +Cc: clm, josef, dsterba, linux-btrfs

 ---- 在 星期一, 2019-10-07 07:28:32 David Sterba <dsterba@suse.cz> 撰写 ----
 > On Sat, Oct 05, 2019 at 01:17:35PM +0800, Chengguang Xu wrote:
 > > Let BTRFS_COMPRESS_TYPES represents the total number
 > > of cmpressoin types and fix related calling places.
 > > It will be more safe when adding new compression type
 > > in the future.
 > 
 > I think we're not going to add a new type anytime soon, zstd provides
 > the choice between fast and good ratio. This itself is not an objection
 > to your patch but is not IMO the true reason for the changes.
 > 
 > Can you please describe the motivation behind the patches? Eg. if it's a
 > general cleanup or if there are other changes planned on top.

Actually, it's just a general cleanup. I found another enum in btrfs code for RAID types
and I think that usage makes me(at least :-)) easy to understand the code. So the
motivation is to keep code style consistency and  make the code a bit more readable.


 > 
 > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
 > > ---
 > >  fs/btrfs/compression.c  |  2 ++
 > >  fs/btrfs/compression.h  | 12 ++++++------
 > >  fs/btrfs/ioctl.c        |  2 +-
 > >  fs/btrfs/tree-checker.c |  4 ++--
 > >  4 files changed, 11 insertions(+), 9 deletions(-)
 > > 
 > > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
 > > index d70c46407420..93deaf0cc2b8 100644
 > > --- a/fs/btrfs/compression.c
 > > +++ b/fs/btrfs/compression.c
 > > @@ -39,6 +39,8 @@ const char* btrfs_compress_type2str(enum btrfs_compression_type type)
 > >      case BTRFS_COMPRESS_ZSTD:
 > >      case BTRFS_COMPRESS_NONE:
 > >          return btrfs_compress_types[type];
 > > +    default:
 > > +        break;
 > >      }
 > >  
 > >      return NULL;
 > > diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
 > > index dd392278ab3f..091ff3f986e5 100644
 > > --- a/fs/btrfs/compression.h
 > > +++ b/fs/btrfs/compression.h
 > > @@ -101,11 +101,11 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 > >  unsigned int btrfs_compress_str2level(unsigned int type, const char *str);
 > >  
 > >  enum btrfs_compression_type {
 > > -    BTRFS_COMPRESS_NONE  = 0,
 > > -    BTRFS_COMPRESS_ZLIB  = 1,
 > > -    BTRFS_COMPRESS_LZO   = 2,
 > > -    BTRFS_COMPRESS_ZSTD  = 3,
 > > -    BTRFS_COMPRESS_TYPES = 3,
 > > +    BTRFS_COMPRESS_NONE,
 > > +    BTRFS_COMPRESS_ZLIB,
 > > +    BTRFS_COMPRESS_LZO,
 > > +    BTRFS_COMPRESS_ZSTD,
 > > +    BTRFS_COMPRESS_TYPES
 > 
 > Please note that the on-disk format values should be expressed by the
 > values, even if it's the same as the automatic enum assignments.

I'll fix in v2.

 > 
 > Regarding change of the BTRFS_COMPRESS_TYPES value, I vaguely remember
 > we had patches for that but I don't recall why it was not changed. The
 > progs have an extra BTRFS_COMPRESS_LAST (== 4) that would be used the
 > same way as you do in this patch.

In previous patch, we had compression type(1-3, skip 0) in the code,
so there may be a bit of  confusion for BTRFS_COMPRESS_TYPES(==4) . 
I think it's not a problem now but maybe  change name to BTRFS_NR_COMPRESS_TYPES(like RAID type enum) 
is better.

 > 
 > BTRFS_COMPRESS_* is not in the public API so changing the value should
 > be safe, but needs double checking.
 > 
 > >  };
 > >  
 > >  struct workspace_manager {
 > > @@ -163,7 +163,7 @@ struct btrfs_compress_op {
 > >  };
 > >  
 > >  /* The heuristic workspaces are managed via the 0th workspace manager */
 > > -#define BTRFS_NR_WORKSPACE_MANAGERS    (BTRFS_COMPRESS_TYPES + 1)
 > > +#define BTRFS_NR_WORKSPACE_MANAGERS    BTRFS_COMPRESS_TYPES
 > >  
 > >  extern const struct btrfs_compress_op btrfs_heuristic_compress;
 > >  extern const struct btrfs_compress_op btrfs_zlib_compress;
 > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
 > > index de730e56d3f5..8c7196ed7ae0 100644
 > > --- a/fs/btrfs/ioctl.c
 > > +++ b/fs/btrfs/ioctl.c
 > > @@ -1411,7 +1411,7 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
 > >          return -EINVAL;
 > >  
 > >      if (do_compress) {
 > > -        if (range->compress_type > BTRFS_COMPRESS_TYPES)
 > > +        if (range->compress_type >= BTRFS_COMPRESS_TYPES)
 > >              return -EINVAL;
 > >          if (range->compress_type)
 > >              compress_type = range->compress_type;
 > > diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
 > > index f28f9725cef1..2d91c34bbf63 100644
 > > --- a/fs/btrfs/tree-checker.c
 > > +++ b/fs/btrfs/tree-checker.c
 > > @@ -168,11 +168,11 @@ static int check_extent_data_item(struct extent_buffer *leaf,
 > >       * Support for new compression/encryption must introduce incompat flag,
 > >       * and must be caught in open_ctree().
 > >       */
 > > -    if (btrfs_file_extent_compression(leaf, fi) > BTRFS_COMPRESS_TYPES) {
 > > +    if (btrfs_file_extent_compression(leaf, fi) >= BTRFS_COMPRESS_TYPES) {
 > >          file_extent_err(leaf, slot,
 > >      "invalid compression for file extent, have %u expect range [0, %u]",
 > >              btrfs_file_extent_compression(leaf, fi),
 > > -            BTRFS_COMPRESS_TYPES);
 > > +            BTRFS_COMPRESS_TYPES - 1);
 > >          return -EUCLEAN;
 > >      }
 > >      if (btrfs_file_extent_encryption(leaf, fi)) {
 > > -- 
 > > 2.21.0
 > > 
 > > 
 > > 
 >


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

* Re: [PATCH 1/3] btrfs: remove unnecessary hash_init()
  2019-10-05  5:17 [PATCH 1/3] btrfs: remove unnecessary hash_init() Chengguang Xu
  2019-10-05  5:17 ` [PATCH 2/3] btrfs: code cleanup for compression type Chengguang Xu
  2019-10-05  5:17 ` [PATCH 3/3] btrfs: using enum to replace macro Chengguang Xu
@ 2019-10-07 15:44 ` David Sterba
  2019-10-08  2:56   ` Chengguang Xu
  2 siblings, 1 reply; 8+ messages in thread
From: David Sterba @ 2019-10-07 15:44 UTC (permalink / raw)
  To: Chengguang Xu; +Cc: clm, josef, dsterba, linux-btrfs

On Sat, Oct 05, 2019 at 01:17:34PM +0800, Chengguang Xu wrote:
> hash_init() is not necessary in btrfs_props_init(),
> so remove it.

The part that explains why it's not necessary is missing in the
changelo. And looking what hash_init and plain DEFINE_HASHTABLE does I
don't think that removing hash_init is safe or making the code more
clear.

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

* Re: [PATCH 1/3] btrfs: remove unnecessary hash_init()
  2019-10-07 15:44 ` [PATCH 1/3] btrfs: remove unnecessary hash_init() David Sterba
@ 2019-10-08  2:56   ` Chengguang Xu
  0 siblings, 0 replies; 8+ messages in thread
From: Chengguang Xu @ 2019-10-08  2:56 UTC (permalink / raw)
  To: dsterba; +Cc: clm, josef, dsterba, linux-btrfs

 ---- 在 星期一, 2019-10-07 23:44:45 David Sterba <dsterba@suse.cz> 撰写 ----
 > On Sat, Oct 05, 2019 at 01:17:34PM +0800, Chengguang Xu wrote:
 > > hash_init() is not necessary in btrfs_props_init(),
 > > so remove it.
 > 
 > The part that explains why it's not necessary is missing in the
 > changelo. And looking what hash_init and plain DEFINE_HASHTABLE does I
 > don't think that removing hash_init is safe or making the code more
 > clear.
 
Sorry for pool explanation in change log.

Look into the code,  DEFINE_HASHTABLE has already included initialization code in it and
I think this is the main difference compare to DECLARE_HASHTABLE which still needs hash_init.


Thanks,
Chengguang


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

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-05  5:17 [PATCH 1/3] btrfs: remove unnecessary hash_init() Chengguang Xu
2019-10-05  5:17 ` [PATCH 2/3] btrfs: code cleanup for compression type Chengguang Xu
2019-10-06 23:28   ` David Sterba
2019-10-07 13:47     ` Chengguang Xu
2019-10-05  5:17 ` [PATCH 3/3] btrfs: using enum to replace macro Chengguang Xu
2019-10-06 23:29   ` David Sterba
2019-10-07 15:44 ` [PATCH 1/3] btrfs: remove unnecessary hash_init() David Sterba
2019-10-08  2:56   ` Chengguang Xu

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org linux-btrfs@archiver.kernel.org
	public-inbox-index linux-btrfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox