Re: oops line 231 of latest readinode.c

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 1356 bytes --]

On Wed, 2004-11-17 at 15:56 +0000, Simon Haynes wrote:
> In my case all arguments appear to be valid kernel addresses. The call to 
> jffs2_lookup_node_frag returns a 0.  So the 'if (this)' takes the else route 
> and lastend is set to 0. We then execute the code in if (lastend <= 
> newfrag->ofs)' and then in the next if as newfrag->ofs contains -1. The oops 
> is produced by the line 'if(this->node)' because this is 0. I have checked 
> this against the latest CVS code and it would seem that this could still 
> happen.
> 
> I don't really know the flow of the code here but could I just put 
> 'if(this)' in front of 'if(this->node)' or is there some other more serious 
> problem here.

Looks like a correct diagnosis and cure; thanks. 

This can only happen 
if ((lastend-1) >> PAGE_CACHE_SHIFT == newfrag->ofs >> PAGE_CACHE_SHIFT)

That's if you have no previous nodes in the fragtree, and the _first_
node it finds is in the range 0xFFFFF000-0xFFFFFFFF (the last possible
page of the file).

Do you have a file with data in that range? This will trigger only if
garbage collection has happened such that a node in that range is the
one with the lowest version number.

Actually I'm not going to make it also depend on (this) but on (lastend)
since it keeps the logic in the above statement consistent, and has much
the same effect.


-- 
dwmw2

[-- Attachment #2: Attached message - mtd/fs/jffs2 readinode.c,1.116,1.117 --]
[-- Type: message/rfc822, Size: 3441 bytes --]

From: David Woodhouse <dwmw2@infradead.org>
To: linux-mtd-cvs@lists.infradead.org
Subject: mtd/fs/jffs2 readinode.c,1.116,1.117
Date: Sat, 20 Nov 2004 18:06:57 +0000
Message-ID: <E1CVZdR-0005FE-0x@phoenix.infradead.org>

Update of /home/cvs/mtd/fs/jffs2
In directory phoenix.infradead.org:/tmp/cvs-serv20158

Modified Files:
	readinode.c 
Log Message:
Fix oops if we happen to have a node in the range 0xfffff000 onwards as the
lowest versioned node in the inode.


Index: readinode.c
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/readinode.c,v
retrieving revision 1.116
retrieving revision 1.117
diff -u -r1.116 -r1.117
--- readinode.c	16 Nov 2004 20:36:12 -0000	1.116
+++ readinode.c	20 Nov 2004 18:06:54 -0000	1.117
@@ -227,7 +227,7 @@
 		   If so, both 'this' and the new node get marked REF_NORMAL so
 		   the GC can take a look.
 		*/
-		if ((lastend-1) >> PAGE_CACHE_SHIFT == newfrag->ofs >> PAGE_CACHE_SHIFT) {
+		if (lastend && (lastend-1) >> PAGE_CACHE_SHIFT == newfrag->ofs >> PAGE_CACHE_SHIFT) {
 			if (this->node)
 				mark_ref_normal(this->node->raw);
 			mark_ref_normal(newfrag->node->raw);


__________________________________________________________
Linux-MTD CVS commit list
http://lists.infradead.org/mailman/listinfo/linux-mtd-cvs/