lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <zxcvscosad6yv2y6km3xsdhh26qse3qcx5u66a4nqybev4u64y@uzdekpsliv35>
Date: Fri, 30 May 2025 18:00:15 -0400
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: George Hu <integral@...hlinux.org>
Cc: linux-bcachefs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] bcachefs: use union for bch_compression_opt to make
 encode & decode easier

On Fri, May 30, 2025 at 10:52:43PM +0800, George Hu wrote:
> Eliminate redundant encode & decode function by using union for
> bch_compression_opt, which reduces code complexity.

order of bitfield members is not fixed, to do this you need #ifdef
__LITTLE_ENDIN_BITFIELD...

> 
> Signed-off-by: George Hu <integral@...hlinux.org>
> ---
>  fs/bcachefs/compress.c | 17 +++++++++--------
>  fs/bcachefs/compress.h | 32 +++++++-------------------------
>  fs/bcachefs/extents.c  |  2 +-
>  3 files changed, 17 insertions(+), 34 deletions(-)
> 
> diff --git a/fs/bcachefs/compress.c b/fs/bcachefs/compress.c
> index 1bca61d17092..10e88e14f5ed 100644
> --- a/fs/bcachefs/compress.c
> +++ b/fs/bcachefs/compress.c
> @@ -336,7 +336,7 @@ static int attempt_compress(struct bch_fs *c,
>  			    void *workspace,
>  			    void *dst, size_t dst_len,
>  			    void *src, size_t src_len,
> -			    struct bch_compression_opt compression)
> +			    union bch_compression_opt compression)
>  {
>  	enum bch_compression_type compression_type =
>  		__bch2_compression_opt_to_type[compression.type];
> @@ -426,7 +426,7 @@ static int attempt_compress(struct bch_fs *c,
>  static unsigned __bio_compress(struct bch_fs *c,
>  			       struct bio *dst, size_t *dst_len,
>  			       struct bio *src, size_t *src_len,
> -			       struct bch_compression_opt compression)
> +			       union bch_compression_opt compression)
>  {
>  	struct bbuf src_data = { NULL }, dst_data = { NULL };
>  	void *workspace;
> @@ -553,7 +553,7 @@ unsigned bch2_bio_compress(struct bch_fs *c,
>  
>  	compression_type =
>  		__bio_compress(c, dst, dst_len, src, src_len,
> -			       bch2_compression_decode(compression_opt));
> +			       (union bch_compression_opt){ .value = compression_opt });
>  
>  	dst->bi_iter.bi_size = orig_dst;
>  	src->bi_iter.bi_size = orig_src;
> @@ -602,7 +602,8 @@ static int __bch2_check_set_has_compressed_data(struct bch_fs *c, u64 f)
>  int bch2_check_set_has_compressed_data(struct bch_fs *c,
>  				       unsigned compression_opt)
>  {
> -	unsigned compression_type = bch2_compression_decode(compression_opt).type;
> +	unsigned int compression_type = ((union bch_compression_opt){ .value = compression_opt })
> +					.type;
>  
>  	BUG_ON(compression_type >= ARRAY_SIZE(bch2_compression_opt_to_feature));
>  
> @@ -683,7 +684,7 @@ static int __bch2_fs_compress_init(struct bch_fs *c, u64 features)
>  
>  static u64 compression_opt_to_feature(unsigned v)
>  {
> -	unsigned type = bch2_compression_decode(v).type;
> +	unsigned int type = ((union bch_compression_opt){ .value = v }).type;
>  
>  	return BIT_ULL(bch2_compression_opt_to_feature[type]);
>  }
> @@ -703,7 +704,7 @@ int bch2_opt_compression_parse(struct bch_fs *c, const char *_val, u64 *res,
>  {
>  	char *val = kstrdup(_val, GFP_KERNEL);
>  	char *p = val, *type_str, *level_str;
> -	struct bch_compression_opt opt = { 0 };
> +	union bch_compression_opt opt = { 0 };
>  	int ret;
>  
>  	if (!val)
> @@ -736,7 +737,7 @@ int bch2_opt_compression_parse(struct bch_fs *c, const char *_val, u64 *res,
>  		opt.level = level;
>  	}
>  
> -	*res = bch2_compression_encode(opt);
> +	*res = opt.value;
>  err:
>  	kfree(val);
>  	return ret;
> @@ -744,7 +745,7 @@ int bch2_opt_compression_parse(struct bch_fs *c, const char *_val, u64 *res,
>  
>  void bch2_compression_opt_to_text(struct printbuf *out, u64 v)
>  {
> -	struct bch_compression_opt opt = bch2_compression_decode(v);
> +	union bch_compression_opt opt = { .value = v };
>  
>  	if (opt.type < BCH_COMPRESSION_OPT_NR)
>  		prt_str(out, bch2_compression_opts[opt.type]);
> diff --git a/fs/bcachefs/compress.h b/fs/bcachefs/compress.h
> index bec2f05bfd52..8cded510b2ac 100644
> --- a/fs/bcachefs/compress.h
> +++ b/fs/bcachefs/compress.h
> @@ -10,41 +10,23 @@ static const unsigned __bch2_compression_opt_to_type[] = {
>  #undef x
>  };
>  
> -struct bch_compression_opt {
> -	u8		type:4,
> -			level:4;
> -};
> -
> -static inline struct bch_compression_opt __bch2_compression_decode(unsigned v)
> -{
> -	return (struct bch_compression_opt) {
> -		.type	= v & 15,
> -		.level	= v >> 4,
> +union bch_compression_opt {
> +	u8 value;
> +	struct {
> +		u8 type:4, level:4;
>  	};
> -}
> +};
>  
>  static inline bool bch2_compression_opt_valid(unsigned v)
>  {
> -	struct bch_compression_opt opt = __bch2_compression_decode(v);
> +	union bch_compression_opt opt = { .value = v };
>  
>  	return opt.type < ARRAY_SIZE(__bch2_compression_opt_to_type) && !(!opt.type && opt.level);
>  }
>  
> -static inline struct bch_compression_opt bch2_compression_decode(unsigned v)
> -{
> -	return bch2_compression_opt_valid(v)
> -		? __bch2_compression_decode(v)
> -		: (struct bch_compression_opt) { 0 };
> -}
> -
> -static inline unsigned bch2_compression_encode(struct bch_compression_opt opt)
> -{
> -	return opt.type|(opt.level << 4);
> -}
> -
>  static inline enum bch_compression_type bch2_compression_opt_to_type(unsigned v)
>  {
> -	return __bch2_compression_opt_to_type[bch2_compression_decode(v).type];
> +	return __bch2_compression_opt_to_type[((union bch_compression_opt){ .value = v }).type];
>  }
>  
>  struct bch_write_op;
> diff --git a/fs/bcachefs/extents.c b/fs/bcachefs/extents.c
> index 677cf453b332..64419c028662 100644
> --- a/fs/bcachefs/extents.c
> +++ b/fs/bcachefs/extents.c
> @@ -1512,7 +1512,7 @@ int bch2_bkey_ptrs_validate(struct bch_fs *c, struct bkey_s_c k,
>  			const struct bch_extent_rebalance *r = &entry->rebalance;
>  
>  			if (!bch2_compression_opt_valid(r->compression)) {
> -				struct bch_compression_opt opt = __bch2_compression_decode(r->compression);
> +				union bch_compression_opt opt = { .value = r->compression };
>  				prt_printf(err, "invalid compression opt %u:%u",
>  					   opt.type, opt.level);
>  				return -BCH_ERR_invalid_bkey;
> -- 
> 2.49.0
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ