From: Joe Perches <joe@perches.com>
To: Jonathan Cameron <jic23@jic23.retrosnub.co.uk>
Cc: SF Markus Elfring <elfring@users.sourceforge.net>,
linux-iio@vger.kernel.org, linux-input@vger.kernel.org,
Jonathan Cameron <jic23@kernel.org>,
Hartmut Knaack <knaack.h@gmx.de>, Jiri Kosina <jikos@kernel.org>,
Lars-Peter Clausen <lars@metafoo.de>,
Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
LKML <linux-kernel@vger.kernel.org>,
kernel-janitors@vger.kernel.org
Subject: Re: Software evolution around “checkpatch.pl”?
Date: Sat, 10 Feb 2018 17:42:57 +0000 [thread overview]
Message-ID: <1518284577.16865.8.camel@perches.com> (raw)
In-Reply-To: <20180210155734.708d01b6@archlinux>
On Sat, 2018-02-10 at 15:57 +0000, Jonathan Cameron wrote:
> On Sat, 10 Feb 2018 06:59:43 -0800
> Joe Perches <joe@perches.com> wrote:
>
> > On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> > > While it would be great to improve checkpatches false
> > > positive rate, it's very nature as a string matcher makes
> > > this hard.
> >
> > true.
> >
> > what are the false positives you see?
> >
>
> This particular case is only 'sort of' a false positive
> in the warning that a message printed on a memory allocation
> failure 'may' not add any information over the generic case.
Right. So it's not a 'false positive' at all.
Are there any actual 'false positives' you know of?
> Very hard to judge on whether it is useful to know more than
> an allocation failed somewhere or not.
>
> Message makes this clear:
> > “WARNING: Possible unnecessary 'out of memory' message”
> > (from the script “checkpatch.pl”)
>
> We also have the balance between any changes to existing code
> adding 'some' maintenance overhead vs changing this stuff
> in a new driver - which is what checkpatch is really intended
> for.
There's almost zero maintenance overhead here.
The time it takes for the back and forth
replies is likely larger.
> So I think checkpatch is striking the right balance here in
> how it warns. Obviously if it could assess the text
> and come to an informed decision that would be great but
> we are some way from that ;)
The 'informed' bit is difficult as it is mostly
a political problem.
This particular message really is unnecessary as
the generic dump_stack on any normal allocation
(ie: without __GFP_WARN) already emits location
specific information.
Removing these messages can help make the kernel
image smaller and thereby help make these OOM
messages a tiny bit less likely.
I just wish Markus would improve his consistently
terrible commit messages that just restate the
action being done and detail _why_ a particular
thing _should_ be done.
His acceptance rate would improve as many of these
back and forth replies for what trivialities he
posts as patches would be minimized.
next prev parent reply other threads:[~2018-02-10 17:42 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-31 21:26 [PATCH] hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_ SF Markus Elfring
2018-02-04 11:23 ` [PATCH] hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel Jonathan Cameron
2018-02-05 18:26 ` hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_prob SF Markus Elfring
2018-02-05 21:51 ` Jonathan Cameron
2018-02-06 8:45 ` Software evolution around “checkpatch.pl”? SF Markus Elfring
2018-02-10 14:53 ` Jonathan Cameron
2018-02-10 14:59 ` Joe Perches
2018-02-10 15:57 ` Jonathan Cameron
2018-02-10 17:42 ` Joe Perches [this message]
2018-02-10 18:30 ` SF Markus Elfring
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=1518284577.16865.8.camel@perches.com \
--to=joe@perches.com \
--cc=elfring@users.sourceforge.net \
--cc=jic23@jic23.retrosnub.co.uk \
--cc=jic23@kernel.org \
--cc=jikos@kernel.org \
--cc=kernel-janitors@vger.kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pmeerw@pmeerw.net \
--cc=srinivas.pandruvada@linux.intel.com \
/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