Implement memory caching for umove* functions

When the data to be fetched by vm_read_mem resides in a single memory
page, fetch the whole page and cache it.  This implementation caches
up to two memory pages.

* defs.h (invalidate_umove_cache): New prototype.
* strace.c (next_event): Call invalidate_umove_cache.
* ucopy.c (cached_idx, cached_raddr): New static variables.
(process_read_mem): New function.
(vm_read_mem): Use them.  Implement fetched page caching.
* tests/umovestr_cached.test: New test.
* tests/Makefile.am (MISC_TESTS): Add umovestr_cached.test.
* tests/umovestr_cached.c: New file.
* tests/pure_executables.list: Add umovestr_cached.
* tests/.gitignore: Likewise.

diff --git a/defs.h b/defs.h
index b8feee6b21f51312388f7fece4ec3b0dfbc5935a..337c9bd74d3e4660628b7ed09bf09b2710f3b17e 100644 (file)
--- a/defs.h
+++ b/defs.h
@@ -645,6 +645,9 @@ umoven_or_printaddr_ignore_syserror(struct tcb *tcp, const kernel_ulong_t addr,
 extern int
 umovestr(struct tcb *, kernel_ulong_t addr, unsigned int len, char *laddr);
 
+/* Invalidate the cache used by umove* functions.  */
+extern void invalidate_umove_cache(void);
+
 extern int upeek(struct tcb *tcp, unsigned long, kernel_ulong_t *);
 extern int upoke(struct tcb *tcp, unsigned long, kernel_ulong_t);
 

diff --git a/strace.c b/strace.c
index 8d9e465c7a950e63e1b7351e2b564e84747c40b1..e8ced366db0cab527c5dd1dc5ee57d9a8c8699f0 100644 (file)
--- a/strace.c
+++ b/strace.c
@@ -2327,6 +2327,8 @@ next_event(void)
        if (interrupted)
                return NULL;
 
+       invalidate_umove_cache();
+
        struct tcb *tcp = NULL;
        struct list_item *elem;
 

diff --git a/tests/.gitignore b/tests/.gitignore
index 4c854db8394f1fcfaac7d92a467d256aff698aa4..774d92be3f1ab0532e25686f2b64d719e8036b9a 100644 (file)
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -624,6 +624,7 @@ umovestr
 umovestr-illptr
 umovestr2
 umovestr3
+umovestr_cached
 uname
 unblock_reset_raise
 unix-pair-send-recv

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 799df074dc0a697d9d299d642b9665b913c6cd13..ec52423a21d0281dc9244262f4d37273a2a12bfd 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -362,6 +362,7 @@ MISC_TESTS = \
        strace-ttt.test \
        termsig.test \
        threads-execve.test \
+       umovestr_cached.test \
        # end of MISC_TESTS
 
 TESTS = $(GEN_TESTS) $(DECODER_TESTS) $(MISC_TESTS) $(STACKTRACE_TESTS)

diff --git a/tests/pure_executables.list b/tests/pure_executables.list
index b565eb9c6378973e822022446623ecd159590ac5..a8b03a825447e6d71e9de5afdc5ccd2c1509232a 100755 (executable)
--- a/tests/pure_executables.list
+++ b/tests/pure_executables.list
@@ -531,6 +531,7 @@ umovestr
 umovestr-illptr
 umovestr2
 umovestr3
+umovestr_cached
 uname
 unlink
 unlinkat

diff --git a/tests/umovestr_cached.c b/tests/umovestr_cached.c
new file mode 100644 (file)
index 0000000..3d7bed3
--- /dev/null
+++ b/tests/umovestr_cached.c
@@ -0,0 +1,47 @@
+/*
+ * Check effectiveness of umovestr memory caching.
+ *
+ * Copyright (c) 2019 Dmitry V. Levin <ldv@altlinux.org>
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "tests.h"
+
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/uio.h>
+
+int
+main(void)
+{
+       char *const buf = tail_alloc(DEFAULT_STRLEN);
+       fill_memory_ex(buf, DEFAULT_STRLEN, 'a', 'z' - 'a' + 1);
+
+       struct iovec *const io = tail_alloc(sizeof(*io) * DEFAULT_STRLEN);
+       for (unsigned int i = 0; i < DEFAULT_STRLEN; ++i) {
+               io[i].iov_base = buf + DEFAULT_STRLEN - i;
+               io[i].iov_len = i;
+       }
+
+       tprintf("%s", "");
+
+       int rc = writev(-1, io, DEFAULT_STRLEN);
+       const char *errstr = sprintrc(rc);
+
+       tprintf("writev(-1, [");
+       for (unsigned int i = 0; i < DEFAULT_STRLEN; ++i) {
+               if (i)
+                       tprintf(", ");
+               tprintf("{iov_base=\"%.*s\", iov_len=%u}",
+                       (int) io[i].iov_len,
+                       (char *) io[i].iov_base,
+                       (unsigned int) io[i].iov_len);
+       }
+       tprintf("], %u) = %s\n", DEFAULT_STRLEN, errstr);
+
+       tprintf("+++ exited with 0 +++\n");
+       return 0;
+}

diff --git a/tests/umovestr_cached.test b/tests/umovestr_cached.test
new file mode 100755 (executable)
index 0000000..d48c900
--- /dev/null
+++ b/tests/umovestr_cached.test
@@ -0,0 +1,24 @@
+#!/bin/sh
+#
+# Check effectiveness of umovestr memory caching.
+#
+# Copyright (c) 2017-2019 Dmitry V. Levin <ldv@altlinux.org>
+# All rights reserved.
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+. "${srcdir=.}/init.sh"
+
+check_prog env
+check_prog grep
+run_strace_match_diff -e trace=writev
+
+run_strace -qq -esignal=none -eprocess_vm_readv -o '|grep -c ^process_vm_readv > count' \
+       -- "$STRACE_EXE" -o "$LOG" $args > /dev/null
+
+count="$(cat count)"
+case "$count" in
+       0) skip_ "$STRACE $args made no process_vm_readv syscall invocations" ;;
+       2) ;;
+       *) fail_ "$STRACE $args made $count != 2 process_vm_readv syscall invocations" ;;
+esac

diff --git a/ucopy.c b/ucopy.c
index 75b28e41015db53e63adc437a271290541d3cedb..1bf4469b6d6569b0589b6d1592a8e22f8a1c6450 100644 (file)
--- a/ucopy.c
+++ b/ucopy.c
@@ -41,24 +41,15 @@ static ssize_t strace_process_vm_readv(pid_t pid,
 #endif /* !HAVE_PROCESS_VM_READV */
 
 static ssize_t
-vm_read_mem(const pid_t pid, void *const laddr,
-           const kernel_ulong_t raddr, const size_t len)
+process_read_mem(const pid_t pid, void *const laddr,
+                void *const raddr, const size_t len)
 {
-       const unsigned long truncated_raddr = raddr;
-
-#if SIZEOF_LONG < SIZEOF_KERNEL_LONG_T
-       if (raddr != (kernel_ulong_t) truncated_raddr) {
-               errno = EIO;
-               return -1;
-       }
-#endif
-
        const struct iovec local = {
                .iov_base = laddr,
                .iov_len = len
        };
        const struct iovec remote = {
-               .iov_base = (void *) truncated_raddr,
+               .iov_base = raddr,
                .iov_len = len
        };
 
@@ -69,6 +60,75 @@ vm_read_mem(const pid_t pid, void *const laddr,
        return rc;
 }
 
+static int cached_idx = -1;
+static unsigned long cached_raddr[2];
+
+void
+invalidate_umove_cache(void)
+{
+       cached_idx = -1;
+}
+
+static ssize_t
+vm_read_mem(const pid_t pid, void *const laddr,
+           const kernel_ulong_t raddr, const size_t len)
+{
+       if (!len)
+               return len;
+
+       const unsigned long taddr = raddr;
+
+#if SIZEOF_LONG < SIZEOF_KERNEL_LONG_T
+       if (raddr != (kernel_ulong_t) taddr) {
+               errno = EIO;
+               return -1;
+       }
+#endif
+
+       const size_t page_size = get_pagesize();
+       const size_t page_mask = page_size - 1;
+       const unsigned long raddr_page_start =
+               taddr & ~page_mask;
+       const unsigned long raddr_page_next =
+               (taddr + len + page_mask) & ~page_mask;
+
+       if (!raddr_page_start ||
+           raddr_page_next < raddr_page_start ||
+           raddr_page_next - raddr_page_start != page_size)
+               return process_read_mem(pid, laddr, (void *) taddr, len);
+
+       int idx = -1;
+       if (cached_idx >= 0) {
+               if (raddr_page_start == cached_raddr[cached_idx])
+                       idx = cached_idx;
+               else if (raddr_page_start == cached_raddr[!cached_idx])
+                       idx = !cached_idx;
+       }
+
+       static char *buf[2];
+
+       if (idx == -1) {
+               idx = !cached_idx;
+
+               if (!buf[idx])
+                       buf[idx] = xmalloc(page_size);
+
+               const ssize_t rc =
+                       process_read_mem(pid, buf[idx],
+                                        (void *) raddr_page_start, page_size);
+               if (rc < 0)
+                       return rc;
+
+               cached_raddr[idx] = raddr_page_start;
+               if (cached_idx < 0)
+                       cached_raddr[!idx] = 0;
+               cached_idx = idx;
+       }
+
+       memcpy(laddr, buf[idx] + (taddr - cached_raddr[idx]), len);
+       return len;
+}
+
 static bool
 tracee_addr_is_invalid(kernel_ulong_t addr)
 {