sdcErase() in HAL 3.0

[ulikoehler]

Hi everyone,
I'm currently playing around with the current git revision of ChibiOS 3.x to see if I can port my source code once it will be released (with 3.0 using the ST USB host library is not a giant pain any more!).

One issue I encountered is that in this commit https://github.com/mabl/ChibiOS/commit/145cae7d32bb11a842820b15c621526fce2c524e the sdcErase() function was removed from the SDC HAL entirely.

Is there a specific reason why it will not be included in the 3.x release? I indeed use it in some of my filesystem code and believe it should be significantly more efficient than overwriting with 0x00 / 0xFF blocks. I believe it might also be relevant to some wear levelling implementations. Currently the CMD_ERASE does not seem to be used anywhere in the sourcecode at all.

Thanks & Best regards,
Uli

[Giovanni]

hi,

It was not part of the block device interface, we thought it was not required. I will look into reintroducing it.

Giovanni

[ulikoehler]

Hi Giovanni,
thanks for your quick reply! I'm looking forward for it to be re-added. Actually the current (trunk) fatfs_diskio.c has this code fragment:

Code: Select all

#if _USE_ERASE
    case CTRL_ERASE_SECTOR:
        sdcErase(&SDCD1, *((DWORD *)buff), *((DWORD *)buff + 1));
        return RES_OK;
#endif


While _USE_ERASE is defined as FALSE, I think having it included in FatFS would be quite valuable.

The MMC SPI driver indeed has a mmcErase() which has not been removed. Besides me/FatFS using the erase function, I think it is advisable to add some symmetry between the drivers so porting from one to another is easy.

Best regards,
Uli

[barthess]

Hi ulikoehler
Erase function was deleted because it was look useless for me.
Could you perform some performance tests with and without erase?
I especially interested in FAT speed results. If there will be some
speed boost (20% or more) than I will be happy to resurrect it.
No sarcasm.

[wild-boar]

I was the one who contributed the erase functionality for SDIO in 2012.

Enabling erase is not solely about performance. The question is: Should the card be notified when the OS decides a block is no longer used?

These cards have an embedded micro-controller and could make better wear-leveling decisions if they are explicitly told of erases. Do they? I do not have enough long-term experience and samples to comment. I do not have notes from my testing.

For my uses, I wanted long-term stability and if I ever needed to fix a corrupted filesystem then I have the knowledge that all non-empty blocks contain valid data.

My application with SDIO has not been ported to 3.0. If I do upgrade, I can submit the patch again. The cards support this functionality. FatFs supports this feature and application developers should have the option to enable it if they believe their application needs it.

[Giovanni]

It is not needed, adding the code again takes no effort, it is still there in the repository.

Giovanni

[barthess]

I am the man wasted lots of hours because of fucking with SD cards and crappy SDC specifications (clock detection... please kill me).
1) I am totally unsure that reported erase block size has correct meaning for all available cards.
2) Will preerasing gains any benefits. If you have some real world benefits/examples/measurements than please post them in this thread.
I just do not want to maintain useless code.

[Giovanni]

I doubt it is useful for filesystems using FAT but probably under some specially designed filesystems meant for flash devices it could make sense, erasing a block with just a command should be faster than transferring a block of FFs in order to achieve the same thing.

Anyway, having users that already use the feature should be a reason good enough to keep it, don't you agree?

Giovanni

[barthess]

don't you agree?

Do not. Until users submit some benchmarks. Modern cards have wear leveling algorithms smarter than users' code.

[ulikoehler]

Sorry, I have not been following this thread over the past view days.

@barthess, Giovanni: I have written a special datalogging filesystem that, for me, solves the problem of crappy SD cards killing FAT so FatFS could not mount it at all (the linux implementation could do it without issues). One reason for this was sudden power loss (probably during write).

While sdcErase() is certainly clearly problem-solver for me, I have to work with a myriad of different SD cards (whatever the customer chose to use). That means, I want to provide as much information to the SD card as possible.

Yeah, sure, some cards do have better wear levelling, others do not. For your application it might be a non-issue if there is no sdcErase(), but I can think of applications I want to implement in the future where sdcErase() might come in handy.

@barthess : I generally do not believe removing a features is the way to go, regardless of who likes it or not. If you don't like it (and I do believe you that there are significant reasons to do so), why not mark it in the documentation like this?

Code: Select all

/**
  * @note: WARNING: There have been issues with some
  */


I'm nowhere near as deep in the SD(IO) specs as you are (I do hope I never have to get into it), but I believe you're saying: Because there are some SD cards who don't benefit from sdcErase, you should not support it for any card.

That being said, I'll try to contribute some benchmarks out of curiosity, however I'm quite occupied with my bachelor's thesis and getting ChibiOS 3.x trunk to work (two separate things), so please don't expect them too soon. I also expect them to be significantly dependant on the SD card in use.

However, I do strongly object to your reasoning that including sdcErase() in 3.x should depend on any benchmarks. As @wild-boar said, this only one of many reasons. I believe that ChibiOS should attempt to support whatever realistically possible and not patronize users by removing some (IMO) well-working 2.x functionality.

Example for another potential usecase:
Last week, I send an inquiry to ATP Inc asking if their secure erase feature (which should securely delete the data) in their industrial uSD card series is implemented by means of ACMD38 (which is what sdcErase() uses). I have not received an answer yet and as a matter of fact I don't have an application for secure erase right now, but I might have one in the future. When I have an application that could benefit from secure erase, I'd love if I would be able to use ChibiOS without modifications right away.

Actually, ACMD38 is called SECURE_ERASE, see for example this patent: https://www.google.com/patents/US20130276130
Therefore I think it is reasonable to rename sdcErase() to sdcSecureErase() before 3.0 is released.

I recognize that including sdcErase() introduces some issues with code maintenance but I believe the source code is neither especially complex nor long and the feature has been well-tested.

For now I changed my filesystem code to overwrite with 0x00. This will most certainly work in the short term, but in the long term with ChibiOS HAL 3.x I'd prefer to see a system which enables me to choose from both possibilities depending on application and application-specific benchmarks.

Best regards, Uli