[PATCH] ubi: Do "dump_stack()" in case of the ubi_io_read()/ubi_io_write()/do_sync_erase() has an error

[PATCH] ubi: Do "dump_stack()" in case of the ubi_io_read()/ubi_io_write()/do_sync_erase() has an error

[Xiaoming Zhang]

Hi Artem,

As you suggested there's a patch to turn "ubi_dbg_dump_stack()" into
"dump_stack()".
This is generated and tested on Linux-2.6.31/mips. The test result:

...
UBI error: ubi_io_read: error -77 while reading 2048 bytes from PEB
0:2048, read 2048 bytes
Call Trace:
[<80016fac>] dump_stack+0x8/0x34
[<80322b70>] ubi_io_read+0x158/0x1ac
[<80322c14>] ubi_io_read_vid_hdr+0x50/0x494
[<80324b04>] wear_leveling_worker+0x14c/0x634
[<80325448>] do_work+0x9c/0x134
[<80325660>] ubi_thread+0x180/0x204
[<8006934c>] kthread+0x84/0x8c
[<80021534>] kernel_thread_helper+0x10/0x18
...

The patch is listed below. Thank you.


ubi: Do "dump_stack()" to always have the dump in case of the
ubi_io_read()/ubi_io_write()/do_sync_erase() has an error.

Signed-off-by: Mark Zhang <mark.zhang@motorola.com>

--- linux-2.6.31.orig/drivers/mtd/ubi/io.c	2012-04-23 08:58:24.000000000 +0800
+++ linux-2.6.31/drivers/mtd/ubi/io.c	2012-04-23 09:01:51.000000000 +0800
@@ -173,7 +173,7 @@ retry:

 		ubi_err("error %d while reading %d bytes from PEB %d:%d, "
 			"read %zd bytes", err, len, pnum, offset, read);
-		ubi_dbg_dump_stack();
+		dump_stack();

 		/*
 		 * The driver should never return -EBADMSG if it failed to read
@@ -259,7 +259,7 @@ int ubi_io_write(struct ubi_device *ubi,
 	if (ubi_dbg_is_write_failure()) {
 		dbg_err("cannot write %d bytes to PEB %d:%d "
 			"(emulated)", len, pnum, offset);
-		ubi_dbg_dump_stack();
+		dump_stack();
 		return -EIO;
 	}

@@ -268,7 +268,7 @@ int ubi_io_write(struct ubi_device *ubi,
 	if (err) {
 		ubi_err("error %d while writing %d bytes to PEB %d:%d, written "
 			"%zd bytes", err, len, pnum, offset, written);
-		ubi_dbg_dump_stack();
+		dump_stack();
 	} else
 		ubi_assert(written == len);

@@ -323,7 +323,7 @@ retry:
 			goto retry;
 		}
 		ubi_err("cannot erase PEB %d, error %d", pnum, err);
-		ubi_dbg_dump_stack();
+		dump_stack();
 		return err;
 	}

@@ -341,7 +341,7 @@ retry:
 			goto retry;
 		}
 		ubi_err("cannot erase PEB %d", pnum);
-		ubi_dbg_dump_stack();
+		dump_stack();
 		return -EIO;
 	}



On Sun, Apr 22, 2012 at 10:37 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Sun, 2012-04-15 at 21:49 +0800, Mark wrote:
>> And, after I patched the nand driver, in most of the situations the
>> UBIFS can be successfully recovered, but I still get this error:
>>
>> ...
>> UBI error: ubi_io_read: error -77 while reading 126976 bytes from PEB
>> 2:4096, read 126976 bytes
>> UBIFS error (pid 481): insert_node: duplicate sqnum in replay
>> mount: mounting ubi13_0 on /usr/local/hmt/appdata failed: Invalid argument
>> ...
>>
>> Does this mean this should never happen to UBIFS? Then what could be the
>> reason?
>> Thank you very much, and my test environment:
>
> I see ubi_dbg_dump_stack() call in ubi_io_read() when it returns an
> error. If you have debugging enabled - you should have the dump in your
> kernel buffer.
>
> Yeah, we should turn it into 'dump_stack()' to always have the dump. I'd
> gratefully accept a patch.
>
> --
> Best Regards,
> Artem Bityutskiy

Re: [PATCH] ubi: Do "dump_stack()" in case of the ubi_io_read()/ubi_io_write()/do_sync_erase() has an error

[Artem Bityutskiy]

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

On Mon, 2012-04-23 at 10:37 +0800, Xiaoming Zhang wrote:
> Hi Artem,
> 
> As you suggested there's a patch to turn "ubi_dbg_dump_stack()" into
> "dump_stack()".
> This is generated and tested on Linux-2.6.31/mips. The test result:

Hi,

thanks, but would you please be kind enough to re-send it in a form
which I can just save and feed to git am. This e-mail has junk text
after the patch, and "Hi Artem" will need to be stripped. Thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] ubi: Do "dump_stack()" in case of the ubi_io_read()/ubi_io_write()/do_sync_erase() has an error

[Mark]

ubi: Do "dump_stack()" to always have the dump in case of the
ubi_io_read()/ubi_io_write()/do_sync_erase() has an error.

Signed-off-by: Mark Zhang <mark.zhang@motorola.com>
---
  drivers/mtd/ubi/io.c |   10 +++++-----
  1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index 43f1a00..4a866db 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -198,7 +198,7 @@ retry:

  		ubi_err("error %d%s while reading %d bytes from PEB %d:%d, "
  			"read %zd bytes", err, errstr, len, pnum, offset, read);
-		ubi_dbg_dump_stack();
+		dump_stack();

  		/*
  		 * The driver should never return -EBADMSG if it failed to read
@@ -284,7 +284,7 @@ int ubi_io_write(struct ubi_device *ubi, const void 
*buf, int pnum, int offset,
  	if (ubi_dbg_is_write_failure(ubi)) {
  		dbg_err("cannot write %d bytes to PEB %d:%d "
  			"(emulated)", len, pnum, offset);
-		ubi_dbg_dump_stack();
+		dump_stack();
  		return -EIO;
  	}

@@ -293,7 +293,7 @@ int ubi_io_write(struct ubi_device *ubi, const void 
*buf, int pnum, int offset,
  	if (err) {
  		ubi_err("error %d while writing %d bytes to PEB %d:%d, written "
  			"%zd bytes", err, len, pnum, offset, written);
-		ubi_dbg_dump_stack();
+		dump_stack();
  		ubi_dbg_dump_flash(ubi, pnum, offset, len);
  	} else
  		ubi_assert(written == len);
@@ -370,7 +370,7 @@ retry:
  			goto retry;
  		}
  		ubi_err("cannot erase PEB %d, error %d", pnum, err);
-		ubi_dbg_dump_stack();
+		dump_stack();
  		return err;
  	}

@@ -388,7 +388,7 @@ retry:
  			goto retry;
  		}
  		ubi_err("cannot erase PEB %d", pnum);
-		ubi_dbg_dump_stack();
+		dump_stack();
  		return -EIO;
  	}

Re: [PATCH] ubi: Do "dump_stack()" in case of the ubi_io_read()/ubi_io_write()/do_sync_erase() has an error

[Artem Bityutskiy]

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

On Mon, 2012-04-23 at 20:31 +0800, Mark wrote:
> ubi: Do "dump_stack()" to always have the dump in case of the
> ubi_io_read()/ubi_io_write()/do_sync_erase() has an error.
> 
> Signed-off-by: Mark Zhang <mark.zhang@motorola.com>

Mark, sorry, but this patch is not applicable because it is
1) line-wrapped
2) against some old UBI version.

To check what I mean - save it from the mailing list as mbox and try to
apply it using git am to the upstream kernel.

Also, keep in mind that the subject line should be short and only give
idea what the patch does without details. In this case I'd prefer "UBI:
dump the stack in case of I/O errors".

BTW, I suggest you to use newer kernel or pull one of the ubifs
back-port trees which also contains all the UBI back-ports.

Anyway, I'll do this change myself now.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]