Patchwork ps3-utils performance issue

login
register
mail settings
Submitter Jiafu Gao
Date Nov. 5, 2008, 3:51 a.m.
Message ID <583206.90742.qm@web35601.mail.mud.yahoo.com>
Download mbox | patch
Permalink /patch/7283/
State New
Headers show

Comments

Jiafu Gao - Nov. 5, 2008, 3:51 a.m.
Hi,

One of the handy tool in ps3-utils [0] is ps3-dump-bootloader. It is implemented as a shell script using dd util. The main command is:

   dd if=/dev/ps3flash bs=1 skip=$ldr_offset count=$ldr_size

Note that the parameter bs=1. Trying to dump the boot loader under Ubuntu Intrepid (size of 3M: 3037844), it takes more than 6 minutes. I am not sure the reason, but I guess it could be dd is not using read/write buffer when bs is set to 1. This translate to 3M reads and writes. Also notes that read from flash is in the form of block. 

There are several solutions:
1. change bs to a reasonable size, say 1024. Then we have an issue of leftover that is not multiple of 1024. We can either read this left over separately by set bs=1 or read an extra block and truncate the file to the proper size.

2. add an option to ps3-flash-util to dump boot loader. The implementation is really simple: just read /dev/ps3flash and write in reasonable blocks. I've came up a quick patch (attached) to do this. 

Both methods only take under 1 second.

Geoff, I've leave to you to decide which method to use. Use the patch if you like.

Thanks for your attention.


[0] http://www.kernel.org/pub/linux/kernel/people/geoff/cell/ps3-utils/, version 2.3
Geoff Levand - Nov. 5, 2008, 5:08 p.m.
Jiafu Gao wrote:
 One of the handy tool in ps3-utils [0] is ps3-dump-bootloader. It is implemented as a shell script using dd util. The main command is:
> 
>    dd if=/dev/ps3flash bs=1 skip=$ldr_offset count=$ldr_size
> 
> Note that the parameter bs=1. Trying to dump the boot loader under Ubuntu Intrepid (size of 3M: 3037844), it takes more than 6 minutes. I am not sure the reason, but I guess it could be dd is not using read/write buffer when bs is set to 1. This translate to 3M reads and writes. Also notes that read from flash is in the form of block. 

Yes, to get the exact number of bytes it uses bs=1, which
is not efficient.

> 2. add an option to ps3-flash-util to dump boot loader. The implementation is really simple: just read /dev/ps3flash and write in reasonable blocks. I've came up a quick patch (attached) to do this. 

Please resend the patch with a description and a Signed-off-by:
in its header.

-Geoff
Jiafu Gao - Nov. 10, 2008, 9:33 a.m.
OK, it has been a while. I made some more changes: fix the ps-dump-bootloader script too, make necessary changes to the files that generates manual pages for ps3-flash-util and ps3-dump-bootloader.

Let me know if you have any concern or comments.

Best regards,

Jiafu



----- Original Message ----
From: Geoff Levand <geoffrey.levand@am.sony.com>
To: Jiafu Gao <jiafu_gao@yahoo.com>
Cc: cbe-oss-dev@ozlabs.org
Sent: Wednesday, November 5, 2008 12:08:55 PM
Subject: Re: [Cbe-oss-dev] ps3-utils performance issue

Jiafu Gao wrote:
One of the handy tool in ps3-utils [0] is ps3-dump-bootloader. It is implemented as a shell script using dd util. The main command is:
> 
>    dd if=/dev/ps3flash bs=1 skip=$ldr_offset count=$ldr_size
> 
> Note that the parameter bs=1. Trying to dump the boot loader under Ubuntu Intrepid (size of 3M: 3037844), it takes more than 6 minutes. I am not sure the reason, but I guess it could be dd is not using read/write buffer when bs is set to 1. This translate to 3M reads and writes. Also notes that read from flash is in the form of block. 

Yes, to get the exact number of bytes it uses bs=1, which
is not efficient.

> 2. add an option to ps3-flash-util to dump boot loader. The implementation is really simple: just read /dev/ps3flash and write in reasonable blocks. I've came up a quick patch (attached) to do this. 

Please resend the patch with a description and a Signed-off-by:
in its header.

-Geoff
Geoff Levand - Dec. 4, 2008, 9:01 p.m.
Jiafu Gao wrote:
> OK, it has been a while. I made some more changes: fix the ps-dump-bootloader script too, make necessary changes to the files that generates manual pages for ps3-flash-util and ps3-dump-bootloader.

Just to let you know, I appreciate your contribution.  I'll
try it next time I do some ps3-utils work, maybe in a few weeks.

-Geoff

Patch

--- ps3-utils-2.3/ps3-flash-util.c	2008-10-31 18:18:11.000000000 -0400
+++ ps3-utils-2.3-new/ps3-flash-util.c	2008-11-04 22:26:49.000000000 -0500
@@ -118,6 +118,7 @@ 
 "SYNOPSIS\n"
 "     ps3-flash-util [-d, --device flash-dev] [-s, --show-settings]\n"
 "                    [-w, --write-image image-file]\n"
+"		     [-b, --backup-image iamge-file]\n"
 "                    [-g, --set-game-os | -o, --set-other-os]\n"
 "                    [-r, --set-raw | -z, --set-gzip] [-t, --game-time]\n"
 "                    [-T, --db-test] [-F, --db-format]\n"
@@ -137,6 +138,8 @@ 
 "             the OS area header with information for the new image.  This is\n"
 "             the option to use to write a new bootloader image to flash mem-\n"
 "             ory.  Use ’-’ for data on stdin.\n"
+"     -b, --backup-image image-file\n"
+"             Backup Other OS image from flash memory to image-file\n" 
 "     -g, --set-game-os\n"
 "             Set the system boot flag to Game OS.\n"
 "     -o, --set-other-os\n"
@@ -189,6 +192,7 @@ 
 	{"device",         required_argument, NULL, 'd'},
 	{"show-settings",  no_argument,       NULL, 's'},
 	{"write-image",    required_argument, NULL, 'w'},
+	{"backup-image",   required_argument, NULL, 'b'},
 	{"set-game-os",    no_argument,       NULL, 'g'},
 	{"set-other-os",   no_argument,       NULL, 'o'},
 	{"set-raw",        no_argument,       NULL, 'r'},
@@ -234,6 +238,7 @@ 
 	struct db_op_entry* db_op_list;
 	const char *device;
 	const char *image;
+	const char *backup_image;
 };
 
 static void show_settings(const struct os_area_header *h,
@@ -280,6 +285,42 @@ 
 	return result;
 }
 
+static int backup_image(FILE* dev, struct os_area_header* h, const char* image)
+{	
+#define BUF_SIZE 1024
+	int left, offset, size;
+	char buf[BUF_SIZE];
+
+	FILE* img = fopen(image, "w");
+	if (!img) {
+		DBG("%s:%d: fopen failed.\n", __func__,__LINE__);
+		perror(image);
+		return -1;
+	}
+
+	offset = 512*h->ldr_area_offset;
+	size = h->ldr_size;
+	if (offset>0)
+		if (fseek(dev,offset,SEEK_SET))
+			return -1;
+	left = size;	
+	while (left>=BUF_SIZE) {
+		if (fread(buf, BUF_SIZE, 1, dev)!=1)
+			return -1;	
+		if (fwrite(buf, BUF_SIZE, 1, img)!=1)
+			return -1;
+		left -= BUF_SIZE;
+	}
+        if (left>0) {
+		if (fread(buf,left,1,dev)!=1)	
+			return -1;
+		if (fwrite(buf,left,1,img)!=1)
+			return -1;
+	}
+				
+	return 0;
+}
+
 static void db_list_owners(void)
 {
 	char s[2048];
@@ -408,6 +449,10 @@ 
 			opts->image = optarg;
 			got_work++;
 			break;
+		case 'b':
+			opts->backup_image = optarg;
+			got_work++;
+			break;
 		case 'g':
 			opts->boot_other_os = opt_no;
 			got_work++;
@@ -662,6 +707,14 @@ 
 			exit(1);
 		}
 	}
+	
+	if (opts.backup_image) {
+		result = backup_image(dev,&header,opts.backup_image);
+		if (result) {
+			fprintf(stderr, "%s:%d: backup_image failed\n", __func__,__LINE__);
+			exit(1);
+		}
+	}
 
 	if (opts.compressed_image) {
 		result = os_area_set_ldr_format(&header, dev,