diff mbox

[U-Boot,V2] JFFS2: accelerate scanning.

Message ID 000001cbfcbd$59578ec0$6401a8c0@LENOVOE5CA6843
State Changes Requested
Headers show

Commit Message

Baidu Boy April 17, 2011, 5:06 a.m. UTC 
Syncs up with jffs2 in the linux kernel:
 1/ Change DEFAULT_EMPTY_SCAN_SIZE from 4KB to 256 Bytes.
 2/ If the 1KB data is 0xFF after the cleanmarker, skip
 and scan the next sector.
 3/ Change the buffer size from 4KB to 128KB which is the
 common size of erase block.

For the 16MB nor flash, the scanning time is accelerated
from about 9s to 1s.

Signed-off-by: Baidu Liu <liucai.lfn@gmail.com>
---
Changes for V2:
      - Add detail description for the patch.
      - Move DEFAULT_EMPTY_SCAN_SIZE to jffs2.h
---
 fs/jffs2/jffs2_1pass.c      |   30 +++++++++++++++++++-----------
 fs/jffs2/jffs2_nand_1pass.c |   11 +++++------
 include/jffs2/jffs2.h       |    2 ++
 3 files changed, 26 insertions(+), 17 deletions(-)

Comments

Detlev Zundel April 19, 2011, 3:15 p.m. UTC | #1 
Hi Baidu,

>  Syncs up with jffs2 in the linux kernel:
>  1/ Change DEFAULT_EMPTY_SCAN_SIZE from 4KB to 256 Bytes.
>  2/ If the 1KB data is 0xFF after the cleanmarker, skip
>  and scan the next sector.
>  3/ Change the buffer size from 4KB to 128KB which is the
>  common size of erase block.

There is no "common size of erase block".  Looking into the Linux code,
it uses "max(erase block size, 128k)" for its buffer to speed up reading
from NAND and the 128k seem to be a kmalloc limit.

So maybe a "increase buffer size from 4KiB to 128KiB to reduce number of
read operations" would be more fitting.  By the way, does this change
contribute to the performance increase at all, or is the increase simply
due to DEFAULT_EMPTY_SCAN_SIZE?

Also as for the other patch, can you split the commit into the
individual changes corresponding to the list items?  In this way, one
could also easily measure which change really speeds up the operation...

Thanks!
  Detlev
Baidu Boy April 24, 2011, 3:43 a.m. UTC | #2 
Hi,Detlev :

2011/4/19 Detlev Zundel <dzu@denx.de>:
> Hi Baidu,
>
>>  Syncs up with jffs2 in the linux kernel:
>>  1/ Change DEFAULT_EMPTY_SCAN_SIZE from 4KB to 256 Bytes.
>>  2/ If the 1KB data is 0xFF after the cleanmarker, skip
>>  and scan the next sector.
>>  3/ Change the buffer size from 4KB to 128KB which is the
>>  common size of erase block.
>
> There is no "common size of erase block".  Looking into the Linux code,
> it uses "max(erase block size, 128k)" for its buffer to speed up reading
> from NAND and the 128k seem to be a kmalloc limit.
>
> So maybe a "increase buffer size from 4KiB to 128KiB to reduce number of
> read operations" would be more fitting.  By the way, does this change
> contribute to the performance increase at all, or is the increase simply
> due to DEFAULT_EMPTY_SCAN_SIZE?
>

Yes, I think it is useful to speed up the scanning.

I have split these changes to individual patch.
Thans.
Detlev Zundel April 27, 2011, 9:43 a.m. UTC | #3 
Hi Baidu,

> Hi,Detlev :
>
> 2011/4/19 Detlev Zundel <dzu@denx.de>:
>> Hi Baidu,
>>
>>>  Syncs up with jffs2 in the linux kernel:
>>>  1/ Change DEFAULT_EMPTY_SCAN_SIZE from 4KB to 256 Bytes.
>>>  2/ If the 1KB data is 0xFF after the cleanmarker, skip
>>>  and scan the next sector.
>>>  3/ Change the buffer size from 4KB to 128KB which is the
>>>  common size of erase block.
>>
>> There is no "common size of erase block".  Looking into the Linux code,
>> it uses "max(erase block size, 128k)" for its buffer to speed up reading
>> from NAND and the 128k seem to be a kmalloc limit.
>>
>> So maybe a "increase buffer size from 4KiB to 128KiB to reduce number of
>> read operations" would be more fitting.  By the way, does this change
>> contribute to the performance increase at all, or is the increase simply
>> due to DEFAULT_EMPTY_SCAN_SIZE?
>>
>
> Yes, I think it is useful to speed up the scanning.

Don't get me wrong, but I was not asking whether you "think" it speeds
up the scanning.  When it comes to performance, I learnt to trust
numbers olnly.  This may in part be because I myself occassionally was
completely wrong in predicting performance issues.

So I am still eager to see actual numbers if this _really_ speeds up
scanning.

Cheers
  Detlev
diff mbox

Patch

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
index 5ddc2b9..2077d2f 100644
--- a/fs/jffs2/jffs2_1pass.c
+++ b/fs/jffs2/jffs2_1pass.c
@@ -1428,8 +1428,6 @@  dump_dirents(struct b_lists *pL)
 }
 #endif
 
-#define DEFAULT_EMPTY_SCAN_SIZE	4096
-
 static inline uint32_t EMPTY_SCAN_SIZE(uint32_t sector_size)
 {
 	if (sector_size < DEFAULT_EMPTY_SCAN_SIZE)
@@ -1449,7 +1447,7 @@  jffs2_1pass_build_lists(struct part_info * part)
 	u32 counterF = 0;
 	u32 counterN = 0;
 	u32 max_totlen = 0;
-	u32 buf_size = DEFAULT_EMPTY_SCAN_SIZE;
+	u32 buf_size = 128*1024;
 	char *buf;
 
 	/* turn off the lcd.  Refreshing the lcd adds 50% overhead to the */
@@ -1540,14 +1538,17 @@  jffs2_1pass_build_lists(struct part_info * part)
 		/* We temporarily use 'ofs' as a pointer into the buffer/jeb */
 		ofs = 0;
 
-		/* Scan only 4KiB of 0xFF before declaring it's empty */
+		/* Scan only EMPTY_SCAN_SIZE of 0xFF before declaring it's empty */
 		while (ofs < EMPTY_SCAN_SIZE(part->sector_size) &&
 				*(uint32_t *)(&buf[ofs]) == 0xFFFFFFFF)
 			ofs += 4;
 
-		if (ofs == EMPTY_SCAN_SIZE(part->sector_size))
+		if (ofs == EMPTY_SCAN_SIZE(part->sector_size)) {
+			printf("Block at 0x%08x is empty (erased)\n", sector_ofs);
 			continue;
+		}
 
+		/* Now ofs is a complete physical flash offset as it always was... */
 		ofs += sector_ofs;
 		prevofs = ofs - 1;
 
@@ -1575,16 +1576,14 @@  jffs2_1pass_build_lists(struct part_info * part)
 
 			if (*(uint32_t *)(&buf[ofs-buf_ofs]) == 0xffffffff) {
 				uint32_t inbuf_ofs;
-				uint32_t empty_start, scan_end;
+				uint32_t empty_start;
 
 				empty_start = ofs;
 				ofs += 4;
-				scan_end = min_t(uint32_t, EMPTY_SCAN_SIZE(
-							part->sector_size)/8,
-							buf_len);
+
 			more_empty:
 				inbuf_ofs = ofs - buf_ofs;
-				while (inbuf_ofs < scan_end) {
+				while (inbuf_ofs < buf_len) {
 					if (*(uint32_t *)(&buf[inbuf_ofs]) !=
 							0xffffffff)
 						goto scan_more;
@@ -1594,6 +1593,15 @@  jffs2_1pass_build_lists(struct part_info * part)
 				}
 				/* Ran off end. */
 
+				/* If we're only checking the beginning of a block with a cleanmarker,
+				   bail now */
+				if((buf_ofs == sector_ofs) && 
+				    (empty_start == sector_ofs +sizeof(struct jffs2_unknown_node))) {  
+				    printf("%d bytes at start of block seems clean... assuming all clean\n",
+						EMPTY_SCAN_SIZE(part->sector_size));
+				    break;
+				}
+
 				/* See how much more there is to read in this
 				 * eraseblock...
 				 */
@@ -1608,12 +1616,12 @@  jffs2_1pass_build_lists(struct part_info * part)
 					 */
 					break;
 				}
-				scan_end = buf_len;
 				get_fl_mem((u32)part->offset + ofs, buf_len,
 					   buf);
 				buf_ofs = ofs;
 				goto more_empty;
 			}
+			
 			if (node->magic != JFFS2_MAGIC_BITMASK ||
 					!hdr_crc(node)) {
 				ofs += 4;
diff --git a/fs/jffs2/jffs2_nand_1pass.c b/fs/jffs2/jffs2_nand_1pass.c
index 740f787..6d205c1 100644
--- a/fs/jffs2/jffs2_nand_1pass.c
+++ b/fs/jffs2/jffs2_nand_1pass.c
@@ -779,7 +779,6 @@  jffs2_fill_scan_buf(nand_info_t *nand, unsigned char *buf,
 	return 0;
 }
 
-#define	EMPTY_SCAN_SIZE	1024
 static u32
 jffs2_1pass_build_lists(struct part_info * part)
 {
@@ -816,17 +815,17 @@  jffs2_1pass_build_lists(struct part_info * part)
 		if (nand_block_isbad(nand, offset))
 			continue;
 
-		if (jffs2_fill_scan_buf(nand, buf, offset, EMPTY_SCAN_SIZE))
+		if (jffs2_fill_scan_buf(nand, buf, offset, DEFAULT_EMPTY_SCAN_SIZE))
 			return 0;
 
 		ofs = 0;
-		/* Scan only 4KiB of 0xFF before declaring it's empty */
-		while (ofs < EMPTY_SCAN_SIZE && *(uint32_t *)(&buf[ofs]) == 0xFFFFFFFF)
+		/* Scan only DEFAULT_EMPTY_SCAN_SIZE of 0xFF before declaring it's empty */
+		while (ofs < DEFAULT_EMPTY_SCAN_SIZE && *(uint32_t *)(&buf[ofs]) == 0xFFFFFFFF)
 			ofs += 4;
-		if (ofs == EMPTY_SCAN_SIZE)
+		if (ofs == DEFAULT_EMPTY_SCAN_SIZE)
 			continue;
 
-		if (jffs2_fill_scan_buf(nand, buf + EMPTY_SCAN_SIZE, offset + EMPTY_SCAN_SIZE, sectorsize - EMPTY_SCAN_SIZE))
+		if (jffs2_fill_scan_buf(nand, buf + DEFAULT_EMPTY_SCAN_SIZE, offset + DEFAULT_EMPTY_SCAN_SIZE, sectorsize - DEFAULT_EMPTY_SCAN_SIZE))
 			return 0;
 		offset += ofs;
 
diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h
index 651f94c..75c68b8 100644
--- a/include/jffs2/jffs2.h
+++ b/include/jffs2/jffs2.h
@@ -41,6 +41,8 @@ 
 #include <asm/types.h>
 #include <jffs2/load_kernel.h>
 
+#define DEFAULT_EMPTY_SCAN_SIZE	256
+
 #define JFFS2_SUPER_MAGIC 0x72b6
 
 /* Values we may expect to find in the 'magic' field */