Unnecessary block erase?

[Frédéric SERGENT]

Hi Ricky,

I came accross something that drew my attention: while writing a rather big buffer with uffs_write to a file, I was amazed that some blocks got erased at that time, even though writing to a new file a freshly formatted partition.

After investigating, I discovered that these blocks get erased in uffs_BufFlush_Exist_With_BlockCover, when called by _BufFlush_NewBlock, which, indeed, passes new blocks not yet written in any way.

So I did this: before erasing them, I test their content by calling uffs_IsThisBlockUsed(), and do the erase step only if necessary.

I would like to know if you think this is safe.

If so, I think you might consider integrating this in the official UFFS for 2 reasons:
- on our hardware we get a huge boost in terms of writing speed on these use cases (almost 50% faster). I suppose it could be the same on other hardwares, and in any case, I don't think it can harm performances on any other.
- this would probably help in terms of reducing the wear of the flash memory by reducing the number of times it gets erased.

I did not have time to check in other places where uffs_FlashEraseBlock is called if this could also be of any benefit there. I could not put the code directly within uffs_FlashEraseBlock because it directly receives an int designating a block, and not a pointer to a uffs_BlockInfo. Maybe ther could a better way to do this.

In any case, I need your opinion on this: is it a good idea, or is there a risk I completely destroy other thing in totally remote places I did not figure while doing this? :-)

Please see below an abstract of what I did.

Thanks in advance,
Fred

static URET uffs_BufFlush_Exist_With_BlockCover(
            uffs_Device *dev,
            int slot,            //!< dirty group slot
            TreeNode *node,        //!< old data node on tree
            uffs_BlockInfo *bc    //!< old data block info
            )
{
(...)
        // if the recovered block is a bad block, it's time to process it.
        if (HAVE_BADBLOCK(dev) && dev->bad.block == newNode->u.list.block) {
            //uffs_Perror(UFFS_ERR_SERIOUS, "Still have bad block ?");
            uffs_BadBlockProcess(dev, newNode);
        }
        else {
            // erase recovered block, put it back to erased block list.
            if (uffs_IsThisBlockUsed(dev, bc))
            {
                uffs_FlashEraseBlock(dev, bc->block);
            }

            if (HAVE_BADBLOCK(dev))
                uffs_BadBlockProcess(dev, newNode);
            else
                uffs_TreeInsertToErasedListTail(dev, newNode);
        }
(...)
}

[Ricky Zheng]

Hi Fred,

Thanks for the bug report. It appears this is the only place the block has unnecessary erased.

Your fix is now in the master branch.

Regards,

Ricky.

2012/2/1 Frédéric SERGENT <fred.se...@gmail.com>

--
You received this message because you are subscribed to the Google Groups "UFFS" group.
To view this discussion on the web visit https://groups.google.com/d/msg/uffs/-/n7OyD74LDEcJ.
To post to this group, send email to uf...@googlegroups.com.
To unsubscribe from this group, send email to uffs+uns...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/uffs?hl=en.

[Frédéric SERGENT]

Great, thanks Ricky!

Fred

Le jeudi 2 février 2012 12:41:58 UTC+1, Ricky Zheng a écrit :

Hi Fred,

Thanks for the bug report. It appears this is the only place the block has unnecessary erased.

Your fix is now in the master branch.

Regards,

Ricky.

2012/2/1 Frédéric SERGENT

Hi Ricky,

To unsubscribe from this group, send email to uffs+unsubscribe@googlegroups.com.