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