Patchwork [U-Boot,6/7] JFFS2: scanning performance improvement

login
register
mail settings
Submitter Baidu Boy
Date April 24, 2011, 3:40 a.m.
Message ID <002101cc0231$704847e0$6401a8c0@LENOVOE5CA6843>
Download mbox | patch
Permalink /patch/92635/
State New
Delegated to: Detlev Zundel
Headers show

Comments

Baidu Boy - April 24, 2011, 3:40 a.m.
1/ Sync with kernel.
 If the 256-(struct jffs2_unknown_node *) bytes are
 0xff after the cleanmarker. We get the conclusion that
 the sector is empty.

Signed-off-by: Baidu Liu <liucai.lfn@gmail.com>
---
 fs/jffs2/jffs2_1pass.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)
Detlev Zundel - April 29, 2011, 1:31 p.m.
Hi Baidu,

>  1/ Sync with kernel.
>  If the 256-(struct jffs2_unknown_node *) bytes are
>  0xff after the cleanmarker. We get the conclusion that
>  the sector is empty.
>
> Signed-off-by: Baidu Liu <liucai.lfn@gmail.com>
> ---
>  fs/jffs2/jffs2_1pass.c |   19 +++++++++++++------
>  1 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
> index 62ba250..bbfab2c 100644
> --- a/fs/jffs2/jffs2_1pass.c
> +++ b/fs/jffs2/jffs2_1pass.c
> @@ -1596,16 +1596,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;
> @@ -1615,6 +1613,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;
> +				}
> +

This has style-problems.  Actually all of your patches have style
problems.  Checking them as one single patch gives:

total: 32 errors, 22 warnings, 369 lines checked

Please rework the whole series.

Thanks
  Detlev
Baidu Boy - April 29, 2011, 3:15 p.m.
Hi,Detlev

>> +                             }
>> +
>
> This has style-problems.  Actually all of your patches have style
> problems.  Checking them as one single patch gives:
>
> total: 32 errors, 22 warnings, 369 lines checked
>
> Please rework the whole series.

I do not know why my patch has style-error. I use Unicode(UTF-8).
What character do you use?
Detlev Zundel - May 2, 2011, 12:44 p.m.
Hi Baidu,

> Hi,Detlev
>
>>> +                             }
>>> +
>>
>> This has style-problems.  Actually all of your patches have style
>> problems.  Checking them as one single patch gives:
>>
>> total: 32 errors, 22 warnings, 369 lines checked
>>
>> Please rework the whole series.
>
> I do not know why my patch has style-error. I use Unicode(UTF-8).
> What character do you use?

This has nothing to do with character encoding.  You should run
checkpatch.pl[1] on your changes as stated on our wiki page[2].  When
you do this, you will see this:

  ERROR: trailing whitespace
  #127: FILE: fs/jffs2/jffs2_1pass.c:964:
  +^I^I^I$
  
  ERROR: trailing whitespace
  #129: FILE: fs/jffs2/jffs2_1pass.c:966:
  +^I^I^Iwe compare the DIRENT's ino with the latest DIRENT's ino $
  
  WARNING: line over 80 characters
  #129: FILE: fs/jffs2/jffs2_1pass.c:966:
  +			we compare the DIRENT's ino with the latest DIRENT's ino 
  
  ERROR: trailing whitespace
  #130: FILE: fs/jffs2/jffs2_1pass.c:967:
  +^I^I^Ito determine whether this DIRENT is the latest one. $
  
  ERROR: trailing whitespace
  #133: FILE: fs/jffs2/jffs2_1pass.c:970:
  +^I^I^Iif(jDir->ino != jffs2_1pass_find_inode(pL, jDir->name, pino))  $
  
  WARNING: line over 80 characters
  #133: FILE: fs/jffs2/jffs2_1pass.c:970:
  +			if(jDir->ino != jffs2_1pass_find_inode(pL, jDir->name, pino))  
  
  ERROR: space required before the open parenthesis '('
  #133: FILE: fs/jffs2/jffs2_1pass.c:970:
  +			if(jDir->ino != jffs2_1pass_find_inode(pL, jDir->name, pino))  
  
  ERROR: trailing whitespace
  #168: FILE: fs/jffs2/jffs2_nand_1pass.c:374:
  +^I^Iif ((pino == jDir->pino) && $
  
  ERROR: trailing whitespace
  #169: FILE: fs/jffs2/jffs2_nand_1pass.c:375:
  +^I^I    (len == jDir->nsize) && $
  
  ERROR: trailing whitespace
  #179: FILE: fs/jffs2/jffs2_nand_1pass.c:488:
  +^I^I^Iwe compare the DIRENT's ino with the latest DIRENT's ino $
  
  WARNING: line over 80 characters
  #179: FILE: fs/jffs2/jffs2_nand_1pass.c:488:
  +			we compare the DIRENT's ino with the latest DIRENT's ino 
  
  ERROR: trailing whitespace
  #180: FILE: fs/jffs2/jffs2_nand_1pass.c:489:
  +^I^I^Ito determine whether this DIRENT is the latest one. $
  
  ERROR: trailing whitespace
  #183: FILE: fs/jffs2/jffs2_nand_1pass.c:492:
  +^I^I^Iif(jDir.ino != jffs2_1pass_find_inode(pL, jDir->name, pino))  $
  
  WARNING: line over 80 characters
  #183: FILE: fs/jffs2/jffs2_nand_1pass.c:492:
  +			if(jDir.ino != jffs2_1pass_find_inode(pL, jDir->name, pino))  
  
  ERROR: space required before the open parenthesis '('
  #183: FILE: fs/jffs2/jffs2_nand_1pass.c:492:
  +			if(jDir.ino != jffs2_1pass_find_inode(pL, jDir->name, pino))  
  
  ERROR: trailing whitespace
  #185: FILE: fs/jffs2/jffs2_nand_1pass.c:494:
  +^I^I^I$
  
  ERROR: trailing whitespace
  #309: FILE: fs/jffs2/jffs2_1pass.c:680:
  +^I^I$
  
  ERROR: space required before the open parenthesis '('
  #311: FILE: fs/jffs2/jffs2_1pass.c:682:
  +		if(!pL->readbuf) {
  
  WARNING: line over 80 characters
  #457: FILE: fs/jffs2/jffs2_1pass.c:1646:
  +					buf_len = min_t(uint32_t, buf_size, sector_ofs
  
  ERROR: trailing whitespace
  #458: FILE: fs/jffs2/jffs2_1pass.c:1647:
  +^I^I^I^I^I^I+ part->sector_size - ofs);^I^I$
  
  WARNING: line over 80 characters
  #458: FILE: fs/jffs2/jffs2_1pass.c:1647:
  +						+ part->sector_size - ofs);		
  
  ERROR: trailing whitespace
  #471: FILE: fs/jffs2/jffs2_1pass.c:1664:
  +^I^I^I^Iif (buf_ofs + buf_len < ofs + $
  
  ERROR: trailing whitespace
  #472: FILE: fs/jffs2/jffs2_1pass.c:1665:
  +^I^I^I^I^Isizeof(struct jffs2_raw_dirent) + $
  
  WARNING: line over 80 characters
  #473: FILE: fs/jffs2/jffs2_1pass.c:1666:
  +					((struct jffs2_raw_dirent *)node)->nsize) {
  
  WARNING: line over 80 characters
  #474: FILE: fs/jffs2/jffs2_1pass.c:1667:
  +					buf_len = min_t(uint32_t, buf_size, sector_ofs
  
  ERROR: trailing whitespace
  #475: FILE: fs/jffs2/jffs2_1pass.c:1668:
  +^I^I^I^I^I^I+ part->sector_size - ofs);^I$
  
  ERROR: space required before the open parenthesis '('
  #575: FILE: fs/jffs2/jffs2_1pass.c:665:
  +		if(pL->readbuf)
  
  ERROR: space prohibited after that '!' (ctx:BxW)
  #585: FILE: fs/jffs2/jffs2_1pass.c:1474:
  +	if(! jffs_init_1pass_list(part))
   	   ^
  
  ERROR: space required before the open parenthesis '('
  #585: FILE: fs/jffs2/jffs2_1pass.c:1474:
  +	if(! jffs_init_1pass_list(part))
  
  ERROR: trailing whitespace
  #587: FILE: fs/jffs2/jffs2_1pass.c:1476:
  +^I$
  
  ERROR: trailing whitespace
  #594: FILE: fs/jffs2/jffs2_1pass.c:1483:
  +^I$
  
  ERROR: space prohibited after that '!' (ctx:BxW)
  #615: FILE: fs/jffs2/jffs2_nand_1pass.c:810:
  +	if(! jffs_init_1pass_list(part))
   	   ^
  
  ERROR: space required before the open parenthesis '('
  #615: FILE: fs/jffs2/jffs2_nand_1pass.c:810:
  +	if(! jffs_init_1pass_list(part))
  
  ERROR: trailing whitespace
  #617: FILE: fs/jffs2/jffs2_nand_1pass.c:812:
  +^I$
  
  ERROR: patch seems to be corrupt (line wrapped?)
  #716: FILE: fs/jffs2/jffs2_1pass.c:800:
  inode, char *dest)
  
  WARNING: line over 80 characters
  #721: FILE: fs/jffs2/jffs2_1pass.c:804:
  +					putLabeledWord("UNKNOWN COMPRESSION METHOD =3D ", jNode->compr);
  
  WARNING: line over 80 characters
  #739: FILE: fs/jffs2/jffs2_1pass.c:1560:
  +		/* Scan only DEFAULT_EMPTY_SCAN_SIZE of 0xFF before declaring it's =
  
  WARNING: line over 80 characters
  #747: FILE: fs/jffs2/jffs2_1pass.c:1565:
  +			printf("Block at 0x%08x is empty (erased)\n", sector_ofs);
  
  WARNING: line over 80 characters
  #751: FILE: fs/jffs2/jffs2_1pass.c:1568:
  +		/* Now ofs is a complete physical flash offset as it always was... */
  
  WARNING: line over 80 characters
  #765: FILE: fs/jffs2/jffs2_nand_1pass.c:354:
  +				putLabeledWord("UNKNOWN COMPRESSION METHOD =3D ", inode->compr);
  
  WARNING: line over 80 characters
  #783: FILE: fs/jffs2/jffs2_nand_1pass.c:829:
  +		if (jffs2_fill_scan_buf(nand, buf, offset, DEFAULT_EMPTY_SCAN_SIZE))
  
  WARNING: line over 80 characters
  #790: FILE: fs/jffs2/jffs2_nand_1pass.c:832:
  +		/* Scan only DEFAULT_EMPTY_SCAN_SIZE of 0xFF before declaring it's =
  
  WARNING: line over 80 characters
  #792: FILE: fs/jffs2/jffs2_nand_1pass.c:833:
  +		while (ofs < DEFAULT_EMPTY_SCAN_SIZE && *(uint32_t *)(&buf[ofs]) =
  
  WARNING: line over 80 characters
  #801: FILE: fs/jffs2/jffs2_nand_1pass.c:837:
  +		if (jffs2_fill_scan_buf(nand, buf + DEFAULT_EMPTY_SCAN_SIZE, offset + =
  
  WARNING: line over 80 characters
  #936: FILE: fs/jffs2/jffs2_1pass.c:1616:
  +				/* If we're only checking the beginning of a block with a cleanmarker,
  
  ERROR: trailing whitespace
  #938: FILE: fs/jffs2/jffs2_1pass.c:1618:
  +^I^I^I^Iif((buf_ofs == sector_ofs) && $
  
  WARNING: suspect code indent for conditional statements (32, 36)
  #938: FILE: fs/jffs2/jffs2_1pass.c:1618:
  +				if((buf_ofs == sector_ofs) && 
  [...]
  +				    printf("%d bytes at start of block seems clean... assuming all clean\n",
  
  ERROR: space required before the open parenthesis '('
  #938: FILE: fs/jffs2/jffs2_1pass.c:1618:
  +				if((buf_ofs == sector_ofs) && 
  
  ERROR: trailing whitespace
  #939: FILE: fs/jffs2/jffs2_1pass.c:1619:
  +^I^I^I^I    (empty_start == sector_ofs +sizeof(struct jffs2_unknown_node))) {  $
  
  WARNING: line over 80 characters
  #939: FILE: fs/jffs2/jffs2_1pass.c:1619:
  +				    (empty_start == sector_ofs +sizeof(struct jffs2_unknown_node))) {  
  
  ERROR: need consistent spacing around '+' (ctx:WxV)
  #939: FILE: fs/jffs2/jffs2_1pass.c:1619:
  +				    (empty_start == sector_ofs +sizeof(struct jffs2_unknown_node))) {  
   				                               ^
  
  WARNING: line over 80 characters
  #940: FILE: fs/jffs2/jffs2_1pass.c:1620:
  +				    printf("%d bytes at start of block seems clean... assuming all clean\n",
  
  WARNING: line over 80 characters
  #941: FILE: fs/jffs2/jffs2_1pass.c:1621:
  +						EMPTY_SCAN_SIZE(part->sector_size));
  
  ERROR: trailing whitespace
  #958: FILE: fs/jffs2/jffs2_1pass.c:1644:
  +^I^I^I$
  
  total: 32 errors, 22 warnings, 369 lines checked
  
  NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
        scripts/cleanfile
  
  /home/dzu/transfer/am has style problems, please review.  If any of these errors
  are false positives report them to the maintainer, see
  CHECKPATCH in MAINTAINERS.

Cheers
  Detlev
  
[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob_plain;f=scripts/checkpatch.pl
[2] http://www.denx.de/wiki/U-Boot/CodingStyle

Patch

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
index 62ba250..bbfab2c 100644
--- a/fs/jffs2/jffs2_1pass.c
+++ b/fs/jffs2/jffs2_1pass.c
@@ -1596,16 +1596,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;
@@ -1615,6 +1613,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...
 				 */
@@ -1629,12 +1636,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;