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@xxxxxxxxxxx> > > 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. -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
