oobavail issues

oobavail issues

[Joshua Wise]

Hi folks,

We're debugging a mysterious crash on NAND writes on an s3c2410-derived 
driver. The culprit seems to be in how oobavail is determined:

line 2524 nand_base.c: mtd->oobavail = mtd->oobsize - 
(this->autooob->eccbytes + 1);

Our oobinfo is the same as the s3c2410's. For reference, that's:

static struct nand_oobinfo nand_hw_eccoob = {
         .useecc = MTD_NANDECC_AUTOPLACE,
         .eccbytes = 3,
         .eccpos = {0, 1, 2 },
         .oobfree = { {8, 8} }
};

We found that making our oobfree match up with oobavail seemed to do the 
trick for avoiding this, and make JFFS2 work, to boot! Here's what we did:

static struct nand_oobinfo nand_hw_eccoob = {
         .useecc = MTD_NANDECC_AUTOPLACE,
         .eccbytes = 3,
         .eccpos = {0, 1, 2 },
         .oobfree = { {8, 8}, {3, 2}, {6, 2} } /* {6, 10} instead? */
};

I suspect that oobavail should be summing oobfree instead of doing that 
arithmetic. Or is there something I'm not seeing?

joshua

Re: oobavail issues

[Thomas Gleixner]

On Wed, 2005-03-30 at 20:18 -0500, Joshua Wise wrote:
> We found that making our oobfree match up with oobavail seemed to do the 
> trick for avoiding this, and make JFFS2 work, to boot! Here's what we did:
>
> I suspect that oobavail should be summing oobfree instead of doing that 
> arithmetic. Or is there something I'm not seeing?

In general you're right. oobavail should be derived from oobfree, but in
case of JFFS2 this does not matter at all.

Which problem went away with your changes ?

tglx

Re: oobavail issues

[Matthew Reimer]

On Thursday 31 March 2005 03:52 am, Thomas Gleixner wrote:
> On Wed, 2005-03-30 at 20:18 -0500, Joshua Wise wrote:
> > We found that making our oobfree match up with oobavail seemed to do the
> > trick for avoiding this, and make JFFS2 work, to boot! Here's what we
> > did:
> >
> > I suspect that oobavail should be summing oobfree instead of doing that
> > arithmetic. Or is there something I'm not seeing?
>
> In general you're right. oobavail should be derived from oobfree, but in
> case of JFFS2 this does not matter at all.
>
> Which problem went away with your changes ?

We're using the nand_oobinfo structure from the s3c2410.c nand driver:

static struct nand_oobinfo nand_hw_eccoob = {
        .useecc         = MTD_NANDECC_AUTOPLACE,
        .eccbytes       = 3,
        .eccpos         = {0, 1, 2 },
        .oobfree        = { {8, 8} }
};

On our platform, it's hooked up to Samsung flash part (k9f5608u0a) with 512 
byte pages and 16 bytes of OOB. We were getting crashes in the memcpy() that 
nand_prepare_oobbuf() does because with the above definition mtd->oobavail = 
12, whereas the total number of oobfree bytes only equals 8. So the first 
memcpy() would be correct and len would advance by 8, but then since len (now 
8) < mtd->oobavail (now 12), it would iterate again. These subsequent 
iterations assume that there are more entries in the oobfree array, and would 
end up reading whatever values happened to follow the oobfree array and using 
those. In our case, it would read zeroes, which would result in len not 
advancing, and eventually it read some impossibly large number which memcpy() 
would choke on and crash the kernel.

We've worked around the crash by making sure that oobfree covers every free 
byte.

I'm not sure what is the correct fix--calculate oobavail differently, or to 
change the loop condition in nand_prepare_oobbuf, or to require 
nand_oobinfo.oobfree to cover every OOB byte. We just copied from the 
s3c2410.c, assuming it was correct.

What would you suggest?

Matt

Re: oobavail issues

[Matthew Reimer]

On Thursday 31 March 2005 11:20 am, Thomas Gleixner wrote:
> On Thu, 2005-03-31 at 09:29 -0800, Matthew Reimer wrote:
> > I'm not sure what is the correct fix--calculate oobavail differently, or
> > to change the loop condition in nand_prepare_oobbuf, or to require
> > nand_oobinfo.oobfree to cover every OOB byte. We just copied from the
> > s3c2410.c, assuming it was correct.
>
> I will fix this. But I still wonder why you hit this problem with JFFS2.
> JFFS2 does not provide a filesystem buffer for oob data, so the memcpy
> in nand_prepare_oobbuf is never invoked.

IIRC it wasn't jffs2 but the MTD_WRITEECC() we were doing to write a kernel to 
a partition, which we had to do in preparation for mounting jffs2. Sorry for 
the confusion.

Matt

Re: oobavail issues

[Thomas Gleixner]

On Thu, 2005-03-31 at 09:29 -0800, Matthew Reimer wrote:
> I'm not sure what is the correct fix--calculate oobavail differently, or to 
> change the loop condition in nand_prepare_oobbuf, or to require 
> nand_oobinfo.oobfree to cover every OOB byte. We just copied from the 
> s3c2410.c, assuming it was correct.

I will fix this. But I still wonder why you hit this problem with JFFS2.
JFFS2 does not provide a filesystem buffer for oob data, so the memcpy
in nand_prepare_oobbuf is never invoked. 

tglx

Re: oobavail issues

[Thomas Gleixner]

On Thu, 2005-03-31 at 10:29 -0800, Matthew Reimer wrote:
> > I will fix this. But I still wonder why you hit this problem with JFFS2.
> > JFFS2 does not provide a filesystem buffer for oob data, so the memcpy
> > in nand_prepare_oobbuf is never invoked.
> 
> IIRC it wasn't jffs2 but the MTD_WRITEECC() we were doing to write a kernel to 
> a partition, which we had to do in preparation for mounting jffs2. Sorry for 
> the confusion.

Ok. Fixed in CVS. Thanks for pointing this out.

tglx

Re: oobavail issues

[Matthew Reimer]

On Friday 01 April 2005 12:23 am, Thomas Gleixner wrote:
> On Thu, 2005-03-31 at 10:29 -0800, Matthew Reimer wrote:
> > > I will fix this. But I still wonder why you hit this problem with
> > > JFFS2. JFFS2 does not provide a filesystem buffer for oob data, so the
> > > memcpy in nand_prepare_oobbuf is never invoked.
> >
> > IIRC it wasn't jffs2 but the MTD_WRITEECC() we were doing to write a
> > kernel to a partition, which we had to do in preparation for mounting
> > jffs2. Sorry for the confusion.
>
> Ok. Fixed in CVS. Thanks for pointing this out.

Thanks, looks good!

Matt