[Yaffs] Re: [PATCH] treat OOB as a single chunk of oobavail bytes

[Charles Manning]

On Wednesday 30 November 2005 21:54, Vitaly Wool wrote:

<snip other stuff on mtd list>

> The problem remaining is that yaffs2 can/t work unmodified with my
> target (ARM926-based board with Sandisk NAND flash controller).
> The reason is that the HW ECC engine uses 40 bytes per 2k page so the
> remaining amount of OOB is no more than 23 (64 - 40 - 1 byte for
> badblockpos). On the other hand, yaffs_PackedTags2 size is 28 bytes, so
> I'm in trouble.
> What I suggest to overcome this issue is:
> - use "__attribute__ (packed) " for all the structures put into the OOB;
> - limit the number of bits for yaffs_PackedTags2TagsPart and members.
>
> For instance, the patch inlined reduces the size of yaffs_PackedTags2 to
> 12 bytes without impacting the functionality.
> One may say that using "packed" limits the portability, but I will argue
> that statement since the alignments within the structure may also change
> from compiler to compiler and from arch to arch (i. e. GNU ABI vs EABI
> for ARM). So the best way would be not to mess with the structures for
> packed tags at all, but as for now, I'm worried just about reducing the
> size of yaffs_PackedTags2 :)

Hi Vitaly

Thanks for your efforts here.

I think the easiest for everyone is going to be adding the packing as a build 
option since I don't want to break anyone using the current packed tags.

Some further comments below:

>
> Best regards,
>    Vitaly
>
> diff -uNr linux.orig/fs/yaffs2/yaffs_ecc.h linux/fs/yaffs2/yaffs_ecc.h
> --- linux.orig/fs/yaffs2/yaffs_ecc.h    2005-11-24 16:51:17.000000000 +0300
> +++ linux/fs/yaffs2/yaffs_ecc.h    2005-11-30 11:05:18.000000000 +0300
> @@ -26,10 +26,10 @@
>  #ifndef __YAFFS_ECC_H__
>  #define __YAFFS_ECC_H__
>
> -typedef struct {
> +typedef struct __attribute__((packed)) {
>      unsigned char colParity;
> -    unsigned lineParity;
> -    unsigned lineParityPrime;
> +    unsigned short lineParity:12;
> +    unsigned short lineParityPrime:12;
>  } yaffs_ECCOther;
>
>  void yaffs_ECCCalculate(const unsigned char *data, unsigned char *ecc);
> diff -uNr linux-2.6.10.orig/fs/yaffs2/yaffs_packedtags2.h
> linux-2.6.10/fs/yaffs2/yaffs_packedtags2.h
> --- linux-2.6.10.orig/fs/yaffs2/yaffs_packedtags2.h    2005-11-24
> 16:51:17.000000000 +0300
> +++ linux-2.6.10/fs/yaffs2/yaffs_packedtags2.h    2005-11-30
> 11:04:52.000000000 +0300
> @@ -6,14 +6,14 @@
>  #include "yaffs_guts.h"
>  #include "yaffs_ecc.h"
>
> -typedef struct {
> -    unsigned sequenceNumber;
> -    unsigned objectId;
> -    unsigned chunkId;
> -    unsigned byteCount;
> +typedef struct __attribute__((packed)) {
> +    unsigned short sequenceNumber;
> +    unsigned short objectId;
> +    unsigned short chunkId;
> +    unsigned short byteCount;
>  } yaffs_PackedTags2TagsPart;

short byteCount is OK. short chunkId limits the file size to 64k  chunks == 
128MBytes (too small for some systems). short objectId limits the fs to 64k 
files (probably OK).
short sequenceNumber is most definitely **not** OK since this is incremented 
every time a new block is allocated. That would limit the fs usage.

There is also an issue that the tag fields get overloaded with other values to 
speed scanning. Making the fields too tight will break that. For example, we 
sometimes stash the whole file size in byteCount. Having file sizes limited 
to 64k would be a bit depressing. If you use your scheme, please disable all 
the extraHeader code in yaffs_PackTags2() and yaffs_UnpackTags2(). yaffs2 
works fine without the extraheader stuff and will just use it if it is 
available.

I'll think a bit about this, but I think a more compressed packed tags 
structure could fit in 16 bytes including the ecc on tags and only minimal 
loss of exta header tags etc.

Perhaps something like the following will work pretty well (but some mods 
required to the extended header stuff):

typedef struct __attribute__((packed)) {
    unsigned sequenceNumber;
    unsigned objectId:24;
    unsigned colParity:8;
    unsigned chunkId:24;
    unsigned lineParity:8;
    unsigned byteCount:24;
    unsigned  lineParityPrime:8;
} yaffs_PackedTags2TagsPart;

-- CHarles