-
Notifications
You must be signed in to change notification settings - Fork 822
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
what to use instead of lfs_file_open() when LFS_NO_MALLOC is defined #720
Comments
Take a look at lfs_file_opencfg.
|
Thank you. I understand correctly that this cache buffer must be allocated per file ? |
If there's potential to have multiple files open at one time then yes. I have multiple mounts, on one of my mounts, I have a buffer that's shared between files, but I have mechanisms to prevent multiple files getting opened at the same time (it's a mount that's reserved for logging). |
does |
Yes, EDIT: Below this line is wrong. See follow up comments. Thanks @Xywzel! You could also provide a custom LFS_MALLOC if you don't want to use // -DLFS_MALLOC=my_malloc
// -DLFS_FREE=my_free
uint8_t buffer[1024];
bool buffer_in_use = false;
void *my_malloc(size_t size) {
assert(size <= 1024);
assert(!buffer_in_use);
buffer_in_use = true;
return buffer;
}
void my_free(void *p) {
assert(p == buffer);
buffer_in_use = false;
} |
It doesn't look like LFS_MALLOC is used anywhere in the codebase. How does whatever is provided with that macro definition get used in the functions? |
Ah! You're right. I should have double checked. That must have been a bit disorienting. You can achieve the same effect by providing your own custom my_lfs_util.h file and defining -DLFS_CONFIG=my_lfs_util.h file during compilation. However this is a bit more tedious. If you do the functions would need to stay named |
Yeah, we opted to just adding our own microcontroller specific malloc and free trough our own version of the lfs_util.h, but for future versions, passing these as either function pointers in struct lfs_config or with macro definition before include of lfs.h would likely be a good option. |
I've been implementing a file system that uses LFS on SPI NOR flash, and FatFS for SD Cards for a little while now. I wanted to stick to static allocations, it took me a bit to understand lfs_file_opencfg vs lfs_file_open to avoid malloc, but seems clear enough now. I understand the need to conserve ram on tiny targets. Might it be clearer to adopt the strategy that FatFS uses, where the buffer is part of the file descriptor and there is a "LFS_FS_TINY" flag that moves the buffer to the file system level if so desired. This way there would be only one File_Open method and possibly be less confusing. Just a suggestion. See: /*---------------------------------------------------------------------------/
/ System Configurations
/---------------------------------------------------------------------------*/
#define FF_FS_TINY 0
/* This option switches tiny buffer configuration. (0:Normal or 1:Tiny)
/ At the tiny configuration, size of file object (FIL) is shrinked FF_MAX_SS bytes.
/ Instead of private sector buffer eliminated from the file object, common sector
/ buffer in the filesystem object (FATFS) is used for the file data transfer. */ |
Merging the static buffer into The main problem right now is that the buffer size is a runtime configuration, forcing the buffer to be a separate "allocation". Though this may change in #491 (needs a lot of work). Looking into what ChaN's FatFS is doing with FF_FS_TINY is really interesting, but if I understand it correctly it risks being extremely costly. In theory you can do the same in LittleFS: have a single shared buffer, and flush any pending writes when another file needs the buffer, but if you have two files open for writing this could create a lot of small flushes that can't be reused due to padding bytes (#725). But it may still be worth adding as an option when the user thinks multiple open files is unlikely. Separate read/write buffers could help for common patterns such as 1 open for reading 1 open for writing. I've also considered having a sort of Another option is allowing users to specify the number of files they will open and adding a simple fixed-sized allocator into littlefs itself. Though maybe that is overkill. There is certainly room for improvement here. |
This is improving over here: #909 Let me know if anything looks amiss. |
#614 made the use of lfs_file_open() not possible when compiling with LFS_NO_MALLOC defined. What function should I use instead to open files?
The text was updated successfully, but these errors were encountered: