diff --git a/NEWS b/NEWS index 7a163c72a..806e3efd6 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,11 @@ GNU coreutils NEWS -*- outline -*- b2sum no longer crashes when processing certain truncated check files. [bug introduced with b2sum coreutils-8.26] + dd now ensures the correct cache ranges are specified for the "nocache" + and "direct" flags. Previously some pages in the page cache were not + invalidated. [bug introduced for "direct" in coreutils-7.5, + and with the "nocache" implementation in coreutils-8.11] + ptx -S no longer infloops for a pattern which returns zero-length matches. [the bug dates back to the initial implementation] diff --git a/THANKS.in b/THANKS.in index b5e11f0a4..46d78ace2 100644 --- a/THANKS.in +++ b/THANKS.in @@ -192,6 +192,7 @@ Eli Zaretskii eliz@is.elta.co.il Emile LeBlanc leblanc@math.toronto.edu Emmanuel Lacour elacour@home-dn.net Eric Backus ericb@lsid.hp.com +Eric Bergen eric.bergen@gmail.com Eric G. Miller egm2@jps.net Eric Pemente pemente@northpark.edu Eric S. Raymond esr@snark.thyrsus.com diff --git a/doc/coreutils.texi b/doc/coreutils.texi index 47a9f6d25..70e2b76c5 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -9187,7 +9187,9 @@ dd if=ifile iflag=nocache count=0 # Ensure drop cache for the whole file dd of=ofile oflag=nocache conv=notrunc,fdatasync count=0 -# Drop cache for part of file +# Advise to drop cache for part of file +# Note the kernel will only consider complete and +# already persisted pages. dd if=ifile iflag=nocache skip=10 count=10 of=/dev/null # Stream data using just the read-ahead cache. diff --git a/src/dd.c b/src/dd.c index 8e3224a21..b6ceb4fd7 100644 --- a/src/dd.c +++ b/src/dd.c @@ -31,6 +31,7 @@ #include "fd-reopen.h" #include "gethrxtime.h" #include "human.h" +#include "ioblksize.h" #include "long-options.h" #include "quote.h" #include "verror.h" @@ -266,6 +267,9 @@ static sig_atomic_t volatile info_signal_count; /* Whether to discard cache for input or output. */ static bool i_nocache, o_nocache; +/* Whether to instruct the kernel to discard the complete file. */ +static bool i_nocache_eof, o_nocache_eof; + /* Function used for read (to handle iflag=fullblock parameter). */ static ssize_t (*iread_fnc) (int fd, char *buf, size_t size); @@ -1000,7 +1004,8 @@ quit (int code) exit (code); } -/* Return LEN rounded down to a multiple of PAGE_SIZE +/* Return LEN rounded down to a multiple of IO_BUFSIZE + (to minimize calls to the expensive posix_fadvise(,POSIX_FADV_DONTNEED), while storing the remainder internally per FD. Pass LEN == 0 to get the current remainder. */ @@ -1013,7 +1018,7 @@ cache_round (int fd, off_t len) if (len) { uintmax_t c_pending = *pending + len; - *pending = c_pending % page_size; + *pending = c_pending % IO_BUFSIZE; if (c_pending > *pending) len = c_pending - *pending; else @@ -1033,55 +1038,65 @@ static bool invalidate_cache (int fd, off_t len) { int adv_ret = -1; + off_t offset; + bool nocache_eof = (fd == STDIN_FILENO ? i_nocache_eof : o_nocache_eof); /* Minimize syscalls. */ off_t clen = cache_round (fd, len); if (len && !clen) return true; /* Don't advise this time. */ - if (!len && !clen && max_records) - return true; /* Nothing pending. */ + else if (! len && ! clen && ! nocache_eof) + return true; off_t pending = len ? cache_round (fd, 0) : 0; if (fd == STDIN_FILENO) { if (input_seekable) - { - /* Note we're being careful here to only invalidate what - we've read, so as not to dump any read ahead cache. */ -#if HAVE_POSIX_FADVISE - adv_ret = posix_fadvise (fd, input_offset - clen - pending, clen, - POSIX_FADV_DONTNEED); -#else - errno = ENOTSUP; -#endif - } + offset = input_offset; else - errno = ESPIPE; + { + offset = -1; + errno = ESPIPE; + } } - else if (fd == STDOUT_FILENO) + else { static off_t output_offset = -2; if (output_offset != -1) { - if (0 > output_offset) - { - output_offset = lseek (fd, 0, SEEK_CUR); - output_offset -= clen + pending; - } - if (0 <= output_offset) - { -#if HAVE_POSIX_FADVISE - adv_ret = posix_fadvise (fd, output_offset, clen, - POSIX_FADV_DONTNEED); -#else - errno = ENOTSUP; -#endif - output_offset += clen + pending; - } + if (output_offset < 0) + output_offset = lseek (fd, 0, SEEK_CUR); + else if (len) + output_offset += clen + pending; } + + offset = output_offset; } + if (0 <= offset) + { + if (! len && clen && nocache_eof) + { + pending = clen; + clen = 0; + } + + /* Note we're being careful here to only invalidate what + we've read, so as not to dump any read ahead cache. + Note also the kernel is conservative and only invalidates + full pages in the specified range. */ +#if HAVE_POSIX_FADVISE + offset = offset - clen - pending; + /* ensure full page specified when invalidating to eof. */ + if (clen == 0) + offset -= offset % page_size; + adv_ret = posix_fadvise (fd, offset, clen, POSIX_FADV_DONTNEED); +#else + errno = ENOTSUP; +#endif + } + return adv_ret != -1 ? true : false; } @@ -1168,15 +1183,19 @@ iwrite (int fd, char const *buf, size_t size) quotef (output_file)); /* Since we have just turned off O_DIRECT for the final write, - here we try to preserve some of its semantics. First, use - posix_fadvise to tell the system not to pollute the buffer - cache with this data. Don't bother to diagnose lseek or - posix_fadvise failure. */ + we try to preserve some of its semantics. */ + + /* Call invalidate_cache() to setup the appropriate offsets + for subsequent calls. */ + o_nocache_eof = true; invalidate_cache (STDOUT_FILENO, 0); /* Attempt to ensure that that final block is committed to disk as quickly as possible. */ conversions_mask |= C_FSYNC; + + /* After the subsequent fsync() we'll call invalidate_cache() + to attempt to clear all data from the page cache. */ } while (total_written < size) @@ -1562,11 +1581,13 @@ scanargs (int argc, char *const *argv) if (input_flags & O_NOCACHE) { i_nocache = true; + i_nocache_eof = (max_records == 0 && max_bytes == 0); input_flags &= ~O_NOCACHE; } if (output_flags & O_NOCACHE) { o_nocache = true; + o_nocache_eof = (max_records == 0 && max_bytes == 0); output_flags &= ~O_NOCACHE; } } @@ -2149,13 +2170,19 @@ dd_copy (void) else nread = iread_fnc (STDIN_FILENO, ibuf, input_blocksize); - if (nread >= 0 && i_nocache) - invalidate_cache (STDIN_FILENO, nread); - - if (nread == 0) - break; /* EOF. */ - - if (nread < 0) + if (nread > 0) + { + advance_input_offset (nread); + if (i_nocache) + invalidate_cache (STDIN_FILENO, nread); + } + else if (nread == 0) + { + i_nocache_eof |= i_nocache; + o_nocache_eof |= o_nocache && ! (conversions_mask & C_NOTRUNC); + break; /* EOF. */ + } + else { if (!(conversions_mask & C_NOERROR) || status_level != STATUS_NONE) error (0, errno, _("error reading %s"), quoteaf (input_file)); @@ -2194,7 +2221,6 @@ dd_copy (void) } n_bytes_read = nread; - advance_input_offset (nread); if (n_bytes_read < input_blocksize) { @@ -2470,13 +2496,12 @@ main (int argc, char **argv) exit_status = EXIT_FAILURE; } } - else if (max_records != (uintmax_t) -1) + else { - /* Invalidate any pending region less than page size, - in case the kernel might round up. */ - if (i_nocache) + /* Invalidate any pending region or to EOF if appropriate. */ + if (i_nocache || i_nocache_eof) invalidate_cache (STDIN_FILENO, 0); - if (o_nocache) + if (o_nocache || o_nocache_eof) invalidate_cache (STDOUT_FILENO, 0); } diff --git a/tests/dd/nocache_eof.sh b/tests/dd/nocache_eof.sh new file mode 100755 index 000000000..96d598825 --- /dev/null +++ b/tests/dd/nocache_eof.sh @@ -0,0 +1,98 @@ +#!/bin/sh +# Ensure dd invalidates to EOF when appropriate + +# Copyright (C) 2017 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src +print_ver_ dd +require_strace_ fadvise64 + +head -c1234567 /dev/zero > in.f || framework_failure_ + +# Check basic operation or skip. +# We could check for 'Operation not supported' error here, +# but that was seen to be brittle. HPUX returns ENOTTY for example. +# So assume that if this basic operation fails, it's due to lack +# of support by the system. +dd if=in.f iflag=nocache count=0 || + skip_ 'this file system lacks support for posix_fadvise()' + +strace_dd() { + strace -o dd.strace -e fadvise64 dd status=none "$@" || fail=1 +} + +advised_to_eof() { + grep -F ' 0, POSIX_FADV_DONTNEED' dd.strace >/dev/null +} + +# The commented fadvise64 calls are what are expected with +# a 4KiB page size and 128KiB IO_BUFSIZE. + +strace_dd if=in.f of=out.f bs=1M oflag=direct +#The first call is redundant but inconsequential +#fadvise64(1, 1048576, 0, POSIX_FADV_DONTNEED) = 0 +#fadvise64(1, 1048576, 0, POSIX_FADV_DONTNEED) = 0 +advised_to_eof || fail=1 + +strace_dd if=in.f of=out.f bs=1M oflag=nocache,sync +#fadvise64(1, 0, 1048576, POSIX_FADV_DONTNEED) = 0 +#fadvise64(1, 1048576, 131072, POSIX_FADV_DONTNEED) = 0 +#fadvise64(1, 1179648, 0, POSIX_FADV_DONTNEED) = 0 +advised_to_eof || fail=1 + +strace_dd if=in.f count=0 iflag=nocache +#fadvise64(0, 0, 0, POSIX_FADV_DONTNEED) = 0 +advised_to_eof || fail=1 + +strace_dd if=in.f of=/dev/null iflag=nocache skip=10 count=300 +#fadvise64(0, 5120, 131072, POSIX_FADV_DONTNEED) = 0 +#fadvise64(0, 136192, 22528, POSIX_FADV_DONTNEED) = 0 +returns_ 1 advised_to_eof || fail=1 + +strace_dd if=in.f of=/dev/null iflag=nocache bs=1M count=3000 +#fadvise64(0, 0, 1048576, POSIX_FADV_DONTNEED) = 0 +#fadvise64(0, 1048576, 131072, POSIX_FADV_DONTNEED) = 0 +#fadvise64(0, 1179648, 0, POSIX_FADV_DONTNEED) = 0 +advised_to_eof || fail=1 + +strace_dd if=in.f of=/dev/null bs=1M count=1 iflag=nocache +#fadvise64(0, 0, 1048576, POSIX_FADV_DONTNEED) = 0 +returns_ 1 advised_to_eof || fail=1 + +strace_dd if=in.f of=out.f bs=1M iflag=nocache oflag=nocache,sync +#fadvise64(0, 0, 1048576, POSIX_FADV_DONTNEED) = 0 +#fadvise64(1, 0, 1048576, POSIX_FADV_DONTNEED) = 0 +#fadvise64(0, 1048576, 131072, POSIX_FADV_DONTNEED) = 0 +#fadvise64(1, 1048576, 131072, POSIX_FADV_DONTNEED) = 0 +#fadvise64(0, 1179648, 0, POSIX_FADV_DONTNEED) = 0 +#fadvise64(1, 1179648, 0, POSIX_FADV_DONTNEED) = 0 +advised_to_eof || fail=1 + +# Ensure sub page size offsets are handled. +# I.e., only page aligned offsets are sent to fadvise. +if ! strace -o dd.strace -e fadvise64 dd status=none \ + if=in.f of=out.f bs=1M oflag=direct seek=512 oflag=seek_bytes 2>err; then + # older XFS had a page size alignment requirement + echo "dd: error writing 'out.f': Invalid argument" > err_ok + compare err_ok err || fail=1 +else + #The first call is redundant but inconsequential + #fadvise64(1, 1048576, 0, POSIX_FADV_DONTNEED) = 0 + #fadvise64(1, 1048576, 0, POSIX_FADV_DONTNEED) = 0 + advised_to_eof || fail=1 +fi + +Exit $fail diff --git a/tests/local.mk b/tests/local.mk index 42e247e6f..74426f608 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -511,6 +511,7 @@ all_tests = \ tests/dd/misc.sh \ tests/dd/no-allocate.sh \ tests/dd/nocache.sh \ + tests/dd/nocache_eof.sh \ tests/dd/not-rewound.sh \ tests/dd/reblock.sh \ tests/dd/skip-seek.pl \