linux-ext4 - Re: [RFC PATCH v2 2/4] jbd2: introduce some new log interfaces

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 25 Jan 2021 23:53:42 +0800
From:   brookxu <brookxu.cn@...il.com>
To:     Jan Kara <jack@...e.cz>
Cc:     tytso@....edu, adilger.kernel@...ger.ca, jack@...e.com,
        harshadshirwadkar@...il.com, linux-ext4@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v2 2/4] jbd2: introduce some new log interfaces


Jan Kara wrote on 2021/1/25 22:54:
> On Sat 23-01-21 20:00:44, Chunguang Xu wrote:
>> From: Chunguang Xu <brookxu@...cent.com>
>>
>> Compared to directly using numbers to indicate levels, using abstract
>> error, warn, notice, info, debug to indicate levels may be more
>> convenient for code reading and writing. Similar to other kernel
>> modules, some basic log interfaces are introduced.
>>
>> Signed-off-by: Chunguang Xu <brookxu@...cent.com>
> 
> One more thing I've noticed when reading this patch:
> 
>> +
>> +#ifdef CONFIG_JBD2_DEBUG
>> +/*
>> + * Define JBD2_EXPENSIVE_CHECKING to enable more expensive internal
>> + * consistency checks.  By default we don't do this unless
>> + * CONFIG_JBD2_DEBUG is on.
>> + */
>> +#define JBD2_EXPENSIVE_CHECKING
>> +extern ushort jbd2_journal_enable_debug;
>> +void jbd2_log(int level, journal_t *j, const char *file, const char *func,
>> +		      unsigned int line, const char *fmt, ...);
>> +
>> +#define JBD2_ERR	1	/* error conditions */
>> +#define JBD2_WARN	2	/* warning conditions */
>> +#define JBD2_NOTICE	3	/* normal but significant condition */
>> +#define JBD2_INFO	4	/* informational */
>> +#define JBD2_DEBUG	5	/* debug-level messages */
> 
> This is actually not true. All the jbd_debug() messages are really debug
> messages, not errors, not warnings. It is just a different level of detail.
> Honestly, these days, I'd rather discard all the levels, use pr_debug()
> function to print these messages inside jdb2_debug() and defer to
> CONFIG_DYNAMIC_DEBUG framework for configuration of which messages are
> interesting for a particular debug session.

>From a certain point, this is indeed, maybe the changes here are not necessary.
Thanks.

> 								Honza
> 
>> +
>> +#define jbd2_err(j, fmt, a...)						\
>> +	jbd2_log(JBD2_ERR, j, __FILE__, __func__, __LINE__, (fmt), ##a)
>> +
>> +#define jbd2_warn(j, fmt, a...)						\
>> +	jbd2_log(JBD2_WARN, j, __FILE__, __func__, __LINE__, (fmt), ##a)
>> +
>> +#define jbd2_notice(j, fmt, a...)					\
>> +	jbd2_log(JBD2_NOTICE, j, __FILE__, __func__, __LINE__, (fmt), ##a)
>> +
>> +#define jbd2_info(j, fmt, a...)						\
>> +	jbd2_log(JBD2_INFO, j, __FILE__, __func__, __LINE__, (fmt), ##a)
>> +
>> +#define jbd2_debug(j, fmt, a...)					\
>> +	jbd2_log(JBD2_DEBUG, j, __FILE__, __func__, __LINE__, (fmt), ##a)
>> +
>> +#else
>> +
>> +#define jbd2_err(j, fmt, a...)
>> +#define jbd2_warn(j, fmt, a...)
>> +#define jbd2_notice(j, fmt, a...)
>> +#define jbd2_info(j, fmt, a...)
>> +#define jbd2_debug(j, fmt, a...)
>> +
>> +#endif
>>  #endif
>>  
>>  /*
>> -- 
>> 2.30.0
>>