Patchwork [3/6] e2fsprogs: only update the icache for ext2_inode

login
register
mail settings
Submitter Robert Yang
Date Aug. 22, 2013, 1:13 p.m.
Message ID <8e9bbd9609efd20317fd4c2617ec293b49ee84bb.1377175027.git.liezhi.yang@windriver.com>
Download mbox | patch
Permalink /patch/56253/
State Accepted
Commit 2cece576b351228cb24bf24a53845646a51f7f6b
Headers show

Comments

Robert Yang - Aug. 22, 2013, 1:13 p.m.
We only read the cache when:

bufsize == sizeof(struct ext2_inode)

then we should only update the cache in the same condition, otherwise
there would be errors, for example:

cache[0]: cached ino 14 when bufsize = 128 by ext2fs_write_inode_full()
cache[1]: cached ino 14 when bufsize = 156 by ext2fs_read_inode_full()

Then update the cache:
cache[0]: cached ino 15 when bufsize = 156 by ext2fs_read_inode_full()

Then the ino 14 would hit the cache[1] when bufsize = 128 (but it was
cached by bufsize = 156), so there would be errors.

[YOCTO #3848]

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 .../e2fsprogs/e2fsprogs-1.42.8/fix-icache.patch    |   69 ++++++++++++++++++++
 .../recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb |    1 +
 2 files changed, 70 insertions(+)
 create mode 100644 meta/recipes-devtools/e2fsprogs/e2fsprogs-1.42.8/fix-icache.patch
Darren Hart - Aug. 22, 2013, 5:23 p.m.
On Thu, 2013-08-22 at 09:13 -0400, Robert Yang wrote:
> We only read the cache when:
> 
> bufsize == sizeof(struct ext2_inode)
> 
> then we should only update the cache in the same condition, otherwise
> there would be errors, for example:
> 
> cache[0]: cached ino 14 when bufsize = 128 by ext2fs_write_inode_full()
> cache[1]: cached ino 14 when bufsize = 156 by ext2fs_read_inode_full()
> 
> Then update the cache:
> cache[0]: cached ino 15 when bufsize = 156 by ext2fs_read_inode_full()
> 
> Then the ino 14 would hit the cache[1] when bufsize = 128 (but it was
> cached by bufsize = 156), so there would be errors.

> [YOCTO #3848]
> 
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>


> +Upstream-Status: [Inappropriate]

Hrm... why is this one inappropriate?
Robert Yang - Aug. 23, 2013, 1:58 a.m.
On 08/23/2013 01:23 AM, Darren Hart wrote:
> On Thu, 2013-08-22 at 09:13 -0400, Robert Yang wrote:
>> We only read the cache when:
>>
>> bufsize == sizeof(struct ext2_inode)
>>
>> then we should only update the cache in the same condition, otherwise
>> there would be errors, for example:
>>
>> cache[0]: cached ino 14 when bufsize = 128 by ext2fs_write_inode_full()
>> cache[1]: cached ino 14 when bufsize = 156 by ext2fs_read_inode_full()
>>
>> Then update the cache:
>> cache[0]: cached ino 15 when bufsize = 156 by ext2fs_read_inode_full()
>>
>> Then the ino 14 would hit the cache[1] when bufsize = 128 (but it was
>> cached by bufsize = 156), so there would be errors.
>
>> [YOCTO #3848]
>>
>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>
>
>> +Upstream-Status: [Inappropriate]
>
> Hrm... why is this one inappropriate?
>

Hi Darren, the upstream has changed the icache lot, so this patch is
inappropriate for the upstream, we can drop this patch when we update
the package, I had put this in the patch head.

// Robert
Darren Hart - Aug. 23, 2013, 5:03 p.m.
On Fri, 2013-08-23 at 09:58 +0800, Robert Yang wrote:
> On 08/23/2013 01:23 AM, Darren Hart wrote:
> > On Thu, 2013-08-22 at 09:13 -0400, Robert Yang wrote:
> >> We only read the cache when:
> >>
> >> bufsize == sizeof(struct ext2_inode)
> >>
> >> then we should only update the cache in the same condition, otherwise
> >> there would be errors, for example:
> >>
> >> cache[0]: cached ino 14 when bufsize = 128 by ext2fs_write_inode_full()
> >> cache[1]: cached ino 14 when bufsize = 156 by ext2fs_read_inode_full()
> >>
> >> Then update the cache:
> >> cache[0]: cached ino 15 when bufsize = 156 by ext2fs_read_inode_full()
> >>
> >> Then the ino 14 would hit the cache[1] when bufsize = 128 (but it was
> >> cached by bufsize = 156), so there would be errors.
> >
> >> [YOCTO #3848]
> >>
> >> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> >
> >
> >> +Upstream-Status: [Inappropriate]
> >
> > Hrm... why is this one inappropriate?
> >
> 
> Hi Darren, the upstream has changed the icache lot, so this patch is
> inappropriate for the upstream, we can drop this patch when we update
> the package, I had put this in the patch head.

Sorry, I must have missed that.

Acked-by: Darren Hart <dvhart@linux.intel.com>

Thanks Robert!

Patch

diff --git a/meta/recipes-devtools/e2fsprogs/e2fsprogs-1.42.8/fix-icache.patch b/meta/recipes-devtools/e2fsprogs/e2fsprogs-1.42.8/fix-icache.patch
new file mode 100644
index 0000000..ad4e343
--- /dev/null
+++ b/meta/recipes-devtools/e2fsprogs/e2fsprogs-1.42.8/fix-icache.patch
@@ -0,0 +1,69 @@ 
+inode.c: only update the icache for ext2_inode
+
+We only read the cache when:
+
+bufsize == sizeof(struct ext2_inode)
+
+then we should only update the cache in the same condition, otherwise
+there would be errors, for example:
+
+cache[0]: cached ino 14 when bufsize = 128 by ext2fs_write_inode_full()
+cache[1]: cached ino 14 when bufsize = 156 by ext2fs_read_inode_full()
+
+Then update the cache:
+cache[0]: cached ino 15 when bufsize = 156 by ext2fs_read_inode_full()
+
+Then the ino 14 would hit the cache[1] when bufsize = 128 (but it was
+cached by bufsize = 156), so there would be errors.
+
+Note: the upstream has changed the icache lot, so this patch is
+inappropriate for the upstream, we can drop this patch when we update
+the package.
+
+Upstream-Status: [Inappropriate]
+
+Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
+---
+ lib/ext2fs/inode.c | 20 ++++++++++++--------
+ 1 file changed, 12 insertions(+), 8 deletions(-)
+
+diff --git a/lib/ext2fs/inode.c b/lib/ext2fs/inode.c
+--- a/lib/ext2fs/inode.c
++++ b/lib/ext2fs/inode.c
+@@ -612,10 +612,12 @@ errcode_t ext2fs_read_inode_full(ext2_filsys fs, ext2_ino_t ino,
+ #endif
+ 
+ 	/* Update the inode cache */
+-	fs->icache->cache_last = (fs->icache->cache_last + 1) %
+-		fs->icache->cache_size;
+-	fs->icache->cache[fs->icache->cache_last].ino = ino;
+-	fs->icache->cache[fs->icache->cache_last].inode = *inode;
++	if (bufsize == sizeof(struct ext2_inode)) {
++		fs->icache->cache_last = (fs->icache->cache_last + 1) %
++			fs->icache->cache_size;
++		fs->icache->cache[fs->icache->cache_last].ino = ino;
++		fs->icache->cache[fs->icache->cache_last].inode = *inode;
++	}
+ 
+ 	return 0;
+ }
+@@ -648,10 +650,12 @@ errcode_t ext2fs_write_inode_full(ext2_filsys fs, ext2_ino_t ino,
+ 
+ 	/* Check to see if the inode cache needs to be updated */
+ 	if (fs->icache) {
+-		for (i=0; i < fs->icache->cache_size; i++) {
+-			if (fs->icache->cache[i].ino == ino) {
+-				fs->icache->cache[i].inode = *inode;
+-				break;
++		if (bufsize == sizeof(struct ext2_inode)) {
++			for (i=0; i < fs->icache->cache_size; i++) {
++				if (fs->icache->cache[i].ino == ino) {
++					fs->icache->cache[i].inode = *inode;
++					break;
++				}
+ 			}
+ 		}
+ 	} else {
+-- 
+1.8.1.2
+
diff --git a/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb b/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb
index 5d65bbc..b063bc0 100644
--- a/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb
+++ b/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb
@@ -5,6 +5,7 @@  SRC_URI += "file://acinclude.m4 \
             file://remove.ldconfig.call.patch \
             file://debugfs-too-short.patch \
             file://debugfs-sparse-copy.patch \
+            file://fix-icache.patch \
 "
 
 SRC_URI[md5sum] = "8ef664b6eb698aa6b733df59b17b9ed4"