FAT: Add support for block sizes of 512-4096 bytes #3972
Conversation
geky
force-pushed
the
fat-big-blocks
branch
3 times, most recently
from
971d19a to
15e38ac
Compare
|
retest uvisor |
This seems pretty serious given its a multi-threaded OS by default, no? |
|
How compatible are these changes if we upgrade to 0.12? |
|
/morph test-nightly |
There is appropriate locking, so it's still is thread safe. The trade off is ram (extra 4kbyte per file) vs some speed, but the driver is already sharing a single spi resource that serializes any storage operations.
It doesn't look like there have been many architectural changes, so it should be an easy update. Though I can look into upstreaming the HEAPBUF option. Here's the chanfs changelog (we're on R0.11a): |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
| @@ -146,6 +146,16 @@ DWORD get_fattime(void) | |||
| | (DWORD)(ptm->tm_sec/2 ); | |||
| } | |||
|
|
|||
| void *ff_memalloc(UINT size) | |||
| { | |||
| return malloc(size); | |||
There was a problem hiding this comment.
should this be new or is the OOM condition caught by the caller?
There was a problem hiding this comment.
Ah yeah, you're right this should probably be new
There was a problem hiding this comment.
Actually I changed this to propogate the error up, it's used for allocating large file buffers, so it's not unreasonable to be propogated as an error code (complies to posix and matches existing use of ff_memalloc in the chanfs layer).
|
One last question around the performance of using _FS_TINY. Is there a speed impact? The comment is a bit unclear. Also I think trying to get the _FS_HEAPBUF mainlined would give a bit more confidence this is a reasonable change. |
|
@geky Update for this one? |
|
@geky bump |
|
It should be updated per feedback. In regards to speed change, here's a quick benchmark using the "basic" tests found in the sd-driver:
In the scale of block device operations there is no speed change. There may be a speed change when multiple files are being written to from multiple threads, but we really aren't there yet. Memory consumptions seems to be the higher priority. |
There was a problem hiding this comment.
LGTM although there is an open issue (enhancement) to upgrade the filesystem. I hope the issue author will make the patch but still makes wonder how long this patch will last before getting overwritten by a copy paste upgrade if not accepted upstream. #4285
|
/morph test-nightly |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputTest failed! |
|
failure looks like udp loss issue fixed in #4369 |
|
|
The issue is sporadic, though I think the core team already killed the job to make way for higher priority prs. |
Result: ABORTEDYour command has finished executing! Here's what you wrote!
OutputBuild failed! |
|
But yeah, we should merge #4369 as soon as we can, currently test-nightly is a bit haywire |
|
/morph test-nightly |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputTest failed! |
This is necessary for support of block devices with >512 byte
blocks, such as most SPI flash parts.
- Enabled support of up to 4096 byte blocks
- Added support for heap-backed buffers using _FS_HEAPBUF
- Necessary to avoid stack overflows
- Avoids over-aggresive allocations of _MAX_SS
- Enabled _FS_TINY to further reduce memory footprint
- Haven't found a downside for this yet except for possible
thread contention
|
Rebased |
|
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
This is necessary for support of block devices with >512 byte blocks, such as most SPI flash parts. This is necessary for FAT support of SPI flash parts.
cc @simonqhughes