oops line 231 of latest readinode.c

oops line 231 of latest readinode.c

[Simon Haynes]

I have been running some systems here for some time error free using a root 
filesystem which is jffs2 on nand. One of them produced an oops today which 
seemed to kill off kupdated. I had a look at the oops and it seems there is a 
possible bug in jffs2_add_frag_to_fragtree.  

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.

Here is the some of the ksymoops output

Oops: kernel access of bad area, sig: 11
NIP: 8007D104 XER: 00000000 LR: 8007D0C8 SP: 802BFD80 REGS: 802bfcd0 TRAP: 
0800    Not tainted
Using defaults from ksymoops -t elf32-powerpc -a powerpc:common
MSR: 00029030 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 11
TASK = 802be000[6] 'kupdated' Last syscall: -1
last math 00000000 last altivec 00000000
GPR00: 000FFFFF 802BFD80 802BE000 00000000 FFFFFFFF 82C8FFB0 00400000 00000000
GPR08: 82C8F000 000FFFFF 00000D90 00000000 82004228 1001F23C 00000000 00000000
GPR16: 00000000 FFF8345A 00000000 00000000 00009032 80FFFF40 00000000 00000000
GPR24: 00000000 80140000 00000000 828EAD14 00000000 82C8FFB0 00000000 83F48CC4
Call backtrace:
83F48CC4 8007CFEC 8007D8D8 8007D6A8 80086B80 8004F4DC 8004F95C
800874F0 800823A4 800886EC 80086FB4 8003CBF0 8003BD44 8003C0EC
800061B0
Warning (Oops_read): Code line not seen, dumping what data is available


>>NIP; 8007d104 <jffs2_add_frag_to_fragtree+68/3b0>   <=====

>>GPR1; 802bfd80 <_end+edcd8/4eaff58>
>>GPR2; 802be000 <_end+ebf58/4eaff58>
>>GPR5; 82c8ffb0 <_end+2abdf08/4eaff58>
>>GPR8; 82c8f000 <_end+2abcf58/4eaff58>
>>GPR12; 82004228 <_end+1e32180/4eaff58>
>>GPR21; 80ffff40 <_end+e2de98/4eaff58>
>>GPR25; 80140000 <crc32_table+3cc/1238>
>>GPR27; 828ead14 <_end+2718c6c/4eaff58>
>>GPR29; 82c8ffb0 <_end+2abdf08/4eaff58>
>>GPR31; 83f48cc4 <_end+3d76c1c/4eaff58>

Trace; 83f48cc4 <_end+3d76c1c/4eaff58>
Trace; 8007cfec <jffs2_add_full_dnode_to_inode+74/124>
Trace; 8007d8d8 <jffs2_do_read_inode_internal+140/698>
Trace; 8007d6a8 <jffs2_do_read_inode+198/1c4>
Trace; 80086b80 <jffs2_read_inode+60/300>
Trace; 8004f4dc <get_new_inode+d4/1bc>
Trace; 8004f95c <iget4+130/144>
Trace; 800874f0 <jffs2_gc_fetch_inode+b0/108>
Trace; 800823a4 <jffs2_garbage_collect_pass+49c/564>
Trace; 800886ec <jffs2_flush_wbuf_gc+cc/178>
Trace; 80086

Re: oops line 231 of latest readinode.c

[Artem B. Bityuckiy]

Is it possible to reproduce this? How if it is?

On Wed, 17 Nov 2004, Simon Haynes wrote:

> 
> I have been running some systems here for some time error free using a root 
> filesystem which is jffs2 on nand. One of them produced an oops today which 
> seemed to kill off kupdated. I had a look at the oops and it seems there is a 
> possible bug in jffs2_add_frag_to_fragtree.  
> 
> 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.
> 
> Here is the some of the ksymoops output
> 
> Oops: kernel access of bad area, sig: 11
> NIP: 8007D104 XER: 00000000 LR: 8007D0C8 SP: 802BFD80 REGS: 802bfcd0 TRAP: 
> 0800    Not tainted
> Using defaults from ksymoops -t elf32-powerpc -a powerpc:common
> MSR: 00029030 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 11
> TASK = 802be000[6] 'kupdated' Last syscall: -1
> last math 00000000 last altivec 00000000
> GPR00: 000FFFFF 802BFD80 802BE000 00000000 FFFFFFFF 82C8FFB0 00400000 00000000
> GPR08: 82C8F000 000FFFFF 00000D90 00000000 82004228 1001F23C 00000000 00000000
> GPR16: 00000000 FFF8345A 00000000 00000000 00009032 80FFFF40 00000000 00000000
> GPR24: 00000000 80140000 00000000 828EAD14 00000000 82C8FFB0 00000000 83F48CC4
> Call backtrace:
> 83F48CC4 8007CFEC 8007D8D8 8007D6A8 80086B80 8004F4DC 8004F95C
> 800874F0 800823A4 800886EC 80086FB4 8003CBF0 8003BD44 8003C0EC
> 800061B0
> Warning (Oops_read): Code line not seen, dumping what data is available
> 
> 
> >>NIP; 8007d104 <jffs2_add_frag_to_fragtree+68/3b0>   <=====
> 
> >>GPR1; 802bfd80 <_end+edcd8/4eaff58>
> >>GPR2; 802be000 <_end+ebf58/4eaff58>
> >>GPR5; 82c8ffb0 <_end+2abdf08/4eaff58>
> >>GPR8; 82c8f000 <_end+2abcf58/4eaff58>
> >>GPR12; 82004228 <_end+1e32180/4eaff58>
> >>GPR21; 80ffff40 <_end+e2de98/4eaff58>
> >>GPR25; 80140000 <crc32_table+3cc/1238>
> >>GPR27; 828ead14 <_end+2718c6c/4eaff58>
> >>GPR29; 82c8ffb0 <_end+2abdf08/4eaff58>
> >>GPR31; 83f48cc4 <_end+3d76c1c/4eaff58>
> 
> Trace; 83f48cc4 <_end+3d76c1c/4eaff58>
> Trace; 8007cfec <jffs2_add_full_dnode_to_inode+74/124>
> Trace; 8007d8d8 <jffs2_do_read_inode_internal+140/698>
> Trace; 8007d6a8 <jffs2_do_read_inode+198/1c4>
> Trace; 80086b80 <jffs2_read_inode+60/300>
> Trace; 8004f4dc <get_new_inode+d4/1bc>
> Trace; 8004f95c <iget4+130/144>
> Trace; 800874f0 <jffs2_gc_fetch_inode+b0/108>
> Trace; 800823a4 <jffs2_garbage_collect_pass+49c/564>
> Trace; 800886ec <jffs2_flush_wbuf_gc+cc/178>
> Trace; 80086
> 
> 
> 
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

--
Best Regards,
Artem B. Bityuckiy,
St.-Petersburg, Russia.

Re: oops line 231 of latest readinode.c

[Estelle HAMMACHE]

Simon Haynes wrote:
> newfrag->ofs)' and then in the next if as newfrag->ofs contains -1. The oops

It seems very strange that newfrag->ofs contains -1. If you can reproduce
it could you check where it was filled with this strange value ?

Estelle

Re: oops line 231 of latest readinode.c

[Simon Haynes]

This crash happened on one of several systems which I have been running for 
over a year. This crash came out of the blue when the system was updating a 
logfile with a message. It has performed many writes during this time and I 
have never seen this problem before. As far as trying to reproduce the 
problem is concerned. 
Some time ago I used a program to exercise the flash to try and track other 
problems and as I said before I have never seen this oops before. Besides 
performing more flash exercising tests I would not have a clue how I could 
reproduce this. 

So in summary this crash was out of the blue and I suspect that I can not 
easily reproduce it without understanding more about what happened.

Cheers

Simon.

On Wednesday 17 Nov 2004 5:48 pm, Estelle HAMMACHE wrote:
> Simon Haynes wrote:
> > newfrag->ofs)' and then in the next if as newfrag->ofs contains -1. The
> > oops
>
> It seems very strange that newfrag->ofs contains -1. If you can reproduce
> it could you check where it was filled with this strange value ?
>
> Estelle

Re: oops line 231 of latest readinode.c

[David Woodhouse]

[-- 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/