The branch main has been updated by des:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=a0439a1b820fa0e742c00d095f5f5c06f5f19432

commit a0439a1b820fa0e742c00d095f5f5c06f5f19432
Author:     Dag-Erling Smørgrav <[email protected]>
AuthorDate: 2024-04-17 01:36:31 +0000
Commit:     Dag-Erling Smørgrav <[email protected]>
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);
-}

Reply via email to