RE: [Yaffs] PATCH!!! PATCH!!! (Was Sorry state of YAFFS2)

Top Page
Attachments:
Message as email
+ (text/plain)
Delete this message
Reply to this message
Author: Martin Fouts
Date:  
To: Sergey Kubushyn, Charles Manning
CC: yaffs-list
Subject: RE: [Yaffs] PATCH!!! PATCH!!! (Was Sorry state of YAFFS2)
Thank you for your patch. I've added it to the queue, and will test it
when I get the chance.

Unfortunately, I'm leaving on vacation on Friday, so it probably won't
be until after the 15th of November.

Marty

> -----Original Message-----
> From:
> [mailto:yaffs-bounces@stoneboat.aleph1.co.uk] On Behalf Of
> Sergey Kubushyn
> Sent: Friday, October 14, 2005 7:07 PM
> To: Charles Manning
> Cc: yaffs-list
> Subject: [Yaffs] PATCH!!! PATCH!!! (Was Sorry state of YAFFS2)
>
> On Fri, 14 Oct 2005, Charles Manning wrote:
>
> > On Friday 14 October 2005 08:54, you wrote:
> > > On Thu, 2005-10-13 at 11:09 -0700, Sergey Kubushyn wrote:
> > > > First of all, I must tell ya that your mailing list is
> extremely
> > > > selective in sending messages. I did NOT get your message in the
> > mail.
> > > > Ian's message also didn't come through. Other messages
> did. That's
> > why
> > > > your message had been copied and inserted into this
> email by hand.
> > > >
> > > > Do you think I'd miss your posting thus making you "right" in
> > having the
> > > > last word? It's kinda childish...
> >
> > Lots of abuse, and now paranoia?
> >
> > I don't own the list, Aleph One does. AFAIK, the list is completely
> > open and unfiltered in any way.
> >
> > As I have said before, I am not prepared to put the energy into
> > reading Sergey's abusive nonsense and I have a filter on my
> personal
> > emailer that does not allow anything from him in (though it
> does not
> > apply any filtersing to outbound stuff). Applying a filter is not
> > something I did lightly.
> > In
> > over 20 years in the business have never done this before
> and hope to
> > never be moved to do it again.
> >
> > Perhaps this filtering is causing some loss of feedback,
> but I think
> > there's probably better feedback coming from the rest of
> the YAFFSers
> > that are more constructive in their approach. Bottom line for me is
> > that I am not prepared to pander to Sergey's outbursts for the few
> > bits of useful feedback.
>
> Sorry for overestimating you, I've been thinking you are smarter :((
>
> > If someone else wants to screen Sergey's stuff and repost
> the useful
> > content I am willing to read that. Otherwise, I don't think
> the signal
> > to noise ratio is high enough to warrant reading.
> >
> > I continue to try work the problem of the mtd interface to
> resolve it
> > the correct way, rather than the ugly hack way.
> Unfortunately this is
> > taking longer than expected (over 2 months now!) so it is probably
> > worth putting the ugly hack into the CVS for now. If someone better
> > equipped with test hardware etc would like to do this, I would not
> > mind at all is someone else works the problem.
>
> I don't know what took you that long to do a trivial thing.
> But looking into that YAFFS code with lot of irregular
> structures casted to (__u8 *) I can now understand -- that
> was lack of knowledge, starting with C programming basics. I
> suggest you should read a good book on C programming. When
> you're through with it, go take some reading on kernel
> programming. It usually helps. Read books, they rule. Then
> you would also probably be able to read THE F#$%%^ING kernel
> source instead of IRC'ing and trying to resolve problems that
> do NOT exist.
>
> Here is the proper patch that makes your wonderOS work with
> _ANY_ large page auto OOB layout. No matter HW, SOFT, bad
> blocks at the beginning, at the end, in the middle of, ECC at
> the left, at the right or wherever you want.
> It does NOT require ANY special MTD or custom crafted kernel
> and works with any of stock ones.
>
> This patch also fixes some spectacularly stupid errors that
> are not errors per se but total lack of understanding how C
> works, less for Linux kernel.
>
> I do, really _DO_ hope to see it applied and that "Fix your
> MTD" mantra thrown away forever. You better start fixing bugs
> in your wonderFS. I'd suggest to start from explaining why 36
> Mbyte tar takes 51 Mbytes on YAFFS2 FS. And here is a hint -
> without that included patch it took 56 Mbytes. If it's too
> complex for you, try to look for any illegal cast usage on
> irregular data structures. I'm sending you a small program
> that illustrates the point of data alignment, you can compile
> and run it to see yourself how it makes a surprise even on ix86...
>
> === Cut ===
> diff -urN linux-2.6.12.orig/fs/yaffs2/yaffs_mtdif2.c 
> linux-2.6.12/fs/yaffs2/yaffs_mtdif2.c
> --- linux-2.6.12.orig/fs/yaffs2/yaffs_mtdif2.c    
> 2005-10-14 11:46:50.000000000 -0700
> +++ linux-2.6.12/fs/yaffs2/yaffs_mtdif2.c    2005-10-14 
> 16:52:40.000000000 -0700
> @@ -29,6 +29,130 @@

>
> #include "yaffs_packedtags2.h"
>
> +#define PT2_BYTES        25
> +
> +void nandmtd2_pt2buf(yaffs_Device *dev, yaffs_PackedTags2 *pt, int 
> +is_raw) {
> +    struct mtd_info *mtd = (struct mtd_info *)(dev->genericDevice);
> +    int    i, j = 0, k, n;
> +    __u8    pt2_byte_buf[PT2_BYTES];
> +
> +    /* Pack buffer with 0xff */
> +    for (i = 0; i < mtd->oobsize; i++)
> +        dev->spareBuffer[i] = 0xff;
> +
> +    if (!is_raw) {
> +        *((unsigned int *) &dev->spareBuffer[0]) = 
> pt->t.sequenceNumber;
> +        *((unsigned int *) &dev->spareBuffer[4]) = 
> pt->t.objectId;
> +        *((unsigned int *) &dev->spareBuffer[8]) = 
> pt->t.chunkId;
> +        *((unsigned int *) &dev->spareBuffer[12]) = 
> pt->t.byteCount;
> +        dev->spareBuffer[16] = pt->ecc.colParity;
> +        dev->spareBuffer[17] = pt->ecc.lineParity & 0xff;
> +        dev->spareBuffer[18] = (pt->ecc.lineParity >> 8) & 0xff;
> +        dev->spareBuffer[19] = (pt->ecc.lineParity >> 
> 16) & 0xff;
> +        dev->spareBuffer[20] = (pt->ecc.lineParity >> 
> 24) & 0xff;
> +        dev->spareBuffer[21] = pt->ecc.lineParityPrime & 0xff;
> +        dev->spareBuffer[22] = (pt->ecc.lineParityPrime 
> >> 8) & 0xff;
> +        dev->spareBuffer[23] = (pt->ecc.lineParityPrime 
> >> 16) & 0xff;
> +        dev->spareBuffer[24] = (pt->ecc.lineParityPrime 
> >> 24) & 0xff;
> +    } else {
> +        *((unsigned int *) &pt2_byte_buf[0]) = 
> pt->t.sequenceNumber;
> +        *((unsigned int *) &pt2_byte_buf[4]) = pt->t.objectId;
> +        *((unsigned int *) &pt2_byte_buf[8]) = pt->t.chunkId;
> +        *((unsigned int *) &pt2_byte_buf[12]) = pt->t.byteCount;
> +        pt2_byte_buf[16] = pt->ecc.colParity;
> +        pt2_byte_buf[17] = pt->ecc.lineParity & 0xff;
> +        pt2_byte_buf[18] = (pt->ecc.lineParity >> 8) & 0xff;
> +        pt2_byte_buf[19] = (pt->ecc.lineParity >> 16) & 0xff;
> +        pt2_byte_buf[20] = (pt->ecc.lineParity >> 24) & 0xff;
> +        pt2_byte_buf[21] = pt->ecc.lineParityPrime & 0xff;
> +        pt2_byte_buf[22] = (pt->ecc.lineParityPrime >> 
> 8) & 0xff;
> +        pt2_byte_buf[23] = (pt->ecc.lineParityPrime >> 
> 16) & 0xff;
> +        pt2_byte_buf[24] = (pt->ecc.lineParityPrime >> 
> 24) & 0xff;
> +
> +        k = mtd->oobinfo.oobfree[j][0];
> +        n = mtd->oobinfo.oobfree[j][1];
> +
> +        if (n == 0) {
> +            T(YAFFS_TRACE_ERROR, (TSTR("No OOB 
> space for tags" TENDSTR)));
> +            YBUG();
> +        }
> +
> +        for (i = 0; i < PT2_BYTES; i++) {
> +            if (n == 0) {
> +                j++;
> +                k = mtd->oobinfo.oobfree[j][0];
> +                n = mtd->oobinfo.oobfree[j][1];
> +                if (n == 0) {
> +                    T(YAFFS_TRACE_ERROR, 
> (TSTR("No OOB space for tags" TENDSTR)));
> +                    YBUG();
> +                }
> +            }
> +            dev->spareBuffer[k++] = pt2_byte_buf[i];
> +            n--;
> +        }
> +    }
> +}
> +
> +void nandmtd2_buf2pt(yaffs_Device *dev, yaffs_PackedTags2 *pt, int 
> +is_raw) {
> +    struct mtd_info *mtd = (struct mtd_info *)(dev->genericDevice);
> +    int    i, j = 0, k, n;
> +    __u8    pt2_byte_buf[PT2_BYTES];
> +
> +
> +    if (!is_raw) {
> +        pt->t.sequenceNumber = *((unsigned int *) 
> &dev->spareBuffer[0]);
> +        pt->t.objectId = *((unsigned int *) 
> &dev->spareBuffer[4]);
> +        pt->t.chunkId = *((unsigned int *) 
> &dev->spareBuffer[8]);
> +        pt->t.byteCount = *((unsigned int *) 
> &dev->spareBuffer[12]);
> +        pt->ecc.colParity = dev->spareBuffer[16];
> +        pt->ecc.lineParity = (dev->spareBuffer[17] & 
> 0x000000ff) |
> +             ((dev->spareBuffer[18] << 8) & 0x0000ff00) |
> +             ((dev->spareBuffer[19] << 16) & 0x00ff0000) |
> +             ((dev->spareBuffer[20] >> 24) & 0xff000000);
> +        pt->ecc.lineParityPrime = (dev->spareBuffer[21] 
> & 0x000000ff) |
> +             ((dev->spareBuffer[22] << 8) & 0x0000ff00) |
> +             ((dev->spareBuffer[23] << 16) & 0x00ff0000) |
> +             ((dev->spareBuffer[24] >> 24) & 0xff000000);
> +    } else {
> +        k = mtd->oobinfo.oobfree[j][0];
> +        n = mtd->oobinfo.oobfree[j][1];
> +
> +        if (n == 0) {
> +            T(YAFFS_TRACE_ERROR, (TSTR("No space in 
> OOB for tags" TENDSTR)));
> +            YBUG();
> +        }
> +
> +        for (i = 0; i < PT2_BYTES; i++) {
> +            if (n == 0) {
> +                j++;
> +                k = mtd->oobinfo.oobfree[j][0];
> +                n = mtd->oobinfo.oobfree[j][1];
> +                if (n == 0) {
> +                    T(YAFFS_TRACE_ERROR, 
> (TSTR("No space in OOB for tags" TENDSTR)));
> +                    YBUG();
> +                }
> +            }
> +            pt2_byte_buf[i] = dev->spareBuffer[k++];
> +            n--;
> +        }
> +        pt->t.sequenceNumber = *((unsigned int *) 
> &pt2_byte_buf[0]);
> +        pt->t.objectId = *((unsigned int *) &pt2_byte_buf[4]);
> +        pt->t.chunkId = *((unsigned int *) &pt2_byte_buf[8]);
> +        pt->t.byteCount = *((unsigned int *) &pt2_byte_buf[12]);
> +        pt->ecc.colParity = pt2_byte_buf[16];
> +        pt->ecc.lineParity = (pt2_byte_buf[17] & 0x000000ff) |
> +             ((pt2_byte_buf[18] << 8) & 0x0000ff00) |
> +             ((pt2_byte_buf[19] << 16) & 0x00ff0000) |
> +             ((pt2_byte_buf[20] << 24) & 0xff000000);
> +        pt->ecc.lineParityPrime = (pt2_byte_buf[21] & 
> 0x000000ff) |
> +             ((pt2_byte_buf[22] << 8) & 0x0000ff00) |
> +             ((pt2_byte_buf[23] << 16) & 0x00ff0000) |
> +             ((pt2_byte_buf[24] << 24) & 0xff000000);
> +    }
> +}
> +
>  int nandmtd2_WriteChunkWithTagsToNAND(yaffs_Device * dev, 
> int chunkInNAND,
>                        const __u8 * data,
>                        const yaffs_ExtendedTags 
> * tags) @@ -51,24 +176,22 @@
>      }

>
>      if (data && tags) {
> -        if (dev->useNANDECC)
> -            retval =
> -                mtd->write_ecc(mtd, addr, 
> dev->nBytesPerChunk,
> -                       &dummy, data, (__u8 
> *) & pt, NULL);
> -        else
> +            nandmtd2_pt2buf(dev, &pt, 0);
>              retval =
>                  mtd->write_ecc(mtd, addr, 
> dev->nBytesPerChunk,
> -                       &dummy, data, (__u8 
> *) & pt, NULL);
> +                       &dummy, data, 
> dev->spareBuffer,
> +                       NULL);
>      } else {
>          if (data)
>              retval =
>                  mtd->write(mtd, addr, 
> dev->nBytesPerChunk, &dummy,
>                         data);
> -        if (tags)
> +        if (tags) {
> +            nandmtd2_pt2buf(dev, &pt, 1);
>              retval =
>                  mtd->write_oob(mtd, addr, 
> mtd->oobsize, &dummy,
> -                       (__u8 *) & pt);
> -
> +                dev->spareBuffer);
> +        }
>      }

>
>      if (retval == 0)
> @@ -94,30 +217,24 @@
>          TENDSTR), chunkInNAND, data, tags));

>
>      if (data && tags) {
> -        if (dev->useNANDECC) {
>              retval =
>                  mtd->read_ecc(mtd, addr, 
> dev->nBytesPerChunk,
>                        &dummy, data, 
> dev->spareBuffer,
>                        NULL);
> -        } else {
> -            retval =
> -                mtd->read_ecc(mtd, addr, 
> dev->nBytesPerChunk,
> -                      &dummy, data, 
> dev->spareBuffer,
> -                      NULL);
> -        }
> +            nandmtd2_buf2pt(dev, &pt, 0);
>      } else {
>          if (data)
>              retval =
>                  mtd->read(mtd, addr, 
> dev->nBytesPerChunk, &dummy,
>                        data);
> -        if (tags)
> +        if (tags) {
>              retval =
>                  mtd->read_oob(mtd, addr, 
> mtd->oobsize, &dummy,
>                        dev->spareBuffer);
> +            nandmtd2_buf2pt(dev, &pt, 1);
> +        }
>      }

>
> -    memcpy(&pt, dev->spareBuffer, sizeof(pt));
> -
>      if (tags)
>          yaffs_UnpackTags2(tags, &pt);

>
> @@ -178,10 +295,11 @@
>              *sequenceNumber = 0;
>              *state = YAFFS_BLOCK_STATE_EMPTY;
>          }
> +
> +        T(YAFFS_TRACE_MTD,
> +          (TSTR("block is OK seq %d state %d" TENDSTR), 
> *sequenceNumber,
> +           *state));
>      }
> -    T(YAFFS_TRACE_MTD,
> -      (TSTR("block is bad seq %d state %d" TENDSTR), 
> *sequenceNumber,
> -       *state));

>
>      if (retval == 0)
>          return YAFFS_OK;
> === Cut ===

>
> ---
> ******************************************************************
> *  KSI@home    KOI8 Net  < >  The impossible we do immediately.  *
> *  Las Vegas   NV, USA   < >  Miracles require 24-hour notice.   *
> ******************************************************************

>
>
> _______________________________________________
> yaffs mailing list
>
> http://stoneboat.aleph1.co.uk/cgi-bin/mailman/listinfo/yaffs
>