On Thursday 18 March 2010 04:50:56 Hugo Etchegoyen wrote:
  

Charles,

As I said in a previous mail, I was looking for means of speeding up
yaffs shutdown during power fail, and that included disabling GC. Your
last modification (the yaffs_gc_control parameter) addresses that issue.

However, although setting this flag to zero will prevent entering GC, it
will not abort a currently active GC. I had originally proposed to
decrease the value of maxCopies in GarbageCollectBlock() to make it
finer grained, but it looks faster and safer to just check your flag in
each iteration of the copying loop:

        for (/* init already done */;
             retVal == YAFFS_OK &&
             dev->gcChunk < dev->param.nChunksPerBlock &&
             (bi->blockState == YAFFS_BLOCK_STATE_COLLECTING) &&
             maxCopies > 0;
             dev->gcChunk++, oldChunk++) {

            if(dev->param.gcControl &&
                (dev->param.gcControl(dev) & 1) == 0)
                    break;

            if (yaffs_CheckChunkBit(dev, block, dev->gcChunk)) {

                /* This page is in use and might need to be copied off */
             ...
    

Doing that should be fine. It should be OK to interrupt gc at any page. That's 
pretty much what the maxCopy stuff is doing anyway.

I would tend to push the gcControl check into the for().




  

As yaffs may now be shut down disabling GC, and depending on how much
data is written with GC disabled (less than a block size in my case),
maybe at initialization time there might be less free blocks than
advisable. So I thought that it might be useful to catch up with delayed
GCs at the end of yaffs_GutsInitialise(). Do you think this is
necessary/advisable?
    

I don't think it is necessary.

  

Would this code handle that correctly? 

    /* Clean up any aborted checkpoint data */
    if(!dev->isCheckpointed && dev->blocksInCheckpoint > 0)
        yaffs_InvalidateCheckpoint(dev);

    

    /* Honor any pending GCs */
    do {
        x = dev->nErasedBlocks;
        yaffs_CheckGarbageCollection(dev);
    } while ( dev->gcBlock > 0 || x < dev->nErasedBlocks );

    

There is no need to resume any gc per se. yaffs_CheckGarbageCollection() 
already does iteration.

If you do it here then the checkpoint will be immediately invalidated which is 
perhaps not a good idea. You might as well just leave this to be sorted out 
on the first write.


  

    T(YAFFS_TRACE_TRACING,
      (TSTR("yaffs: yaffs_GutsInitialise() done.\n" TENDSTR)));

    return YAFFS_OK;

I also noticed that there is a possible source of delay when updating
dirty directories, since once started it will iterate over a list of
pending updates with the yaffs lock taken. I propose adding another
flag, yaffs_udd_control ("udd" for Update Dirty Directories) and the
corresponding callback (dev->uddControl). Then
yaffs_UpdateDirtyDirectories() can check this flag in each iteration of
the updating loop:

    while(!ylist_empty(&dev->dirtyDirectories)){

        if(dev->param.uddControl &&
            (dev->param.uddControl(dev) & 1) == 0)
            break;

        link = dev->dirtyDirectories.next;
        ylist_del_init(link);
        ...
    

That should be fine to do,
I'll be adding flags like these when I flesh out the background task with 
background garbage collection etc.

  

Please advise if these modifications are OK. I have attached the
modified files (yaffs_guts.h, yaffs_guts.c and yaffs_fs.c). I have
compiled and tried the module and it seems to work properly, including
disabling GC and UDD, writing some data, then enabling them again and
writing some more. I haven't yet simulated a power fail situation when
one would abort an ongoing GC or UDD, then write some data, sync and
unmount. I'll report back as soon as I can try that.

Best regards,
Hugo