git: a0439a1b820f - main - install: Remove the mmap(2) option.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 17 Apr 2024 02:04:05 UTC
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);
-}