Hi,Detlev : 2011/4/19 Detlev Zundel <d...@denx.de>: > Hi Baidu, > >> This patch fixes some issues with JFFS2 summary support in U-Boot. >> 1/ Bug fix for summary support: we need to 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. >> 3/ Avoid allocate too big memory if the biggest file in JFFS2 is too >> long. We only allocate one node size for pL->readbuf. >> For example, if the biggest file in the jffs2 is 15MB in the 16MB jffs2 >> system. Our previous code will allocate a buffer(pL->readbuf) as 15MB >> length. >> 4/ Improve error checking in the scanning. > > I missed saying this explicitely in my first review that we should > strive to isolate changes into single commits. As you have added even > more changes into a single commit this has now become somewhat > untolerable. Please split into individual functional changes (git add > -i can work wonders here), i.e. the list items in the changelog should > become individual commits. > > This also includes the added WATCHDOG_RESET not mentioned in the > changelog at all ;) > >> Changes for v2: >> - Add detail description for the patch. > > Are you sure you only changed the description? The diffstat from this > and the last time disagree - this time: > >> --- >> fs/jffs2/jffs2_1pass.c | 60 >> +++++++++++++++++++++++++----------------- >> fs/jffs2/jffs2_nand_1pass.c | 28 +++++++++++++++----- >> include/jffs2/jffs2.h | 10 +++++++ >> 3 files changed, 67 insertions(+), 31 deletions(-) > > Last time: > >> fs/jffs2/jffs2_1pass.c | 53 >> +++++++++++++++++++++++++----------------- >> fs/jffs2/jffs2_nand_1pass.c | 24 ++++++++++++++----- >> include/jffs2/jffs2.h | 11 +++++++++ >> 3 files changed, 60 insertions(+), 28 deletions(-) > > Looking over the changes, I do _see_ changes in code, so you should tell > us about them. > >> [...] > >> 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 */ > > I liked the previous version better: > >> diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h >> index 651f94c..c01a76e 100644 >> --- a/include/jffs2/jffs2.h >> +++ b/include/jffs2/jffs2.h >> @@ -41,6 +41,17 @@ >> #include <asm/types.h> >> #include <jffs2/load_kernel.h> >> >> +#ifdef CONFIG_JFFS2_SUMMARY >> +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS >> +/* >> + * if we define summary in jffs2, we also need to define >> + * CONFIG_SYS_JFFS2_SORT_FRAGMENTS. If not, the data in latest inode may >> be >> + * overwritten by the old one. >> +*/ >> +#error "need to define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if summary is >> enabled" >> +#endif >> +#endif >> + >> #define JFFS2_SUPER_MAGIC 0x72b6 > > Why did you change this to the worse? >
If we use summary in uboot, we MUST also define CONFIG_SYS_JFFS2_SORT_FRAGMENTS. The reason is : 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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot