Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Uwe Kleine-König 
Hello Joe,

On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
 Emitting an OOM message isn't necessary after input_allocate_device
 as there's a generic OOM and a dump_stack already done.
 
 [...]
 Signed-off-by: Joe Perches j...@perches.com
 diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
 index 005d852..3b9c709 100644
 --- a/drivers/input/joystick/as5011.c
 +++ b/drivers/input/joystick/as5011.c
 @@ -254,8 +254,6 @@ static int as5011_probe(struct i2c_client *client,
   as5011 = kmalloc(sizeof(struct as5011_device), GFP_KERNEL);
   input_dev = input_allocate_device();
   if (!as5011 || !input_dev) {
 - dev_err(client-dev,
 - Can't allocate memory for device structure\n);
Don't know if that can happen, but if as5011 is NULL but input_dev isn't
the message would still be sensible, wouldn't it? There are several more
that suffer the same problem.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Dmitry Torokhov 
Hi Joe,

On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
 Emitting an OOM message isn't necessary after input_allocate_device
 as there's a generic OOM and a dump_stack already done.

No, please don't. The kzalloc may get changed in the future to not dump
stack (that was added originally because not everyone was handling OOM
properly, right?), input core might get changed to use something else
than kzalloc, etc, etc.

The majority of errors use dev_err so we also get idea what device
failed (if there are several), and more.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Joe Perches 
On Thu, 2013-10-24 at 20:26 +0200, Uwe Kleine-König wrote:
 Hello Joe,
 
 On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
  Emitting an OOM message isn't necessary after input_allocate_device
  as there's a generic OOM and a dump_stack already done.
  
  [...]
  Signed-off-by: Joe Perches j...@perches.com
  diff --git a/drivers/input/joystick/as5011.c 
  b/drivers/input/joystick/as5011.c
  index 005d852..3b9c709 100644
  --- a/drivers/input/joystick/as5011.c
  +++ b/drivers/input/joystick/as5011.c
  @@ -254,8 +254,6 @@ static int as5011_probe(struct i2c_client *client,
  as5011 = kmalloc(sizeof(struct as5011_device), GFP_KERNEL);
  input_dev = input_allocate_device();
  if (!as5011 || !input_dev) {
  -   dev_err(client-dev,
  -   Can't allocate memory for device structure\n);
 Don't know if that can happen, but if as5011 is NULL but input_dev isn't
 the message would still be sensible, wouldn't it? There are several more
 that suffer the same problem.

Any k.alloc without __GFP_NOWARN does a generic OOM message
and a dump_stack() so there could already be 2 messages anyway.

cheers, Joe

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Joe Perches 
On Thu, 2013-10-24 at 11:37 -0700, Dmitry Torokhov wrote:
 Hi Joe,
 
 On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
  Emitting an OOM message isn't necessary after input_allocate_device
  as there's a generic OOM and a dump_stack already done.
 
 No, please don't. The kzalloc may get changed in the future to not dump
 stack (that was added originally because not everyone was handling OOM
 properly, right?), input core might get changed to use something else
 than kzalloc, etc, etc.
 
 The majority of errors use dev_err so we also get idea what device
 failed (if there are several), and more.

I think that's not valuable as input_allocate_device already has
dozens of locations that don't emit a specific OOM and centralizing
the location for any generic message would work anyway.



--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Uwe Kleine-König 
On Thu, Oct 24, 2013 at 11:43:38AM -0700, Joe Perches wrote:
 On Thu, 2013-10-24 at 20:26 +0200, Uwe Kleine-König wrote:
  Hello Joe,
  
  On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
   Emitting an OOM message isn't necessary after input_allocate_device
   as there's a generic OOM and a dump_stack already done.
   
   [...]
   Signed-off-by: Joe Perches j...@perches.com
   diff --git a/drivers/input/joystick/as5011.c 
   b/drivers/input/joystick/as5011.c
   index 005d852..3b9c709 100644
   --- a/drivers/input/joystick/as5011.c
   +++ b/drivers/input/joystick/as5011.c
   @@ -254,8 +254,6 @@ static int as5011_probe(struct i2c_client *client,
 as5011 = kmalloc(sizeof(struct as5011_device), GFP_KERNEL);
 input_dev = input_allocate_device();
 if (!as5011 || !input_dev) {
   - dev_err(client-dev,
   - Can't allocate memory for device structure\n);
  Don't know if that can happen, but if as5011 is NULL but input_dev isn't
  the message would still be sensible, wouldn't it? There are several more
  that suffer the same problem.
 
 Any k.alloc without __GFP_NOWARN does a generic OOM message
 and a dump_stack() so there could already be 2 messages anyway.
Then mention that in the commit log if you still want this patch?!

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Joe Perches 
On Thu, 2013-10-24 at 20:46 +0200, Uwe Kleine-König wrote:
 On Thu, Oct 24, 2013 at 11:43:38AM -0700, Joe Perches wrote:
[]
  Any k.alloc without __GFP_NOWARN does a generic OOM message
  and a dump_stack() so there could already be 2 messages anyway.
 Then mention that in the commit log if you still want this patch?!

How do you think this commit message should be changed:

Emitting an OOM message isn't necessary after input_allocate_device
as there's a generic OOM and a dump_stack already done.


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Dmitry Torokhov 
On Thu, Oct 24, 2013 at 11:45:39AM -0700, Joe Perches wrote:
 On Thu, 2013-10-24 at 11:37 -0700, Dmitry Torokhov wrote:
  Hi Joe,
  
  On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
   Emitting an OOM message isn't necessary after input_allocate_device
   as there's a generic OOM and a dump_stack already done.
  
  No, please don't. The kzalloc may get changed in the future to not dump
  stack (that was added originally because not everyone was handling OOM
  properly, right?), input core might get changed to use something else
  than kzalloc, etc, etc.
  
  The majority of errors use dev_err so we also get idea what device
  failed (if there are several), and more.
 
 I think that's not valuable as input_allocate_device already has
 dozens of locations that don't emit a specific OOM and centralizing
 the location for any generic message would work anyway.

Not having diagnostic messages in some of the drivers is hardly a
justification to remove them from everywhere else.

-- 
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Uwe Kleine-König 
On Thu, Oct 24, 2013 at 11:48:48AM -0700, Joe Perches wrote:
 On Thu, 2013-10-24 at 20:46 +0200, Uwe Kleine-König wrote:
  On Thu, Oct 24, 2013 at 11:43:38AM -0700, Joe Perches wrote:
 []
   Any k.alloc without __GFP_NOWARN does a generic OOM message
   and a dump_stack() so there could already be 2 messages anyway.
  Then mention that in the commit log if you still want this patch?!
 
 How do you think this commit message should be changed:
 
 Emitting an OOM message isn't necessary after input_allocate_device
 as there's a generic OOM and a dump_stack already done.
after input_allocate_device and/or k.alloc ...

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Joe Perches 
On Thu, 2013-10-24 at 12:10 -0700, Dmitry Torokhov wrote:
 On Thu, Oct 24, 2013 at 11:45:39AM -0700, Joe Perches wrote:
  On Thu, 2013-10-24 at 11:37 -0700, Dmitry Torokhov wrote:
   Hi Joe,
   
   On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
Emitting an OOM message isn't necessary after input_allocate_device
as there's a generic OOM and a dump_stack already done.
   
   No, please don't. The kzalloc may get changed in the future to not dump
   stack (that was added originally because not everyone was handling OOM
   properly, right?), input core might get changed to use something else
   than kzalloc, etc, etc.
   
   The majority of errors use dev_err so we also get idea what device
   failed (if there are several), and more.
  
  I think that's not valuable as input_allocate_device already has
  dozens of locations that don't emit a specific OOM and centralizing
  the location for any generic message would work anyway.
 
 Not having diagnostic messages in some of the drivers is hardly a
 justification to remove them from everywhere else.

But standardization is.

If a diagnostic message is really desired
without the already existing dump_stack(),
it's a small matter of adding something like:

 drivers/input/input.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index c044699..98570c2 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1736,19 +1736,23 @@ struct input_dev *input_allocate_device(void)
 {
struct input_dev *dev;
 
-   dev = kzalloc(sizeof(struct input_dev), GFP_KERNEL);
-   if (dev) {
-   dev-dev.type = input_dev_type;
-   dev-dev.class = input_class;
-   device_initialize(dev-dev);
-   mutex_init(dev-mutex);
-   spin_lock_init(dev-event_lock);
-   INIT_LIST_HEAD(dev-h_list);
-   INIT_LIST_HEAD(dev-node);
-
-   __module_get(THIS_MODULE);
+   dev = kzalloc(sizeof(struct input_dev), GFP_KERNEL | __GFP_NOWARN);
+   if (!dev) {
+   pr_err(%pf: input_allocate_device failed\n,
+  __builtin_return_address(1));
+   return NULL;
}
 
+   dev-dev.type = input_dev_type;
+   dev-dev.class = input_class;
+   device_initialize(dev-dev);
+   mutex_init(dev-mutex);
+   spin_lock_init(dev-event_lock);
+   INIT_LIST_HEAD(dev-h_list);
+   INIT_LIST_HEAD(dev-node);
+
+   __module_get(THIS_MODULE);
+
return dev;
 }
 EXPORT_SYMBOL(input_allocate_device);



--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-23 Thread Joe Perches 
Emitting an OOM message isn't necessary after input_allocate_device
as there's a generic OOM and a dump_stack already done.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/input/joystick/as5011.c | 2 --
 drivers/input/joystick/db9.c| 1 -
 drivers/input/joystick/gamecon.c| 4 +---
 drivers/input/joystick/turbografx.c | 1 -
 drivers/input/joystick/walkera0701.c| 1 -
 drivers/input/keyboard/amikbd.c | 4 +---
 drivers/input/keyboard/davinci_keyscan.c| 1 -
 drivers/input/keyboard/gpio_keys.c  | 1 -
 drivers/input/keyboard/lpc32xx-keys.c   | 1 -
 drivers/input/keyboard/max7359_keypad.c | 1 -
 drivers/input/keyboard/mcs_touchkey.c   | 1 -
 drivers/input/keyboard/mpr121_touchkey.c| 1 -
 drivers/input/keyboard/nomadik-ske-keypad.c | 1 -
 drivers/input/keyboard/opencores-kbd.c  | 1 -
 drivers/input/keyboard/pmic8xxx-keypad.c| 1 -
 drivers/input/keyboard/pxa27x_keypad.c  | 1 -
 drivers/input/keyboard/pxa930_rotary.c  | 1 -
 drivers/input/keyboard/qt1070.c | 1 -
 drivers/input/keyboard/qt2160.c | 1 -
 drivers/input/keyboard/sh_keysc.c   | 1 -
 drivers/input/keyboard/tc3589x-keypad.c | 1 -
 drivers/input/keyboard/tnetv107x-keypad.c   | 1 -
 drivers/input/keyboard/w90p910_keypad.c | 1 -
 drivers/input/misc/88pm80x_onkey.c  | 1 -
 drivers/input/misc/88pm860x_onkey.c | 1 -
 drivers/input/misc/arizona-haptics.c| 4 +---
 drivers/input/misc/atlas_btns.c | 4 +---
 drivers/input/misc/da9052_onkey.c   | 1 -
 drivers/input/misc/da9055_onkey.c   | 4 +---
 drivers/input/misc/ideapad_slidebar.c   | 1 -
 drivers/input/misc/ims-pcu.c| 7 +--
 drivers/input/misc/kxtj9.c  | 4 +---
 drivers/input/misc/max8997_haptic.c | 1 -
 drivers/input/misc/mc13783-pwrbutton.c  | 4 +---
 drivers/input/misc/mpu3050.c| 1 -
 drivers/input/misc/pcf8574_keypad.c | 1 -
 drivers/input/misc/pm8xxx-vibrator.c| 1 -
 drivers/input/misc/pmic8xxx-pwrkey.c| 1 -
 drivers/input/misc/pwm-beeper.c | 1 -
 drivers/input/misc/twl4030-pwrbutton.c  | 4 +---
 drivers/input/misc/twl6040-vibra.c  | 1 -
 drivers/input/mouse/appletouch.c| 4 +---
 drivers/input/mouse/bcm5974.c   | 4 +---
 drivers/input/mouse/cyapa.c | 4 +---
 drivers/input/mouse/inport.c| 1 -
 drivers/input/mouse/logibm.c| 1 -
 drivers/input/mouse/pc110pad.c  | 1 -
 drivers/input/mouse/pxa930_trkball.c| 1 -
 drivers/input/tablet/aiptek.c   | 5 +
 drivers/input/tablet/gtco.c | 1 -
 drivers/input/touchscreen/88pm860x-ts.c | 1 -
 drivers/input/touchscreen/atmel_mxt_ts.c| 1 -
 drivers/input/touchscreen/atmel_tsadcc.c| 1 -
 drivers/input/touchscreen/bu21013_ts.c  | 1 -
 drivers/input/touchscreen/cyttsp4_core.c| 2 --
 drivers/input/touchscreen/da9034-ts.c   | 1 -
 drivers/input/touchscreen/edt-ft5x06.c  | 1 -
 drivers/input/touchscreen/eeti_ts.c | 5 +
 drivers/input/touchscreen/htcpen.c  | 1 -
 drivers/input/touchscreen/intel-mid-touch.c | 1 -
 drivers/input/touchscreen/lpc32xx_ts.c  | 1 -
 drivers/input/touchscreen/mcs5000_ts.c  | 1 -
 drivers/input/touchscreen/migor_ts.c| 1 -
 drivers/input/touchscreen/mk712.c   | 1 -
 drivers/input/touchscreen/pixcir_i2c_ts.c   | 1 -
 drivers/input/touchscreen/s3c2410_ts.c  | 1 -
 drivers/input/touchscreen/ti_am335x_tsc.c   | 1 -
 drivers/input/touchscreen/tnetv107x-ts.c| 1 -
 68 files changed, 14 insertions(+), 103 deletions(-)

diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
index 005d852..3b9c709 100644
--- a/drivers/input/joystick/as5011.c
+++ b/drivers/input/joystick/as5011.c
@@ -254,8 +254,6 @@ static int as5011_probe(struct i2c_client *client,
as5011 = kmalloc(sizeof(struct as5011_device), GFP_KERNEL);
input_dev = input_allocate_device();
if (!as5011 || !input_dev) {
-   dev_err(client-dev,
-   Can't allocate memory for device structure\n);
error = -ENOMEM;
goto err_free_mem;
}
diff --git a/drivers/input/joystick/db9.c b/drivers/input/joystick/db9.c
index 8e7de5c..b0f18f2 100644
--- a/drivers/input/joystick/db9.c
+++ b/drivers/input/joystick/db9.c
@@ -609,7 +609,6 @@ static struct db9 __init *db9_probe(int parport, int mode)
 
db9-dev[i] = input_dev = input_allocate_device();
if (!input_dev) {
-   printk(KERN_ERR db9.c: Not enough memory for input 
device\n);
err = -ENOMEM;
goto err_unreg_devs;
}
diff --git a/drivers/input/joystick/gamecon.c b/drivers/input/joystick/gamecon.c
index e68e497..ded4f23 100644
---