From 6cff3c1c2a31e2a92f5987f00d107d8fd251359a Mon Sep 17 00:00:00 2001 From: Andrew Wood Date: Wed, 26 Jul 2023 22:17:00 +0100 Subject: [PATCH] First draft of new "--sync" option. --- autoconf/configure.in | 2 +- autoconf/header.in | 3 ++ doc/NEWS.md | 3 +- doc/VERSION | 2 +- doc/quickref.1.in | 7 ++++ src/include/options.h | 1 + src/include/pv-internal.h | 1 + src/include/pv.h | 1 + src/main/help.c | 2 + src/main/main.c | 1 + src/main/options.c | 6 ++- src/nls/pv.pot | 82 ++++++++++++++++++++------------------- src/pv/display.c | 12 +++--- src/pv/state.c | 5 +++ src/pv/transfer.c | 47 ++++++++++++++++++++-- 15 files changed, 123 insertions(+), 52 deletions(-) diff --git a/autoconf/configure.in b/autoconf/configure.in index 2fb99a3..b0eeb72 100644 --- a/autoconf/configure.in +++ b/autoconf/configure.in @@ -39,7 +39,7 @@ AC_SYS_LARGEFILE AC_HEADER_TIOCGWINSZ AC_CHECK_FUNCS(getopt_long getopt) AC_CHECK_HEADERS(getopt.h) -AC_CHECK_FUNCS(memcpy basename vsnprintf strlcat) +AC_CHECK_FUNCS(memcpy basename vsnprintf strlcat fdatasync) AC_CHECK_HEADERS(limits.h) AC_ARG_ENABLE(debugging, diff --git a/autoconf/header.in b/autoconf/header.in index 3686777..43d27c0 100644 --- a/autoconf/header.in +++ b/autoconf/header.in @@ -21,6 +21,9 @@ /* Define to 1 if you have the `basename' function. */ #undef HAVE_BASENAME +/* Define to 1 if you have the `fdatasync' function. */ +#undef HAVE_FDATASYNC + /* Define to 1 if you have the `getopt' function. */ #undef HAVE_GETOPT diff --git a/doc/NEWS.md b/doc/NEWS.md index 29fb6f1..b74d7f2 100644 --- a/doc/NEWS.md +++ b/doc/NEWS.md @@ -1,4 +1,5 @@ -0.0.20230725-UNRELEASED +0.0.20230726-UNRELEASED + * feature: new "`--sync`" option to flush cache to disk after every write (related to [GH#6](https://github.com/a-j-wood/pv/issues/6), to improve accuracy when writing to slow disks) * fix: correct byte prefix size to 2 spaces in rate display, so progress display size remains constant at low transfer rates * cleanup: rewrote `configure.in` as per suggestions in newer "`autoconf`" manuals * cleanup: replaced `header.in` with one generated by "`autoheader`", moving custom logic to a separate header file "`config-aux.h`" diff --git a/doc/VERSION b/doc/VERSION index 83b3f21..afe24da 100644 --- a/doc/VERSION +++ b/doc/VERSION @@ -1 +1 @@ -0.0.20230725-UNRELEASED +0.0.20230726-UNRELEASED diff --git a/doc/quickref.1.in b/doc/quickref.1.in index 2eb7e41..f123aa6 100644 --- a/doc/quickref.1.in +++ b/doc/quickref.1.in @@ -395,6 +395,13 @@ If a size was specified with stop transferring data once that many bytes have been written, instead of continuing to the end of input. .TP +.B \-Y, \-\-sync +After every write operation, synchronise the buffer caches to disk - see +.BR fdatasync (2). +This has no effect when the output is a pipe. Using +.B \-Y +may improve the accuracy of the progress bar when writing to a slow disk. +.TP .B \-d PID[:FD], \-\-watchfd PID[:FD] Instead of transferring data, watch file descriptor .B FD diff --git a/src/include/options.h b/src/include/options.h index 0c6ae04..eb09a67 100644 --- a/src/include/options.h +++ b/src/include/options.h @@ -43,6 +43,7 @@ struct opts_s { /* structure describing run-time options */ bool no_splice; /* flag set if never to use splice */ unsigned int skip_errors; /* skip read errors counter */ bool stop_at_size; /* set if we stop at "size" bytes */ + bool sync_after_write; /* set if we sync after every write */ double interval; /* interval between updates */ double delay_start; /* delay before first display */ unsigned int watch_pid; /* process to watch fds of */ diff --git a/src/include/pv-internal.h b/src/include/pv-internal.h index 632256c..cb73113 100644 --- a/src/include/pv-internal.h +++ b/src/include/pv-internal.h @@ -93,6 +93,7 @@ struct pvstate_s { bool no_op; /* do nothing other than pipe data */ unsigned int skip_errors; /* skip read errors counter */ bool stop_at_size; /* set if we stop at "size" bytes */ + bool sync_after_write; /* set if we sync after every write */ bool no_splice; /* never use splice() */ unsigned long long rate_limit; /* rate limit, in bytes per second */ unsigned long long target_buffer_size; /* buffer size (0=default) */ diff --git a/src/include/pv.h b/src/include/pv.h index 2897831..ab4f9ab 100644 --- a/src/include/pv.h +++ b/src/include/pv.h @@ -106,6 +106,7 @@ extern void pv_state_null_set(pvstate_t, bool); extern void pv_state_no_op_set(pvstate_t, bool); extern void pv_state_skip_errors_set(pvstate_t, unsigned int); extern void pv_state_stop_at_size_set(pvstate_t, bool); +extern void pv_state_sync_after_write_set(pvstate_t, bool); extern void pv_state_rate_limit_set(pvstate_t, unsigned long long); extern void pv_state_target_buffer_size_set(pvstate_t, unsigned long long); extern void pv_state_no_splice_set(pvstate_t, bool); diff --git a/src/main/help.c b/src/main/help.c index 4e5884e..9db009e 100644 --- a/src/main/help.c +++ b/src/main/help.c @@ -91,6 +91,8 @@ void display_help(void) N_("skip read errors in input")}, {"-S", "--stop-at-size", 0, N_("stop after --size bytes have been transferred")}, + {"-Y", "--sync", 0, + N_("flush cache to disk after every write")}, #ifdef HAVE_IPC {"-R", "--remote", N_("PID"), N_("update settings of process PID")}, diff --git a/src/main/main.c b/src/main/main.c index cd8385a..73a01da 100644 --- a/src/main/main.c +++ b/src/main/main.c @@ -201,6 +201,7 @@ int main(int argc, char **argv) pv_state_null_set(state, opts->null); pv_state_skip_errors_set(state, opts->skip_errors); pv_state_stop_at_size_set(state, opts->stop_at_size); + pv_state_sync_after_write_set(state, opts->sync_after_write); pv_state_rate_limit_set(state, opts->rate_limit); pv_state_target_buffer_size_set(state, opts->buffer_size); pv_state_no_splice_set(state, opts->no_splice); diff --git a/src/main/options.c b/src/main/options.c index 136defc..10287fd 100644 --- a/src/main/options.c +++ b/src/main/options.c @@ -82,6 +82,7 @@ opts_t opts_parse(int argc, char **argv) {"no-splice", 0, NULL, (int) 'C'}, {"skip-errors", 0, NULL, (int) 'E'}, {"stop-at-size", 0, NULL, (int) 'S'}, + {"sync", 0, NULL, (int) 'Y'}, {"remote", 1, NULL, (int) 'R'}, {"pidfile", 1, NULL, (int) 'P'}, {"watchfd", 1, NULL, (int) 'd'}, @@ -91,7 +92,7 @@ opts_t opts_parse(int argc, char **argv) int option_index = 0; #endif char *short_options = - "hVpteIrab8TA:fnqcWD:s:l0i:w:H:N:F:L:B:CESR:P:d:m:"; + "hVpteIrab8TA:fnqcWD:s:l0i:w:H:N:F:L:B:CESYR:P:d:m:"; int c, numopts; unsigned int check_pid; int check_fd; @@ -333,6 +334,9 @@ opts_t opts_parse(int argc, char **argv) case 'S': opts->stop_at_size = true; break; + case 'Y': + opts->sync_after_write = true; + break; case 'R': opts->remote = pv_getnum_ui(optarg); break; diff --git a/src/nls/pv.pot b/src/nls/pv.pot index 4c19f52..0cb66cd 100644 --- a/src/nls/pv.pot +++ b/src/nls/pv.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2023-07-23 22:39+0100\n" +"POT-Creation-Date: 2023-07-26 22:09+0100\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -44,7 +44,7 @@ msgstr "" msgid "failed to read file" msgstr "" -#: src/pv/file.c:253 src/main/options.c:291 +#: src/pv/file.c:253 src/main/options.c:292 msgid "failed to stat file" msgstr "" @@ -56,39 +56,39 @@ msgstr "" msgid "input file is output file" msgstr "" -#: src/pv/transfer.c:347 +#: src/pv/transfer.c:387 msgid "read failed" msgstr "" -#: src/pv/transfer.c:365 +#: src/pv/transfer.c:405 msgid "warning: read errors detected" msgstr "" -#: src/pv/transfer.c:380 +#: src/pv/transfer.c:420 msgid "file is not seekable" msgstr "" -#: src/pv/transfer.c:443 +#: src/pv/transfer.c:483 msgid "failed to seek past error" msgstr "" -#: src/pv/transfer.c:462 +#: src/pv/transfer.c:502 msgid "skipped past read error" msgstr "" -#: src/pv/transfer.c:464 src/pv/display.c:723 +#: src/pv/transfer.c:504 src/pv/display.c:723 msgid "B" msgstr "" -#: src/pv/transfer.c:637 +#: src/pv/transfer.c:678 msgid "write failed" msgstr "" -#: src/pv/transfer.c:685 src/pv/display.c:646 +#: src/pv/transfer.c:726 src/pv/display.c:646 msgid "buffer allocation failed" msgstr "" -#: src/pv/transfer.c:776 +#: src/pv/transfer.c:817 msgid "select call failed" msgstr "" @@ -327,55 +327,59 @@ msgid "stop after --size bytes have been transferred" msgstr "" #: src/main/help.c:95 +msgid "flush cache to disk after every write" +msgstr "" + +#: src/main/help.c:97 msgid "PID" msgstr "" -#: src/main/help.c:96 +#: src/main/help.c:98 msgid "update settings of process PID" msgstr "" -#: src/main/help.c:99 +#: src/main/help.c:101 msgid "FILE" msgstr "" -#: src/main/help.c:100 +#: src/main/help.c:102 msgid "save process ID in FILE" msgstr "" -#: src/main/help.c:102 +#: src/main/help.c:104 msgid "PID[:FD]" msgstr "" -#: src/main/help.c:103 +#: src/main/help.c:105 msgid "watch file FD opened by process PID" msgstr "" -#: src/main/help.c:106 +#: src/main/help.c:108 msgid "show this help and exit" msgstr "" -#: src/main/help.c:108 +#: src/main/help.c:110 msgid "show version information and exit" msgstr "" -#: src/main/help.c:115 +#: src/main/help.c:117 #, c-format msgid "Usage: %s [OPTION] [FILE]..." msgstr "" -#: src/main/help.c:118 +#: src/main/help.c:120 msgid "" "Concatenate FILE(s), or standard input, to standard output,\n" "with monitoring." msgstr "" -#: src/main/help.c:211 +#: src/main/help.c:213 msgid "" "Debugging is enabled; export the DEBUG environment variable to define the\n" "output filename.\n" msgstr "" -#: src/main/help.c:215 +#: src/main/help.c:217 #, c-format msgid "Please report any bugs to %s." msgstr "" @@ -384,62 +388,62 @@ msgstr "" msgid "state allocation failed" msgstr "" -#: src/main/main.c:250 +#: src/main/main.c:251 msgid "failed to read terminal attributes" msgstr "" -#: src/main/options.c:104 +#: src/main/options.c:105 msgid "option structure allocation failed" msgstr "" -#: src/main/options.c:118 +#: src/main/options.c:119 msgid "option structure argv allocation failed" msgstr "" -#: src/main/options.c:163 +#: src/main/options.c:164 msgid "integer argument expected" msgstr "" -#: src/main/options.c:174 +#: src/main/options.c:175 msgid "numeric argument expected" msgstr "" -#: src/main/options.c:185 +#: src/main/options.c:186 msgid "process ID or pid:fd pair expected" msgstr "" -#: src/main/options.c:192 +#: src/main/options.c:193 msgid "invalid process ID" msgstr "" -#: src/main/options.c:358 -#, c-format -msgid "Try `%s --help' for more information." -msgstr "" - #: src/main/options.c:362 #, c-format +msgid "Try `%s --help' for more information." +msgstr "" + +#: src/main/options.c:366 +#, c-format msgid "Try `%s -h' for more information." msgstr "" -#: src/main/options.c:378 +#: src/main/options.c:382 msgid "" "cannot use line mode or transfer modifier options when watching file " "descriptors" msgstr "" -#: src/main/options.c:386 +#: src/main/options.c:390 msgid "cannot use cursor positioning when watching file descriptors" msgstr "" -#: src/main/options.c:394 +#: src/main/options.c:398 msgid "cannot use remote control when watching file descriptors" msgstr "" -#: src/main/options.c:402 +#: src/main/options.c:406 msgid "cannot transfer files when watching file descriptors" msgstr "" -#: src/main/options.c:410 +#: src/main/options.c:414 msgid "not available on systems without /proc/self/fdinfo" msgstr "" diff --git a/src/pv/display.c b/src/pv/display.c index b77e002..5fac68a 100644 --- a/src/pv/display.c +++ b/src/pv/display.c @@ -188,12 +188,12 @@ static void pv__si_prefix(long double *value, char *prefix, * in bytes mode. */ if (0.0 == *value) { - if (is_bytes) { - prefix[1] = ' '; - prefix[2] = 0; - } + if (is_bytes) { + prefix[1] = ' '; + prefix[2] = 0; + } return; - } + } cutoff = ratio * 0.97; @@ -213,7 +213,7 @@ static void pv__si_prefix(long double *value, char *prefix, * is two spaces, not one. */ if (is_bytes) { - prefix[1] = (prefix[0] == ' ' ? ' ' : 'i'); + prefix[1] = (prefix[0] == ' ' ? ' ' : 'i'); prefix[2] = 0; } } diff --git a/src/pv/state.c b/src/pv/state.c index 3454a51..414b3f4 100644 --- a/src/pv/state.c +++ b/src/pv/state.c @@ -201,6 +201,11 @@ void pv_state_stop_at_size_set(pvstate_t state, bool val) state->stop_at_size = val; }; +void pv_state_sync_after_write_set(pvstate_t state, bool val) +{ + state->sync_after_write = val; +}; + void pv_state_rate_limit_set(pvstate_t state, unsigned long long val) { state->rate_limit = val; diff --git a/src/pv/transfer.c b/src/pv/transfer.c index c3099c9..598b7a8 100644 --- a/src/pv/transfer.c +++ b/src/pv/transfer.c @@ -103,10 +103,13 @@ static ssize_t pv__transfer_read_repeated(int fd, void *buf, size_t count) * see if we can write any more, and keep trying, to make sure we empty the * buffer as much as we can. * + * If "sync_after_write" is true, we call fdatasync() after each write(). + * * We stop retrying if the time elapsed since this function was entered * reaches TRANSFER_WRITE_TIMEOUT microseconds. */ -static ssize_t pv__transfer_write_repeated(int fd, void *buf, size_t count) +static ssize_t pv__transfer_write_repeated(int fd, void *buf, size_t count, + bool sync_after_write) { struct timeval start_time; ssize_t total_written; @@ -125,6 +128,20 @@ static ssize_t pv__transfer_write_repeated(int fd, void *buf, size_t count) MAX_WRITE_AT_ONCE ? MAX_WRITE_AT_ONCE : count; nwritten = write(fd, buf, asked_to_write); + +#ifdef HAVE_FDATASYNC + if (sync_after_write && nwritten >= 0) { + /* + * Ignore non IO errors, such as EBADFD (bad file + * descriptor), EINVAL (non syncable fd, such as a + * pipe), etc - only return an error on EIO. + */ + if ((fdatasync(fd) < 0) && (EIO == errno)) { + return -1; + } + } +#endif /* HAVE_FDATASYNC */ + if (nwritten < 0) { if ((EINTR == errno) || (EAGAIN == errno)) { /* @@ -220,6 +237,7 @@ static int pv__transfer_read(pvstate_t state, int fd, int *eof_in, int *eof_out, unsigned long long allowed) { + bool do_not_skip_errors; unsigned long bytes_can_read; unsigned long amount_to_skip; long amount_skipped; @@ -230,6 +248,10 @@ static int pv__transfer_read(pvstate_t state, int fd, size_t bytes_to_splice; #endif /* HAVE_SPLICE */ + do_not_skip_errors = false; + if (0 == state->skip_errors) + do_not_skip_errors = true; + bytes_can_read = state->buffer_size - state->read_position; #ifdef HAVE_SPLICE @@ -256,6 +278,24 @@ static int pv__transfer_read(pvstate_t state, int fd, */ } else if (nread > 0) { state->written = nread; +# ifdef HAVE_FDATASYNC + if (state->sync_after_write) { + /* + * Ignore non IO errors, such as EBADFD (bad file + * descriptor), EINVAL (non syncable fd, such as a + * pipe), etc - only treat EIO as a failure. + * + * Since this is a write error, not a read + * error, we cannot skip it, so set + * "do_not_skip_errors". + */ + if ((fdatasync(STDOUT_FILENO) < 0) + && (EIO == errno)) { + nread = -1; + do_not_skip_errors = true; + } + } +# endif /* HAVE_FDATASYNC */ } else if ((-1 == nread) && (EAGAIN == errno)) { /* nothing read yet - do nothing */ } else { @@ -341,7 +381,7 @@ static int pv__transfer_read(pvstate_t state, int fd, * If we aren't skipping errors, show the error and pretend we * reached the end of this file. */ - if (0 == state->skip_errors) { + if (do_not_skip_errors) { pv_error(state, "%s: %s: %s", state->current_file, _("read failed"), strerror(errno)); @@ -502,7 +542,8 @@ static int pv__transfer_write(pvstate_t state, nwritten = pv__transfer_write_repeated(STDOUT_FILENO, state->transfer_buffer + state->write_position, - state->to_write); + state->to_write, + state->sync_after_write); alarm(0);