Skip to contentSkip to conversation

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

PATCH In review Xiaoming Zhang 2 participants 4 messages Apr 23, 2012 – Apr 24, 2012

Subsystems (2)
Memory technology devices (MTD) Unsorted block images (UBI)
lore
Share & tools
lore

Xiaoming Zhang

author
#0 lore raw

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

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 <[email protected]>
drivers/mtd/ubi/io.c +5 -5
@@ -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;
 	}
Quoted 25 lines · Artem Bityutskiy
Quoted 10 lines · Mark

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
...

Quoted 3 lines

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

#1 lore raw
Quoted 5 lines · 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:

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!

Signature · 2 lines
Best Regards,
Artem Bityutskiy
author
#2 lore raw

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

In reply to Xiaoming Zhang

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 <[email protected]>
---
 drivers/mtd/ubi/io.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
drivers/mtd/ubi/io.c +5 -5
index 43f1a00..4a866db 100644
@@ -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;
 	}
#3 lore raw
Quoted 4 lines · 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 <[email protected]>

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.

Signature · 2 lines
Best Regards,
Artem Bityutskiy