From 42904eef310313eeed6f8d91d5306488fd016d2e Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Mon, 9 Nov 2020 13:01:01 -0800 Subject: cache: introduce "syncer" thread for cache writers This adds a new flag: --periodic-datasync-ms with a default of 100ms. When a new CacheFile is created for writing, and the datasync period is non-zero, a thread is created for the sole purpose of calling fdatasync() on the underlying fd in a loop separated by sleeps of the specified duration. The purpose of this is to prevent a large backlog of dirty buffers from accumulating until the operating system's normal background dirty sync kicks in. Depending on how the underlying filesystem and storage stack is configured, these bulk writebacks can result in very long stalls on a subsequent write operation. This is easily observed on ext4+lvm+dmcrypt setups, even using a simple test like `dd if=/dev/urandom of=testfile bs=8M`. Despite having plenty of available buffers, once ext4's internal journal fills while a bulk writeback is underway, dd's progress will completely stall until the entire writeback is completed, usually it's in the jbd2 wchan of do_get_write_access(). When this occurs during a recording by recordMyDesktop, the result is dropping frames for the entire duration of the stall. One thing recordMyDesktop could do to insulate from this is perform its own buffering of sampled frames at the YUV stage, with the cache writer consuming from this pool of buffered frames. Then when the cache writer gets stalled on jbd2/dmcrypt holdups, the buffer pool just grows instead of frames being dropped. I may explore this option in the future, but for now simply syncing regularly has been sufficient in my usage, as it keeps the storage subsystem more continuously utilized and spreads out the writebacks so they don't completely back up the journal. --- src/rmd_cache.c | 57 ++++++++++++++++++++++++++++++++++++++++++----- src/rmd_initialize_data.c | 1 + src/rmd_parseargs.c | 5 +++++ src/rmd_types.h | 4 ++++ 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/rmd_cache.c b/src/rmd_cache.c index db7682e..9e42be3 100644 --- a/src/rmd_cache.c +++ b/src/rmd_cache.c @@ -28,18 +28,48 @@ #include "rmd_cache.h" #include "rmd_specsfile.h" +#include "rmd_threads.h" #include "rmd_types.h" -#include -#include -#include +#include #include +#include #include #include +#include +#include #define CACHE_FILE_SIZE_LIMIT (500 * 1024 * 1024) +/* periodic fdatasync thread for cache writers when fdatasyncing is enabled */ +static void * rmdCacheFileSyncer(CacheFile *file) +{ + struct timespec delay; + int fd; + + assert(file); + assert(file->periodic_datasync_ms); + + rmdThreadsSetName("rmdCacheSyncer"); + + delay.tv_sec = file->periodic_datasync_ms / 1000; + delay.tv_nsec = (file->periodic_datasync_ms - delay.tv_sec * 1000) * 1000000; + + if (file->gzfp) + fd = file->gzfd; + else + fd = fileno(file->fp); + + for (;;) { + nanosleep(&delay, NULL); + fdatasync(fd); + } + + return NULL; +} + + /* open file @ path storing the file handles in *file, * this doesn't store the new path since it's primarily * for the purposes of opening new chapters for the same @@ -48,10 +78,12 @@ static int _rmdCacheFileOpen(CacheFile *file, const char *path) { const char *modestr = "rb"; + int flags = O_RDONLY; assert(file); if (file->mode == RMD_CACHE_FILE_MODE_WRITE) { + flags = O_CREAT|O_WRONLY; modestr = "wb"; if (file->compressed) @@ -59,7 +91,12 @@ static int _rmdCacheFileOpen(CacheFile *file, const char *path) } if (file->compressed) { - file->gzfp = gzopen(path, modestr); + /* zlib doesn't expose a fileno() equivalent for the syncer */ + file->gzfd = open(path, flags, S_IRUSR|S_IWUSR); + if (file->gzfd < 0) + return -1; + + file->gzfp = gzdopen(file->gzfd, modestr); } else { file->fp = fopen(path, modestr); } @@ -69,6 +106,9 @@ static int _rmdCacheFileOpen(CacheFile *file, const char *path) file->chapter_n_bytes = 0; + if (file->mode == RMD_CACHE_FILE_MODE_WRITE && file->periodic_datasync_ms) + pthread_create(&file->syncer_thread, NULL, (void *(*)(void *))rmdCacheFileSyncer, file); + return 0; } @@ -78,6 +118,11 @@ static int _rmdCacheFileClose(CacheFile *file) { assert(file); + if (file->mode == RMD_CACHE_FILE_MODE_WRITE && file->periodic_datasync_ms) { + pthread_cancel(file->syncer_thread); + pthread_join(file->syncer_thread, NULL); + } + /* TODO: return meaningful -errno on errors? */ if (file->gzfp) { if (gzclose(file->gzfp) != Z_OK) @@ -114,8 +159,8 @@ CacheFile * rmdCacheFileOpen(ProgData *pdata, const char *path, CacheFileMode mo } f->mode = mode; - if (!pdata->args.zerocompression) - f->compressed = 1; + f->compressed = !pdata->args.zerocompression; + f->periodic_datasync_ms = pdata->args.periodic_datasync_ms; if (_rmdCacheFileOpen(f, path) < 0) return NULL; diff --git a/src/rmd_initialize_data.c b/src/rmd_initialize_data.c index 4cd015f..6f2af89 100644 --- a/src/rmd_initialize_data.c +++ b/src/rmd_initialize_data.c @@ -211,6 +211,7 @@ void rmdSetupDefaultArgs(ProgArgs *args) args->jack_nports = 0; args->jack_ringbuffer_secs = 3.0; args->zerocompression = 1; + args->periodic_datasync_ms = 100; args->no_quick_subsample = 1; args->cursor_color = 1; args->have_dummy_cursor = 0; diff --git a/src/rmd_parseargs.c b/src/rmd_parseargs.c index 5a39f2a..e4cfdd7 100644 --- a/src/rmd_parseargs.c +++ b/src/rmd_parseargs.c @@ -262,6 +262,11 @@ boolean rmdParseArgs(int argc, char **argv, ProgArgs *arg_return) "Image data are cached with light compression.", NULL }, + { "periodic-datasync-ms", '\0', + POPT_ARG_INT, &arg_return->periodic_datasync_ms, 0, + "Asynchronously fdatasync() cache files every specified ms while writing, 0 disables (default 100).", + "N>=0"}, + { "workdir", '\0', POPT_ARG_STRING, &arg_return->workdir, 0, "Location where a temporary directory will be created to hold project files(default $HOME).", diff --git a/src/rmd_types.h b/src/rmd_types.h index 252861c..9be5ca7 100644 --- a/src/rmd_types.h +++ b/src/rmd_types.h @@ -153,6 +153,7 @@ typedef struct _ProgArgs{ char *stop_shortcut; //stop shortcut sequence(Control+Alt+s) int noframe; //don't draw a frame around the recording area int zerocompression; //image data are always flushed uncompressed + unsigned periodic_datasync_ms; //interval between background async fdatasync calls while writing cache files, when zero no periodic fdatasync is performed int overwrite; //overwite a previously existing file //(do not add a .number postfix) int use_jack; //record audio with jack @@ -195,9 +196,12 @@ typedef struct CacheFile { unsigned chapter; size_t chapter_n_bytes, total_n_bytes; unsigned compressed:1; + unsigned periodic_datasync_ms; CacheFileMode mode; gzFile gzfp; + int gzfd; FILE *fp; + pthread_t syncer_thread; } CacheFile; //this struct will hold a few basic -- cgit v1.2.3