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

[David Sterba <dsterba@suse.cz>]

On Sun, Nov 02, 2025 at 10:38:52AM +0300, Gladyshev Ilya wrote:
> The current definition of `ASSERT(cond)` as `(void)(cond)` is redundant,
> since these checks have no side effects and don't affect code logic.

Have you checked that none of the assert expressions really don't have
side effects other than touching the memory?

> However, some checks contain READ_ONCE or other compiler-unfriendly
> constructs. For example, ASSERT(list_empty) in btrfs_add_dealloc_inode
> was compiled to a redundant mov instruction due to this issue.
> 
> This patch defines ASSERT as BUILD_BUG_ON_INVALID for !CONFIG_BTRFS_ASSERT
> builds. It also marks `full_page_sectors_uptodate` as __maybe_unused to
> suppress "unneeded declaration" warning (it's needed in compile time)
> 
> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
> ---
> Changes from v1:
> - Annotate full_page_sectors_uptodate as __maybe_unused to avoid
>   compiler warning
> 
> Link to v1: https://lore.kernel.org/linux-btrfs/20251030182322.4085697-1-foxido@foxido.dev/
> ---
>  fs/btrfs/messages.h | 2 +-
>  fs/btrfs/raid56.c   | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> 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)

I'd rather have the expression open coded rather than using
BUILD_BUG_ON_INVALID, the name is confusing as it's not checking build
time condtitons.