OK, thanks a lot.

Hugo

Charles Manning escribió:
> 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
>>     
>
>
>
>
>   

-- 

Ing. Hugo Eduardo Etchegoyen*
*Gerente Dto. Software de Base

Compañía Hasar | Grupo Hasar*
*Marcos Sastre y José Ingenieros
El Talar. Pacheco
[B1618CSD] Buenos Aires. Argentina
Tel [54 11] 4117 8900 | Fax [54 11] 4117 8998
E-mail: hetchegoyen@hasar.com
Visítenos en: www.hasar.com
<http://www.hasar.com>Información legal y política de confidencialidad: 
www.grupohasar.com/disclaimer <http://www.grupohasar.com/disclaimer>