Patchwork [4/9] pwm: renesas-tpu: Remove unnecessary OOM messages

login
register
mail settings
Submitter Jingoo Han
Date Feb. 26, 2014, 1:08 a.m.
Message ID <000c01cf328f$376b8860$a6429920$%han@samsung.com>
Download mbox | patch
Permalink /patch/324157/
State Accepted
Headers show

Comments

Jingoo Han - Feb. 26, 2014, 1:08 a.m.
The site-specific OOM messages are unnecessary, because they
duplicate the MM subsystem generic OOM message.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/pwm/pwm-renesas-tpu.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
Laurent Pinchart - Feb. 26, 2014, 2:22 a.m.
Hi Jingoo,

Thank you for the patch.

On Wednesday 26 February 2014 10:08:10 Jingoo Han wrote:
> The site-specific OOM messages are unnecessary, because they
> duplicate the MM subsystem generic OOM message.

While an allocation failure for such a small piece of memory will mean that 
we're in trouble far too big for an error message to matter, have you made 
sure that all devm_kzalloc() error paths lead to an OOM message being printed 
?

> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  drivers/pwm/pwm-renesas-tpu.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
> index aff6ba9..cc13ff4 100644
> --- a/drivers/pwm/pwm-renesas-tpu.c
> +++ b/drivers/pwm/pwm-renesas-tpu.c
> @@ -405,10 +405,8 @@ static int tpu_probe(struct platform_device *pdev)
>  	int ret;
> 
>  	tpu = devm_kzalloc(&pdev->dev, sizeof(*tpu), GFP_KERNEL);
> -	if (tpu == NULL) {
> -		dev_err(&pdev->dev, "failed to allocate driver data\n");
> +	if (tpu == NULL)
>  		return -ENOMEM;
> -	}
> 
>  	spin_lock_init(&tpu->lock);
>  	tpu->pdev = pdev;
Jingoo Han - Feb. 26, 2014, 2:34 a.m.
On Wednesday, February 26, 2014 11:22 AM, Laurent Pinchart wrote:
> On Wednesday 26 February 2014 10:08:10 Jingoo Han wrote:
> > The site-specific OOM messages are unnecessary, because they
> > duplicate the MM subsystem generic OOM message.
> 
> While an allocation failure for such a small piece of memory will mean that
> we're in trouble far too big for an error message to matter, have you made
> sure that all devm_kzalloc() error paths lead to an OOM message being printed
> ?

(+cc Joe Perches, Andrew Morton)

Hi Laurent Pinchart,
Long time no see! Thank you for your comment.
I am not sure that I understand your question exactly.
I believe that all devm_kzalloc() error paths lead to
an OOM message being printed, because k.alloc and v.alloc
failures use dump_stack().

Joe Perches,
Would you add some comments on this?
If I am wrong, please let me know.

Best regards,
Jingoo Han

> 
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> >  drivers/pwm/pwm-renesas-tpu.c |    4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
> > index aff6ba9..cc13ff4 100644
> > --- a/drivers/pwm/pwm-renesas-tpu.c
> > +++ b/drivers/pwm/pwm-renesas-tpu.c
> > @@ -405,10 +405,8 @@ static int tpu_probe(struct platform_device *pdev)
> >  	int ret;
> >
> >  	tpu = devm_kzalloc(&pdev->dev, sizeof(*tpu), GFP_KERNEL);
> > -	if (tpu == NULL) {
> > -		dev_err(&pdev->dev, "failed to allocate driver data\n");
> > +	if (tpu == NULL)
> >  		return -ENOMEM;
> > -	}
> >
> >  	spin_lock_init(&tpu->lock);
> >  	tpu->pdev = pdev;

--
To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Pinchart - Feb. 26, 2014, 10:02 p.m.
Hi Jingoo,

On Wednesday 26 February 2014 11:34:54 Jingoo Han wrote:
> On Wednesday, February 26, 2014 11:22 AM, Laurent Pinchart wrote:
> > On Wednesday 26 February 2014 10:08:10 Jingoo Han wrote:
> > > The site-specific OOM messages are unnecessary, because they
> > > duplicate the MM subsystem generic OOM message.
> > 
> > While an allocation failure for such a small piece of memory will mean
> > that we're in trouble far too big for an error message to matter, have you
> > made sure that all devm_kzalloc() error paths lead to an OOM message being
> > printed ?
> 
> (+cc Joe Perches, Andrew Morton)
> 
> Hi Laurent Pinchart,
> Long time no see! Thank you for your comment.

You're welcome.

> I am not sure that I understand your question exactly.
> I believe that all devm_kzalloc() error paths lead to
> an OOM message being printed, because k.alloc and v.alloc
> failures use dump_stack().

There's so many error code paths in the slab allocator that I got a bit lost, 
I was just wondering if all of them ended up with a message being printed. 
It's very probably needless worrying from my side.

> Joe Perches,
> Would you add some comments on this?
> If I am wrong, please let me know.
> 
> Best regards,
> Jingoo Han
> 
> > > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > > ---
> > > 
> > >  drivers/pwm/pwm-renesas-tpu.c |    4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/pwm/pwm-renesas-tpu.c
> > > b/drivers/pwm/pwm-renesas-tpu.c
> > > index aff6ba9..cc13ff4 100644
> > > --- a/drivers/pwm/pwm-renesas-tpu.c
> > > +++ b/drivers/pwm/pwm-renesas-tpu.c
> > > @@ -405,10 +405,8 @@ static int tpu_probe(struct platform_device *pdev)
> > >  	int ret;
> > >  	
> > >  	tpu = devm_kzalloc(&pdev->dev, sizeof(*tpu), GFP_KERNEL);
> > > -	if (tpu == NULL) {
> > > -		dev_err(&pdev->dev, "failed to allocate driver data\n");
> > > +	if (tpu == NULL)
> > >  		return -ENOMEM;
> > > -	}
> > > 
> > >  	spin_lock_init(&tpu->lock);
> > >  	tpu->pdev = pdev;

Patch

diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
index aff6ba9..cc13ff4 100644
--- a/drivers/pwm/pwm-renesas-tpu.c
+++ b/drivers/pwm/pwm-renesas-tpu.c
@@ -405,10 +405,8 @@  static int tpu_probe(struct platform_device *pdev)
 	int ret;
 
 	tpu = devm_kzalloc(&pdev->dev, sizeof(*tpu), GFP_KERNEL);
-	if (tpu == NULL) {
-		dev_err(&pdev->dev, "failed to allocate driver data\n");
+	if (tpu == NULL)
 		return -ENOMEM;
-	}
 
 	spin_lock_init(&tpu->lock);
 	tpu->pdev = pdev;