Hello Mark,

Am 29.06.2015 um 07:02 schrieb Mark Tomlinson:

If a destination is not provided, jffs2_1pass_read_inode() only returns the length of the file. In this case, avoid reading all the data nodes, and return as soon as the length of the file is known.

Signed-off-by: Mark Tomlinson mark.tomlinson@alliedtelesis.co.nz

fs/jffs2/jffs2_1pass.c | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c index b1d6470..2335db1 100644 --- a/fs/jffs2/jffs2_1pass.c +++ b/fs/jffs2/jffs2_1pass.c @@ -719,6 +719,12 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest) } put_fl_mem(jNode, pL->readbuf); }

• /* If no destination is provided, we are done.

• * Just return the total size.
  

• */
  

please change this into

/* * If no destination is provided, we are done. * Just return the total size. */

to fit with Coding style.

• if (!dest) {

• return totalSize;
  

• }

no {} needed

Beside of this:

Acked-by: Heiko Schocher hs@denx.de

bye, Heiko

#endif

for (b = pL->frag.listHead; b != NULL; b = b->next) {

Hello Mark,

Am 29.06.2015 um 07:02 schrieb Mark Tomlinson:

Copying complete nodes from flash can be slow if the flash is slow to read. By only reading the data needed, the sorting operation can be made much faster.

The directory entry comparison function also had a two bugs. First, it did not ensure the name was copied, so the name comparison may have been faulty (although it would have worked with NOR flash). Second, setting the ino to zero to ignore the entry did not work, since this was either writing to a temporary buffer, or (for NOR flash) directly to flash. Either way, the change was not remembered.

Signed-off-by: Mark Tomlinson mark.tomlinson@alliedtelesis.co.nz

fs/jffs2/jffs2_1pass.c | 82 ++++++++++++++++++++++++++------------------------ 1 file changed, 42 insertions(+), 40 deletions(-)

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c index 2335db1..079bb73 100644 --- a/fs/jffs2/jffs2_1pass.c +++ b/fs/jffs2/jffs2_1pass.c @@ -598,14 +598,17 @@ insert_node(struct b_list *list, u32 offset) */ static int compare_inodes(struct b_node *new, struct b_node *old) {

• struct jffs2_raw_inode ojNew;
• struct jffs2_raw_inode ojOld;
• struct jffs2_raw_inode *jNew =

• (struct jffs2_raw_inode *)get_fl_mem(new->offset, sizeof(ojNew), &ojNew);
  

• struct jffs2_raw_inode *jOld =

• (struct jffs2_raw_inode *)get_fl_mem(old->offset, sizeof(ojOld), &ojOld);
  

• return jNew->version > jOld->version;

• /* Only read in the version info from flash, not the entire inode.

please fix your comment style globally, thanks1

• * This can make a big difference to speed if flash is slow.
  

• */
  

• u32 new_version;

• u32 old_version;

• get_fl_mem(new->offset + offsetof(struct jffs2_raw_inode, version),

•    sizeof(new_version), &new_version);
  

• get_fl_mem(old->offset + offsetof(struct jffs2_raw_inode, version),

•    sizeof(old_version), &old_version);
  

• return new_version > old_version; }

  /* Sort directory entries so all entries in the same directory

@@ -615,42 +618,41 @@ static int compare_inodes(struct b_node *new, struct b_node *old) */ static int compare_dirents(struct b_node *new, struct b_node *old) {

• struct jffs2_raw_dirent ojNew;
• struct jffs2_raw_dirent ojOld;
• struct jffs2_raw_dirent *jNew =

• (struct jffs2_raw_dirent *)get_fl_mem(new->offset, sizeof(ojNew), &ojNew);
  

• struct jffs2_raw_dirent *jOld =

• (struct jffs2_raw_dirent *)get_fl_mem(old->offset, sizeof(ojOld), &ojOld);
  

• int cmp;
• /* ascending sort by pino */
• if (jNew->pino != jOld->pino)

• return jNew->pino > jOld->pino;
  

• /* pino is the same, so use ascending sort by nsize, so

• * we don't do strncmp unless we really must.
  

• */
  

• if (jNew->nsize != jOld->nsize)

• return jNew->nsize > jOld->nsize;
  

• /* length is also the same, so use ascending sort by name

• */
  

• cmp = strncmp((char *)jNew->name, (char *)jOld->name, jNew->nsize);
• if (cmp != 0)

• return cmp > 0;
  

• /* we have duplicate names in this directory, so use ascending

• * sort by version
  

• /* Using NULL as the buffer for NOR flash prevents the entire node

• * being read. This makes most comparisons much quicker as only one
  

• * or two entries from the node will be used most of the time.
  

  */

• if (jNew->version > jOld->version) {

• /* since jNew is newer, we know jOld is not valid, so
  

•  * mark it with inode 0 and it will not be used
  

• struct jffs2_raw_dirent *jNew = get_node_mem(new->offset, NULL);
• struct jffs2_raw_dirent *jOld = get_node_mem(old->offset, NULL);
• int cmp;
• int ret;
• if (jNew->pino != jOld->pino) {

• /* ascending sort by pino */
  

• ret = jNew->pino > jOld->pino;
  

• } else if (jNew->nsize != jOld->nsize) {

• /* pino is the same, so use ascending sort by nsize, so
  

•  * we don't do strncmp unless we really must.
  

  */

• jOld->ino = 0;
  

• return 1;
  

• ret = jNew->nsize > jOld->nsize;
  

• } else {

• /* length is also the same, so use ascending sort by name
  

•  */
  

• cmp = strncmp((char *)jNew->name, (char *)jOld->name,
  

• 	jNew->nsize);
  

• if (cmp != 0) {
  

• 	ret = cmp > 0;
  

• } else {
  

• 	/* we have duplicate names in this directory,
  

• 	 * so use ascending sort by version
  

• 	 */
  

• 	ret = jNew->version > jOld->version;
  

• }
  

  }
• put_fl_mem(jNew, NULL);
• put_fl_mem(jOld, NULL);

• return 0;

• return ret; } #endif

Reviewed-by: Heiko Schocher hs@denx.de

bye, Heiko

Hello Mark,

Am 29.06.2015 um 07:02 schrieb Mark Tomlinson:

If multiple versions of a file exist, only the most recent version should be used. The scheme to write 0 for the inode in older versions did not work, since this would have required writing to flash.

The only time this caused an issue was listing a directory, where older versions of the file would still be seen. Since the directory entries are sorted, just look at the next entry in the list, and if it's the same move to that entry instead.

Signed-off-by: Mark Tomlinson mark.tomlinson@alliedtelesis.co.nz

fs/jffs2/jffs2_1pass.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-)

Reviewed-by: Heiko Schocher hs@denx.de

bye, Heiko

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c index 079bb73..1f6eea7 100644 --- a/fs/jffs2/jffs2_1pass.c +++ b/fs/jffs2/jffs2_1pass.c @@ -840,7 +840,6 @@ jffs2_1pass_find_inode(struct b_lists * pL, const char *name, u32 pino) jDir = (struct jffs2_raw_dirent *) get_node_mem(b->offset, pL->readbuf); if ((pino == jDir->pino) && (len == jDir->nsize) &&

•     (jDir->ino) &&	/* 0 for unlink */
  

  (!strncmp((char *)jDir->name, name, len))) { /* a match */ if (jDir->version < version) { put_fl_mem(jDir, pL->readbuf);

@@ -961,13 +960,42 @@ jffs2_1pass_list_inodes(struct b_lists * pL, u32 pino) for (b = pL->dir.listHead; b; b = b->next) { jDir = (struct jffs2_raw_dirent *) get_node_mem(b->offset, pL->readbuf);

• if ((pino == jDir->pino) && (jDir->ino)) { /* ino=0 -> unlink */
  

• if (pino == jDir->pino) {
  u32 i_version = 0;
  struct jffs2_raw_inode ojNode;
  struct jffs2_raw_inode *jNode, *i = NULL;
  

• 	struct b_node *b2 = pL->frag.listHead;
  

• 	struct b_node *b2;
  
  

• 	while (b2) {
  

+#ifdef CONFIG_SYS_JFFS2_SORT_FRAGMENTS

• 	/* Check for more recent versions of this file */
  

• 	int match;
  

• 	do {
  

• 		struct b_node *next = b->next;
  

• 		struct jffs2_raw_dirent *jDirNext;
  

• 		if (!next)
  

• 			break;
  

• 		jDirNext = (struct jffs2_raw_dirent *)
  

• 			get_node_mem(next->offset, NULL);
  

• 		match = jDirNext->pino == jDir->pino &&
  

• 			jDirNext->nsize == jDir->nsize &&
  

• 			strncmp((char *)jDirNext->name,
  

• 				(char *)jDir->name,
  

• 				jDir->nsize) == 0;
  

• 		if (match) {
  

• 			/* Use next. It is more recent */
  

• 			b = next;
  

• 			/* Update buffer with the new info */
  

• 			*jDir = *jDirNext;
  

• 			put_fl_mem(jDirNext, NULL);
  

• 		}
  

• 	} while (match);
  

+#endif

• 	if (jDir->ino == 0) {
  

• 		/* Deleted file */
  

• 		continue;
  

• 	}
  

• 	for (b2 = pL->frag.listHead; b2; b2 = b2->next) {
  	jNode = (struct jffs2_raw_inode *)
  		get_fl_mem(b2->offset, sizeof(ojNode), &ojNode);
  	if (jNode->ino == jDir->ino && jNode->version >= i_version) {
  

@@ -983,7 +1011,6 @@ jffs2_1pass_list_inodes(struct b_lists * pL, u32 pino) sizeof(*i), NULL); }

• 		b2 = b2->next;
  }
  
  dump_inode(pL, jDir, i);
  
  

Hello Mark,

Am 29.06.2015 um 07:02 schrieb Mark Tomlinson:

jffs2_1pass_read_inode() would read the entire data for each node in the filesystem, regardless of whether it was part of the file to be loaded or not. By only reading the header data for an inode, and then reading the data only when it is found to be part of the file to be loaded, much copying of data is saved.

jffs2_1pass_list_inodes() read each inode for every file in the directory into a buffer. By using NULL as a buffer pointer, NOR flash simply returns a pointer, and therefore avoids a memory copy.

Signed-off-by: Mark Tomlinson mark.tomlinson@alliedtelesis.co.nz

fs/jffs2/jffs2_1pass.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)

Reviewed-by: Heiko Schocher hs@denx.de

bye, Heiko

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c index 1f6eea7..80210be 100644 --- a/fs/jffs2/jffs2_1pass.c +++ b/fs/jffs2/jffs2_1pass.c @@ -730,8 +730,12 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest) #endif

for (b = pL->frag.listHead; b != NULL; b = b->next) {

• jNode = (struct jffs2_raw_inode *) get_node_mem(b->offset,
  

• 						pL->readbuf);
  

• /* Copy just the node and not the data at this point,
  

•  * since we don't yet know if we need this data.
  

•  */
  

• jNode = (struct jffs2_raw_inode *)get_fl_mem(b->offset,
  

• 		sizeof(struct jffs2_raw_inode),
  

• 		pL->readbuf);
  

  if (inode == jNode->ino) { #if 0 putLabeledWord("\r\n\r\nread_inode: totlen = ", jNode->totlen);

@@ -755,7 +759,14 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest) #endif

	if(dest) {

• 		src = ((uchar *) jNode) + sizeof(struct jffs2_raw_inode);
  

• 		/* Now that the inode has been checked,
  

• 		 * read the entire inode, including data.
  

• 		 */
  

• 		put_fl_mem(jNode, pL->readbuf);
  

• 		jNode = (struct jffs2_raw_inode *)
  

• 			get_node_mem(b->offset, pL->readbuf);
  

• 		src = ((uchar *)jNode) +
  

• 			sizeof(struct jffs2_raw_inode);
  	/* ignore data behind latest known EOF */
  	if (jNode->offset > totalSize) {
  		put_fl_mem(jNode, pL->readbuf);
  

@@ -962,7 +973,6 @@ jffs2_1pass_list_inodes(struct b_lists * pL, u32 pino) pL->readbuf); if (pino == jDir->pino) { u32 i_version = 0;

• 	struct jffs2_raw_inode ojNode;
  struct jffs2_raw_inode *jNode, *i = NULL;
  struct b_node *b2;
  
  

@@ -997,8 +1007,10 @@ jffs2_1pass_list_inodes(struct b_lists * pL, u32 pino)

	for (b2 = pL->frag.listHead; b2; b2 = b2->next) {
		jNode = (struct jffs2_raw_inode *)

• 			get_fl_mem(b2->offset, sizeof(ojNode), &ojNode);
  

• 		if (jNode->ino == jDir->ino && jNode->version >= i_version) {
  

• 			get_fl_mem(b2->offset, sizeof(*jNode),
  

• 				   NULL);
  

• 		if (jNode->ino == jDir->ino &&
  

• 		    jNode->version >= i_version) {
  		i_version = jNode->version;
  		if (i)
  			put_fl_mem(i, NULL);
  

@@ -1011,6 +1023,7 @@ jffs2_1pass_list_inodes(struct b_lists * pL, u32 pino) sizeof(*i), NULL); }

• 		put_fl_mem(jNode, NULL);
  }
  
  dump_inode(pL, jDir, i);
  
  

Hello Mark,

please fix Tom rinis mail address globally for your patchset into

Tom Rini trini@konsulko.com

I fixed it for my response manually, thanks!

BTW: You can use patman for creating patches/patchset. Look into u-boot:tools/patman

Am 29.06.2015 um 07:02 schrieb Mark Tomlinson:

If the flash is slow, reading less from the flash into buffers makes the process faster.

Signed-off-by: Mark Tomlinson mark.tomlinson@alliedtelesis.co.nz

fs/jffs2/jffs2_1pass.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)

Reviewed-by: Heiko Schocher hs@denx.de

bye, Heiko

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c index 80210be..10bd7be 100644 --- a/fs/jffs2/jffs2_1pass.c +++ b/fs/jffs2/jffs2_1pass.c @@ -1493,7 +1493,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; char *buf;

  nr_sectors = lldiv(part->size, part->sector_size);

@@ -1505,7 +1505,7 @@ jffs2_1pass_build_lists(struct part_info * part) /* if we are building a list we need to refresh the cache. */ jffs_init_1pass_list(part); pL = (struct b_lists *)part->jffs2_priv;

• buf = malloc(buf_size);

• buf = malloc(DEFAULT_EMPTY_SCAN_SIZE); puts ("Scanning JFFS2 FS: ");

  /* start at the beginning of the partition */

@@ -1521,6 +1521,8 @@ jffs2_1pass_build_lists(struct part_info * part) int ret; #endif

• /* Set buf_size to maximum length */
  

• buf_size = DEFAULT_EMPTY_SCAN_SIZE;
  

  WATCHDOG_RESET();

  #ifdef CONFIG_JFFS2_SUMMARY

@@ -1595,6 +1597,10 @@ jffs2_1pass_build_lists(struct part_info * part)

ofs += sector_ofs;
prevofs = ofs - 1;

• /* Set buf_size down to the minimum size required.
  

•  * This prevents reading in chunks of flash data unnecessarily.
  

•  */
  

• buf_size = sizeof(union jffs2_node_union);
  
  

  scan_more: while (ofs < sector_ofs + part->sector_size) {

@@ -1675,13 +1681,18 @@ 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,
  

• 					sizeof(struct jffs2_raw_inode),
  

• 					sector_ofs +
  

• 					part->sector_size -
  

• 					ofs);
  		get_fl_mem((u32)part->offset + ofs,
  			   buf_len, buf);
  		buf_ofs = ofs;
  		node = (void *)buf;
  	}
  

• 		if (!inode_crc((struct jffs2_raw_inode *) node))
  

• 		       break;
  

• 		if (!inode_crc((struct jffs2_raw_inode *)node))
  

• 			break;
  
  	if (insert_node(&pL->frag, (u32) part->offset +
  			ofs) == NULL) {
  

@@ -1698,6 +1709,11 @@ jffs2_1pass_build_lists(struct part_info * part) ((struct jffs2_raw_dirent *) node)->nsize) {

• 			buf_len = min_t(uint32_t,
  

• 					node->totlen,
  

• 					sector_ofs +
  

• 					part->sector_size -
  

• 					ofs);
  		get_fl_mem((u32)part->offset + ofs,
  			   buf_len, buf);
  		buf_ofs = ofs;
  
  

Hello Mark,

Am 29.06.2015 um 07:02 schrieb Mark Tomlinson:

The scan code is similar to the linux kernel, but the kernel defines a much smaller size to scan through before deciding a sector is blank. Assuming that what is in the kernel is OK, make these two match.

On its own, this change makes no difference to scanning of any sectors which have a clean marker at the beginning, since the entire sector is not blank.

Signed-off-by: Mark Tomlinson mark.tomlinson@alliedtelesis.co.nz

fs/jffs2/jffs2_1pass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Heiko Schocher hs@denx.de

bye, Heiko

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c index 10bd7be..d78fb06 100644 --- a/fs/jffs2/jffs2_1pass.c +++ b/fs/jffs2/jffs2_1pass.c @@ -1472,7 +1472,7 @@ dump_dirents(struct b_lists *pL) } #endif

-#define DEFAULT_EMPTY_SCAN_SIZE 4096 +#define DEFAULT_EMPTY_SCAN_SIZE 256

static inline uint32_t EMPTY_SCAN_SIZE(uint32_t sector_size) {

Hello Mark,

Am 29.06.2015 um 07:02 schrieb Mark Tomlinson:

If a sector has a CLEANMARKER at the beginning, it indicates that the entire sector has been erased. Therefore, if this is found, we can skip the entire block. This was not being done before this patch.

The code now does the same as the kernel does when encountering a CLEANMARKER. It still checks that the next few words are FFFFFFFF, and if so, the block is assumed to be empty, and so is skipped.

Signed-off-by: Mark Tomlinson mark.tomlinson@alliedtelesis.co.nz

fs/jffs2/jffs2_1pass.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)

Beside of your comment style

Reviewed-by: Heiko Schocher hs@denx.de

bye, Heiko

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c index d78fb06..26a748f 100644 --- a/fs/jffs2/jffs2_1pass.c +++ b/fs/jffs2/jffs2_1pass.c @@ -1520,6 +1520,8 @@ jffs2_1pass_build_lists(struct part_info * part) uint32_t sumlen; int ret; #endif

• /* Indicates a sector with a CLEANMARKER was found */
  

• int clean_sector = 0;
  
  

  /* Set buf_size to maximum length */ buf_size = DEFAULT_EMPTY_SCAN_SIZE;

@@ -1643,6 +1645,13 @@ jffs2_1pass_build_lists(struct part_info * part) ofs += 4; } /* Ran off end. */

• 		/* If this sector had a clean marker at the
  

• 		 * beginning, and immediately following this
  

• 		 * have been a bunch of FF bytes, treat the
  

• 		 * entire sector as empty.
  

• 		 */
  

• 		if (clean_sector)
  

• 			break;
  
  	/* See how much more there is to read in this
  	 * eraseblock...
  

@@ -1664,6 +1673,10 @@ jffs2_1pass_build_lists(struct part_info * part) buf_ofs = ofs; goto more_empty; }

• 	/* Found something not erased in the sector, so reset
  

• 	 * the 'clean_sector' flag.
  

• 	 */
  

• 	clean_sector = 0;
  if (node->magic != JFFS2_MAGIC_BITMASK ||
  		!hdr_crc(node)) {
  	ofs += 4;
  

@@ -1745,6 +1758,15 @@ jffs2_1pass_build_lists(struct part_info * part) "%d != %zu\n", node->totlen, sizeof(struct jffs2_unknown_node));

• 		if ((node->totlen ==
  

• 		     sizeof(struct jffs2_unknown_node)) &&
  

• 		    (ofs == sector_ofs)) {
  

• 			/* Found a CLEANMARKER at the beginning
  

• 			 * of the sector. It's in the correct
  

• 			 * place with correct size and CRC.
  

• 			 */
  

• 			clean_sector = 1;
  

• 		}
  	break;
  case JFFS2_NODETYPE_PADDING:
  	if (node->totlen < sizeof(struct jffs2_unknown_node))