| Message ID | 000c01cf328f$376b8860$a6429920$%han@samsung.com |
|---|---|
| State | Accepted |
| Headers | show |
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;
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
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;
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;
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(-)