linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] drivers/base: Remove unnecessary OOM message
@ 2015-02-06 15:39 Quentin Lambert
  2015-02-06 20:58 ` Pavel Machek
  0 siblings, 1 reply; 6+ messages in thread
From: Quentin Lambert @ 2015-02-06 15:39 UTC (permalink / raw)
  To: Ming Lei, Greg Kroah-Hartman, Rafael J. Wysocki, Len Brown, Pavel Machek
  Cc: kernel-janitors, linux-kernel, linux-pm

This patch fix checkpatch warnings concerning the possible
duplication of an "out of memory" message.

A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)

@@
identifier f,print,l;
expression e;
constant char[] c;
@@

e = \(kzalloc\|kmalloc\|devm_kzalloc\|devm_kmalloc\)(...);
if (e == NULL) {
  <+...
-  print(...,c,...);
  ... when any
(
  goto l;
|
  return ...;
)
  ...+> }

Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
 drivers/base/firmware_class.c  | 1 -
 drivers/base/power/clock_ops.c | 4 +---
 drivers/base/power/opp.c       | 8 ++------
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 58470c3..c3293f0 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -855,7 +855,6 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
 
 	fw_priv = kzalloc(sizeof(*fw_priv), GFP_KERNEL);
 	if (!fw_priv) {
-		dev_err(device, "%s: kmalloc failed\n", __func__);
 		fw_priv = ERR_PTR(-ENOMEM);
 		goto exit;
 	}
diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index d626576..7fdd017 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -81,10 +81,8 @@ static int __pm_clk_add(struct device *dev, const char *con_id,
 		return -EINVAL;
 
 	ce = kzalloc(sizeof(*ce), GFP_KERNEL);
-	if (!ce) {
-		dev_err(dev, "Not enough memory for clock entry.\n");
+	if (!ce)
 		return -ENOMEM;
-	}
 
 	if (con_id) {
 		ce->con_id = kstrdup(con_id, GFP_KERNEL);
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 15bf299..677fb28 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -474,10 +474,8 @@ static int _opp_add_dynamic(struct device *dev, unsigned long freq,
 
 	/* allocate new OPP node */
 	new_opp = kzalloc(sizeof(*new_opp), GFP_KERNEL);
-	if (!new_opp) {
-		dev_warn(dev, "%s: Unable to create new OPP node\n", __func__);
+	if (!new_opp)
 		return -ENOMEM;
-	}
 
 	/* Hold our list modification lock here */
 	mutex_lock(&dev_opp_list_lock);
@@ -695,10 +693,8 @@ static int _opp_set_availability(struct device *dev, unsigned long freq,
 
 	/* keep the node allocated */
 	new_opp = kmalloc(sizeof(*new_opp), GFP_KERNEL);
-	if (!new_opp) {
-		dev_warn(dev, "%s: Unable to create OPP\n", __func__);
+	if (!new_opp)
 		return -ENOMEM;
-	}
 
 	mutex_lock(&dev_opp_list_lock);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] drivers/base: Remove unnecessary OOM message
  2015-02-06 15:39 [PATCH 1/1] drivers/base: Remove unnecessary OOM message Quentin Lambert
@ 2015-02-06 20:58 ` Pavel Machek
  2015-02-06 21:04   ` Joe Perches
  0 siblings, 1 reply; 6+ messages in thread
From: Pavel Machek @ 2015-02-06 20:58 UTC (permalink / raw)
  To: Quentin Lambert
  Cc: Ming Lei, Greg Kroah-Hartman, Rafael J. Wysocki, Len Brown,
	kernel-janitors, linux-kernel, linux-pm

On Fri 2015-02-06 16:39:12, Quentin Lambert wrote:
> This patch fix checkpatch warnings concerning the possible
> duplication of an "out of memory" message.

So, instead of nice and readable "not enough memory for clock..." we
get OOM, stackdump, and backtrace...? Not sure it is improvement.

							Pavel

> --- a/drivers/base/power/clock_ops.c
> +++ b/drivers/base/power/clock_ops.c
> @@ -81,10 +81,8 @@ static int __pm_clk_add(struct device *dev, const char *con_id,
>  		return -EINVAL;
>  
>  	ce = kzalloc(sizeof(*ce), GFP_KERNEL);
> -	if (!ce) {
> -		dev_err(dev, "Not enough memory for clock entry.\n");
> +	if (!ce)
>  		return -ENOMEM;
> -	}
>  
>  	if (con_id) {
>  		ce->con_id = kstrdup(con_id, GFP_KERNEL);
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index 15bf299..677fb28 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -474,10 +474,8 @@ static int _opp_add_dynamic(struct device *dev, unsigned long freq,
>  
>  	/* allocate new OPP node */
>  	new_opp = kzalloc(sizeof(*new_opp), GFP_KERNEL);
> -	if (!new_opp) {
> -		dev_warn(dev, "%s: Unable to create new OPP node\n", __func__);
> +	if (!new_opp)
>  		return -ENOMEM;
> -	}
>  
>  	/* Hold our list modification lock here */
>  	mutex_lock(&dev_opp_list_lock);
> @@ -695,10 +693,8 @@ static int _opp_set_availability(struct device *dev, unsigned long freq,
>  
>  	/* keep the node allocated */
>  	new_opp = kmalloc(sizeof(*new_opp), GFP_KERNEL);
> -	if (!new_opp) {
> -		dev_warn(dev, "%s: Unable to create OPP\n", __func__);
> +	if (!new_opp)
>  		return -ENOMEM;
> -	}
>  
>  	mutex_lock(&dev_opp_list_lock);
>  

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] drivers/base: Remove unnecessary OOM message
  2015-02-06 20:58 ` Pavel Machek
@ 2015-02-06 21:04   ` Joe Perches
  2015-02-06 21:17     ` Pavel Machek
  0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2015-02-06 21:04 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Quentin Lambert, Ming Lei, Greg Kroah-Hartman, Rafael J. Wysocki,
	Len Brown, kernel-janitors, linux-kernel, linux-pm

On Fri, 2015-02-06 at 21:58 +0100, Pavel Machek wrote:
> On Fri 2015-02-06 16:39:12, Quentin Lambert wrote:
> > This patch fix checkpatch warnings concerning the possible
> > duplication of an "out of memory" message.
> 
> So, instead of nice and readable "not enough memory for clock..." we
> get OOM, stackdump, and backtrace...? Not sure it is improvement.

All allocs without __GFP_NOWARN already gets an OOM and stackdump.
These are just unnecessary.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] drivers/base: Remove unnecessary OOM message
  2015-02-06 21:04   ` Joe Perches
@ 2015-02-06 21:17     ` Pavel Machek
  2015-02-07 10:24       ` Dan Carpenter
  0 siblings, 1 reply; 6+ messages in thread
From: Pavel Machek @ 2015-02-06 21:17 UTC (permalink / raw)
  To: Joe Perches
  Cc: Quentin Lambert, Ming Lei, Greg Kroah-Hartman, Rafael J. Wysocki,
	Len Brown, kernel-janitors, linux-kernel, linux-pm

On Fri 2015-02-06 13:04:30, Joe Perches wrote:
> On Fri, 2015-02-06 at 21:58 +0100, Pavel Machek wrote:
> > On Fri 2015-02-06 16:39:12, Quentin Lambert wrote:
> > > This patch fix checkpatch warnings concerning the possible
> > > duplication of an "out of memory" message.
> > 
> > So, instead of nice and readable "not enough memory for clock..." we
> > get OOM, stackdump, and backtrace...? Not sure it is improvement.
> 
> All allocs without __GFP_NOWARN already gets an OOM and stackdump.
> These are just unnecessary.

"These" being human readable messages or ugly stackdumps?

Read what I have said. Figuring out what allocation failed from
unreliable backtrace is not fun.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] drivers/base: Remove unnecessary OOM message
  2015-02-06 21:17     ` Pavel Machek
@ 2015-02-07 10:24       ` Dan Carpenter
  2015-02-08 23:28         ` Rafael J. Wysocki
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2015-02-07 10:24 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Joe Perches, Quentin Lambert, Ming Lei, Greg Kroah-Hartman,
	Rafael J. Wysocki, Len Brown, kernel-janitors, linux-kernel,
	linux-pm

Facts:
1) None of these four error messages will ever be printed.
   http://lwn.net/Articles/627419/
2) These messages are a waste of RAM.

Debatable:
1) These messages are bad style and distracting.  Simple code is better.
2) The normal OOM message is enough to find which allocation failed.

Not everbody, but a lot of people write these kinds of error messages
with their brain on auto-pilot because they think they *should* do it.
It's quite common to do things like printk("dev is NULL.  %s",
dev->name);.  I'm happy for this checkpatch warning because it hopefully
bumps people out of mindless mode and makes them think about error
messages.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] drivers/base: Remove unnecessary OOM message
  2015-02-07 10:24       ` Dan Carpenter
@ 2015-02-08 23:28         ` Rafael J. Wysocki
  0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2015-02-08 23:28 UTC (permalink / raw)
  To: Dan Carpenter, Quentin Lambert
  Cc: Pavel Machek, Joe Perches, Ming Lei, Greg Kroah-Hartman,
	Len Brown, kernel-janitors, linux-kernel, linux-pm

On Saturday, February 07, 2015 01:24:36 PM Dan Carpenter wrote:
> Facts:
> 1) None of these four error messages will ever be printed.
>    http://lwn.net/Articles/627419/
> 2) These messages are a waste of RAM.

So this is what the changelog should be saying, not that it "fixes
checkpatch warnings".

checkpatch warnings in *existing* code are meaningless, unless they
uncover a *real* problem in it which quite arguably this one is.  But
in those cases patch changelogs should talk about the *real* problems
and they don't even need to mention checkpatch then as far as I'm
concerned.

Send this one with a better changelog and I'll apply it.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-02-08 23:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-06 15:39 [PATCH 1/1] drivers/base: Remove unnecessary OOM message Quentin Lambert
2015-02-06 20:58 ` Pavel Machek
2015-02-06 21:04   ` Joe Perches
2015-02-06 21:17     ` Pavel Machek
2015-02-07 10:24       ` Dan Carpenter
2015-02-08 23:28         ` Rafael J. Wysocki

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).