From: "Ted Ts'o" <tytso@mit.edu>
To: Joe Perches <joe@perches.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Andy Whitcroft <apw@shadowen.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
Date: Wed, 14 Mar 2012 08:34:29 -0400 [thread overview]
Message-ID: <20120314123429.GG15379@thunk.org> (raw)
In-Reply-To: <1331694074.27389.42.camel@joe2Laptop>
On Tue, Mar 13, 2012 at 08:01:14PM -0700, Joe Perches wrote:
> > That's a debug message which is never by anyone other than ext4
> > developers. Your patch also hacked the Makefile to enable it by
> > default,
>
> It's just an example and no it didn't.
> That output is still in an #ifdef EXT4FS_DEBUG
> block and is unchanged.
I looked at your patch, and nearly all of them were in debug code. I
know, because in practice the messages that come up with any kind of
regularity are all properly prefixed.
Look, the reason why I care about patch noise is because I have a huge
patch backlog. Take a look at this:
http://patchwork.ozlabs.org/project/linux-ext4/list/
Very few other people review patches, and even patches that survive
review, I've found problems that could potentially lead to data loss
or system instability. This is not like your average device driver,
where if the machine panics once a week, "oh well", and you reboot.
Linus would get very cranky if he lost data as a result of a bad patch
slipping through. Hence, patches don't go in until after significant
review and testing.
As a result, #1, patches that are don't add value, and are large,
simply won't get applied. Period. Avoiding the downside of lots of
people losing data is ****far**** more than your OCD wanting me to use
pr_warn(...) instead of printk(KERN_WARN, ...).
If you want your style patches to go in, break them into smaller
chunks, or I *will* ignore them.
#2, patches that don't add substantial value, and make it harder for
me to review and apply patches from my very substantial patch
backlog, I consider as adding ***substantial*** negative value.
That being said, I use checkpatch.pl as an initial screen, but it's
already been the case that there are warnings that I consider pure
noise, and I really, REALLY, rather not add more noise to
checkpatch.pl. There is no group consensus about things like pr_foo
--- not as far as I've seen --- and to impose it from on high by some
patch wankers making changes to checkpatch.pl very much offends me.
- Ted
next prev parent reply other threads:[~2012-03-14 12:34 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20120302025554.GA13493@joana>
[not found] ` <20120301.221643.881299898523907213.davem@davemloft.net>
[not found] ` <20120301.222316.1877216960521396397.davem@davemloft.net>
[not found] ` <20120301.222604.1508242694024394849.davem@davemloft.net>
[not found] ` <1330661602.1939.13.camel@joe2Laptop>
2012-03-02 5:35 ` [PATCH] checkpatch: Add --strict tests for braces, comments and casts Joe Perches
2012-03-02 5:54 ` [PATCH] checkpatch: Add --strict test for strings split across multiple lines Joe Perches
2012-03-13 6:23 ` [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL> Joe Perches
2012-03-13 12:05 ` Ted Ts'o
2012-03-13 21:55 ` Andrew Morton
2012-03-13 22:01 ` Ted Ts'o
2012-03-13 22:03 ` Andrew Morton
2012-03-14 0:31 ` Ted Ts'o
2012-03-14 0:47 ` Joe Perches
2012-03-14 1:07 ` Ted Ts'o
2012-03-14 1:17 ` Joe Perches
2012-03-14 2:19 ` Ted Ts'o
2012-03-14 2:31 ` Joe Perches
2012-03-14 2:41 ` Ted Ts'o
2012-03-14 3:01 ` Joe Perches
2012-03-14 12:34 ` Ted Ts'o [this message]
2012-03-14 13:05 ` Joe Perches
2012-03-14 13:45 ` Ted Ts'o
2012-03-14 14:06 ` Joe Perches
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=20120314123429.GG15379@thunk.org \
--to=tytso@mit.edu \
--cc=akpm@linux-foundation.org \
--cc=apw@shadowen.org \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
/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