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

[David Sterba <dsterba@suse.cz>]

On Fri, Oct 31, 2025 at 07:35:29AM +1030, Qu Wenruo wrote:
> 在 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.

It's supposed to be no-op but also compile checked, so the do/while(0)
will not do that. What BUILD_BUG_ON_INVALID is basically a sizeof(cond)
so it's the right thing but the naming is confusing, we can possibly
open code it.