[RFC,v2,1/4] jbd2: make jdb2_debug module parameter per device

From: Chunguang Xu <brookxu@tencent.com>

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 <brookxu@tencent.com>
Reviewed-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
---
 fs/jbd2/journal.c     | 55 ++++++++++++++++++++++++++++++++++++++++---
 fs/jbd2/transaction.c |  2 +-
 include/linux/jbd2.h  |  7 ++++++
 3 files changed, 60 insertions(+), 4 deletions(-)

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 <brookxu.cn@gmail.com> wrote:
>
> From: Chunguang Xu <brookxu@tencent.com>
>
> 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 <brookxu@tencent.com>
> Reviewed-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> ---
>  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 <brookxu.cn@gmail.com> wrote:
>>
>> From: Chunguang Xu <brookxu@tencent.com>
>>
>> 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 <brookxu@tencent.com>
>> Reviewed-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
>> ---
>>  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
>>