[PATCH 2/2] trivial: Changed the printk loglevel when not able to allocate memory

[PATCH 2/2] trivial: Changed the printk loglevel when not able to allocate memory

[anish]

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

When not able to allocate memory we were using KERN_INFO as 
log level in printk so changed to KERN_ERR
 Signed-off-by: anish kumar<anish198519851985@gmail.com>

---
 drivers/video/amba-clcd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c
index cb7ec86..e87d98b 100644
--- a/drivers/video/amba-clcd.c
+++ b/drivers/video/amba-clcd.c
@@ -551,7 +551,7 @@ static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id)
 
 	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
 	if (!fb) {
-		printk(KERN_INFO "CLCD: could not allocate new clcd_fb struct\n");
+		printk(KERN_ERR "CLCD: could not allocate new clcd_fb struct\n");
 		ret = -ENOMEM;
 		goto free_region;
 	}
-- 
1.7.0.4



[-- Attachment #2: 0002-Changed-the-printk-loglevel-when-not-able-to-allocat.patch --]
[-- Type: text/x-patch, Size: 984 bytes --]

>From 19bbb666a6067a6ba280e2cb984c924bcaf06f52 Mon Sep 17 00:00:00 2001
From: anish kumar <anish198519851985@gmail.com>
Date: Tue, 17 May 2011 22:50:59 +0530
Subject: [PATCH 2/2] Changed the printk loglevel when not able to allocate memory
 When not able to allocate memory we were using KERN_INFO so just
 changed to KERN_ERR
 Signed-off-by: anish kumar<anish198519851985@gmail.com>

---
 drivers/video/amba-clcd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c
index cb7ec86..e87d98b 100644
--- a/drivers/video/amba-clcd.c
+++ b/drivers/video/amba-clcd.c
@@ -551,7 +551,7 @@ static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id)
 
 	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
 	if (!fb) {
-		printk(KERN_INFO "CLCD: could not allocate new clcd_fb struct\n");
+		printk(KERN_ERR "CLCD: could not allocate new clcd_fb struct\n");
 		ret = -ENOMEM;
 		goto free_region;
 	}
-- 
1.7.0.4

Re: [PATCH 2/2] trivial: Changed the printk loglevel when not able to allocate memory

[Catalin Marinas]

On 17 May 2011 18:29, anish <anish198519851985@gmail.com> wrote:
> When not able to allocate memory we were using KERN_INFO as
> log level in printk so changed to KERN_ERR
>  Signed-off-by: anish kumar<anish198519851985@gmail.com>

Maybe KERN_WARN?

-- 
Catalin

Re: [PATCH 2/2] trivial: Changed the printk loglevel when not able to allocate memory

[Catalin Marinas]

On Wed, 2011-05-18 at 04:19 +0100, anish singh wrote:
> On Tue, May 17, 2011 at 11:13 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>         On 17 May 2011 18:29, anish <anish198519851985@gmail.com> wrote:
>         > When not able to allocate memory we were using KERN_INFO as
>         > log level in printk so changed to KERN_ERR
>         >  Signed-off-by: anish kumar<anish198519851985@gmail.com>
>         
>         
>         Maybe KERN_WARN?
> Then shouldn't we change below case also?
> 467         ret = amba_request_regions(dev, NULL);
> 468         if (ret) {
> 469                 printk(KERN_ERR "CLCD: unable to reserve regs region\n");
> 470                 goto out;
> 
> If yes,then i will resend the patch for this also.

I think the register reserving is less likely to fail because of memory
allocations but more because of overlapping regions, in which case it
could be a programming error.

Allocating a big framebuffer is likely to fail in some memory constraint
systems but I don't consider this a kernel error. That's why I suggested
warning.

-- 
Catalin

Re: [PATCH 2/2] trivial: Changed the printk loglevel when not able to allocate memory

[Catalin Marinas]

On Wed, 2011-05-18 at 11:57 +0100, anish singh wrote:
> On Wed, May 18, 2011 at 3:45 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>         
>         On Wed, 2011-05-18 at 04:19 +0100, anish singh wrote:
>         > On Tue, May 17, 2011 at 11:13 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>         >         On 17 May 2011 18:29, anish <anish198519851985@gmail.com> wrote:
>         >         > When not able to allocate memory we were using KERN_INFO as
>         >         > log level in printk so changed to KERN_ERR
>         >         >  Signed-off-by: anish kumar<anish198519851985@gmail.com>
>         >
>         >
>         >         Maybe KERN_WARN?
>         > Then shouldn't we change below case also?
>         > 467         ret = amba_request_regions(dev, NULL);
>         > 468         if (ret) {
>         > 469                 printk(KERN_ERR "CLCD: unable to reserve regs region\n");
>         > 470                 goto out;
>         >
>         > If yes,then i will resend the patch for this also.
>         
>         
>         I think the register reserving is less likely to fail because of memory
>         allocations but more because of overlapping regions, in which case it
>         could be a programming error.
>         
>         Allocating a big framebuffer is likely to fail in some memory constraint
>         systems but I don't consider this a kernel error. That's why I suggested
>         warning.
>  
> Got it.So should i change both as KERN_WARN and resend?

No, just the one KERN_INFO one. Anyway, Russell King is the maintainer
of this driver so it's up to him to ack the patch (and don't forget to
cc him).

-- 
Catalin

Re: [PATCH 2/2] trivial: Changed the printk loglevel when not able to allocate memory

[Russell King - ARM Linux]

On Wed, May 18, 2011 at 04:27:47PM +0530, anish singh wrote:
> Got it.So should i change both as KERN_WARN and resend?

Much better would be to change it to pr_warn() or even
dev_warn(&dev->dev, ...).

Re: [PATCH 2/2] trivial: Changed the printk loglevel when not able to allocate memory

[Russell King - ARM Linux]

On Wed, May 18, 2011 at 12:24:36PM +0100, Russell King - ARM Linux wrote:
> On Wed, May 18, 2011 at 04:27:47PM +0530, anish singh wrote:
> > Got it.So should i change both as KERN_WARN and resend?
> 
> Much better would be to change it to pr_warn() or even
> dev_warn(&dev->dev, ...).

Actually, make that pr_err() or dev_err().  Both printks in clcdfb_probe()
are errors and so should be reported at error severity - they cause things
to stop working and so they're not just a warning.