On 2019/1/17 下午9:14, Johannes Thumshirn wrote:
> On 17/01/2019 11:15, Qu Wenruo wrote:
>> @@ -449,9 +449,10 @@ static int verify_level_key(struct btrfs_fs_info *fs_info,
>> btrfs_item_key_to_cpu(eb, &found_key, 0);
>> ret = btrfs_comp_cpu_keys(first_key, &found_key);
>>
>> -#ifdef CONFIG_BTRFS_DEBUG
>> if (ret) {
>> +#ifdef CONFIG_BTRFS_DEBUG
>> WARN_ON(1);
>> +#endif
>> btrfs_err(fs_info,
>> "tree first key mismatch detected, bytenr=%llu parent_transid=%llu key expected=(%llu,%u,%llu) has=(%llu,%u,%llu)",
>> eb->start, parent_transid, first_key->objectid,
>> @@ -459,7 +460,6 @@ static int verify_level_key(struct btrfs_fs_info *fs_info,
>> found_key.objectid, found_key.type,
>> found_key.offset);
>> }
>> -#endif
>> return ret;
>> }
>
>
> Hmm I think the resulting code looks a bit odd now, doesn't it?
> if (ret) {
> #ifdef CONFIG_BTRFS_DEBUG
> WARN_ON(1);
> #endif
>
> btrfs_err(fs_info,
>
>
> Why not something like:
> if (ret) {
> if (IS_ENABLED(CONFIG_BTRFS_DEBUG))
> WARN_ON(1);
Didn't know that macro, and indeed it looks much better.
>
> btrfs_err(fs_info,
>
> Or even hide the if (IS_ENABLED(CONFIG_BTRFS_DEBUG)) WARN_ON(1); in a macro
> #define BTRFS_DEBUG_WARN_ON(x) do { \
> if (IS_ENABLED(CONFIG_BTRFS_DEBUG)) \
> WARN_ON((x)); \
> } while(0)
Although I'd say such WARN_ON() for DEBUG build will finally get moved
to any build.
Thanks,
Qu
>
