[U-Boot] [PATCH] JFFS2: accelerate scanning.
Baidu Liu
liucai.lfn at gmail.com
Wed Apr 13 15:39:07 CEST 2011
Hi, Detlev
2011/4/13 Detlev Zundel <dzu at denx.de>:
> Hi Leo,
>
>> This patch make the JFFS2 scanning faster in U-Boot.
>> 1). if we find 1KB 0xFF data from the beginning of the erase block,skip it.
>> 2). if the 1KB data is 0xFF after the cleanmarker, ship this erase block.
>
> Ship it to where? ;) Typo, should be skip.
>
Skip to the next erase block
>> For the 16MB nor flash, the scanning time is changed from about 9s to 1s.
>>
>> Signed-off-by: Leo Liu <liucai.lfn at gmail.com>
>
> checkpatch says:
>
> total: 7 errors, 9 warnings, 103 lines checked
>
> NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
> scripts/cleanfile
>
> Is there a corresponding thing in the Linux kernel?
>
>> ---
>> fs/jffs2/jffs2_1pass.c | 31 +++++++++++++++++++++----------
>> fs/jffs2/jffs2_nand_1pass.c | 2 +-
>> 2 files changed, 22 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
>> index dfb1745..f38f755 100644
>> --- a/fs/jffs2/jffs2_1pass.c
>> +++ b/fs/jffs2/jffs2_1pass.c
>> @@ -1441,7 +1441,7 @@ dump_dirents(struct b_lists *pL)
>> }
>> #endif
>>
>> -#define DEFAULT_EMPTY_SCAN_SIZE 4096
>> +#define DEFAULT_EMPTY_SCAN_SIZE 1024
>
> Hm, this is not mentioned in the commit log. So is this detection
> already there and you simply decrease the amount of empty space you are
> looking for?
>
1KB FF is enough to get the conclusion that the erase block is empty.
I refer kenel to do this change
>>
>> static inline uint32_t EMPTY_SCAN_SIZE(uint32_t sector_size)
>> {
>> @@ -1461,7 +1461,7 @@ jffs2_1pass_build_lists(struct part_info * part)
>> u32 counter4 = 0;
>> u32 counterF = 0;
>> u32 counterN = 0;
>> - u32 buf_size = DEFAULT_EMPTY_SCAN_SIZE;
>> + u32 buf_size = 128*1024;;
>
> This is a 128k buffer size which previously was a 4k buffer. Why is
> this change needed? Where does the size come from?
>
I refer the kenel jffs2 to do this change. we read 128 KB or size of a
erase block if the block size is less than 128 KB.
>> char *buf;
>>
>> /* turn off the lcd. Refreshing the lcd adds 50% overhead to the */
>> @@ -1559,14 +1559,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 1KiB of 0xFF before declaring it's empty */
>
> Change the comment to refer to the macro so we don't need to change it
> every time the macro changes.
Ok, i will do the change in the next patch
>
>> 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);
>
> Hm, you add this output so lets consider what it says - wouldn't it be
> better to say "is considered to be empty"? After all, we only infer the
> emptiness in the code.
>
Yes, thanks. You description is better.
>> continue;
>> + }
>>
>> + /* Now ofs is a complete physical flash offset as it always was... */
>> ofs += sector_ofs;
>> prevofs = ofs - 1;
>>
>> @@ -1594,16 +1597,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;
>> @@ -1613,6 +1614,12 @@ 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) &&
>> + (node ==(struct jffs2_unknown_node *)&buf[ofs-buf_ofs]))
>> + break;
>> +
It is wrong here. I test in my board today. I will send a new patch to
correct this.
>> /* See how much more there is to read in this
>> * eraseblock...
>> */
>> @@ -1627,12 +1634,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;
>> @@ -1650,6 +1657,8 @@ jffs2_1pass_build_lists(struct part_info * part)
>> case JFFS2_NODETYPE_INODE:
>> if (buf_ofs + buf_len < ofs + sizeof(struct
>> jffs2_raw_inode)) {
>> + buf_len = min_t(uint32_t, buf_size, sector_ofs
>> + + part->sector_size - ofs);
>>
>> get_fl_mem((u32)part->offset + ofs,
>> buf_len, buf);
>> buf_ofs = ofs;
>> @@ -1671,6 +1680,8 @@ jffs2_1pass_build_lists(struct part_info * part)
>> ((struct
>> jffs2_raw_dirent *)
>> node)->nsize) {
>> + buf_len = min_t(uint32_t, buf_size, sector_ofs
>> + + part->sector_size - ofs);
>>
>> get_fl_mem((u32)part->offset + ofs,
>> buf_len, buf);
>> buf_ofs = ofs;
>> diff --git a/fs/jffs2/jffs2_nand_1pass.c b/fs/jffs2/jffs2_nand_1pass.c
>> index e3bc536..16bb2e9 100644
>> --- a/fs/jffs2/jffs2_nand_1pass.c
>> +++ b/fs/jffs2/jffs2_nand_1pass.c
>> @@ -833,7 +833,7 @@ jffs2_1pass_build_lists(struct part_info * part)
>> return 0;
>>
>> ofs = 0;
>> - /* Scan only 4KiB of 0xFF before declaring it's empty */
>> + /* Scan only 1KiB of 0xFF before declaring it's empty */
>> while (ofs < EMPTY_SCAN_SIZE && *(uint32_t *)(&buf[ofs]) == 0xFFFFFFFF)
>> ofs += 4;
>> if (ofs == EMPTY_SCAN_SIZE)
>
> On a whole - is this optimization safe with regard to uncompressed files
> containing only 0xFFs?
>
Yes, I think it is ok. Most of the change is porting from kernel JFFS2.
We can see in the kernel we mount a 16MB , we need about 0.09s, but in
uboot, we need 9s
I wil send a new patch soon. Thanks
More information about the U-Boot
mailing list