Hey hi! I don't see my previous comments being handled here or am I
missing something? It'd be really handy to have the device name
printed in jbd2 logs.
On Sat, Jan 23, 2021 at 4:01 AM Chunguang Xu <[email protected]> wrote:
>
> From: Chunguang Xu <[email protected]>
>
> On a multi-disk machine, because jbd2's debugging switch is global,this
> confuses the logs of multiple disks. It is not easy to distinguish the
> logs of each disk and the amount of generated logs is very large. Maybe
> a separate debugging switch for each disk would be better, so that we
> can easily distinguish the logs of a certain disk.
>
> Signed-off-by: Chunguang Xu <[email protected]>
> Reviewed-by: Harshad Shirwadkar <[email protected]>
> ---
> fs/jbd2/journal.c | 55 ++++++++++++++++++++++++++++++++++++++++---
> fs/jbd2/transaction.c | 2 +-
> include/linux/jbd2.h | 7 ++++++
> 3 files changed, 60 insertions(+), 4 deletions(-)
>
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 2dc944442802..1f32b854ea28 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -101,13 +101,13 @@ EXPORT_SYMBOL(jbd2_inode_cache);
> static int jbd2_journal_create_slab(size_t slab_size);
>
> #ifdef CONFIG_JBD2_DEBUG
> -void __jbd2_debug(int level, const char *file, const char *func,
> +void jbd2_log(int level, journal_t *j, const char *file, const char *func,
> unsigned int line, const char *fmt, ...)
> {
> struct va_format vaf;
> va_list args;
>
> - if (level > jbd2_journal_enable_debug)
> + if (!j || (level > jbd2_journal_enable_debug && level > j->j_debug_level))
> return;
> va_start(args, fmt);
> vaf.fmt = fmt;
> @@ -115,7 +115,7 @@ void __jbd2_debug(int level, const char *file, const char *func,
> printk(KERN_DEBUG "%s: (%s, %u): %pV", file, func, line, &vaf);
> va_end(args);
> }
> -EXPORT_SYMBOL(__jbd2_debug);
> +EXPORT_SYMBOL(jbd2_log);
> #endif
>
> /* Checksumming functions */
> @@ -1257,6 +1257,48 @@ static int jbd2_seq_info_release(struct inode *inode, struct file *file)
> return seq_release(inode, file);
> }
>
> +#ifdef CONFIG_JBD2_DEBUG
> +static int jbd2_proc_debug_show(struct seq_file *m, void *v)
> +{
> + journal_t *j = m->private;
> +
> + seq_printf(m, "%d\n", j->j_debug_level);
> + return 0;
> +}
> +
> +static int jbd2_proc_debug_open(struct inode *inode, struct file *file)
> +{
> + journal_t *journal = PDE_DATA(inode);
> +
> + return single_open(file, jbd2_proc_debug_show, journal);
> +}
> +
> +static ssize_t jbd2_proc_debug_write(struct file *file,
> + const char __user *buffer, size_t count, loff_t *ppos)
> +{
> + struct seq_file *seq = file->private_data;
> + journal_t *j = seq->private;
> + char c;
> +
> + if (get_user(c, buffer))
> + return -EFAULT;
> +
> + if (c < '0' || c > '5')
> + return -EINVAL;
> +
> + j->j_debug_level = c - '0';
> + return count;
> +}
> +
> +static const struct proc_ops jbd2_debug_proc_ops = {
> + .proc_open = jbd2_proc_debug_open,
> + .proc_read = seq_read,
> + .proc_write = jbd2_proc_debug_write,
> + .proc_release = single_release,
> + .proc_lseek = seq_lseek,
> +};
> +#endif
> +
> static const struct proc_ops jbd2_info_proc_ops = {
> .proc_open = jbd2_seq_info_open,
> .proc_read = seq_read,
> @@ -1272,12 +1314,19 @@ static void jbd2_stats_proc_init(journal_t *journal)
> if (journal->j_proc_entry) {
> proc_create_data("info", S_IRUGO, journal->j_proc_entry,
> &jbd2_info_proc_ops, journal);
> +#ifdef CONFIG_JBD2_DEBUG
> + proc_create_data("jbd2_debug", S_IRUGO, journal->j_proc_entry,
> + &jbd2_debug_proc_ops, journal);
> +#endif
> }
> }
>
> static void jbd2_stats_proc_exit(journal_t *journal)
> {
> remove_proc_entry("info", journal->j_proc_entry);
> +#ifdef CONFIG_JBD2_DEBUG
> + remove_proc_entry("jbd2_debug", journal->j_proc_entry);
> +#endif
> remove_proc_entry(journal->j_devname, proc_jbd2_stats);
> }
>
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 9396666b7314..71787e826788 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -150,7 +150,7 @@ static inline void update_t_max_wait(transaction_t *transaction,
> unsigned long ts)
> {
> #ifdef CONFIG_JBD2_DEBUG
> - if (jbd2_journal_enable_debug &&
> + if ((jbd2_journal_enable_debug || transaction->t_journal->j_debug_level) &&
> time_after(transaction->t_start, ts)) {
> ts = jbd2_time_diff(ts, transaction->t_start);
> spin_lock(&transaction->t_handle_lock);
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 99d3cd051ac3..600a2ea8324a 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -1211,6 +1211,13 @@ struct journal_s
> */
> struct transaction_stats_s j_stats;
>
> +#ifdef CONFIG_JBD2_DEBUG
> + /**
> + * @j_debug_level: debugging level for jbd2.
> + */
> + unsigned int j_debug_level;
> +#endif
> +
> /**
> * @j_failed_commit: Failed journal commit ID.
> */
> --
> 2.30.0
>
harshad shirwadkar wrote on 2021/1/26 1:15:
> Hey hi! I don't see my previous comments being handled here or am I
> missing something? It'd be really handy to have the device name
> printed in jbd2 logs.
Maybe I miss something..,the origin switch has been reserved, the new added
switch and the origin switch together determine whether the log of a certain
device is finally output. . Also, I will add device information to the next
version.thanks.
> On Sat, Jan 23, 2021 at 4:01 AM Chunguang Xu <[email protected]> wrote:
>>
>> From: Chunguang Xu <[email protected]>
>>
>> On a multi-disk machine, because jbd2's debugging switch is global,this
>> confuses the logs of multiple disks. It is not easy to distinguish the
>> logs of each disk and the amount of generated logs is very large. Maybe
>> a separate debugging switch for each disk would be better, so that we
>> can easily distinguish the logs of a certain disk.
>>
>> Signed-off-by: Chunguang Xu <[email protected]>
>> Reviewed-by: Harshad Shirwadkar <[email protected]>
>> ---
>> fs/jbd2/journal.c | 55 ++++++++++++++++++++++++++++++++++++++++---
>> fs/jbd2/transaction.c | 2 +-
>> include/linux/jbd2.h | 7 ++++++
>> 3 files changed, 60 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
>> index 2dc944442802..1f32b854ea28 100644
>> --- a/fs/jbd2/journal.c
>> +++ b/fs/jbd2/journal.c
>> @@ -101,13 +101,13 @@ EXPORT_SYMBOL(jbd2_inode_cache);
>> static int jbd2_journal_create_slab(size_t slab_size);
>>
>> #ifdef CONFIG_JBD2_DEBUG
>> -void __jbd2_debug(int level, const char *file, const char *func,
>> +void jbd2_log(int level, journal_t *j, const char *file, const char *func,
>> unsigned int line, const char *fmt, ...)
>> {
>> struct va_format vaf;
>> va_list args;
>>
>> - if (level > jbd2_journal_enable_debug)
>> + if (!j || (level > jbd2_journal_enable_debug && level > j->j_debug_level))
>> return;
>> va_start(args, fmt);
>> vaf.fmt = fmt;
>> @@ -115,7 +115,7 @@ void __jbd2_debug(int level, const char *file, const char *func,
>> printk(KERN_DEBUG "%s: (%s, %u): %pV", file, func, line, &vaf);
>> va_end(args);
>> }
>> -EXPORT_SYMBOL(__jbd2_debug);
>> +EXPORT_SYMBOL(jbd2_log);
>> #endif
>>
>> /* Checksumming functions */
>> @@ -1257,6 +1257,48 @@ static int jbd2_seq_info_release(struct inode *inode, struct file *file)
>> return seq_release(inode, file);
>> }
>>
>> +#ifdef CONFIG_JBD2_DEBUG
>> +static int jbd2_proc_debug_show(struct seq_file *m, void *v)
>> +{
>> + journal_t *j = m->private;
>> +
>> + seq_printf(m, "%d\n", j->j_debug_level);
>> + return 0;
>> +}
>> +
>> +static int jbd2_proc_debug_open(struct inode *inode, struct file *file)
>> +{
>> + journal_t *journal = PDE_DATA(inode);
>> +
>> + return single_open(file, jbd2_proc_debug_show, journal);
>> +}
>> +
>> +static ssize_t jbd2_proc_debug_write(struct file *file,
>> + const char __user *buffer, size_t count, loff_t *ppos)
>> +{
>> + struct seq_file *seq = file->private_data;
>> + journal_t *j = seq->private;
>> + char c;
>> +
>> + if (get_user(c, buffer))
>> + return -EFAULT;
>> +
>> + if (c < '0' || c > '5')
>> + return -EINVAL;
>> +
>> + j->j_debug_level = c - '0';
>> + return count;
>> +}
>> +
>> +static const struct proc_ops jbd2_debug_proc_ops = {
>> + .proc_open = jbd2_proc_debug_open,
>> + .proc_read = seq_read,
>> + .proc_write = jbd2_proc_debug_write,
>> + .proc_release = single_release,
>> + .proc_lseek = seq_lseek,
>> +};
>> +#endif
>> +
>> static const struct proc_ops jbd2_info_proc_ops = {
>> .proc_open = jbd2_seq_info_open,
>> .proc_read = seq_read,
>> @@ -1272,12 +1314,19 @@ static void jbd2_stats_proc_init(journal_t *journal)
>> if (journal->j_proc_entry) {
>> proc_create_data("info", S_IRUGO, journal->j_proc_entry,
>> &jbd2_info_proc_ops, journal);
>> +#ifdef CONFIG_JBD2_DEBUG
>> + proc_create_data("jbd2_debug", S_IRUGO, journal->j_proc_entry,
>> + &jbd2_debug_proc_ops, journal);
>> +#endif
>> }
>> }
>>
>> static void jbd2_stats_proc_exit(journal_t *journal)
>> {
>> remove_proc_entry("info", journal->j_proc_entry);
>> +#ifdef CONFIG_JBD2_DEBUG
>> + remove_proc_entry("jbd2_debug", journal->j_proc_entry);
>> +#endif
>> remove_proc_entry(journal->j_devname, proc_jbd2_stats);
>> }
>>
>> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
>> index 9396666b7314..71787e826788 100644
>> --- a/fs/jbd2/transaction.c
>> +++ b/fs/jbd2/transaction.c
>> @@ -150,7 +150,7 @@ static inline void update_t_max_wait(transaction_t *transaction,
>> unsigned long ts)
>> {
>> #ifdef CONFIG_JBD2_DEBUG
>> - if (jbd2_journal_enable_debug &&
>> + if ((jbd2_journal_enable_debug || transaction->t_journal->j_debug_level) &&
>> time_after(transaction->t_start, ts)) {
>> ts = jbd2_time_diff(ts, transaction->t_start);
>> spin_lock(&transaction->t_handle_lock);
>> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
>> index 99d3cd051ac3..600a2ea8324a 100644
>> --- a/include/linux/jbd2.h
>> +++ b/include/linux/jbd2.h
>> @@ -1211,6 +1211,13 @@ struct journal_s
>> */
>> struct transaction_stats_s j_stats;
>>
>> +#ifdef CONFIG_JBD2_DEBUG
>> + /**
>> + * @j_debug_level: debugging level for jbd2.
>> + */
>> + unsigned int j_debug_level;
>> +#endif
>> +
>> /**
>> * @j_failed_commit: Failed journal commit ID.
>> */
>> --
>> 2.30.0
>>