Date: Wed, 17 Apr 2024 02:04:05 GMT From: Dag-Erling =?utf-8?Q?Sm=C3=B8rgrav?= <des@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: a0439a1b820f - main - install: Remove the mmap(2) option. Message-ID: <202404170204.43H245tn033971@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=a0439a1b820fa0e742c00d095f5f5c06f5f19432 commit a0439a1b820fa0e742c00d095f5f5c06f5f19432 Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2024-04-17 01:36:31 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-04-17 02:03:31 +0000 install: Remove the mmap(2) option. We already removed it from cp(1) over a year ago but never followed up here. Do so now, for the same reasons: significant complexity for little to no benefit. MFC after: 1 week Sponsored by: Klara, Inc. Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D44809 --- usr.bin/xinstall/install.1 | 3 +- usr.bin/xinstall/xinstall.c | 212 ++++++++++++++------------------------------ 2 files changed, 67 insertions(+), 148 deletions(-) diff --git a/usr.bin/xinstall/install.1 b/usr.bin/xinstall/install.1 index c87a1f464555..c923321f20fe 100644 --- a/usr.bin/xinstall/install.1 +++ b/usr.bin/xinstall/install.1 @@ -25,7 +25,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd April 10, 2024 +.Dd April 16, 2024 .Dt INSTALL 1 .Os .Sh NAME @@ -325,7 +325,6 @@ The default was changed to copy in .Xr cp 1 , .Xr mv 1 , .Xr strip 1 , -.Xr mmap 2 , .Xr getgrnam 3 , .Xr getpwnam 3 , .Xr chown 8 diff --git a/usr.bin/xinstall/xinstall.c b/usr.bin/xinstall/xinstall.c index d696a8429c88..b824c860e9a8 100644 --- a/usr.bin/xinstall/xinstall.c +++ b/usr.bin/xinstall/xinstall.c @@ -30,9 +30,6 @@ * SUCH DAMAGE. */ -#include <sys/param.h> -#include <sys/mman.h> -#include <sys/mount.h> #include <sys/stat.h> #include <sys/time.h> #include <sys/wait.h> @@ -159,7 +156,6 @@ static void metadata_log(const char *, const char *, struct timespec *, const char *, const char *, off_t); static int parseid(const char *, id_t *); static int strip(const char *, int, const char *, char **); -static int trymmap(size_t); static void usage(void); int @@ -1093,86 +1089,62 @@ compare(int from_fd, const char *from_name __unused, size_t from_len, int to_fd, const char *to_name __unused, size_t to_len, char **dresp) { - char *p, *q; int rv; - int do_digest, done_compare; + int do_digest; DIGEST_CTX ctx; - rv = 0; if (from_len != to_len) return 1; do_digest = (digesttype != DIGEST_NONE && dresp != NULL && *dresp == NULL); if (from_len <= MAX_CMP_SIZE) { + static char *buf, *buf1, *buf2; + static size_t bufsize; + int n1, n2; + if (do_digest) digest_init(&ctx); - done_compare = 0; - if (trymmap(from_len) && trymmap(to_len)) { - p = mmap(NULL, from_len, PROT_READ, MAP_SHARED, - from_fd, (off_t)0); - if (p == MAP_FAILED) - goto out; - q = mmap(NULL, from_len, PROT_READ, MAP_SHARED, - to_fd, (off_t)0); - if (q == MAP_FAILED) { - munmap(p, from_len); - goto out; - } - rv = memcmp(p, q, from_len); - if (do_digest) - digest_update(&ctx, p, from_len); - munmap(p, from_len); - munmap(q, from_len); - done_compare = 1; + 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 * 2); + if (buf == NULL) + err(1, "Not enough memory"); + buf1 = buf; + buf2 = buf + bufsize; } - out: - if (!done_compare) { - static char *buf, *buf1, *buf2; - static size_t bufsize; - int n1, n2; - - 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); + rv = 0; + lseek(from_fd, 0, SEEK_SET); + lseek(to_fd, 0, SEEK_SET); + while (rv == 0) { + n1 = read(from_fd, buf1, bufsize); + if (n1 == 0) + break; /* EOF */ + else if (n1 > 0) { + n2 = read(to_fd, buf2, n1); + if (n2 == n1) + rv = memcmp(buf1, buf2, n1); else - bufsize = BUFSIZE_SMALL; - buf = malloc(bufsize * 2); - if (buf == NULL) - err(1, "Not enough memory"); - buf1 = buf; - buf2 = buf + bufsize; - } - rv = 0; - lseek(from_fd, 0, SEEK_SET); - lseek(to_fd, 0, SEEK_SET); - while (rv == 0) { - n1 = read(from_fd, buf1, bufsize); - if (n1 == 0) - break; /* EOF */ - else if (n1 > 0) { - n2 = read(to_fd, buf2, n1); - if (n2 == n1) - rv = memcmp(buf1, buf2, n1); - else - rv = 1; /* out of sync */ - } else - rv = 1; /* read failure */ - if (do_digest) - digest_update(&ctx, buf1, n1); - } - lseek(from_fd, 0, SEEK_SET); - lseek(to_fd, 0, SEEK_SET); + rv = 1; /* out of sync */ + } else + rv = 1; /* read failure */ + if (do_digest) + digest_update(&ctx, buf1, n1); } - } else + lseek(from_fd, 0, SEEK_SET); + lseek(to_fd, 0, SEEK_SET); + } else { rv = 1; /* don't bother in this case */ + } if (do_digest) { if (rv == 0) @@ -1219,8 +1191,6 @@ copy(int from_fd, const char *from_name, int to_fd, const char *to_name, #ifndef BOOTSTRAP_XINSTALL ssize_t ret; #endif - char *p; - int done_copy; DIGEST_CTX ctx; /* Rewind file descriptors. */ @@ -1246,69 +1216,45 @@ copy(int from_fd, const char *from_name, int to_fd, const char *to_name, } /* Fall back */ } - #endif digest_init(&ctx); - done_copy = 0; - if (trymmap((size_t)size) && - (p = mmap(NULL, (size_t)size, PROT_READ, MAP_SHARED, - from_fd, (off_t)0)) != MAP_FAILED) { - nw = write(to_fd, p, size); - if (nw != size) { + 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"); + } + while ((nr = read(from_fd, buf, bufsize)) > 0) { + if ((nw = write(to_fd, buf, nr)) != nr) { serrno = errno; (void)unlink(to_name); if (nw >= 0) { errx(EX_OSERR, - "short write to %s: %jd bytes written, %jd bytes asked to write", - to_name, (uintmax_t)nw, (uintmax_t)size); + "short write to %s: %jd bytes written, " + "%jd bytes asked to write", + to_name, (uintmax_t)nw, + (uintmax_t)size); } else { errno = serrno; err(EX_OSERR, "%s", to_name); } } - digest_update(&ctx, p, size); - (void)munmap(p, size); - done_copy = 1; + digest_update(&ctx, buf, nr); } - if (!done_copy) { - 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"); - } - while ((nr = read(from_fd, buf, bufsize)) > 0) { - if ((nw = write(to_fd, buf, nr)) != nr) { - serrno = errno; - (void)unlink(to_name); - if (nw >= 0) { - errx(EX_OSERR, - "short write to %s: %jd bytes written, %jd bytes asked to write", - to_name, (uintmax_t)nw, - (uintmax_t)size); - } else { - errno = serrno; - err(EX_OSERR, "%s", to_name); - } - } - digest_update(&ctx, buf, nr); - } - if (nr != 0) { - serrno = errno; - (void)unlink(to_name); - errno = serrno; - err(EX_OSERR, "%s", from_name); - } + if (nr != 0) { + serrno = errno; + (void)unlink(to_name); + errno = serrno; + err(EX_OSERR, "%s", from_name); } done: if (safecopy && fsync(to_fd) == -1) { @@ -1543,29 +1489,3 @@ usage(void) exit(EX_USAGE); /* NOTREACHED */ } - -/* - * trymmap -- - * return true (1) if mmap should be tried, false (0) if not. - */ -static int -trymmap(size_t filesize) -{ - /* - * This function existed to skip mmap() for NFS file systems whereas - * nowadays mmap() should be perfectly safe. Nevertheless, using mmap() - * only reduces the number of system calls if we need multiple read() - * syscalls, i.e. if the file size is > MAXBSIZE. However, mmap() is - * more expensive than read() so set the threshold at 4 fewer syscalls. - * Additionally, for larger file size mmap() can significantly increase - * the number of page faults, so avoid it in that case. - * - * Note: the 8MB limit is not based on any meaningful benchmarking - * results, it is simply reusing the same value that was used before - * and also matches bin/cp. - * - * XXX: Maybe we shouldn't bother with mmap() at all, since we use - * MAXBSIZE the syscall overhead of read() shouldn't be too high? - */ - return (filesize > 4 * MAXBSIZE && filesize < 8 * 1024 * 1024); -}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202404170204.43H245tn033971>