From: Dmitry V. Levin Date: Sun, 15 Sep 2019 15:47:01 +0000 (+0000) Subject: Implement memory caching for umove* functions X-Git-Tag: v5.3~25 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e99ac2bd2b055c7804d22e3519d7ba23c8f34df8;p=strace 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 b8feee6b..337c9bd7 100644 --- 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 8d9e465c..e8ced366 100644 --- 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 4c854db8..774d92be 100644 --- 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 799df074..ec52423a 100644 --- 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 b565eb9c..a8b03a82 100755 --- 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 index 00000000..3d7bed3b --- /dev/null +++ b/tests/umovestr_cached.c @@ -0,0 +1,47 @@ +/* + * Check effectiveness of umovestr memory caching. + * + * Copyright (c) 2019 Dmitry V. Levin + * All rights reserved. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "tests.h" + +#include +#include +#include +#include + +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 index 00000000..d48c900a --- /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 +# 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 75b28e41..1bf4469b 100644 --- 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) {