[U-Boot] [PATCH] JFFS2: accelerate scanning.

[Detlev Zundel <dzu@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.

> For the 16MB nor flash, the scanning time is changed from about 9s to 1s.
>
> Signed-off-by: Leo Liu <liucai.lfn@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?

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

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

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

>  			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;
> +	
>  				/* 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?

Cheers
  Detlev

-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de