Re: [PATCH] btrfs: make ASSERT no-op in release builds

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2025/10/31 04:53, Gladyshev Ilya 写道:
> The current definition of `ASSERT(cond)` as `(void)(cond)` is redundant,
> because all checks are without side effects and don't affect code logic.
> 
> However, some checks has READ_ONCE in them or other 'compiler-unfriendly'
> behaviour. For example, ASSERT(list_empty) in btrfs_add_dealloc_inode
> was compiled to redundant mov because of this.
> 
> This patch replaces ASSERT with BUILD_BUG_ON_INVALID for
> !CONFIG_BTRFS_ASSERT builds.
> 
> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
> 
> ---
> .o size reductions are not that big, for example on defconfig + btrfs
> fs/btrfs/*.o size went from 3280528 to 3277936, so compiler was pretty
> efficient on his own
> ---
>   fs/btrfs/messages.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/messages.h b/fs/btrfs/messages.h
> index 4416c165644f..f80fe40a2c2b 100644
> --- a/fs/btrfs/messages.h
> +++ b/fs/btrfs/messages.h
> @@ -168,7 +168,7 @@ do {										\
>   #endif
>   
>   #else
> -#define ASSERT(cond, args...)			(void)(cond)
> +#define ASSERT(cond, args...)			BUILD_BUG_ON_INVALID(cond)

And I do not think it's a good idea to use BUILD_BUG_ON_INVALID() here, 
most ASSERT()s are checking runtime conditions, I understand you want to 
avoid real code generation, but in that case there are more 
straightforward solutions, like "do {} while (0)" as no-op.

Thanks,
Qu


>   #endif
>   
>   #ifdef CONFIG_BTRFS_DEBUG
> 
> base-commit: e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6