[sdlog] fix some bugs

- fix dma buffer allocation for STM32F7 in sd logger and usb storage
- fix sdLog race condition when sdLogWrite are done at high frequency before sdLogInit/sdOpenLog are done
- fix calls to APi that shluld have been atomic : two fonctions are merged in one at the expense of long argument list
- add attribute format to sdLogWriteLog function (add more static checking)
This commit is contained in:
Alexandre Bustico
2017-03-03 18:09:53 +01:00
committed by Gautier Hattenberger
parent 210f94befa
commit 409ae11f6a
6 changed files with 47 additions and 42 deletions
@@ -232,7 +232,7 @@
/ These options have no effect at read-only configuration (_FS_READONLY = 1). */
#define _FS_LOCK 0
#define _FS_LOCK 6
/* The option _FS_LOCK switches file lock function to control duplicated file open
/ and illegal operation to open objects. This option must be 0 when _FS_READONLY
/ is 1.
@@ -385,9 +385,9 @@
sdlog message buffer and queue configuration
*/
#define SDLOG_QUEUE_BUCKETS 1024
#define SDLOG_MAX_MESSAGE_LEN 252
#define SDLOG_MAX_MESSAGE_LEN 300
#define SDLOG_NUM_FILES 2
#define SDLOG_ALL_BUFFERS_SIZE (SDLOG_NUM_FILES*4096)
#define SDLOG_ALL_BUFFERS_SIZE (SDLOG_NUM_FILES*8192)
+6 -6
View File
@@ -217,18 +217,18 @@ static void thd_startlog(void *arg)
sdOk = false;
} else {
removeEmptyLogs(PPRZ_LOG_DIR, PPRZ_LOG_NAME, 50);
if (sdLogOpenLog(&pprzLogFile, PPRZ_LOG_DIR, PPRZ_LOG_NAME, SDLOG_AUTO_FLUSH_PERIOD, true) != SDLOG_OK) {
if (sdLogOpenLog(&pprzLogFile, PPRZ_LOG_DIR,
PPRZ_LOG_NAME, SDLOG_AUTO_FLUSH_PERIOD, true,
SDLOG_CONTIGUOUS_STORAGE_MEM, false) != SDLOG_OK) {
sdOk = false;
}
// try to reserve contiguous mass storage memory
sdLogExpandLogFile(pprzLogFile, SDLOG_CONTIGUOUS_STORAGE_MEM, false);
#if FLIGHTRECORDER_SDLOG
removeEmptyLogs(FR_LOG_DIR, FLIGHTRECORDER_LOG_NAME, 50);
if (sdLogOpenLog(&flightRecorderLogFile, FR_LOG_DIR, FLIGHTRECORDER_LOG_NAME, SDLOG_AUTO_FLUSH_PERIOD, false) != SDLOG_OK) {
if (sdLogOpenLog(&flightRecorderLogFile, FR_LOG_DIR, FLIGHTRECORDER_LOG_NAME,
SDLOG_AUTO_FLUSH_PERIOD, false,
SDLOG_CONTIGUOUS_STORAGE_MEM, false) != SDLOG_OK) {
sdOk = false;
}
// try to reserve contiguous mass storage memory
sdLogExpandLogFile(flightRecorderLogFile, SDLOG_CONTIGUOUS_STORAGE_MEM, false);
#endif
}
@@ -176,6 +176,8 @@ static thread_t *sdLogThd = NULL;
static SdioError getNextFIL(FileDes *fd);
static void removeFromQueue(const size_t nbMsgToRFemov);
static void cleanQueue(const bool allQueue);
static SdioError sdLogExpandLogFile(const FileDes fileObject, const size_t sizeInMo,
const bool preallocate);
#if (CH_KERNEL_MAJOR > 2)
static void thdSdLog(void *arg) ;
@@ -270,18 +272,25 @@ SdioError sdLogFinish(void)
#ifdef SDLOG_NEED_QUEUE
SdioError sdLogOpenLog(FileDes *fd, const char *directoryName, const char *prefix,
const uint32_t autoFlushPeriod, const bool appendTagAtClose)
const uint32_t autoFlushPeriod, const bool appendTagAtClose,
const size_t sizeInMo, const bool preallocate)
{
FRESULT rc; /* fatfs result code */
SdioError sde; /* sdio result code */
SdioError sde = SDLOG_OK; /* sdio result code */
//DIR dir; /* Directory object */
//FILINFO fno; /* File information object */
char fileName[32];
sde = getNextFIL(fd);
/* local file descriptor
using fd is a bad idea since fd is set before fatfs objets are coherents
in a multithreaded application where sdLogXXX are done before sdLogWriteLog is done
we can have a race condition. setting fd only when fatfs files are opened resolve the problem
*/
FileDes ldf;
sde = getNextFIL(&ldf);
if (sde != SDLOG_OK) {
storageStatus = sde;
return sde;
return storageStatus = sde;
}
sde = getFileName(prefix, directoryName, fileName, sizeof(fileName), +1);
@@ -291,17 +300,19 @@ SdioError sdLogOpenLog(FileDes *fd, const char *directoryName, const char *prefi
}
rc = f_open(&fileDes[*fd].fil, fileName, FA_WRITE | FA_CREATE_ALWAYS);
rc = f_open(&fileDes[ldf].fil, fileName, FA_WRITE | FA_CREATE_ALWAYS);
if (rc) {
fileDes[*fd].inUse = false;
fileDes[ldf].inUse = false;
return storageStatus = SDLOG_FATFS_ERROR;
} else {
fileDes[*fd].tagAtClose = appendTagAtClose;
fileDes[*fd].autoFlushPeriod = autoFlushPeriod;
fileDes[*fd].lastFlushTs = 0;
fileDes[ldf].tagAtClose = appendTagAtClose;
fileDes[ldf].autoFlushPeriod = autoFlushPeriod;
fileDes[ldf].lastFlushTs = 0;
sde = sdLogExpandLogFile(ldf, sizeInMo, preallocate);
}
return storageStatus = SDLOG_OK;
*fd = ldf;
return storageStatus = sde;
}
SdioError sdLogCloseAllLogs(bool flush)
@@ -870,7 +881,7 @@ static void cleanQueue(const bool allQueue)
}
} while (true);
} else {
removeFromQueue(10000);
removeFromQueue(SDLOG_QUEUE_BUCKETS);
}
}
@@ -919,7 +930,7 @@ static msg_t thdSdLog(void *arg)
} ;
UINT bw;
static IN_STD_SECTION_CLEAR(struct PerfBuffer perfBuffers[SDLOG_NUM_FILES]);
static IN_DMA_SECTION_CLEAR(struct PerfBuffer perfBuffers[SDLOG_NUM_FILES]);
storageStatus = SDLOG_OK;
chRegSetThreadName("thdSdLog");
while (true) {
@@ -986,7 +997,8 @@ static msg_t thdSdLog(void *arg)
}
}
if (rc) {
chThdExit(storageStatus = SDLOG_FATFS_ERROR);
//chThdExit(storageStatus = SDLOG_FATFS_ERROR);
storageStatus = SDLOG_FATFS_ERROR;
} else if (bw != SDLOG_WRITE_BUFFER_SIZE) {
chThdExit(storageStatus = SDLOG_FSFULL);
}
@@ -65,7 +65,6 @@ extern "C" {
use of the api :
sdLogInit (initialize peripheral, verify sdCard availibility)
sdLogOpenLog : open file
sdLogExpandLogFile : reserve contiguous space on storage
sdLogWriteXXX : write log using one off the many function of the API
sdLogCloseLog : close log
sdLogFinish : terminate logging thread
@@ -192,26 +191,18 @@ SdioError sdLogFinish(void);
* @param[in] appendTagAtClose : at close, a marker will be added to prove that the file is complete
* and not corrupt. useful for text logging purpose, but probably not wanted for binary
* files.
* @return status (always check status)
*/
SdioError sdLogOpenLog(FileDes *fileObject, const char *directoryName, const char *fileName,
const uint32_t autoFlushPeriod,
const bool appendTagAtClose);
/**
* @brief expand underlying file to maximise throughtput
* @details ask underlying file system to prepare a contiguous data area to the file.
* if expand fail, file system is still avalaible but performance may suffer
* @param[in] fileObject : file descriptor returned by sdLogOpenLog
* @param[in] sizeInMo : size of the contiguous storage
* @param[in] preallocate : if true, the file is preallocated at asked size, more efficient
* but take room on storage ans is not easy to manipulate afterward because of big files
* but take room on storage ans is not easy to manipulate afterward because
* of big files
* even if no log id recorded on file
* @return status (always check status)
*/
SdioError sdLogExpandLogFile(const FileDes fileObject, const size_t sizeInMo,
const bool preallocate);
SdioError sdLogOpenLog(FileDes *fileObject, const char *directoryName, const char *fileName,
const uint32_t autoFlushPeriod, const bool appendTagAtClose,
const size_t sizeInMo, const bool preallocate);
/**
@@ -256,7 +247,8 @@ SdioError sdLogCloseAllLogs(bool flush);
* @param[in] fmt : format and args in printf convention
* @return status (always check status)
*/
SdioError sdLogWriteLog(const FileDes fileObject, const char *fmt, ...);
SdioError sdLogWriteLog(const FileDes fileObject, const char *fmt, ...)
__attribute__ ((format (printf, 2, 3)));;
/**
@@ -25,6 +25,7 @@
*/
#include "modules/loggers/sdlog_chibios/usb_msd.h"
#include "mcu_periph/ram_arch.h"
static THD_WORKING_AREA(mass_storage_thread_wa, 1024);
@@ -127,7 +128,7 @@ PACK_STRUCT_BEGIN typedef struct {
/**
* @brief Read-write buffers (TODO: need a way of specifying the size of this)
*/
static uint8_t rw_buf[2][512];
static uint8_t IN_DMA_SECTION_CLEAR(rw_buf[2][512]);
typedef uint32_t DWORD __attribute__((__may_alias__));;
typedef uint16_t WORD __attribute__((__may_alias__));