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>
(text/plain)