Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support no validation to improve write performance #381

Closed
wants to merge 1 commit into from
Closed

Support no validation to improve write performance #381

wants to merge 1 commit into from

Conversation

gmpy
Copy link

@gmpy gmpy commented Feb 21, 2020

It can improve write performance if no validation, but drivers
must ensure data integrity.

I test lfs on spinand with NFTL. The spinand has 128M capacity
with 2K page size and 128K block size. The actually page size
changes to 4K on my using NFTL. And lfs is configured as follows:

.read_size = 4096
.write_szie = 4096
.block_szie = 4096
.block_count = 25680
.block_cycles = -1
.lookahead_size = 256

The performance on raw device without littlefs:

read average speed: 14361.85 KB/s
write average speed: 2994.74 KB/s

The performance on lfs with validation as follow:

read average speed: 12094.49 KB/s
write average speed: 2074.97 KB/s

The performance on lfs without validation as follow:

read average speed: 12427.18 KB/s
write average speed: 2674.33 KB/s

It really improves 600 KB/s to write if disable validation.
To me, the NFTL has ensure data integrity, there is no need
to validate again on lfs. It is also helpfull to flash like
MMC.

It can improve write performance if no validation, but drivers
must ensure data integrity.

I test lfs on spinand with NFTL. The spinand has 128M capacity
with 2K page size and 128K block size. The actually page size
changes to 4K on my using NFTL. And lfs is configured as follows:

    .read_size = 4096
    .write_szie = 4096
    .block_szie = 4096
    .block_count = 25680
    .block_cycles = -1
    .lookahead_size = 256

The performance on raw device without littlefs:

    read average speed: 14361.85 KB/s
    write average speed: 2994.74 KB/s

The performance on lfs with validation as follow:

    read average speed: 12094.49 KB/s
    write average speed: 2074.97 KB/s

The performance on lfs without validation as follow:

    read average speed: 12427.18 KB/s
    write average speed: 2674.33 KB/s

It really improves 600 KB/s to write if disable validation.
To me, the NFTL has ensure data integrity, there is no need
to validate again on lfs. It is also helpfull to flash like
MMC.
@gmpy gmpy requested a review from geky February 21, 2020 10:35
@geky
Copy link
Member

geky commented Feb 24, 2020

This is interesting, thanks for creating a pr!

I haven't considered a non-validation option a priority since usually the time spent erasing greatly outweighs the overhead of bus transactions. But I don't see a reason to not bring this in. It may also have more benefits for a multi-tasking system that can perform other operations during the erase time.

I was hurt before working on a project with many configuration options, as it made testing every configuration a pain. Though maybe littlefs is mature enough that it should expand the available configurations?

Currently #372 and related bug fixes is my priority, so I may wait on this until after the next release. Sorry about the delay.


It would also be interesting to look into improvements on how littlefs is configured in the same release #158 (comment) (though this should be a different PR)

At first glance it seems like you could balance the compile time and runtime concerns with something like this in lfs_utils.h

#define LFS_ALIGNMENT(lfs) lfs->config->cache_alignment

and similar for many other configurables.

You wouldn't be able to entirely get rid of the runtime cost using this mechanism, but for people who don't need these features, replacing the macros to return constants would potentially allow constant folding to collapse some of the work.

// validation, but drivers must ensure data integrity.
int validate;
#define LFS_ENABLE_VALIDATE 1
#define LFS_DISABLE_VALIDATE -1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these defines up to? It looks like they're unused.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are defined for user to assign option validate. To me, using these macro is more clarity than number.

@geky
Copy link
Member

geky commented Feb 24, 2020

There's actually two different methods littlefs uses to check disk-writes. You found the one used by files, but a separate method is used in lfs_dir_commitcrc to check dir commits without need to store the data in RAM.

You can probably skip this loop as well:
https://github.com/ARMmbed/littlefs/blob/88bc3335599bc4d4655cde38b4965099ec28ce00/lfs.c##L1297

@gmpy
Copy link
Author

gmpy commented Feb 26, 2020

There's actually two different methods littlefs uses to check disk-writes. You found the one used by files, but a separate method is used in lfs_dir_commitcrc to check dir commits without need to store the data in RAM.

You can probably skip this loop as well:
https://github.com/ARMmbed/littlefs/blob/88bc3335599bc4d4655cde38b4965099ec28ce00/lfs.c##L1297

I did miss it. But it has little impact on write performance. On my test on a 10M file, lfs_dir_commitcrc() is called twice and lfs_bd_flush() at lfs_bd_prog() is called 2565 times.

The performance as follow:

read average speed: 12362.17 KB/s
write average speed: 2691.20 KB/s

I suggest that keep checking here, which makes the cache of dir on flash drivers keeping hot since dir is read frequently. It pays little performance to check dir.

@pjsg
Copy link

pjsg commented Feb 26, 2020

One important feature of the current verification is that it catches logical errors where the programmed data is not written over 'erased' memory, but over 'used' memory. It would be a pity to trade off detection of corruption for a bit of performance.

@gmpy
Copy link
Author

gmpy commented Feb 28, 2020

One important feature of the current verification is that it catches logical errors where the programmed data is not written over 'erased' memory, but over 'used' memory. It would be a pity to trade off detection of corruption for a bit of performance.

I think 30% increase has been a lot. This is very important for some products with performance requirements. This is a switch, the user can choose whether to turn it on or not. To me, it can be enabled during the development phase, and can be closed after the product is formed for the highest experience.

@pjsg
Copy link

pjsg commented Mar 4, 2020

You might want to check that (at least) the first byte of the area that you are about to program is set to the erase value. If it isn't, then you should abort.

@gmpy
Copy link
Author

gmpy commented Mar 9, 2020

You might want to check that (at least) the first byte of the area that you are about to program is set to the erase value. If it isn't, then you should abort.

The minimum read size for nand/mmc is equal to page size. Take my using spinand as am example, the page size is 2KB. So reading one byte may be no difference to reading a page.

In addition, during my analysis, the nand flash driver always has ram cache. The lfs writes to cache on driver and reads for validation from cache. It should not be much different. There may be some unexpected situations. Please put this patch aside before i find it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants