diff mbox

[U-Boot,1/7] JFFS2: Bug fix for summary support

Message ID 001c01cc0230$7dba7840$6401a8c0@LENOVOE5CA6843
State Deferred
Delegated to: Detlev Zundel
Headers show

Commit Message

Baidu Boy April 24, 2011, 3:34 a.m. UTC
1/ Get the latest DIRENT
 For example, if you create a file in linux jffs2 which config summary
 support, then you delete the file , you will not see the file  in
 linux jffs2. But you can also see this file in uboot after you reset
 the system. That is because all the nodes in jffs2 which config summary
 will not be marked as obsolete. The deleted file's DIRENT node will be
 seen in uboot. So what we need to do is to get the latest DIRENT whose
 ino in DIRENT is 0. Than we will not see this file in uboot which is
 what we want.
 2/ Add CONFIG_SYS_JFFS2_SORT_FRAGMENTS definition,if we config jffs2
 summary in uboot.
 All the inodes of a file will not marked as obsolete, if they do not
 sort in the list struct b_node *, the latest data in inode may be
 overwritten by the older one.

Signed-off-by: Baidu Liu <liucai.lfn@gmail.com>
---
 fs/jffs2/jffs2_1pass.c      |   17 ++++++++++++-----
 fs/jffs2/jffs2_nand_1pass.c |   17 +++++++++++++----
 include/jffs2/jffs2.h       |   10 ++++++++++
 3 files changed, 35 insertions(+), 9 deletions(-)

Comments

Detlev Zundel April 29, 2011, 12:57 p.m. UTC | #1
Hi,

>  1/ Get the latest DIRENT
>  For example, if you create a file in linux jffs2 which config summary
>  support, then you delete the file , you will not see the file  in
>  linux jffs2. But you can also see this file in uboot after you reset
>  the system. That is because all the nodes in jffs2 which config summary
>  will not be marked as obsolete. The deleted file's DIRENT node will be
>  seen in uboot. So what we need to do is to get the latest DIRENT whose
>  ino in DIRENT is 0.

Sorry, but I do not understand that last sentence.  Can you clarify this
please?

>  Than we will not see this file in uboot which is
>  what we want.
>
>  2/ Add CONFIG_SYS_JFFS2_SORT_FRAGMENTS definition,if we config jffs2
>  summary in uboot.
>  All the inodes of a file will not marked as obsolete, if they do not
>  sort in the list struct b_node *, the latest data in inode may be
>  overwritten by the older one.

Also I have trouble making sense of this text.

>
> Signed-off-by: Baidu Liu <liucai.lfn@gmail.com>
> ---
>  fs/jffs2/jffs2_1pass.c      |   17 ++++++++++++-----
>  fs/jffs2/jffs2_nand_1pass.c |   17 +++++++++++++----
>  include/jffs2/jffs2.h       |   10 ++++++++++
>  3 files changed, 35 insertions(+), 9 deletions(-)

[...]

> diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h
> index 651f94c..5b006c0 100644
> --- a/include/jffs2/jffs2.h
> +++ b/include/jffs2/jffs2.h
> @@ -41,6 +41,16 @@
>  #include <asm/types.h>
>  #include <jffs2/load_kernel.h>
>  
> +#ifdef CONFIG_JFFS2_SUMMARY
> +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
> +/*
> +we should define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if
> +CONFIG_JFFS2_SUMMARY is enabled.
> +*/
> +#define CONFIG_SYS_JFFS2_SORT_FRAGMENTS
> +#endif
> +#endif
> +
>  #define JFFS2_SUPER_MAGIC 0x72b6
>  
>  /* Values we may expect to find in the 'magic' field */

If JFFS2_SUMMARY _needs_ SORT_FRAGMENTS, then we should say so, i.e.

/*
CONFIG_JFFS2_SUMMARY will not work correctly without
CONFIG_SYS_JFFS2_SORT_FRAGMENTS
*/

Alas, technically I do not understand why that is the case.  So I invite
people more knowledgeable with JFFS2 to comment on this bit.

[time passes]

Wait a minute - I tried to understand the code here - is it possible
that SORT_FRAGMENTS really is needed _whenever_ we have a read-write
JFFS2 filesystem?  I.e. even without summary support we will have
problems without SORT_FRAGMENTS?

Wow, if this is true, then the option is certainly named completely
misleading and most boards using JFFS2 actually use incorrect code....

We should define this option by default and only let people undefine it
if they know exactly what they do.

Cheers
  Detlev
Baidu Boy April 29, 2011, 1:49 p.m. UTC | #2
Hi,  Detlev :

2011/4/29 Detlev Zundel <dzu@denx.de>:
> Hi,
>
>>  1/ Get the latest DIRENT
>>  For example, if you create a file in linux jffs2 which config summary
>>  support, then you delete the file , you will not see the file  in
>>  linux jffs2. But you can also see this file in uboot after you reset
>>  the system. That is because all the nodes in jffs2 which config summary
>>  will not be marked as obsolete. The deleted file's DIRENT node will be
>>  seen in uboot. So what we need to do is to get the latest DIRENT whose
>>  ino in DIRENT is 0.
>
> Sorry, but I do not understand that last sentence.  Can you clarify this
> please?

This is just give an example. If you create a file, then you DELETE it
in jffs2 .
In previous code where you do NOT config SUMMARY. There will be two
DIRENT nodes in flash. One is obsoleted where the ino=1, another one
is valid where the ino=0. When we type "ls " command, the file will
NOT shown because uboot will not show the DIRENT where the ino =0
which means the file is deleted.
But when we config SUMMARY in  kernel jffs2. The two DIRENTs are BOTH
valid. When we type "ls " command, the file will be  SHOWN  because we
find the first DIRENT where the ino is not 0. The result is  obviously
wrong.
So what we need to do is to find the DIRENT with the latest version.
In this example ,the second DIRENT has highest version which means
latest. And the ino is 0. Then we will not show it.

>>  Than we will not see this file in uboot which is
>>  what we want.
>>
>>  2/ Add CONFIG_SYS_JFFS2_SORT_FRAGMENTS definition,if we config jffs2
>>  summary in uboot.
>>  All the inodes of a file will not marked as obsolete, if they do not
>>  sort in the list struct b_node *, the latest data in inode may be
>>  overwritten by the older one.
>
> Also I have trouble making sense of this text.
>
There may have many data inode in jfffs2 which represent the same
range in the same file, when we use SUMMARY. They are overlapped. But
only the data inode which has the latest version represent the actual
data. So we need to sort the data inode, where we make the latest data
inode after the obsoleted data inode. Then the latest data inode will
not be overwritten when use use "fsload" command in uboot.

>>
>> Signed-off-by: Baidu Liu <liucai.lfn@gmail.com>
>> ---
>>  fs/jffs2/jffs2_1pass.c      |   17 ++++++++++++-----
>>  fs/jffs2/jffs2_nand_1pass.c |   17 +++++++++++++----
>>  include/jffs2/jffs2.h       |   10 ++++++++++
>>  3 files changed, 35 insertions(+), 9 deletions(-)
>
> [...]
>
>> diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h
>> index 651f94c..5b006c0 100644
>> --- a/include/jffs2/jffs2.h
>> +++ b/include/jffs2/jffs2.h
>> @@ -41,6 +41,16 @@
>>  #include <asm/types.h>
>>  #include <jffs2/load_kernel.h>
>>
>> +#ifdef CONFIG_JFFS2_SUMMARY
>> +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
>> +/*
>> +we should define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if
>> +CONFIG_JFFS2_SUMMARY is enabled.
>> +*/
>> +#define CONFIG_SYS_JFFS2_SORT_FRAGMENTS
>> +#endif
>> +#endif
>> +
>>  #define JFFS2_SUPER_MAGIC 0x72b6
>>
>>  /* Values we may expect to find in the 'magic' field */
>
> If JFFS2_SUMMARY _needs_ SORT_FRAGMENTS, then we should say so, i.e.
>
> /*
> CONFIG_JFFS2_SUMMARY will not work correctly without
> CONFIG_SYS_JFFS2_SORT_FRAGMENTS
> */
>
> Alas, technically I do not understand why that is the case.  So I invite
> people more knowledgeable with JFFS2 to comment on this bit.
>
> [time passes]
>
> Wait a minute - I tried to understand the code here - is it possible
> that SORT_FRAGMENTS really is needed _whenever_ we have a read-write
> JFFS2 filesystem?  I.e. even without summary support we will have
> problems without SORT_FRAGMENTS?
>
No, SORT_FRAGMENTS only need when SUMMARY is configed.
Because all the nodes including the DIRENT inode and DATA inode are
all valid. So we need to sort them.

> Wow, if this is true, then the option is certainly named completely
> misleading and most boards using JFFS2 actually use incorrect code....
>
The previous code works well when SUMAMRY is not configed. We have
plenty of  board use
JFFS2 in uboot where the SUMMARY is not configed. But the uboot does
not work when sumamry configed, so I submit these patches to fix this
issue.

> We should define this option by default and only let people undefine it
> if they know exactly what they do.
>
We do not need to define SORT when we do not use SUMMARY. But we mush
use SORT when we use SUMMARY.

This patch is most importent in the 7 patches.

The jffs2 inode is not easy to understand. And I think the comments in
the patch is enough if the reader know about jffs2. Too many commants
is not need just as previous code did not give us too many comments.
In kernel there are even little comment to explain how the jffs2
works.

Thanks
Detlev Zundel April 29, 2011, 5:18 p.m. UTC | #3
Hi Baidu,

> Hi,  Detlev :
>
> 2011/4/29 Detlev Zundel <dzu@denx.de>:
>> Hi,
>>
>>>  1/ Get the latest DIRENT
>>>  For example, if you create a file in linux jffs2 which config summary
>>>  support, then you delete the file , you will not see the file  in
>>>  linux jffs2. But you can also see this file in uboot after you reset
>>>  the system. That is because all the nodes in jffs2 which config summary
>>>  will not be marked as obsolete. The deleted file's DIRENT node will be
>>>  seen in uboot. So what we need to do is to get the latest DIRENT whose
>>>  ino in DIRENT is 0.
>>
>> Sorry, but I do not understand that last sentence.  Can you clarify this
>> please?
>
> This is just give an example. If you create a file, then you DELETE it
> in jffs2 .
> In previous code where you do NOT config SUMMARY. There will be two
> DIRENT nodes in flash. One is obsoleted where the ino=1, another one
> is valid where the ino=0. When we type "ls " command, the file will
> NOT shown because uboot will not show the DIRENT where the ino =0
> which means the file is deleted.
> But when we config SUMMARY in  kernel jffs2. The two DIRENTs are BOTH
> valid. When we type "ls " command, the file will be  SHOWN  because we
> find the first DIRENT where the ino is not 0. The result is  obviously
> wrong.

The more I read the jffs2 code, the more I do not understand.  In my
eyes, the summary feature should just be a shortcut for not scanning the
whole eraseblock, but I fail to see substantial differences.
Specifically, I do not understand why your example should be a problem.
So maybe you should correct me where I'm wrong:

- we create a file in Linux.  So we get a dirent with versino = 1, ino
  <> 0.  This will be in the summary of its jeb.

- we delete the file in Linux.  So we get a dirent with version = 2 and
  ino = 0.  If this is not in the same jeb than the previous dirent,
  then it will be in the summary of its own jeb, otherwise it will
  obsolete the old dirent in the summary directly.

Is this correct?

Now when U-Boot reads the summaries, it will enter both dirents into its
list and 'jffs2_1pass_find_inode' will find the dirent with version 2
having ino = 0.  So the file should not be listed.

Where is this going wrong?

I'm really trying hard to understand the changes, but to comment on them
sensibly, I have to understand what is going on...

> So what we need to do is to find the DIRENT with the latest version.
> In this example ,the second DIRENT has highest version which means
> latest. And the ino is 0. Then we will not show it.

Yes, I think I understand what you try to do, I explicitely said that I
did not understand this sentence:

"So what we need to do is to get the latest DIRENT whose ino in DIRENT
is 0"

What does this mean? Do you mean "Before we list any DIRENT, we first
check if we find a later DIRENT which has ino = 0.  If this is the case,
then the file has been erased and should not be displayed"?

Alas as I wrote above, I do not understand why this is not handled
already by current code.

>>>  Than we will not see this file in uboot which is
>>>  what we want.
>>>
>>>  2/ Add CONFIG_SYS_JFFS2_SORT_FRAGMENTS definition,if we config jffs2
>>>  summary in uboot.
>>>  All the inodes of a file will not marked as obsolete, if they do not
>>>  sort in the list struct b_node *, the latest data in inode may be
>>>  overwritten by the older one.
>>
>> Also I have trouble making sense of this text.
>>
> There may have many data inode in jfffs2 which represent the same
> range in the same file, when we use SUMMARY. They are overlapped. But
> only the data inode which has the latest version represent the actual
> data. So we need to sort the data inode, where we make the latest data
> inode after the obsoleted data inode. Then the latest data inode will
> not be overwritten when use use "fsload" command in uboot.

Sorry, I am still not understanding this.  Can you please state
explicitely where you think there is an error and how your changes fix
the error?

Using summary or not, we can have multiple fragments for a file in a
JFFS2 filesystem and the code seems to handle this fine.  The summary
should only be a optimization for the scanning phase, it should not
behave differently.  This is why I want to understand this.  SUMMARY and
SORT_FRAGMENTS should be orthogonal.  If they are not, then we most
certainly have another problem.

[...]

>>> diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h
>>> index 651f94c..5b006c0 100644
>>> --- a/include/jffs2/jffs2.h
>>> +++ b/include/jffs2/jffs2.h
>>> @@ -41,6 +41,16 @@
>>>  #include <asm/types.h>
>>>  #include <jffs2/load_kernel.h>
>>>
>>> +#ifdef CONFIG_JFFS2_SUMMARY
>>> +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
>>> +/*
>>> +we should define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if
>>> +CONFIG_JFFS2_SUMMARY is enabled.
>>> +*/
>>> +#define CONFIG_SYS_JFFS2_SORT_FRAGMENTS
>>> +#endif
>>> +#endif
>>> +
>>>  #define JFFS2_SUPER_MAGIC 0x72b6
>>>
>>>  /* Values we may expect to find in the 'magic' field */
>>
>> If JFFS2_SUMMARY _needs_ SORT_FRAGMENTS, then we should say so, i.e.
>>
>> /*
>> CONFIG_JFFS2_SUMMARY will not work correctly without
>> CONFIG_SYS_JFFS2_SORT_FRAGMENTS
>> */
>>
>> Alas, technically I do not understand why that is the case.  So I invite
>> people more knowledgeable with JFFS2 to comment on this bit.
>>
>> [time passes]
>>
>> Wait a minute - I tried to understand the code here - is it possible
>> that SORT_FRAGMENTS really is needed _whenever_ we have a read-write
>> JFFS2 filesystem?  I.e. even without summary support we will have
>> problems without SORT_FRAGMENTS?
>>
> No, SORT_FRAGMENTS only need when SUMMARY is configed.
> Because all the nodes including the DIRENT inode and DATA inode are
> all valid. So we need to sort them.

What?  On JFFS2 we can always have multiple DIRENT and DATA nodes and we
always have to find the highest version.  

As far as I can see the only effect of using SORT_FRAGMENTS is that the
compare function sets ino of known obsolete dirents to 0 as a _side
effect_.  This is rather nasty coding practice and but completely
independent of Linux and SUMMARY.

>> Wow, if this is true, then the option is certainly named completely
>> misleading and most boards using JFFS2 actually use incorrect code....
>>
> The previous code works well when SUMAMRY is not configed. We have
> plenty of  board use
> JFFS2 in uboot where the SUMMARY is not configed. But the uboot does
> not work when sumamry configed, so I submit these patches to fix this
> issue.

Ok, I was overly pessimistic interpreting this comment:

 * The fragment sorting feature must be enabled by CONFIG_SYS_JFFS2_SORT_FRAGMENTS.
 * Sorting is done while adding fragments to the lists, which is more or less a
 * bubble sort. This takes a lot of time, and is most probably not an issue if
 * the boot filesystem is always mounted readonly.
 *
 * You should define it if the boot filesystem is mounted writable, and updates
 * to the boot files are done by copying files to that filesystem.

>> We should define this option by default and only let people undefine it
>> if they know exactly what they do.
>>
> We do not need to define SORT when we do not use SUMMARY. But we mush
> use SORT when we use SUMMARY.

I am not convinced.  Both options should be orthogonal.

> This patch is most importent in the 7 patches.
>
> The jffs2 inode is not easy to understand. And I think the comments in
> the patch is enough if the reader know about jffs2. Too many commants
> is not need just as previous code did not give us too many comments.
> In kernel there are even little comment to explain how the jffs2
> works.

You must be joking.  Either you can explain why your changes are indeed
the correct thing or the community will not consider them.  It is that
easy.

Best wishes
  Detlev
Baidu Boy April 30, 2011, 12:46 a.m. UTC | #4
Hi,Detlev :

2011/4/30 Detlev Zundel <dzu@denx.de>:
> Hi Baidu,
>
>> Hi,  Detlev :
>>
>> 2011/4/29 Detlev Zundel <dzu@denx.de>:
>>> Hi,
>>>
>>>>  1/ Get the latest DIRENT
>>>>  For example, if you create a file in linux jffs2 which config summary
>>>>  support, then you delete the file , you will not see the file  in
>>>>  linux jffs2. But you can also see this file in uboot after you reset
>>>>  the system. That is because all the nodes in jffs2 which config summary
>>>>  will not be marked as obsolete. The deleted file's DIRENT node will be
>>>>  seen in uboot. So what we need to do is to get the latest DIRENT whose
>>>>  ino in DIRENT is 0.
>>>
>>> Sorry, but I do not understand that last sentence.  Can you clarify this
>>> please?
>>
>> This is just give an example. If you create a file, then you DELETE it
>> in jffs2 .
>> In previous code where you do NOT config SUMMARY. There will be two
>> DIRENT nodes in flash. One is obsoleted where the ino=1, another one
>> is valid where the ino=0. When we type "ls " command, the file will
>> NOT shown because uboot will not show the DIRENT where the ino =0
>> which means the file is deleted.
>> But when we config SUMMARY in  kernel jffs2. The two DIRENTs are BOTH
>> valid. When we type "ls " command, the file will be  SHOWN  because we
>> find the first DIRENT where the ino is not 0. The result is  obviously
>> wrong.
>
> The more I read the jffs2 code, the more I do not understand.  In my
> eyes, the summary feature should just be a shortcut for not scanning the
> whole eraseblock, but I fail to see substantial differences.
> Specifically, I do not understand why your example should be a problem.
> So maybe you should correct me where I'm wrong:
>
> - we create a file in Linux.  So we get a dirent with versino = 1, ino
>  <> 0.  This will be in the summary of its jeb.
NO, the summary inode is only written by jffs2 when one jeb is almost full.
In some jeb which are not full, there is no summary node.

> - we delete the file in Linux.  So we get a dirent with version = 2 and
>  ino = 0.  If this is not in the same jeb than the previous dirent,
>  then it will be in the summary of its own jeb, otherwise it will
>  obsolete the old dirent in the summary directly.
Firstly, if the jeb is not almost full, there is NO summary inode.
secondly, The most important different between the jffs2 not configed
SUMMARY and the jffs2 configed jffs2 is :
There is no obsolete inode in jffs2 configed SUMMARY.

> Is this correct?
>
> Now when U-Boot reads the summaries, it will enter both dirents into its
> list and 'jffs2_1pass_find_inode' will find the dirent with version 2
> having ino = 0.  So the file should not be listed.
Acturally, the both dirent is belong to the same file. The latest
version represent the last status of the file

> Where is this going wrong?
>
> I'm really trying hard to understand the changes, but to comment on them
> sensibly, I have to understand what is going on...
>
>> So what we need to do is to find the DIRENT with the latest version.
>> In this example ,the second DIRENT has highest version which means
>> latest. And the ino is 0. Then we will not show it.
>
> Yes, I think I understand what you try to do, I explicitely said that I
> did not understand this sentence:
>
> "So what we need to do is to get the latest DIRENT whose ino in DIRENT
> is 0"
>
> What does this mean? Do you mean "Before we list any DIRENT, we first
> check if we find a later DIRENT which has ino = 0.  If this is the case,
> then the file has been erased and should not be displayed"?
Yes, this is happened when the summary is configed. Because all the
inode will be scanned when summary is configed.

> Alas as I wrote above, I do not understand why this is not handled
> already by current code.
>
>>>>  Than we will not see this file in uboot which is
>>>>  what we want.
>>>>
>>>>  2/ Add CONFIG_SYS_JFFS2_SORT_FRAGMENTS definition,if we config jffs2
>>>>  summary in uboot.
>>>>  All the inodes of a file will not marked as obsolete, if they do not
>>>>  sort in the list struct b_node *, the latest data in inode may be
>>>>  overwritten by the older one.
>>>
>>> Also I have trouble making sense of this text.
>>>
>> There may have many data inode in jfffs2 which represent the same
>> range in the same file, when we use SUMMARY. They are overlapped. But
>> only the data inode which has the latest version represent the actual
>> data. So we need to sort the data inode, where we make the latest data
>> inode after the obsoleted data inode. Then the latest data inode will
>> not be overwritten when use use "fsload" command in uboot.
>
> Sorry, I am still not understanding this.  Can you please state
> explicitely where you think there is an error and how your changes fix
> the error?
>
> Using summary or not, we can have multiple fragments for a file in a
> JFFS2 filesystem and the code seems to handle this fine.  The summary
> should only be a optimization for the scanning phase, it should not
> behave differently.  This is why I want to understand this.  SUMMARY and
> SORT_FRAGMENTS should be orthogonal.  If they are not, then we most
> certainly have another problem.
>
> [...]
>
>>>> diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h
>>>> index 651f94c..5b006c0 100644
>>>> --- a/include/jffs2/jffs2.h
>>>> +++ b/include/jffs2/jffs2.h
>>>> @@ -41,6 +41,16 @@
>>>>  #include <asm/types.h>
>>>>  #include <jffs2/load_kernel.h>
>>>>
>>>> +#ifdef CONFIG_JFFS2_SUMMARY
>>>> +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
>>>> +/*
>>>> +we should define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if
>>>> +CONFIG_JFFS2_SUMMARY is enabled.
>>>> +*/
>>>> +#define CONFIG_SYS_JFFS2_SORT_FRAGMENTS
>>>> +#endif
>>>> +#endif
>>>> +
>>>>  #define JFFS2_SUPER_MAGIC 0x72b6
>>>>
>>>>  /* Values we may expect to find in the 'magic' field */
>>>
>>> If JFFS2_SUMMARY _needs_ SORT_FRAGMENTS, then we should say so, i.e.
>>>
>>> /*
>>> CONFIG_JFFS2_SUMMARY will not work correctly without
>>> CONFIG_SYS_JFFS2_SORT_FRAGMENTS
>>> */
>>>
>>> Alas, technically I do not understand why that is the case.  So I invite
>>> people more knowledgeable with JFFS2 to comment on this bit.
>>>
>>> [time passes]
>>>
>>> Wait a minute - I tried to understand the code here - is it possible
>>> that SORT_FRAGMENTS really is needed _whenever_ we have a read-write
>>> JFFS2 filesystem?  I.e. even without summary support we will have
>>> problems without SORT_FRAGMENTS?
>>>
>> No, SORT_FRAGMENTS only need when SUMMARY is configed.
>> Because all the nodes including the DIRENT inode and DATA inode are
>> all valid. So we need to sort them.
>
> What?  On JFFS2 we can always have multiple DIRENT and DATA nodes and we
> always have to find the highest version.
For DIRENT, we shall find the latest DIRENT with latest version.
For INODE, because we do not create fully structure to represent the
information in the jffs2 while the kernel does. So we should sort the
inode to avoid the latest data be overwritten by the old one

You can dump the flash image in the flash where the summary is
configed, the cleanmark is still 0x1985 2003  after the jffs2 write
inode in the jeb.
And all the DIRENTs are 0xe001, all the data inode is 0xe002.

While the summary is not configed, the obsoleted DIRENT  is 0xc001.
And the obsoleted data inode is 0xc002.

We should do our best to  make the UBOOT work well. And for jffs2, the
best way to know the principle is to exercise, after this, then you
will know why I do these changes.

We can see the summary MACRO has been added in UBOOT for a long time.
But it does not work. So we can get the conclusion that people do not
use the uboot to read file from jffs2 when summary is configed.

UBOOT should be more frequently used in board. Not just put some
unused functions in it.
diff mbox

Patch

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
index c4f7445..a34756e 100644
--- a/fs/jffs2/jffs2_1pass.c
+++ b/fs/jffs2/jffs2_1pass.c
@@ -748,8 +748,8 @@  jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest)
 
 			if(dest) {
 				src = ((uchar *) jNode) + sizeof(struct jffs2_raw_inode);
-				/* ignore data behind latest known EOF */
-				if (jNode->offset > totalSize) {
+				/* ignore data which exceed file length */
+				if (jNode->offset + jNode->dsize > totalSize) {
 					put_fl_mem(jNode, pL->readbuf);
 					continue;
 				}
@@ -836,9 +836,8 @@  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) {
+			if (jDir->version < version) { /*ignore the old DIRENT*/
 				put_fl_mem(jDir, pL->readbuf);
 				continue;
 			}
@@ -962,6 +961,14 @@  jffs2_1pass_list_inodes(struct b_lists * pL, u32 pino)
 			struct jffs2_raw_inode ojNode;
 			struct jffs2_raw_inode *jNode, *i = NULL;
 			struct b_node *b2 = pL->frag.listHead;
+			
+			/*
+			we compare the DIRENT's ino with the latest DIRENT's ino 
+			to determine whether this DIRENT is the latest one. 
+			If the DIRENT is not the latest one,ignore it.
+			*/
+			if(jDir->ino != jffs2_1pass_find_inode(pL, jDir->name, pino))  
+				continue;
 
 			while (b2) {
 				jNode = (struct jffs2_raw_inode *)
@@ -1520,7 +1527,7 @@  jffs2_1pass_build_lists(struct part_info * part)
 			ret = jffs2_sum_scan_sumnode(part, sector_ofs, sumptr,
 					sumlen, pL);
 
-			if (buf_size && sumlen > buf_size)
+			if (sumlen > buf_size)
 				free(sumptr);
 			if (ret < 0) {
 				free(buf);
diff --git a/fs/jffs2/jffs2_nand_1pass.c b/fs/jffs2/jffs2_nand_1pass.c
index 3982003..9bad690 100644
--- a/fs/jffs2/jffs2_nand_1pass.c
+++ b/fs/jffs2/jffs2_nand_1pass.c
@@ -305,8 +305,8 @@  jffs2_1pass_read_inode(struct b_lists *pL, u32 ino, char *dest,
 			if (dest)
 				len += jNode->csize;
 			nand_read(nand, jNode->offset, &len, inode);
-			/* ignore data behind latest known EOF */
-			if (inode->offset > totalSize)
+			/* ignore data which exceed file length */
+			if (inode->offset + inode->dsize > totalSize)
 				continue;
 
 			if (stat) {
@@ -371,8 +371,9 @@  jffs2_1pass_find_inode(struct b_lists * pL, const char *name, u32 pino)
 
 	/* we need to search all and return the inode with the highest version */
 	for (jDir = (struct b_dirent *)pL->dir.listHead; jDir; jDir = jDir->next) {
-		if ((pino == jDir->pino) && (jDir->ino) &&	/* 0 for unlink */
-		    (len == jDir->nsize) && (nhash == jDir->nhash)) {
+		if ((pino == jDir->pino) && 
+		    (len == jDir->nsize) && 
+		    (nhash == jDir->nhash)) {
 			/* TODO: compare name */
 			if (jDir->version < version)
 				continue;
@@ -483,6 +484,14 @@  jffs2_1pass_list_inodes(struct b_lists * pL, u32 pino)
 			struct b_inode *jNode = (struct b_inode *)pL->frag.listHead;
 			struct b_inode *i = NULL;
 
+			/*
+			we compare the DIRENT's ino with the latest DIRENT's ino 
+			to determine whether this DIRENT is the latest one. 
+			If the DIRENT is not the latest one,ignore it.
+			*/
+			if(jDir.ino != jffs2_1pass_find_inode(pL, jDir->name, pino))  
+				continue;
+			
 			while (jNode) {
 				if (jNode->ino == jDir->ino && jNode->version >= i_version) {
 					i_version = jNode->version;
diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h
index 651f94c..5b006c0 100644
--- a/include/jffs2/jffs2.h
+++ b/include/jffs2/jffs2.h
@@ -41,6 +41,16 @@ 
 #include <asm/types.h>
 #include <jffs2/load_kernel.h>
 
+#ifdef CONFIG_JFFS2_SUMMARY
+#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
+/*
+we should define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if
+CONFIG_JFFS2_SUMMARY is enabled.
+*/
+#define CONFIG_SYS_JFFS2_SORT_FRAGMENTS
+#endif
+#endif
+
 #define JFFS2_SUPER_MAGIC 0x72b6
 
 /* Values we may expect to find in the 'magic' field */