From 6c2ef5f4c57d44ec7576da4c01e886a135e70d6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrik=20Dahlstr=C3=B6m?= Date: Sat, 19 Jun 2021 01:05:48 +0200 Subject: [PATCH] Add AFATFS_ASYNC_IO flag to make AsyncFatFS even more asynchronous This change introduces a new feature which enables afatfs_poll() to become an internal static function, and everything else works (almost) as before. The one difference is that callbacks may be called directly from the routine where it was provided, even before the routine returns. E.g. the callback for "unlink complete" may be called before afatfs_funlink() returns. The feature can be enabled by compiling AsyncFatFS with AFATFS_ASYNC_IO defined. --- Readme.md | 16 +++++++ lib/asyncfatfs.c | 79 ++++++++++++++++++++++++++++---- lib/asyncfatfs.h | 2 + tests/common.c | 12 +++++ tests/common.h | 2 + tests/sdcard_sim.c | 4 +- tests/test_file_delete.c | 31 ++++++++----- tests/test_file_modes.c | 3 +- tests/test_file_size.c | 5 +- tests/test_file_size_powerloss.c | 5 +- tests/test_logging_workload.c | 3 +- tests/test_root_fill.c | 3 +- tests/test_subdir_fill.c | 3 +- tests/test_volume_fill.c | 3 +- 14 files changed, 140 insertions(+), 31 deletions(-) diff --git a/Readme.md b/Readme.md index 93ce74c..f0060eb 100644 --- a/Readme.md +++ b/Readme.md @@ -75,3 +75,19 @@ Cleanflight / Betaflight's "blackbox" logging system: [filesystem consumer code] You'll notice that since most filesystem operations will fail and ask you to retry when the card is busy, it becomes natural to call it using a state-machine from your app's main loop - where you only advance to the next state once the current operation succeeds, calling afatfs_poll() in-between so that the filesystem can complete its queued tasks. + +### Asynchronous I/O + +There is experimental support for removing the need to call `afatfs_poll()` and to remove the `sdcard_poll()` +primitive. This feature can be activated by compiling AsyncFatFS with the "AFATFS_ASYNC_IO" flag. When this flag is +set, `afatfs_poll()` will be called internally from the callback function provided to `sdcard_readBlock()` and +`sdcard_writeBlock()`. AsyncFatFS will no longer call `sdcard_poll()` if this is enabled. + +It is **strongly recommended** that `sdcard_readBlock()` and `sdcard_writeBlock()` implement some kind of scheduling +and never call the provided callback directly. Since the callback functions themselves will call `afatfs_poll()` +internally (in this mode), another call to any of the SD card primitives might be triggered. This will lead to +AsyncFatFS performing an unbounded amount of work (particularly at card init when writing the freefile) and using an +unbounded amount of stack size. If you have an infinite stack, then please go ahead. If not, then make sure that you +use the callbacks further up the stack, e.g. from your main loop. + +Similarly, it is important that you update any internal state variables *before* you use any callback. diff --git a/lib/asyncfatfs.c b/lib/asyncfatfs.c index 37002e7..3a45e76 100644 --- a/lib/asyncfatfs.c +++ b/lib/asyncfatfs.c @@ -490,6 +490,10 @@ static afatfs_t afatfs; static void afatfs_fileOperationContinue(afatfsFile_t *file); static uint8_t* afatfs_fileLockCursorSectorForWrite(afatfsFilePtr_t file); static uint8_t* afatfs_fileRetainCursorSectorForRead(afatfsFilePtr_t file); +static void afatfs_fileOperationsPoll(); +#ifdef AFATFS_ASYNC_IO +static void afatfs_poll(); +#endif static uint32_t roundUpTo(uint32_t value, uint32_t rounding) { @@ -639,6 +643,9 @@ static void afatfs_sdcardReadComplete(sdcardBlockOperation_e operation, uint32_t break; } } +#ifdef AFATFS_ASYNC_IO + afatfs_poll(); +#endif } /** @@ -670,6 +677,9 @@ static void afatfs_sdcardWriteComplete(sdcardBlockOperation_e operation, uint32_ break; } } +#ifdef AFATFS_ASYNC_IO + afatfs_poll(); +#endif } /** @@ -904,6 +914,7 @@ static afatfsOperationStatus_e afatfs_cacheSector(uint32_t physicalSectorIndex, if (cacheSectorIndex == -1) { // We don't have enough free cache to service this request right now, try again later + afatfs_flush(); return AFATFS_OPERATION_IN_PROGRESS; } @@ -1454,7 +1465,7 @@ static afatfsOperationStatus_e afatfs_saveDirectoryEntry(afatfsFilePtr_t file, a entry->fileSize = file->physicalSize; break; case AFATFS_SAVE_DIRECTORY_DELETED: - entry->filename[0] = FAT_DELETED_FILE_MARKER; + entry->filename[0] = (char)FAT_DELETED_FILE_MARKER; //Fall through case AFATFS_SAVE_DIRECTORY_FOR_CLOSE: @@ -1560,6 +1571,9 @@ static afatfsOperationStatus_e afatfs_appendRegularFreeClusterContinue(afatfsFil file->operation.operation = AFATFS_FILE_OPERATION_NONE; } + // flush any pending writes + afatfs_flush(); + return AFATFS_OPERATION_SUCCESS; break; case AFATFS_APPEND_FREE_CLUSTER_PHASE_FAILURE: @@ -2054,7 +2068,13 @@ static afatfsOperationStatus_e afatfs_fseekInternal(afatfsFilePtr_t file, uint32 opState->callback = callback; opState->seekOffset = offset; - return AFATFS_OPERATION_IN_PROGRESS; + // Do an initial round of processing + if (afatfs_fseekInternalContinue(file)) { + return AFATFS_OPERATION_SUCCESS; + } + else { + return AFATFS_OPERATION_IN_PROGRESS; + } } } @@ -2262,6 +2282,9 @@ static afatfsOperationStatus_e afatfs_extendSubdirectoryContinue(afatfsFile_t *d opState->callback(directory); } + // flush any pending writes + afatfs_flush(); + return AFATFS_OPERATION_SUCCESS; break; case AFATFS_EXTEND_SUBDIRECTORY_PHASE_FAILURE: @@ -2475,6 +2498,9 @@ static afatfsOperationStatus_e afatfs_ftruncateContinue(afatfsFilePtr_t file, bo opState->callback(file); } + // flush any pending writes + afatfs_flush(); + return AFATFS_OPERATION_SUCCESS; break; } @@ -2491,7 +2517,7 @@ static afatfsOperationStatus_e afatfs_ftruncateContinue(afatfsFilePtr_t file, bo * * Returns true if the operation was successfully queued or false if the file is busy (try again later). * - * The callback is called once the file has been truncated (some time after this routine returns). + * The callback is called once the file has been truncated, which MAY occur before this routine returns. */ bool afatfs_ftruncate(afatfsFilePtr_t file, afatfsFileCallback_t callback) { @@ -2677,6 +2703,9 @@ static void afatfs_createFileContinue(afatfsFile_t *file) } } + // flush any pending writes + afatfs_flush(); + file->operation.operation = AFATFS_FILE_OPERATION_NONE; opState->callback(file); break; @@ -2714,8 +2743,10 @@ static void afatfs_funlinkContinue(afatfsFilePtr_t file) /** * Delete and close the file. * - * Returns true if the operation was successfully queued (callback will be called some time after this routine returns) + * Returns true if the operation was successfully completed or queued, * or false if the file is busy and you should try again later. + * + * If provided, the callback will be called after the operation completes (pass NULL for no callback). */ bool afatfs_funlink(afatfsFilePtr_t file, afatfsCallback_t callback) { @@ -2742,6 +2773,9 @@ bool afatfs_funlink(afatfsFilePtr_t file, afatfsCallback_t callback) file->operation.operation = AFATFS_FILE_OPERATION_UNLINK; + // Start the unlink operation before returning + afatfs_funlinkContinue(file); + return true; } @@ -2795,6 +2829,11 @@ static afatfsFilePtr_t afatfs_createFile(afatfsFilePtr_t file, const char *name, afatfs_createFileContinue(file); + if (file->operation.operation != AFATFS_FILE_OPERATION_CREATE_FILE && file->operation.operation != AFATFS_FILE_OPERATION_NONE) { + // file operation changed - process it + afatfs_fileOperationsPoll(); + } + return file; } @@ -3394,6 +3433,7 @@ static void afatfs_initContinue() afatfs_chdir(NULL); afatfs.initPhase++; + goto doMore; } else { afatfs.lastError = AFATFS_ERROR_BAD_FILESYSTEM_HEADER; afatfs.filesystemState = AFATFS_FILESYSTEM_STATE_FATAL; @@ -3407,9 +3447,20 @@ static void afatfs_initContinue() afatfs_createFile(&afatfs.freeFile, AFATFS_FREESPACE_FILENAME, FAT_FILE_ATTRIBUTE_SYSTEM | FAT_FILE_ATTRIBUTE_READ_ONLY, AFATFS_FILE_MODE_CREATE | AFATFS_FILE_MODE_RETAIN_DIRECTORY, afatfs_freeFileCreated); + // Check if our callback was called before we could return + if (afatfs.filesystemState != AFATFS_FILESYSTEM_STATE_FATAL + && afatfs.initPhase != AFATFS_INITIALIZATION_FREEFILE_CREATING + ) { + goto doMore; + } break; case AFATFS_INITIALIZATION_FREEFILE_CREATING: afatfs_fileOperationContinue(&afatfs.freeFile); + if (afatfs.filesystemState != AFATFS_FILESYSTEM_STATE_FATAL + && afatfs.initPhase != AFATFS_INITIALIZATION_FREEFILE_CREATING + ) { + goto doMore; + } break; case AFATFS_INITIALIZATION_FREEFILE_FAT_SEARCH: if (afatfs_findLargestContiguousFreeBlockContinue() == AFATFS_OPERATION_SUCCESS) { @@ -3491,16 +3542,26 @@ static void afatfs_initContinue() } } +static bool afatfs_continue() +{ + bool cont = true; +#ifndef AFATFS_ASYNC_IO + cont = sdcard_poll(); +#endif + return cont ? afatfs_flush() : false; +} + /** * Check to see if there are any pending operations on the filesystem and perform a little work (without waiting on the - * sdcard). You must call this periodically. + * sdcard). */ +#ifdef AFATFS_ASYNC_IO +static +#endif void afatfs_poll() { // Only attempt to continue FS operations if the card is present & ready, otherwise we would just be wasting time - if (sdcard_poll()) { - afatfs_flush(); - + if (afatfs_continue()) { switch (afatfs.filesystemState) { case AFATFS_FILESYSTEM_STATE_INITIALIZATION: afatfs_initContinue(); @@ -3577,6 +3638,8 @@ void afatfs_init() #ifdef AFATFS_USE_INTROSPECTIVE_LOGGING sdcard_setProfilerCallback(afatfs_sdcardProfilerCallback); #endif + + afatfs_poll(); } /** diff --git a/lib/asyncfatfs.h b/lib/asyncfatfs.h index 06659c4..c671a0d 100644 --- a/lib/asyncfatfs.h +++ b/lib/asyncfatfs.h @@ -65,7 +65,9 @@ void afatfs_findLast(afatfsFilePtr_t directory); bool afatfs_flush(); void afatfs_init(); bool afatfs_destroy(bool dirty); +#ifndef AFATFS_ASYNC_IO void afatfs_poll(); +#endif uint32_t afatfs_getFreeBufferSpace(); uint32_t afatfs_getContiguousFreeSpace(); diff --git a/tests/common.c b/tests/common.c index 6699d60..37f6332 100644 --- a/tests/common.c +++ b/tests/common.c @@ -1,4 +1,5 @@ #include "common.h" +#include "sdcard.h" #include #include @@ -65,3 +66,14 @@ void testAssert(bool condition, const char *errorMessage) exit(-1); } } + +void testPoll() +{ +#ifdef AFATFS_ASYNC_IO + // Let's reuse the sdcard_poll() function for asynchronous I/O mode, even if it's not strictly necessary. + // It could as well be implemented as a separate worker thread. + sdcard_poll(); +#else + afatfs_poll(); +#endif +} diff --git a/tests/common.h b/tests/common.h index c9ae5cd..f8e6dbc 100644 --- a/tests/common.h +++ b/tests/common.h @@ -7,4 +7,6 @@ bool validateLogTestEntries(afatfsFilePtr_t file, uint32_t *entryIndex, uint32_t void testAssert(bool condition, const char *errorMessage); +void testPoll(); + #define TEST_LOG_ENTRY_SIZE 16 diff --git a/tests/sdcard_sim.c b/tests/sdcard_sim.c index be9d81e..e721cb0 100644 --- a/tests/sdcard_sim.c +++ b/tests/sdcard_sim.c @@ -80,6 +80,8 @@ static void sdcard_continueReadBlock() if (--sdcard.currentOperation.countdownTimer <= 0) { uint64_t byteIndex = (uint64_t) sdcard.currentOperation.blockIndex * SDCARD_SIM_BLOCK_SIZE; + sdcard.state = SDCARD_STATE_READY; + fseeko(simFile, byteIndex, SEEK_SET); if (fread(sdcard.currentOperation.buffer, sizeof(uint8_t), SDCARD_SIM_BLOCK_SIZE, simFile) == SDCARD_SIM_BLOCK_SIZE) { @@ -93,8 +95,6 @@ static void sdcard_continueReadBlock() fprintf(stderr, "SDCardSim: fread failed on underlying file\n"); exit(-1); } - - sdcard.state = SDCARD_STATE_READY; } } diff --git a/tests/test_file_delete.c b/tests/test_file_delete.c index e5fb23d..c75d032 100644 --- a/tests/test_file_delete.c +++ b/tests/test_file_delete.c @@ -182,9 +182,10 @@ bool continueSpaceReclaimTest(bool start) afatfs_fopen("test.txt", "w+", spaceReclaimTestFileCreatedForEmpty); break; case SPACE_RECLAIM_TEST_STAGE_EMPTY_DELETE: - if (afatfs_funlink(testFile, spaceReclaimTestFileEmptyDeleted)) { - // Wait for the unlink to complete - reclaimTestStage = SPACE_RECLAIM_TEST_STAGE_IDLE; + reclaimTestStage = SPACE_RECLAIM_TEST_STAGE_IDLE; + if (!afatfs_funlink(testFile, spaceReclaimTestFileEmptyDeleted)) { + // retry later + reclaimTestStage = SPACE_RECLAIM_TEST_STAGE_EMPTY_DELETE; } break; case SPACE_RECLAIM_TEST_STAGE_SOLID_APPEND_INIT: @@ -204,9 +205,10 @@ bool continueSpaceReclaimTest(bool start) } break; case SPACE_RECLAIM_TEST_STAGE_SOLID_APPEND_DELETE: - if (afatfs_funlink(testFile, spaceReclaimTestFileSolidAppendDeleted)) { - // Wait for the unlink to complete - reclaimTestStage = SPACE_RECLAIM_TEST_STAGE_IDLE; + reclaimTestStage = SPACE_RECLAIM_TEST_STAGE_IDLE; + if (!afatfs_funlink(testFile, spaceReclaimTestFileSolidAppendDeleted)) { + // retry later + reclaimTestStage = SPACE_RECLAIM_TEST_STAGE_SOLID_APPEND_DELETE; } break; case SPACE_RECLAIM_TEST_STAGE_APPEND_INIT: @@ -226,9 +228,10 @@ bool continueSpaceReclaimTest(bool start) } break; case SPACE_RECLAIM_TEST_STAGE_APPEND_DELETE: - if (afatfs_funlink(testFile, spaceReclaimTestFileAppendDeleted)) { - // Wait for the unlink to complete - reclaimTestStage = SPACE_RECLAIM_TEST_STAGE_IDLE; + reclaimTestStage = SPACE_RECLAIM_TEST_STAGE_IDLE; + if (!afatfs_funlink(testFile, spaceReclaimTestFileAppendDeleted)) { + // retry later + reclaimTestStage = SPACE_RECLAIM_TEST_STAGE_APPEND_DELETE; } break; case SPACE_RECLAIM_TEST_STAGE_IDLE: @@ -392,8 +395,11 @@ bool continueSpaceRetainTest(bool start, const char *fileMode) break; case SPACE_RETAIN_TEST_STAGE_UNLINK_B: if (afatfs_funlink(retainTestFileB, retainTestFileBDeleted)) { - retainTestFileB = NULL; - retainTestStage = SPACE_RETAIN_TEST_STAGE_IDLE; + // need to check state again in case callback has already been triggered + if (retainTestStage == SPACE_RETAIN_TEST_STAGE_UNLINK_B) { + retainTestFileB = NULL; + retainTestStage = SPACE_RETAIN_TEST_STAGE_IDLE; + } } break; case SPACE_RETAIN_TEST_STAGE_VERIFY_A_OPEN: @@ -506,7 +512,7 @@ int main(int argc, char **argv) bool keepGoing = true; while (keepGoing) { - afatfs_poll(); + testPoll(); switch (afatfs_getFilesystemState()) { case AFATFS_FILESYSTEM_STATE_READY: @@ -524,6 +530,7 @@ int main(int argc, char **argv) } while (!afatfs_destroy(false)) { + testPoll(); } sdcard_sim_destroy(); diff --git a/tests/test_file_modes.c b/tests/test_file_modes.c index fc5a6a4..6fff276 100644 --- a/tests/test_file_modes.c +++ b/tests/test_file_modes.c @@ -217,7 +217,7 @@ int main(int argc, char **argv) bool keepGoing = true; while (keepGoing) { - afatfs_poll(); + testPoll(); switch (afatfs_getFilesystemState()) { case AFATFS_FILESYSTEM_STATE_READY: @@ -235,6 +235,7 @@ int main(int argc, char **argv) } while (!afatfs_destroy(false)) { + testPoll(); } sdcard_sim_destroy(); diff --git a/tests/test_file_size.c b/tests/test_file_size.c index 8e230ff..d300fea 100644 --- a/tests/test_file_size.c +++ b/tests/test_file_size.c @@ -57,7 +57,7 @@ static void initFilesystem() afatfs_init(); while (afatfs_getFilesystemState() != AFATFS_FILESYSTEM_STATE_READY) { - afatfs_poll(); + testPoll(); if (afatfs_getFilesystemState() == AFATFS_FILESYSTEM_STATE_FATAL) { fprintf(stderr, "[Fail] Fatal filesystem error during init\n"); @@ -252,7 +252,7 @@ int main(int argc, char **argv) bool keepGoing = true; while (keepGoing) { - afatfs_poll(); + testPoll(); switch (afatfs_getFilesystemState()) { case AFATFS_FILESYSTEM_STATE_READY: @@ -270,6 +270,7 @@ int main(int argc, char **argv) } while (!afatfs_destroy(false)) { + testPoll(); } sdcard_sim_destroy(); diff --git a/tests/test_file_size_powerloss.c b/tests/test_file_size_powerloss.c index 6d1d236..1e265f8 100644 --- a/tests/test_file_size_powerloss.c +++ b/tests/test_file_size_powerloss.c @@ -58,7 +58,7 @@ static void initFilesystem() afatfs_init(); while (afatfs_getFilesystemState() != AFATFS_FILESYSTEM_STATE_READY) { - afatfs_poll(); + testPoll(); if (afatfs_getFilesystemState() == AFATFS_FILESYSTEM_STATE_FATAL) { fprintf(stderr, "[Fail] Fatal filesystem error during init\n"); @@ -248,7 +248,7 @@ int main(int argc, char **argv) bool keepGoing = true; while (keepGoing) { - afatfs_poll(); + testPoll(); switch (afatfs_getFilesystemState()) { case AFATFS_FILESYSTEM_STATE_READY: @@ -266,6 +266,7 @@ int main(int argc, char **argv) } while (!afatfs_destroy(false)) { + testPoll(); } sdcard_sim_destroy(); diff --git a/tests/test_logging_workload.c b/tests/test_logging_workload.c index f28f3dd..09b88a5 100644 --- a/tests/test_logging_workload.c +++ b/tests/test_logging_workload.c @@ -207,7 +207,7 @@ int main(int argc, char **argv) bool keepGoing = true; while (keepGoing) { - afatfs_poll(); + testPoll(); switch (afatfs_getFilesystemState()) { case AFATFS_FILESYSTEM_STATE_READY: @@ -225,6 +225,7 @@ int main(int argc, char **argv) } while (!afatfs_destroy(false)) { + testPoll(); } sdcard_sim_destroy(); diff --git a/tests/test_root_fill.c b/tests/test_root_fill.c index 681e348..d1d09ea 100644 --- a/tests/test_root_fill.c +++ b/tests/test_root_fill.c @@ -134,7 +134,7 @@ int main(int argc, char **argv) bool keepGoing = true; while (keepGoing) { - afatfs_poll(); + testPoll(); switch (afatfs_getFilesystemState()) { case AFATFS_FILESYSTEM_STATE_READY: @@ -152,6 +152,7 @@ int main(int argc, char **argv) } while (!afatfs_destroy(false)) { + testPoll(); } sdcard_sim_destroy(); diff --git a/tests/test_subdir_fill.c b/tests/test_subdir_fill.c index 34e5538..6bd1623 100644 --- a/tests/test_subdir_fill.c +++ b/tests/test_subdir_fill.c @@ -153,7 +153,7 @@ int main(int argc, char **argv) bool keepGoing = true; while (keepGoing) { - afatfs_poll(); + testPoll(); switch (afatfs_getFilesystemState()) { case AFATFS_FILESYSTEM_STATE_READY: @@ -171,6 +171,7 @@ int main(int argc, char **argv) } while (!afatfs_destroy(false)) { + testPoll(); } sdcard_sim_destroy(); diff --git a/tests/test_volume_fill.c b/tests/test_volume_fill.c index 126b8b8..22f94f5 100644 --- a/tests/test_volume_fill.c +++ b/tests/test_volume_fill.c @@ -218,7 +218,7 @@ int main(int argc, char **argv) bool keepGoing = true; while (keepGoing) { - afatfs_poll(); + testPoll(); switch (afatfs_getFilesystemState()) { case AFATFS_FILESYSTEM_STATE_READY: @@ -236,6 +236,7 @@ int main(int argc, char **argv) } while (!afatfs_destroy(false)) { + testPoll(); } sdcard_sim_destroy();