Skip to contentSkip to conversation

Remove #define hex_asc from kernel.h, update lib/hexdump.c

PATCH In review Joe Perches 5 participants 16 messages Nov 29, 2007 – Dec 6, 2007

Subsystems (11)
Networking Network drivers SCSI USB +7 more subsystems
lore
Share & tools
lore

Joe Perches

author
#0 lore raw

PATCH 1/1 Remove #define hex_asc from kernel.h, update lib/hexdump.c

It's only use is in lib/hexdump.c
Instead, use scnprintf in lib/hexdump.c

Use "%0nx" not "%n.nx" (Smaller kernel, one byte at a time, woohoo!) Use true not 1 for print_hex_dump
A bit of indentation style in lib/hexdump.c

Size before/after
text data bss dec hex filename
1037 0 0 1037 40d lib/hexdump.o

text data bss dec hex filename
1019 0 0 1019 3fb lib/hexdump.o

Signed-off-by Joe Perches <[email protected]>

---

 include/linux/kernel.h |    1 -
 lib/hexdump.c          |   23 ++++++++++-------------
 2 files changed, 10 insertions(+), 14 deletions(-)
include/linux/kernel.h +0 -1
index 94bc996..77b187f 100644
@@ -256,7 +256,6 @@ extern void print_hex_dump(const char *level, const char *prefix_str,
 				const void *buf, size_t len, bool ascii);
 extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 			const void *buf, size_t len);
-#define hex_asc(x)	"0123456789abcdef"[x]
 
 #define pr_emerg(fmt, arg...) \
 	printk(KERN_EMERG fmt, ##arg)
lib/hexdump.c +10 -13
index bd5edae..70e23fb 100644
@@ -41,7 +41,6 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 			bool ascii)
 {
 	const u8 *ptr = buf;
-	u8 ch;
 	int j, lx = 0;
 	int ascii_column;
 

@@ -62,7 +61,9 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
-				"%16.16llx ", (unsigned long long)*(ptr8 + j));
+					"%016llx ",
+					(unsigned long long)*(ptr8 + j) &
+					0xffffffffffffffffULL);
 		ascii_column = 17 * ngroups + 2;
 		break;
 	}

@@ -73,7 +74,7 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
-				"%8.8x ", *(ptr4 + j));
+					"%08x ", *(ptr4 + j) & 0xffffffffU);
 		ascii_column = 9 * ngroups + 2;
 		break;
 	}

@@ -84,19 +85,15 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
-				"%4.4x ", *(ptr2 + j));
+					"%04x ", *(ptr2 + j) & 0xffff);
 		ascii_column = 5 * ngroups + 2;
 		break;
 	}
 
 	default:
-		for (j = 0; (j < rowsize) && (j < len) && (lx + 4) < linebuflen;
-		     j++) {
-			ch = ptr[j];
-			linebuf[lx++] = hex_asc(ch >> 4);
-			linebuf[lx++] = hex_asc(ch & 0x0f);
-			linebuf[lx++] = ' ';
-		}
+		for (j = 0; (j < rowsize) && (j < len) < linebuflen; j++)
+			lx += scnprintf(linebuf + lx, linebuflen - lx,
+					"%02x ", ptr[j] & 0xff);
 		ascii_column = 3 * rowsize + 2;
 		break;
 	}

@@ -163,7 +160,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 		switch (prefix_type) {
 		case DUMP_PREFIX_ADDRESS:
 			printk("%s%s%*p: %s\n", level, prefix_str,
-				(int)(2 * sizeof(void *)), ptr + i, linebuf);
+			       (int)(2 * sizeof(void *)), ptr + i, linebuf);
 			break;
 		case DUMP_PREFIX_OFFSET:
 			printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);

@@ -192,6 +189,6 @@ void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 			const void *buf, size_t len)
 {
 	print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, 16, 1,
-			buf, len, 1);
+		       buf, len, true);
 }
 EXPORT_SYMBOL(print_hex_dump_bytes);
#1 lore raw
Quoted 6 lines · On Thu, 29 Nov 2007 10:08:44 -0800 Joe Perches wrote:

It's only use is in lib/hexdump.c
Instead, use scnprintf in lib/hexdump.c

Use "%0nx" not "%n.nx" (Smaller kernel, one byte at a time, woohoo!) Use true not 1 for print_hex_dump
A bit of indentation style in lib/hexdump.c

I don't care for the mixed spaces/tabs.

Quoted 68 lines

Size before/after
text data bss dec hex filename
1037 0 0 1037 40d lib/hexdump.o

text data bss dec hex filename
1019 0 0 1019 3fb lib/hexdump.o

Signed-off-by Joe Perches <[email protected]>

---

 include/linux/kernel.h |    1 -
 lib/hexdump.c          |   23 ++++++++++-------------
 2 files changed, 10 insertions(+), 14 deletions(-)
include/linux/kernel.h +0 -1
index 94bc996..77b187f 100644
@@ -256,7 +256,6 @@ extern void print_hex_dump(const char *level, const char *prefix_str,
 				const void *buf, size_t len, bool ascii);
 extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 			const void *buf, size_t len);
-#define hex_asc(x)	"0123456789abcdef"[x]
 
 #define pr_emerg(fmt, arg...) \
 	printk(KERN_EMERG fmt, ##arg)
lib/hexdump.c +5 -4
index bd5edae..70e23fb 100644
@@ -41,7 +41,6 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 			bool ascii)
 {
 	const u8 *ptr = buf;
-	u8 ch;
 	int j, lx = 0;
 	int ascii_column;
 

@@ -62,7 +61,9 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
-				"%16.16llx ", (unsigned long long)*(ptr8 + j));
+					"%016llx ",
+					(unsigned long long)*(ptr8 + j) &
+					0xffffffffffffffffULL);
 		ascii_column = 17 * ngroups + 2;
 		break;
 	}

@@ -73,7 +74,7 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
-				"%8.8x ", *(ptr4 + j));
+					"%08x ", *(ptr4 + j) & 0xffffffffU);
 		ascii_column = 9 * ngroups + 2;
 		break;
 	}

@@ -84,19 +85,15 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
-				"%4.4x ", *(ptr2 + j));
+					"%04x ", *(ptr2 + j) & 0xffff);
 		ascii_column = 5 * ngroups + 2;
 		break;
 	}

lib/hexdump.c void hex_dump_to_buffer(const void *buf, size_t len, int rowsize, · line 94

Quoted 10 lines
lib/hexdump.c +1 -7
 
 	default:
-		for (j = 0; (j < rowsize) && (j < len) && (lx + 4) < linebuflen;
-		     j++) {
-			ch = ptr[j];
-			linebuf[lx++] = hex_asc(ch >> 4);
-			linebuf[lx++] = hex_asc(ch & 0x0f);
-			linebuf[lx++] = ' ';
-		}
+		for (j = 0; (j < rowsize) && (j < len) < linebuflen; j++)

What does

					(j < len) < linebuflen;

do? I don't see that C construct very often.

Quoted 15 lines
lib/hexdump.c +3 -1
+			lx += scnprintf(linebuf + lx, linebuflen - lx,
+					"%02x ", ptr[j] & 0xff);
 		ascii_column = 3 * rowsize + 2;
 		break;
 	}

@@ -163,7 +160,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 		switch (prefix_type) {
 		case DUMP_PREFIX_ADDRESS:
 			printk("%s%s%*p: %s\n", level, prefix_str,
-				(int)(2 * sizeof(void *)), ptr + i, linebuf);
+			       (int)(2 * sizeof(void *)), ptr + i, linebuf);
 			break;
 		case DUMP_PREFIX_OFFSET:
 			printk("%s%s%.8x: %s\n", level, prefix_str, i, linebuf);

lib/hexdump.c void print_hex_dump_bytes(const char *prefix_str, int prefix_type, · line 194

Quoted 7 lines
lib/hexdump.c +1 -1
@@ -192,6 +189,6 @@ void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 			const void *buf, size_t len)
 {
 	print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, 16, 1,
-			buf, len, 1);
+		       buf, len, true);
 }
 EXPORT_SYMBOL(print_hex_dump_bytes);

---
~Randy

author
#2 lore raw
Quoted 1 line · Randy Dunlap

I don't care for the mixed spaces/tabs.

I like myself, but I removed it just for you...

Quoted 1 line

What does (j < len) < linebuflen; do?

Nothing good. It's a silly mistake. I fixed it.

Size before/after:

   text    data     bss     dec     hex filename
   1037       0       0    1037     40d lib/hexdump.o
   1004       0       0    1004     3ec lib/hexdump.o

---

 include/linux/kernel.h |    1 -
 lib/hexdump.c          |   23 ++++++++++-------------
 2 files changed, 10 insertions(+), 14 deletions(-)
include/linux/kernel.h +0 -1
index 94bc996..77b187f 100644
@@ -256,7 +256,6 @@ extern void print_hex_dump(const char *level, const char *prefix_str,
 				const void *buf, size_t len, bool ascii);
 extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 			const void *buf, size_t len);
-#define hex_asc(x)	"0123456789abcdef"[x]
 
 #define pr_emerg(fmt, arg...) \
 	printk(KERN_EMERG fmt, ##arg)
lib/hexdump.c +9 -12
index bd5edae..125c137 100644
@@ -41,7 +41,6 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 			bool ascii)
 {
 	const u8 *ptr = buf;
-	u8 ch;
 	int j, lx = 0;
 	int ascii_column;
 

@@ -62,7 +61,9 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
-				"%16.16llx ", (unsigned long long)*(ptr8 + j));
+					"%016llx ",
+					(unsigned long long)*(ptr8 + j) &
+					0xffffffffffffffffULL);
 		ascii_column = 17 * ngroups + 2;
 		break;
 	}

@@ -73,7 +74,7 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
-				"%8.8x ", *(ptr4 + j));
+					"%08x ", *(ptr4 + j) & 0xffffffffU);
 		ascii_column = 9 * ngroups + 2;
 		break;
 	}

@@ -84,19 +85,15 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
-				"%4.4x ", *(ptr2 + j));
+					"%04x ", *(ptr2 + j) & 0xffff);
 		ascii_column = 5 * ngroups + 2;
 		break;
 	}
 
 	default:
-		for (j = 0; (j < rowsize) && (j < len) && (lx + 4) < linebuflen;
-		     j++) {
-			ch = ptr[j];
-			linebuf[lx++] = hex_asc(ch >> 4);
-			linebuf[lx++] = hex_asc(ch & 0x0f);
-			linebuf[lx++] = ' ';
-		}
+		for (j = 0; j < rowsize; j++)
+			lx += scnprintf(linebuf + lx, linebuflen - lx,
+					"%02x ", ptr[j] & 0xff);
 		ascii_column = 3 * rowsize + 2;
 		break;
 	}

@@ -192,6 +189,6 @@ void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 			const void *buf, size_t len)
 {
 	print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, 16, 1,
-			buf, len, 1);
+			buf, len, true);
 }
 EXPORT_SYMBOL(print_hex_dump_bytes);
#3 lore raw
Quoted 4 lines · Joe Perches
Quoted 1 line · Randy Dunlap

I don't care for the mixed spaces/tabs.

I like myself, but I removed it just for you...

Thanks. Acked-by: Randy Dunlap <[email protected]>

but needs an S-O-B:

Quoted 94 lines
Quoted 1 line

What does (j < len) < linebuflen; do?

Nothing good. It's a silly mistake. I fixed it.

Size before/after:

   text    data     bss     dec     hex filename
   1037       0       0    1037     40d lib/hexdump.o
   1004       0       0    1004     3ec lib/hexdump.o

---

 include/linux/kernel.h |    1 -
 lib/hexdump.c          |   23 ++++++++++-------------
 2 files changed, 10 insertions(+), 14 deletions(-)
include/linux/kernel.h +0 -1
index 94bc996..77b187f 100644
@@ -256,7 +256,6 @@ extern void print_hex_dump(const char *level, const char *prefix_str,
 				const void *buf, size_t len, bool ascii);
 extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 			const void *buf, size_t len);
-#define hex_asc(x)	"0123456789abcdef"[x]
 
 #define pr_emerg(fmt, arg...) \
 	printk(KERN_EMERG fmt, ##arg)
lib/hexdump.c +9 -12
index bd5edae..125c137 100644
@@ -41,7 +41,6 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 			bool ascii)
 {
 	const u8 *ptr = buf;
-	u8 ch;
 	int j, lx = 0;
 	int ascii_column;
 

@@ -62,7 +61,9 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
-				"%16.16llx ", (unsigned long long)*(ptr8 + j));
+					"%016llx ",
+					(unsigned long long)*(ptr8 + j) &
+					0xffffffffffffffffULL);
 		ascii_column = 17 * ngroups + 2;
 		break;
 	}

@@ -73,7 +74,7 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
-				"%8.8x ", *(ptr4 + j));
+					"%08x ", *(ptr4 + j) & 0xffffffffU);
 		ascii_column = 9 * ngroups + 2;
 		break;
 	}

@@ -84,19 +85,15 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
-				"%4.4x ", *(ptr2 + j));
+					"%04x ", *(ptr2 + j) & 0xffff);
 		ascii_column = 5 * ngroups + 2;
 		break;
 	}
 
 	default:
-		for (j = 0; (j < rowsize) && (j < len) && (lx + 4) < linebuflen;
-		     j++) {
-			ch = ptr[j];
-			linebuf[lx++] = hex_asc(ch >> 4);
-			linebuf[lx++] = hex_asc(ch & 0x0f);
-			linebuf[lx++] = ' ';
-		}
+		for (j = 0; j < rowsize; j++)
+			lx += scnprintf(linebuf + lx, linebuflen - lx,
+					"%02x ", ptr[j] & 0xff);
 		ascii_column = 3 * rowsize + 2;
 		break;
 	}

@@ -192,6 +189,6 @@ void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 			const void *buf, size_t len)
 {
 	print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, 16, 1,
-			buf, len, 1);
+			buf, len, true);
 }
 EXPORT_SYMBOL(print_hex_dump_bytes);
Signature · 1 line
~Randy
author
#5 lore raw

PATCH 1/1 Reduce stack used by lib/hexdump.c

In reply to Randy Dunlap

200 bytes on stack might be a bit much.

Size goes up to
text data bss dec hex filename
1142 0 0 1142 476 lib/hexdump.o
Without the WARN_ON
1053 0 0 1053 41d lib/hexdump.o
Before this patch
1004 0 0 1004 3ec lib/hexdump.o

Win some/lose some...

Signed-off-by Joe Perches <[email protected]>
lib/hexdump.c +10 -1
index 70e23fb..be94934 100644
@@ -140,13 +140,20 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  * Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode:
  * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c  pqrstuvwxyz{|}~.
  */
+
+#define HEX_LINE_SIZE 200
+
 void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 			int rowsize, int groupsize,
 			const void *buf, size_t len, bool ascii)
 {
 	const u8 *ptr = buf;
 	int i, linelen, remaining = len;
-	unsigned char linebuf[200];
+	unsigned char *linebuf;
+
+	linebuf = kmalloc(HEX_LINE_SIZE, GFP_KERNEL);
+	if (!linebuf) {
+		WARN_ON(1);
+		return;
+	}

if (rowsize != 16 && rowsize != 32)
rowsize = 16;
@@ -155,7 +162,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
linelen = min(remaining, rowsize);
remaining -= rowsize;
hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
- linebuf, sizeof(linebuf), ascii);
+ linebuf, HEX_LINE_SIZE, ascii);

switch (prefix_type) {
case DUMP_PREFIX_ADDRESS:
@@ -170,6 +177,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
break;
}
}
+ kfree(linebuf);
}
EXPORT_SYMBOL(print_hex_dump);

Acked-by on v1 1/1
#6 lore raw

Re: [PATCH] Reduce stack used by lib/hexdump.c

Quoted 10 lines · Joe Perches

200 bytes on stack might be a bit much.

Size goes up to
text data bss dec hex filename
1142 0 0 1142 476 lib/hexdump.o
Without the WARN_ON
1053 0 0 1053 41d lib/hexdump.o
Before this patch
1004 0 0 1004 3ec lib/hexdump.o

Quoted 3 lines

Win some/lose some...

Signed-off-by Joe Perches <[email protected]>
Acked-by Randy Dunlap <[email protected]>
Quoted 44 lines
lib/hexdump.c +12 -2
index 70e23fb..be94934 100644
@@ -140,13 +140,20 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  * Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode:
  * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c  pqrstuvwxyz{|}~.
  */
+
+#define HEX_LINE_SIZE 200
+
 void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 			int rowsize, int groupsize,
 			const void *buf, size_t len, bool ascii)
 {
 	const u8 *ptr = buf;
 	int i, linelen, remaining = len;
-	unsigned char linebuf[200];
+	unsigned char *linebuf;
+
+	linebuf = kmalloc(HEX_LINE_SIZE, GFP_KERNEL);
+	if (!linebuf) {
+		WARN_ON(1);
+		return;
+	}
 
 	if (rowsize != 16 && rowsize != 32)
 		rowsize = 16;

@@ -155,7 +162,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 		linelen = min(remaining, rowsize);
 		remaining -= rowsize;
 		hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
-				linebuf, sizeof(linebuf), ascii);
+				linebuf, HEX_LINE_SIZE, ascii);
 
 		switch (prefix_type) {
 		case DUMP_PREFIX_ADDRESS:

@@ -170,6 +177,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 			break;
 		}
 	}
+	kfree(linebuf);
 }
 EXPORT_SYMBOL(print_hex_dump);
Signature · 1 line
~Randy
#7 lore raw

Re: [PATCH] Reduce stack used by lib/hexdump.c

Quoted 15 lines · Randy Dunlap
Quoted 15 lines
lib/hexdump.c +3 -1
@@ -140,13 +140,20 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  * Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode:
  * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c  pqrstuvwxyz{|}~.
  */
+
+#define HEX_LINE_SIZE 200
+
 void print_hex_dump(const char *level, const char *prefix_str, int
 prefix_type,
    int rowsize, int groupsize,
    const void *buf, size_t len, bool ascii)
 {
  const u8 *ptr = buf;
  int i, linelen, remaining = len;
-	unsigned char linebuf[200];
Quoted 8 lines

lib/hexdump.c EXPORT_SYMBOL(hex_dump_to_buffer); · line 158

Quoted 8 lines
lib/hexdump.c +7 -0
+	unsigned char *linebuf;
+
+	linebuf = kmalloc(HEX_LINE_SIZE, GFP_KERNEL);
+	if (!linebuf) {
+		WARN_ON(1);
+		return;
+	}

I'd add GFP_ATOMIC here. Who knows whether tomorrow, the oops dumper or warn_on will use print_hex_dump.

author
#8 lore raw

Re: [PATCH] Reduce stack used by lib/hexdump.c

Quoted 2 lines · Jan Engelhardt

I'd add GFP_ATOMIC here. Who knows whether tomorrow, the oops dumper or warn_on will use print_hex_dump.

Signed-off-by Joe Perches <[email protected]>
lib/hexdump.c +10 -1
index 70e23fb..be94934 100644
@@ -140,13 +140,20 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  * Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode:
  * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c  pqrstuvwxyz{|}~.
  */
+
+#define HEX_LINE_SIZE 200
+
 void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 			int rowsize, int groupsize,
 			const void *buf, size_t len, bool ascii)
 {
 	const u8 *ptr = buf;
 	int i, linelen, remaining = len;
-	unsigned char linebuf[200];
+	unsigned char *linebuf;
+
+	linebuf = kmalloc(HEX_LINE_SIZE, GFP_ATOMIC);
+	if (!linebuf) {
+		WARN_ON(1);
+		return;
+	}

if (rowsize != 16 && rowsize != 32)
rowsize = 16;
@@ -155,7 +162,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
linelen = min(remaining, rowsize);
remaining -= rowsize;
hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
- linebuf, sizeof(linebuf), ascii);
+ linebuf, HEX_LINE_SIZE, ascii);

switch (prefix_type) {
case DUMP_PREFIX_ADDRESS:
@@ -170,6 +177,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
break;
}
}
+ kfree(linebuf);
}
EXPORT_SYMBOL(print_hex_dump);

maintainer
#9 lore raw

Re: [PATCH] Reduce stack used by lib/hexdump.c

Quoted 21 lines · On Thu, 29 Nov 2007 15:28:42 -0800 Joe Perches <[email protected]> wrote:
Quoted 2 lines · Jan Engelhardt

I'd add GFP_ATOMIC here. Who knows whether tomorrow, the oops dumper or warn_on will use print_hex_dump.

Signed-off-by Joe Perches <[email protected]>
lib/hexdump.c +3 -0
index 70e23fb..be94934 100644
@@ -140,13 +140,20 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  * Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode:
  * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c  pqrstuvwxyz{|}~.
  */
+
+#define HEX_LINE_SIZE 200
+
 void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 			int rowsize, int groupsize,
 			const void *buf, size_t len, bool ascii)
 {

lib/hexdump.c EXPORT_SYMBOL(hex_dump_to_buffer); · line 158

Quoted 10 lines
lib/hexdump.c +7 -1
 	const u8 *ptr = buf;
 	int i, linelen, remaining = len;
-	unsigned char linebuf[200];
+	unsigned char *linebuf;
+
+	linebuf = kmalloc(HEX_LINE_SIZE, GFP_ATOMIC);
+	if (!linebuf) {
+		WARN_ON(1);
+		return;
+	}

No, I think print_hex_dump() is too low-level to be doing allocations. For example, one could easily choose to call print_hex_dump() at oops time, and then what happens if we oops in kmalloc() (as we often do...)?

You could trim linebuf[] to 80 chars or so. Extra points for making it very clear when someone tries to exceed that - strcpy(linebuf, "stop being stupid").

author
#10 lore raw

Re: [PATCH] Reduce stack used by lib/hexdump.c

Quoted 3 lines · Andrew Morton

You could trim linebuf[] to 80 chars or so. Extra points for making it very clear when someone tries to exceed that - strcpy(linebuf, "stop being stupid").

Maybe just eliminate the 16 or 32 byte width option and force it to only 16 byte widths.

That'd keep it down to ~ 100 bytes nicely.

Prefix + address + dump + ascii: 20 + 18 + 48 + 10.

#11 lore raw

Re: [PATCH] Reduce stack used by lib/hexdump.c

Quoted 11 lines · Joe Perches
Quoted 3 lines · Andrew Morton

You could trim linebuf[] to 80 chars or so. Extra points for making it very clear when someone tries to exceed that - strcpy(linebuf, "stop being stupid").

Maybe just eliminate the 16 or 32 byte width option and force it to only 16 byte widths.

That'd keep it down to ~ 100 bytes nicely.

Prefix + address + dump + ascii: 20 + 18 + 48 + 10.

Have you checked users (callers)? I'm pretty sure that one of the callers wanted 32 and that's why it's there.

Signature · 2 lines
~Randy
Features and documentation: http://lwn.net/Articles/260136/
author
#12 lore raw

Re: [PATCH] Reduce stack used by lib/hexdump.c

Quoted 5 lines · Randy Dunlap
Quoted 2 lines · Joe Perches

Maybe just eliminate the 16 or 32 byte width option and force it to only 16 byte widths.

Have you checked users (callers)? I'm pretty sure that one of the callers wanted 32 and that's why it's there.

I did. There is only 1 subsystem. That's easy to change.

drivers/mtd/ubi/debug.c:  print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1,
drivers/mtd/ubi/io.c:     print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1,

Long lines in the log file are not too easy to read anyway. Using 16 byte dumps per line instead of 32 isn't painful.

It gets rid of the allocation, reduces the argument count and makes the kernel smaller. I think it's all good.

Every current caller would have to change though.

#13 lore raw

Re: [PATCH] Reduce stack used by lib/hexdump.c

Quoted 7 lines · Joe Perches
Quoted 3 lines · Randy Dunlap
Quoted 1 line · Joe Perches

Maybe just eliminate the 16 or 32 byte width option and force it to only 16 byte widths.

Have you checked users (callers)? I'm pretty sure that one of the callers wanted 32 and that's why it's there.

I did. There is only 1 subsystem. That's easy to change.

Quoted 8 lines
drivers/mtd/ubi/debug.c:  print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1,
drivers/mtd/ubi/io.c:     print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1,

Long lines in the log file are not too easy to read anyway. Using 16 byte dumps per line instead of 32 isn't painful.

It gets rid of the allocation, reduces the argument count and makes the kernel smaller. I think it's all good.

Every current caller would have to change though.

Alternatively, since print_hex_dump is not a performance-critical path (and usually indicates an error/debug condition), you could probably just make a static "hexdump_lock" spinlock and spin_lock_irqsave()/spin_unlock_irqrestore(). It would always nest inside any other lock (except during crash, where we break locks already for printk()), and I doubt any of the callers would notice the serialization since they're already serialized on the printk buffer.

Cheers,
Kyle Moffett

maintainer
#14 lore raw

Re: [PATCH] Reduce stack used by lib/hexdump.c

Quoted 23 lines · On Thu, 6 Dec 2007 00:58:38 -0500 Kyle Moffett <[email protected]> wrote:
Quoted 14 lines · Joe Perches
Quoted 5 lines · Randy Dunlap
Quoted 2 lines · Joe Perches

Maybe just eliminate the 16 or 32 byte width option and force it to only 16 byte widths.

Have you checked users (callers)? I'm pretty sure that one of the callers wanted 32 and that's why it's there.

I did. There is only 1 subsystem. That's easy to change.

drivers/mtd/ubi/debug.c: print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1,
drivers/mtd/ubi/io.c: print_hex_dump(KERN_DEBUG, "",
DUMP_PREFIX_OFFSET, 32, 1,

Quoted 7 lines

Long lines in the log file are not too easy to read anyway. Using 16 byte dumps per line instead of 32 isn't painful.

It gets rid of the allocation, reduces the argument count and makes the kernel smaller. I think it's all good.

Every current caller would have to change though.

Quoted 8 lines

Alternatively, since print_hex_dump is not a performance-critical path (and usually indicates an error/debug condition), you could probably just make a static "hexdump_lock" spinlock and spin_lock_irqsave()/spin_unlock_irqrestore(). It would always nest inside any other lock (except during crash, where we break locks already for printk()), and I doubt any of the callers would notice the serialization since they're already serialized on the printk buffer.

Yup, that'd work.

author
#15 lore raw

Re: [PATCH] Reduce stack used by lib/hexdump.c

In reply to Andrew Morton
Quoted 7 lines · Andrew Morton

No, I think print_hex_dump() is too low-level to be doing allocations. For example, one could easily choose to call print_hex_dump() at oops time, and then what happens if we oops in kmalloc() (as we often do...)?

You could trim linebuf[] to 80 chars or so. Extra points for making it very clear when someone tries to exceed that - strcpy(linebuf, "stop being stupid").

No extra points, but here's a revised patch to hexdump against Linus' current:

hex_dump_to_buffer:
Removes casts to type for non-1 group sizes
Used by: fs/ext(3|4)super.c, fs/jfs
If someone really dislikes this change, please say so. I think casting to type in a hex dump odd, especially for mixed type structures.
If you want an array of type dumper, it probably shouldn't be called hex_dump_to_buffer.
Groups by arbitrary size

print_hex_dump:
Removes rowsize argument
Reduces linebuf stack use to ~120 bytes
prefix:25 + address:20 + data:48 + ascii:20)
Aligns multiline ascii output
Changes return to size_t, number of bytes actually output

include/linux/kernel.h
Removes hex_asc define
Updates hex_dump prototypes

The rest are trivial conversions to new argument list.

size before:
text data bss dec hex filename
1142 0 0 1142 476 lib/hexdump.o

size after:

   text    data     bss     dec     hex filename
    823       0       0     823     337 lib/hexdump.o
Signed-off-by Joe Perches <[email protected]>
---
 include/linux/kernel.h                      |   13 +-
 lib/hexdump.c                               |  164 ++++++++++++---------------
 drivers/mtd/ubi/debug.c                     |    2 +-
 drivers/mtd/ubi/io.c                        |    2 +-
 drivers/net/wireless/iwlwifi/iwl3945-base.c |    4 +-
 drivers/net/wireless/iwlwifi/iwl4965-base.c |    4 +-
 drivers/scsi/ide-scsi.c                     |    8 +-
 drivers/usb/gadget/file_storage.c           |    4 +-
 fs/ext3/super.c                             |    6 +-
 fs/ext4/super.c                             |    6 +-
 fs/jffs2/wbuf.c                             |    4 +-
 fs/jfs/jfs_imap.c                           |    2 +-
 fs/jfs/jfs_logmgr.c                         |    6 +-
 fs/jfs/jfs_metapage.c                       |    2 +-
 fs/jfs/jfs_txnmgr.c                         |    8 +-
 fs/jfs/xattr.c                              |    4 +-
 16 files changed, 110 insertions(+), 129 deletions(-)
include/linux/kernel.h +6 -7
index 94bc996..ab45524 100644
@@ -248,15 +248,14 @@ enum {
 	DUMP_PREFIX_ADDRESS,
 	DUMP_PREFIX_OFFSET
 };
-extern void hex_dump_to_buffer(const void *buf, size_t len,
-				int rowsize, int groupsize,
-				char *linebuf, size_t linebuflen, bool ascii);
+extern size_t hex_dump_to_buffer(const void *buf, size_t len,
+				 size_t rowsize, size_t groupsize,
+				 char *linebuf, size_t linebuflen, bool ascii);
 extern void print_hex_dump(const char *level, const char *prefix_str,
-				int prefix_type, int rowsize, int groupsize,
-				const void *buf, size_t len, bool ascii);
+			   int prefix_type, size_t groupsize,
+			   const void *buf, size_t len, bool ascii);
 extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
-			const void *buf, size_t len);
-#define hex_asc(x)	"0123456789abcdef"[x]
+				 const void *buf, size_t len);
 
 #define pr_emerg(fmt, arg...) \
 	printk(KERN_EMERG fmt, ##arg)
lib/hexdump.c +73 -91

243 lines not on this page · raw message

drivers/mtd/ubi/debug.c +1 -1
index 56956ec..d99c4d5 100644
@@ -42,7 +42,7 @@ void ubi_dbg_dump_ec_hdr(const struct ubi_ec_hdr *ec_hdr)
 	dbg_msg("data_offset    %d",    be32_to_cpu(ec_hdr->data_offset));
 	dbg_msg("hdr_crc        %#08x", be32_to_cpu(ec_hdr->hdr_crc));
 	dbg_msg("erase counter header hexdump:");
-	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1,
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 1,
 		       ec_hdr, UBI_EC_HDR_SIZE, 1);
 }
drivers/mtd/ubi/io.c +1 -1
index 7c304ee..20cf7b2 100644
@@ -1243,7 +1243,7 @@ static int paranoid_check_all_ff(struct ubi_device *ubi, int pnum, int offset,
 fail:
 	ubi_err("paranoid check failed for PEB %d", pnum);
 	dbg_msg("hex dump of the %d-%d region", offset, offset + len);
-	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1,
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 1,
 		       ubi->dbg_peb_buf, len, 1);
 	err = 1;
 error:
drivers/net/wireless/iwlwifi/iwl3945-base.c +2 -2
index 4bdf237..563c30f 100644
@@ -184,8 +184,8 @@ static void iwl_print_hex_dump(int level, void *p, u32 len)
 	if (!(iwl_debug_level & level))
 		return;
 
-	print_hex_dump(KERN_DEBUG, "iwl data: ", DUMP_PREFIX_OFFSET, 16, 1,
-			p, len, 1);
+	print_hex_dump(KERN_DEBUG, "iwl data: ", DUMP_PREFIX_OFFSET, 1,
+		       p, len, 1);
 #endif
 }
drivers/net/wireless/iwlwifi/iwl4965-base.c +2 -2
index 8f85564..150f6e4 100644
@@ -183,8 +183,8 @@ static void iwl_print_hex_dump(int level, void *p, u32 len)
 	if (!(iwl_debug_level & level))
 		return;
 
-	print_hex_dump(KERN_DEBUG, "iwl data: ", DUMP_PREFIX_OFFSET, 16, 1,
-			p, len, 1);
+	print_hex_dump(KERN_DEBUG, "iwl data: ", DUMP_PREFIX_OFFSET, 1,
+		       p, len, 1);
 #endif
 }
drivers/scsi/ide-scsi.c +4 -4
index 7a835a3..cc2da8a 100644
@@ -272,7 +272,7 @@ static int idescsi_check_condition(ide_drive_t *drive, struct request *failed_co
 	pc->scsi_cmd = ((idescsi_pc_t *) failed_command->special)->scsi_cmd;
 	if (test_bit(IDESCSI_LOG_CMD, &scsi->log)) {
 		printk ("ide-scsi: %s: queue cmd = ", drive->name);
-		print_hex_dump(KERN_CONT, "", DUMP_PREFIX_NONE, 16, 1, pc->c,
+		print_hex_dump(KERN_CONT, "", DUMP_PREFIX_NONE, 1, pc->c,
 			       6, 0);
 	}
 	rq->rq_disk = scsi->disk;

@@ -328,7 +328,7 @@ static int idescsi_end_request (ide_drive_t *drive, int uptodate, int nrsecs)
 		idescsi_pc_t *opc = (idescsi_pc_t *) rq->buffer;
 		if (log) {
 			printk ("ide-scsi: %s: wrap up check %lu, rst = ", drive->name, opc->scsi_cmd->serial_number);
-			print_hex_dump(KERN_CONT, "", DUMP_PREFIX_NONE, 16, 1,
+			print_hex_dump(KERN_CONT, "", DUMP_PREFIX_NONE, 1,
 				       pc->buffer, 16, 0);
 		}
 		memcpy((void *) opc->scsi_cmd->sense_buffer, pc->buffer, SCSI_SENSE_BUFFERSIZE);

@@ -808,11 +808,11 @@ static int idescsi_queue (struct scsi_cmnd *cmd,
 
 	if (test_bit(IDESCSI_LOG_CMD, &scsi->log)) {
 		printk ("ide-scsi: %s: que %lu, cmd = ", drive->name, cmd->serial_number);
-		print_hex_dump(KERN_CONT, "", DUMP_PREFIX_NONE, 16, 1,
+		print_hex_dump(KERN_CONT, "", DUMP_PREFIX_NONE, 1,
 			       cmd->cmnd, cmd->cmd_len, 0);
 		if (memcmp(pc->c, cmd->cmnd, cmd->cmd_len)) {
 			printk ("ide-scsi: %s: que %lu, tsl = ", drive->name, cmd->serial_number);
-			print_hex_dump(KERN_CONT, "", DUMP_PREFIX_NONE, 16, 1,
+			print_hex_dump(KERN_CONT, "", DUMP_PREFIX_NONE, 1,
 				       pc->c, 12, 0);
 		}
 	}
drivers/usb/gadget/file_storage.c +2 -2
index 1d174dc..e690949 100644
@@ -723,7 +723,7 @@ static void dump_msg(struct fsg_dev *fsg, const char *label,
 	if (length < 512) {
 		DBG(fsg, "%s, length %u:\n", label, length);
 		print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET,
-				16, 1, buf, length, 0);
+			       1, buf, length, 0);
 	}
 }
 

@@ -741,7 +741,7 @@ static void dump_msg(struct fsg_dev *fsg, const char *label,
 static void dump_cdb(struct fsg_dev *fsg)
 {
 	print_hex_dump(KERN_DEBUG, "SCSI CDB: ", DUMP_PREFIX_NONE,
-			16, 1, fsg->cmnd, fsg->cmnd_size, 0);
+		       1, fsg->cmnd, fsg->cmnd_size, 0);
 }
 
 #else
fs/ext3/super.c +3 -3
index de55da9..f5022a3 100644
@@ -464,9 +464,9 @@ static void ext3_destroy_inode(struct inode *inode)
 	if (!list_empty(&(EXT3_I(inode)->i_orphan))) {
 		printk("EXT3 Inode %p: orphan list check failed!\n",
 			EXT3_I(inode));
-		print_hex_dump(KERN_INFO, "", DUMP_PREFIX_ADDRESS, 16, 4,
-				EXT3_I(inode), sizeof(struct ext3_inode_info),
-				false);
+		print_hex_dump(KERN_INFO, "", DUMP_PREFIX_ADDRESS, 4,
+			       EXT3_I(inode), sizeof(struct ext3_inode_info),
+			       false);
 		dump_stack();
 	}
 	kmem_cache_free(ext3_inode_cachep, EXT3_I(inode));
fs/ext4/super.c +3 -3
index 8031dc0..edd6a80 100644
@@ -517,9 +517,9 @@ static void ext4_destroy_inode(struct inode *inode)
 	if (!list_empty(&(EXT4_I(inode)->i_orphan))) {
 		printk("EXT4 Inode %p: orphan list check failed!\n",
 			EXT4_I(inode));
-		print_hex_dump(KERN_INFO, "", DUMP_PREFIX_ADDRESS, 16, 4,
-				EXT4_I(inode), sizeof(struct ext4_inode_info),
-				true);
+		print_hex_dump(KERN_INFO, "", DUMP_PREFIX_ADDRESS, 4,
+			       EXT4_I(inode), sizeof(struct ext4_inode_info),
+			       true);
 		dump_stack();
 	}
 	kmem_cache_free(ext4_inode_cachep, EXT4_I(inode));
fs/jffs2/wbuf.c +2 -2
index d1d4f27..c03977e 100644
@@ -248,11 +248,11 @@ static int jffs2_verify_write(struct jffs2_sb_info *c, unsigned char *buf,
 
 	printk(KERN_WARNING "Write verify error (ECC %s) at %08x. Wrote:\n",
 	       eccstr, c->wbuf_ofs);
-	print_hex_dump(KERN_WARNING, "", DUMP_PREFIX_OFFSET, 16, 1,
+	print_hex_dump(KERN_WARNING, "", DUMP_PREFIX_OFFSET, 1,
 		       c->wbuf, c->wbuf_pagesize, 0);
 
 	printk(KERN_WARNING "Read back:\n");
-	print_hex_dump(KERN_WARNING, "", DUMP_PREFIX_OFFSET, 16, 1,
+	print_hex_dump(KERN_WARNING, "", DUMP_PREFIX_OFFSET, 1,
 		       c->wbuf_verify, c->wbuf_pagesize, 0);
 
 	return -EIO;
fs/jfs/jfs_imap.c +1 -1
index 3870ba8..c22907d 100644
@@ -890,7 +890,7 @@ int diFree(struct inode *ip)
 	 * the map.
 	 */
 	if (iagno >= imap->im_nextiag) {
-		print_hex_dump(KERN_ERR, "imap: ", DUMP_PREFIX_ADDRESS, 16, 4,
+		print_hex_dump(KERN_ERR, "imap: ", DUMP_PREFIX_ADDRESS, 4,
 			       imap, 32, 0);
 		jfs_error(ip->i_sb,
 			  "diFree: inum = %d, iagno = %d, nextiag = %d",
fs/jfs/jfs_logmgr.c +3 -3
index 15a3974..a54e19f 100644
@@ -1627,15 +1627,15 @@ void jfs_flush_journal(struct jfs_log *log, int wait)
 			if (lp->xflag & COMMIT_PAGE) {
 				struct metapage *mp = (struct metapage *)lp;
 				print_hex_dump(KERN_ERR, "metapage: ",
-					       DUMP_PREFIX_ADDRESS, 16, 4,
+					       DUMP_PREFIX_ADDRESS, 4,
 					       mp, sizeof(struct metapage), 0);
 				print_hex_dump(KERN_ERR, "page: ",
-					       DUMP_PREFIX_ADDRESS, 16,
+					       DUMP_PREFIX_ADDRESS,
 					       sizeof(long), mp->page,
 					       sizeof(struct page), 0);
 			} else
 				print_hex_dump(KERN_ERR, "tblock:",
-					       DUMP_PREFIX_ADDRESS, 16, 4,
+					       DUMP_PREFIX_ADDRESS, 4,
 					       lp, sizeof(struct tblock), 0);
 		}
 	}
fs/jfs/jfs_metapage.c +1 -1
index f5cd8d3..9e5fb41 100644
@@ -461,7 +461,7 @@ add_failed:
 	printk(KERN_ERR "JFS: bio_add_page failed unexpectedly\n");
 	goto skip;
 dump_bio:
-	print_hex_dump(KERN_ERR, "JFS: dump of bio: ", DUMP_PREFIX_ADDRESS, 16,
+	print_hex_dump(KERN_ERR, "JFS: dump of bio: ", DUMP_PREFIX_ADDRESS,
 		       4, bio, sizeof(*bio), 0);
 skip:
 	bio_put(bio);
fs/jfs/jfs_txnmgr.c +4 -4
index e7c60ae..a105198 100644
@@ -830,14 +830,14 @@ struct tlock *txLock(tid_t tid, struct inode *ip, struct metapage * mp,
 	/* assert(jfs_ip->fileset == AGGREGATE_I); */
 	if (jfs_ip->fileset != AGGREGATE_I) {
 		printk(KERN_ERR "txLock: trying to lock locked page!");
-		print_hex_dump(KERN_ERR, "ip: ", DUMP_PREFIX_ADDRESS, 16, 4,
+		print_hex_dump(KERN_ERR, "ip: ", DUMP_PREFIX_ADDRESS, 4,
 			       ip, sizeof(*ip), 0);
-		print_hex_dump(KERN_ERR, "mp: ", DUMP_PREFIX_ADDRESS, 16, 4,
+		print_hex_dump(KERN_ERR, "mp: ", DUMP_PREFIX_ADDRESS, 4,
 			       mp, sizeof(*mp), 0);
 		print_hex_dump(KERN_ERR, "Locker's tblock: ",
-			       DUMP_PREFIX_ADDRESS, 16, 4, tid_to_tblock(tid),
+			       DUMP_PREFIX_ADDRESS, 4, tid_to_tblock(tid),
 			       sizeof(struct tblock), 0);
-		print_hex_dump(KERN_ERR, "Tlock: ", DUMP_PREFIX_ADDRESS, 16, 4,
+		print_hex_dump(KERN_ERR, "Tlock: ", DUMP_PREFIX_ADDRESS, 4,
 			       tlck, sizeof(*tlck), 0);
 		BUG();
 	}
fs/jfs/xattr.c +2 -2
index 9b7f2cd..43c0d70 100644
@@ -590,8 +590,8 @@ static int ea_get(struct inode *inode, struct ea_buffer *ea_buf, int min_size)
       size_check:
 	if (EALIST_SIZE(ea_buf->xattr) != ea_size) {
 		printk(KERN_ERR "ea_get: invalid extended attribute\n");
-		print_hex_dump(KERN_ERR, "", DUMP_PREFIX_ADDRESS, 16, 1,
-				     ea_buf->xattr, ea_size, 1);
+		print_hex_dump(KERN_ERR, "", DUMP_PREFIX_ADDRESS, 1,
+			       ea_buf->xattr, ea_size, 1);
 		ea_release(inode, ea_buf);
 		rc = -EIO;
 		goto clean_up;