Re: [PATCH] Fix oops on reboot in physmap

Re: [PATCH] Fix oops on reboot in physmap

[Jörn Engel]

On Thu, 13 March 2008 09:42:42 +0100, Arnaud Patard wrote:
> 
> I sent this patch to David Woodhouse, but I forgot to put the list in
> CC: (and I'm not even sure that I used the right email address :( ). So,
> I'm sending the patch here so everyone can have my fix.

There were various small errors in it, afaics.  Does this patch achieve
the same result for you?

Added dwmw2 on Cc:.  This could be material for -stable.

Jörn

-- 
Prosperity makes friends, adversity tries them.
-- Publilius Syrus

Commit df66e7167ac756baf14d2b8ea7a2cfa056600a93 is adding support for multiple
resources in physmap. On shutdown/supend/resume, it's suspending all resources
by calling mtd[]->suspend() without checking the mtd[] is not null.
This makes oopsing my kernel.

Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
Signed-Off-By: Joern Engel <joern@logfs.org>

diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c
index f00e04e..1abecd5 100644
--- a/drivers/mtd/maps/physmap.c
+++ b/drivers/mtd/maps/physmap.c
@@ -204,7 +204,8 @@ static int physmap_flash_suspend(struct platform_device *dev, pm_message_t state
 
 	if (info)
 		for (i = 0; i < MAX_RESOURCES; i++)
-			ret |= info->mtd[i]->suspend(info->mtd[i]);
+			if (info->mtd[i])
+				ret |= info->mtd[i]->suspend(info->mtd[i]);
 
 	return ret;
 }
@@ -216,7 +217,8 @@ static int physmap_flash_resume(struct platform_device *dev)
 
 	if (info)
 		for (i = 0; i < MAX_RESOURCES; i++)
-			info->mtd[i]->resume(info->mtd[i]);
+			if (info->mtd[i])
+				info->mtd[i]->resume(info->mtd[i]);
 	return 0;
 }
 
@@ -225,9 +227,10 @@ static void physmap_flash_shutdown(struct platform_device *dev)
 	struct physmap_flash_info *info = platform_get_drvdata(dev);
 	int i;
 
-	for (i = 0; i < MAX_RESOURCES; i++)
-		if (info && info->mtd[i]->suspend(info->mtd[i]) == 0)
-			info->mtd[i]->resume(info->mtd[i]);
+	if (info)
+		for (i = 0; i < MAX_RESOURCES; i++)
+			if (info->mtd[i] && info->mtd[i]->suspend(info->mtd[i]) == 0)
+				info->mtd[i]->resume(info->mtd[i]);
 }
 #else
 #define physmap_flash_suspend NULL

[PATCH] Fix oops on reboot in physmap

[Arnaud Patard]

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

Hi,

I sent this patch to David Woodhouse, but I forgot to put the list in
CC: (and I'm not even sure that I used the right email address :( ). So,
I'm sending the patch here so everyone can have my fix.


Thanks,
Arnaud


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: physmap_change.patch --]
[-- Type: text/x-diff, Size: 1538 bytes --]

Commit df66e7167ac756baf14d2b8ea7a2cfa056600a93 is adding support for multiple 
resources in physmap. On shutdown/supend/resume, it's suspending all resources
by calling mtd[]->suspend() without checking the mtd[] is not null.
This makes oopsing my kernel.


Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
---
 arch/arm/mach-iop32x/em7210.c |   10 	6 +	4 -	0 !
 drivers/mtd/maps/physmap.c    |    8 	5 +	3 -	0 !
 2 files changed, 11 insertions(+), 7 deletions(-)

Index: linux-2.6/drivers/mtd/maps/physmap.c
===================================================================
--- linux-2.6.orig/drivers/mtd/maps/physmap.c	2008-03-02 15:37:20.000000000 +0100
+++ linux-2.6/drivers/mtd/maps/physmap.c	2008-03-02 15:43:18.000000000 +0100
@@ -204,7 +204,8 @@ static int physmap_flash_suspend(struct 
 
 	if (info)
 		for (i = 0; i < MAX_RESOURCES; i++)
-			ret |= info->mtd[i]->suspend(info->mtd[i]);
+			if (info && info->mtd[i])
+				ret |= info->mtd[i]->suspend(info->mtd[i]);
 
 	return ret;
 }
@@ -216,7 +217,8 @@ static int physmap_flash_resume(struct p
 
 	if (info)
 		for (i = 0; i < MAX_RESOURCES; i++)
-			info->mtd[i]->resume(info->mtd[i]);
+			if (info && info->mtd[i])
+				info->mtd[i]->resume(info->mtd[i]);
 	return 0;
 }
 
@@ -226,7 +228,7 @@ static void physmap_flash_shutdown(struc
 	int i;
 
 	for (i = 0; i < MAX_RESOURCES; i++)
-		if (info && info->mtd[i]->suspend(info->mtd[i]) == 0)
+		if (info && info->mtd[i] &&  info->mtd[i]->suspend(info->mtd[i]) == 0)
 			info->mtd[i]->resume(info->mtd[i]);
 }
 #else

Re: [PATCH] Fix oops on reboot in physmap

[Jörn Engel]

On Thu, 13 March 2008 11:10:37 +0100, Arnaud Patard wrote:
> >
> > Added dwmw2 on Cc:.  This could be material for -stable.
> 
> -stable ? why ? iirc, the faulty commit has been merged for
> 25-rc..something so 2.6.24 is not affected. Are you talking about 2.6.25?

No, I simply didn't check when the commit happened.  But yes, this is
.25 material.

Jörn

-- 
He that composes himself is wiser than he that composes a book.
-- B. Franklin

Re: [PATCH] Fix oops on reboot in physmap

[Arnaud Patard]

Jörn Engel <joern@logfs.org> writes:

Hi,

> On Thu, 13 March 2008 09:42:42 +0100, Arnaud Patard wrote:
>> 
>> I sent this patch to David Woodhouse, but I forgot to put the list in
>> CC: (and I'm not even sure that I used the right email address :( ). So,
>> I'm sending the patch here so everyone can have my fix.
>
> There were various small errors in it, afaics.  Does this patch achieve
> the same result for you?

>From what I can see, this patch is fine. I can't test it now but will do asap.

>
> Added dwmw2 on Cc:.  This could be material for -stable.

-stable ? why ? iirc, the faulty commit has been merged for
25-rc..something so 2.6.24 is not affected. Are you talking about 2.6.25?

Arnaud