lkml.org 
[lkml]   [2012]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>
On Mon, Mar 19, 2012 at 06:33:22PM -0700, Joe Perches wrote:
> They _were_ doubly prefixed.
>
> from ext4#dev
>
> commit 2504a4a9c0c096e11bcc24691b85bf6d942df9fe
> Author: Joe Perches <joe@perches.com>
> Date: Mon Mar 19 00:12:00 2012 -0400
>
> ext4: remove redundant "EXT4-fs: " from uses of ext4_msg
>
> ext4_msg adds "EXT4-fs: " to the messsage output.
> Remove the redundant bits from uses.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

Yes, and I accepted that patch. I was referring to your complaints of
printk's such as this:

#ifdef EXT4FS_DEBUG
WARN_ON(ret <= 0);
printk(KERN_ERR "%s: ext4_ext_map_blocks "
"returned error inode#%lu, block=%u, "
"max_blocks=%u", __func__,
inode->i_ino, map.m_lblk, max_blocks);
#endif

... and in that case, even if I was going to fix it, I wouldn't be
fixing it via pr_err(). I'd be fixing using ext4_msg(). Why?
Because then the message would include the block device and not just
the EXT4-fs prefix.

> > Yes, but we can't do structured notifications with the current
> > pr_<foo>. So why change literally tens of thousands of callsites when
> > in order to really realize the full promise of structured
> > notifications, we'll have to change them *again*?
>
> So that they are consistent and extensible and can use
> something like pr_<level>_notify, just like pr_<level>_once
> and pr_<level>_ratelimited, or some other similar form.

Changing to pr_err() is pointless, because it doesn't do anything
functional. You *have* to have an interface like ext4_msg(sb, ...) if
you're going to send a semi-structured notification, or include
relevant information about which ext4 file system was responsible for
issuing the warning.

If you're going to change huge numbers of lines of code, you might as
well do it in a way that significantly improves things. The change to
pr_foo() is just syntactic sugar, and that's a whitespace-level change
in my book. Adding a struct super * or or a struct block device *,
which gets passed to the notification functions? That's ***far***
more interesting.

- Ted


\
 
 \ /
  Last update: 2012-03-20 02:49    [W:0.075 / U:0.332 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site