| Message ID | 093bba8a2b23f5bb678aaa9e6824e2bed3b4d2a5.1744794336.git.dsterba@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Assertion and debugging helpers | expand |
On 16.04.25 11:09, David Sterba wrote: > + > +/* Verbose assert, use to print any relevant values of the condition. */ > +#define VASSERT(expr, fmt, ...) \ > + (likely(expr) ? (void)0 : btrfs_assertfail_verbose(#expr, __FILE__, __LINE__, \ > + fmt, __VA_ARGS__)) > #else > #define ASSERT(expr) (void)(expr) > +#define VASSERT(expr, fmt, ...) (void)(expr) > #endif Ahm stupid question (applies for ASSERT() as well), doesn't that generate code as well when CONFIG_BTRFS_ASSERT=n? So we're doing a lot of potentially unneeded tests?
On Wed, Apr 16, 2025 at 03:03:18PM +0000, Johannes Thumshirn wrote: > On 16.04.25 11:09, David Sterba wrote: > > + > > +/* Verbose assert, use to print any relevant values of the condition. */ > > +#define VASSERT(expr, fmt, ...) \ > > + (likely(expr) ? (void)0 : btrfs_assertfail_verbose(#expr, __FILE__, __LINE__, \ > > + fmt, __VA_ARGS__)) > > #else > > #define ASSERT(expr) (void)(expr) > > +#define VASSERT(expr, fmt, ...) (void)(expr) > > #endif > > Ahm stupid question (applies for ASSERT() as well), doesn't that > generate code as well when CONFIG_BTRFS_ASSERT=n? So we're doing a lot > of potentially unneeded tests? It should not generate any code. It's parsed, syntax-checked and as the result is cast to void it's thrown out and optimized out (-O2).
diff --git a/fs/btrfs/messages.h b/fs/btrfs/messages.h index 08a9272399d26f..8cca9d834b274d 100644 --- a/fs/btrfs/messages.h +++ b/fs/btrfs/messages.h @@ -175,10 +175,33 @@ do { \ BUG(); \ }) +__printf(4, 5) +__cold +static inline void btrfs_assertfail_verbose(const char *str_expr, + const char *file, int line, + const char *fmt, ...) { + struct va_format vaf; + va_list args; + + va_start(args, fmt); + vaf.fmt = fmt; + vaf.va = &args; + + pr_err("assertion failed: %s, in %s:%d (%pV)\n", str_expr, file, line, &vaf); + va_end(args); + BUG(); +} + #define ASSERT(expr) \ (likely(expr) ? (void)0 : btrfs_assertfail(#expr, __FILE__, __LINE__)) + +/* Verbose assert, use to print any relevant values of the condition. */ +#define VASSERT(expr, fmt, ...) \ + (likely(expr) ? (void)0 : btrfs_assertfail_verbose(#expr, __FILE__, __LINE__, \ + fmt, __VA_ARGS__)) #else #define ASSERT(expr) (void)(expr) +#define VASSERT(expr, fmt, ...) (void)(expr) #endif __printf(5, 6)
Currently ASSERT prints the stringified condition and without macro expansions so simple constants like BTRFS_MAX_METADATA_BLOCKSIZE remain readable in the output. There are expressions where we'd like to see the exact values but all we get is someting like: assertion failed: em->start <= start && start < extent_map_end(em), in fs/btrfs/extent_map.c:613 It would be nice to be able to print any additional information to help understand the problem. For that there's now VASSERT and can be used like: VASSERT(value > limit, "value=%llu limit=%llu", value, limit); with free-form printk arguments that will be part of the assertion message. Pros: - helps debugging and understanding reported problems Cons: - increases the .ko size - writing the message is repetitive (condition, format, values) - format and variable type must match (extra lookup) Recommended use is for non-trivial expressions, so basic ASSERT(value) can be used for pointers or sometimes integers. The helper btrfs_assertfail_verbose() is inlined because it generates a bit better assembly (avoids a function call). Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/messages.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)