From: Joe Perches <joe@perches.com>
To: Ted Ts'o <tytso@mit.edu>
Cc: David Miller <davem@davemloft.net>,
anca.emanuel@gmail.com, adilger.kernel@dilger.ca,
linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/9] ext4: Use pr_fmt and pr_<level>
Date: Mon, 19 Mar 2012 18:59:01 -0700 [thread overview]
Message-ID: <1332208741.7847.46.camel@joe2Laptop> (raw)
In-Reply-To: <20120320014707.GD14363@thunk.org>
On Mon, 2012-03-19 at 21:47 -0400, Ted Ts'o wrote:
> 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.
> 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 I was not. Doubly prefixed was as I said, doubly prefixed.
ext4_msg wasn't a consistently used interface.
> 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.
Umm, ext4_msg does call printk.
> 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.
It's hard to say that's true.
Look at the the trace_<foo> mechanisms.
Very useful stuff but once set, there's
been a strong desire to set the output
as an unchangeable ABI.
So I think defining the output correctly
_first_ is the most important element of
any notification mechanism. TLV use in
the output generally isn't human parsable
and there's value in that readability.
next prev parent reply other threads:[~2012-03-20 1:59 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-16 0:07 [PATCH 0/9] ext4: Message logging corrections and neatening Joe Perches
2012-03-16 0:07 ` [PATCH 1/9] ext4: Add -DDEBUG to Makefile Joe Perches
2012-03-16 16:03 ` David Daney
2012-03-16 16:29 ` Joe Perches
2012-03-19 4:39 ` Ted Ts'o
2012-03-19 16:26 ` Joe Perches
2012-03-19 18:48 ` David Daney
2012-03-20 1:05 ` Ted Ts'o
2012-03-16 0:07 ` [PATCH 2/9] ext4: Use pr_fmt and pr_<level> Joe Perches
2012-03-19 4:09 ` Ted Ts'o
2012-03-19 4:14 ` David Miller
2012-03-19 4:34 ` Ted Ts'o
2012-03-19 5:12 ` David Miller
2012-03-19 15:31 ` Ted Ts'o
2012-03-19 15:51 ` Anca Emanuel
2012-03-19 16:14 ` Joe Perches
2012-03-19 16:14 ` Ted Ts'o
2012-03-19 18:14 ` David Miller
2012-03-19 18:31 ` Ted Ts'o
2012-03-19 18:46 ` Joe Perches
2012-03-20 1:04 ` Ted Ts'o
2012-03-20 1:33 ` Joe Perches
2012-03-20 1:47 ` Ted Ts'o
2012-03-20 1:59 ` Joe Perches [this message]
2012-03-20 2:58 ` Ted Ts'o
2012-03-20 3:02 ` Joe Perches
2012-03-20 5:46 ` Valdis.Kletnieks
2012-03-20 7:10 ` David Miller
2012-03-20 8:47 ` Jiri Slaby
2012-03-20 9:44 ` Joe Perches
2012-03-20 9:27 ` Geert Uytterhoeven
2012-03-20 13:03 ` Ted Ts'o
2012-03-20 18:47 ` Valdis.Kletnieks
2012-03-20 1:46 ` Al Viro
2012-03-19 17:53 ` David Miller
2012-03-19 4:25 ` Joe Perches
2012-03-19 4:36 ` Ted Ts'o
2012-03-19 16:46 ` Jiri Slaby
2012-03-19 17:09 ` Joe Perches
2012-03-19 17:36 ` Valdis.Kletnieks
2012-03-19 17:44 ` Joe Perches
2012-03-20 1:06 ` Ted Ts'o
2012-03-20 1:28 ` david
2012-03-20 1:51 ` Joe Perches
2012-03-20 1:33 ` Joe Perches
2012-03-20 8:57 ` Jiri Slaby
2012-03-20 9:21 ` Joe Perches
2012-03-20 9:25 ` Jiri Slaby
2012-03-20 9:46 ` Joe Perches
2012-03-22 17:02 ` Jiri Slaby
2012-03-22 17:42 ` Joe Perches
2012-03-19 4:55 ` Ted Ts'o
2012-03-19 5:13 ` David Miller
2012-03-19 5:39 ` Joe Perches
2012-03-16 0:07 ` [PATCH 3/9] ext4: Fix indentation Joe Perches
2012-03-19 4:10 ` Ted Ts'o
2012-03-19 4:30 ` Joe Perches
2012-03-16 0:07 ` [PATCH 4/9] ext4: Add no_printk argument validation, fix fallout Joe Perches
2012-03-19 4:16 ` Ted Ts'o
2012-03-16 0:07 ` [PATCH 5/9] ext4: Avoid output message interleaving in ext4_error_<foo> Joe Perches
2012-03-19 4:51 ` Ted Ts'o
2012-03-16 0:07 ` [PATCH 6/9] ext4: Remove redundant "EXT4-fs: " from uses of ext4_msg Joe Perches
2012-03-19 4:13 ` Ted Ts'o
2012-03-16 0:07 ` [PATCH 7/9] ext4: Format neatening for easier grep Joe Perches
2012-03-19 4:26 ` Ted Ts'o
2012-03-19 4:30 ` Joe Perches
2012-03-16 0:07 ` [PATCH 8/9] ext4: Neaten ext4_error uses Joe Perches
2012-03-16 0:07 ` [PATCH 9/9] ext4: Rename ext4_warning to ext4_warn and ext4_error to ext4_err Joe Perches
2012-03-19 4:51 ` Ted Ts'o
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1332208741.7847.46.camel@joe2Laptop \
--to=joe@perches.com \
--cc=adilger.kernel@dilger.ca \
--cc=anca.emanuel@gmail.com \
--cc=davem@davemloft.net \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).