Date: Thu, 2 Feb 2023 14:46:41 GMT From: =?utf-8?Q?Dag-Erling=20Sm=C3=B8rgrav?= <des@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 6c85042afcbb - main - cp: Simplify the common case. Message-ID: <202302021446.312EkfiB077240@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=6c85042afcbbf4cd0fb7e7c03226c7249081e690 commit 6c85042afcbbf4cd0fb7e7c03226c7249081e690 Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2023-02-01 20:06:28 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2023-02-02 14:46:04 +0000 cp: Simplify the common case. * The allocated buffer is only used in the fallback case, so move it there. The argument for passing it in from the caller was that if malloc(3) were to fail, we'd want it to fail before we started copying anything, but firstly, it was already not in the right place to ensure that, and secondly, malloc(3) never fails (except in very contrived circumstances, such as an unreasonable RLIMIT_AS or RLIMIT_DATA). * Remove the mmap(2) option. It is almost never beneficial, especially when the alternative is copy_file_range(2), and it adds needless complexity and indentation. MFC after: 1 week Sponsored by: Klara, Inc. Reviewed by: rmacklem, mav Differential Revision: https://reviews.freebsd.org/D38291 --- bin/cp/Makefile | 2 +- bin/cp/utils.c | 122 +++++++++++++++----------------------------------------- 2 files changed, 34 insertions(+), 90 deletions(-) diff --git a/bin/cp/Makefile b/bin/cp/Makefile index 0c4c7b5eff53..a73ee5447769 100644 --- a/bin/cp/Makefile +++ b/bin/cp/Makefile @@ -6,7 +6,7 @@ PACKAGE=runtime PROG= cp SRCS= cp.c utils.c -CFLAGS+= -DVM_AND_BUFFER_CACHE_SYNCHRONIZED -D_ACL_PRIVATE +CFLAGS+= -D_ACL_PRIVATE HAS_TESTS= SUBDIR.${MK_TESTS}= tests diff --git a/bin/cp/utils.c b/bin/cp/utils.c index 07de0495ba9e..a3a498714f43 100644 --- a/bin/cp/utils.c +++ b/bin/cp/utils.c @@ -41,9 +41,6 @@ __FBSDID("$FreeBSD$"); #include <sys/acl.h> #include <sys/param.h> #include <sys/stat.h> -#ifdef VM_AND_BUFFER_CACHE_SYNCHRONIZED -#include <sys/mman.h> -#endif #include <err.h> #include <errno.h> @@ -75,11 +72,22 @@ __FBSDID("$FreeBSD$"); #define BUFSIZE_SMALL (MAXPHYS) static ssize_t -copy_fallback(int from_fd, int to_fd, char *buf, size_t bufsize) +copy_fallback(int from_fd, int to_fd) { + static char *buf = NULL; + static size_t bufsize; ssize_t rcount, wresid, wcount = 0; char *bufp; + if (buf == NULL) { + if (sysconf(_SC_PHYS_PAGES) > PHYSPAGES_THRESHOLD) + bufsize = MIN(BUFSIZE_MAX, MAXPHYS * 8); + else + bufsize = BUFSIZE_SMALL; + buf = malloc(bufsize); + if (buf == NULL) + err(1, "Not enough memory"); + } rcount = read(from_fd, buf, bufsize); if (rcount <= 0) return (rcount); @@ -96,16 +104,10 @@ copy_fallback(int from_fd, int to_fd, char *buf, size_t bufsize) int copy_file(const FTSENT *entp, int dne) { - static char *buf = NULL; - static size_t bufsize; struct stat *fs; ssize_t wcount; off_t wtotal; int ch, checkch, from_fd, rval, to_fd; -#ifdef VM_AND_BUFFER_CACHE_SYNCHRONIZED - size_t wresid; - char *bufp, *p; -#endif int use_copy_file_range = 1; from_fd = to_fd = -1; @@ -174,89 +176,31 @@ copy_file(const FTSENT *entp, int dne) rval = 0; if (!lflag && !sflag) { - /* - * Mmap and write if less than 8M (the limit is so we don't - * totally trash memory on big files. This is really a minor - * hack, but it wins some CPU back. - * Some filesystems, such as smbnetfs, don't support mmap, - * so this is a best-effort attempt. - */ -#ifdef VM_AND_BUFFER_CACHE_SYNCHRONIZED - if (S_ISREG(fs->st_mode) && fs->st_size > 0 && - fs->st_size <= 8 * 1024 * 1024 && - (p = mmap(NULL, (size_t)fs->st_size, PROT_READ, - MAP_SHARED, from_fd, (off_t)0)) != MAP_FAILED) { - wtotal = 0; - for (bufp = p, wresid = fs->st_size; ; - bufp += wcount, wresid -= (size_t)wcount) { - wcount = write(to_fd, bufp, wresid); - if (wcount <= 0) - break; - wtotal += wcount; - if (info) { - info = 0; - (void)fprintf(stderr, - "%s -> %s %3d%%\n", - entp->fts_path, to.p_path, - cp_pct(wtotal, fs->st_size)); + wtotal = 0; + do { + if (use_copy_file_range) { + wcount = copy_file_range(from_fd, NULL, + to_fd, NULL, SSIZE_MAX, 0); + if (wcount < 0 && errno == EINVAL) { + /* Prob a non-seekable FD */ + use_copy_file_range = 0; } - if (wcount >= (ssize_t)wresid) - break; } - if (wcount != (ssize_t)wresid) { - warn("%s", to.p_path); - rval = 1; + if (!use_copy_file_range) { + wcount = copy_fallback(from_fd, to_fd); } - /* Some systems don't unmap on close(2). */ - if (munmap(p, fs->st_size) < 0) { - warn("%s", entp->fts_path); - rval = 1; - } - } else -#endif - { - if (buf == NULL) { - /* - * Note that buf and bufsize are static. If - * malloc() fails, it will fail at the start - * and not copy only some files. - */ - if (sysconf(_SC_PHYS_PAGES) > - PHYSPAGES_THRESHOLD) - bufsize = MIN(BUFSIZE_MAX, MAXPHYS * 8); - else - bufsize = BUFSIZE_SMALL; - buf = malloc(bufsize); - if (buf == NULL) - err(1, "Not enough memory"); - } - wtotal = 0; - do { - if (use_copy_file_range) { - wcount = copy_file_range(from_fd, NULL, - to_fd, NULL, SSIZE_MAX, 0); - if (wcount < 0 && errno == EINVAL) { - /* Prob a non-seekable FD */ - use_copy_file_range = 0; - } - } - if (!use_copy_file_range) { - wcount = copy_fallback(from_fd, to_fd, - buf, bufsize); - } - wtotal += wcount; - if (info) { - info = 0; - (void)fprintf(stderr, - "%s -> %s %3d%%\n", - entp->fts_path, to.p_path, - cp_pct(wtotal, fs->st_size)); - } - } while (wcount > 0); - if (wcount < 0) { - warn("%s", entp->fts_path); - rval = 1; + wtotal += wcount; + if (info) { + info = 0; + (void)fprintf(stderr, + "%s -> %s %3d%%\n", + entp->fts_path, to.p_path, + cp_pct(wtotal, fs->st_size)); } + } while (wcount > 0); + if (wcount < 0) { + warn("%s", entp->fts_path); + rval = 1; } } else if (lflag) { if (link(entp->fts_path, to.p_path)) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202302021446.312EkfiB077240>