From: Miroslav Benes <mbenes@suse.cz>
To: Joe Lawrence <joe.lawrence@redhat.com>
Cc: rostedt@goodmis.org, mingo@redhat.com, jpoimboe@redhat.com,
jikos@kernel.org, pmladek@suse.com, linux-kernel@vger.kernel.org,
live-patching@vger.kernel.org
Subject: Re: [PATCH v2] ftrace: Introduce PERMANENT ftrace_ops flag
Date: Tue, 15 Oct 2019 13:23:10 +0200 (CEST) [thread overview]
Message-ID: <alpine.LSU.2.21.1910151259220.30206@pobox.suse.cz> (raw)
In-Reply-To: <20191014223100.GA16608@redhat.com>
> Hi Miroslav,
>
> Maybe we should add a test to verify this new behavior? See sample
> version below (lightly tested). We can add to this one, or patch
> seperately if you prefer.
Thanks a lot, Joe. It looks nice. I'll include it in v3. One question
below.
> -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--
>
>
> >From c8c9f22e3816ca4c90ab7e7159d2ce536eaa5fad Mon Sep 17 00:00:00 2001
> From: Joe Lawrence <joe.lawrence@redhat.com>
> Date: Mon, 14 Oct 2019 18:25:01 -0400
> Subject: [PATCH] selftests/livepatch: test interaction with ftrace_enabled
>
> Since livepatching depends upon ftrace handlers to implement "patched"
> functionality, verify that the ftrace_enabled sysctl value interacts
> with livepatch registration as expected.
>
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> ---
> tools/testing/selftests/livepatch/Makefile | 3 +-
> .../testing/selftests/livepatch/functions.sh | 18 +++++
> .../selftests/livepatch/test-ftrace.sh | 65 +++++++++++++++++++
> 3 files changed, 85 insertions(+), 1 deletion(-)
> create mode 100755 tools/testing/selftests/livepatch/test-ftrace.sh
>
> diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
> index fd405402c3ff..1886d9d94b88 100644
> --- a/tools/testing/selftests/livepatch/Makefile
> +++ b/tools/testing/selftests/livepatch/Makefile
> @@ -4,6 +4,7 @@ TEST_PROGS_EXTENDED := functions.sh
> TEST_PROGS := \
> test-livepatch.sh \
> test-callbacks.sh \
> - test-shadow-vars.sh
> + test-shadow-vars.sh \
> + test-ftrace.sh
>
> include ../lib.mk
> diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
> index 79b0affd21fb..556252efece0 100644
> --- a/tools/testing/selftests/livepatch/functions.sh
> +++ b/tools/testing/selftests/livepatch/functions.sh
> @@ -52,6 +52,24 @@ function set_dynamic_debug() {
> EOF
> }
>
> +function push_ftrace_enabled() {
> + FTRACE_ENABLED=$(sysctl --values kernel.ftrace_enabled)
> +}
Shouldn't we call push_ftrace_enabled() somewhere at the beginning of the
test script? set_dynamic_debug() calls its push_dynamic_debug() directly,
but set_ftrace_enabled() is different, because it is called more than once
in the script.
One could argue that ftrace_enabled has to be true at the beginning of
testing anyway, but I think it would be cleaner. Btw, we should probably
guarantee that ftrace_enabled is true when livepatch selftests are
invoked.
> +function pop_ftrace_enabled() {
> + if [[ -n "$FTRACE_ENABLED" ]]; then
> + sysctl kernel.ftrace_enabled="$FTRACE_ENABLED"
> + fi
> +}
> +# set_ftrace_enabled() - save the current ftrace_enabled config and tweak
> +# it for the self-tests. Set a script exit trap
> +# that restores the original value.
> +function set_ftrace_enabled() {
> + local sysctl="$1"
> + trap pop_ftrace_enabled EXIT INT TERM HUP
> + result=$(sysctl kernel.ftrace_enabled="$1" 2>&1 | paste --serial --delimiters=' ')
> + echo "livepatch: $result" > /dev/kmsg
> +}
> +
> # loop_until(cmd) - loop a command until it is successful or $MAX_RETRIES,
> # sleep $RETRY_INTERVAL between attempts
> # cmd - command and its arguments to run
Miroslav
next prev parent reply other threads:[~2019-10-15 11:23 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-14 10:59 [PATCH v2] ftrace: Introduce PERMANENT ftrace_ops flag Miroslav Benes
2019-10-14 14:26 ` Petr Mladek
2019-10-14 15:17 ` Steven Rostedt
2019-10-15 7:45 ` Petr Mladek
2019-10-15 10:50 ` Miroslav Benes
2019-10-15 13:36 ` Steven Rostedt
2019-10-14 22:31 ` Joe Lawrence
2019-10-15 11:23 ` Miroslav Benes [this message]
2019-10-15 13:25 ` Joe Lawrence
2019-10-15 14:02 ` Miroslav Benes
2019-10-15 14:58 ` Joe Lawrence
2019-10-16 5:02 ` Kamalesh Babulal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LSU.2.21.1910151259220.30206@pobox.suse.cz \
--to=mbenes@suse.cz \
--cc=jikos@kernel.org \
--cc=joe.lawrence@redhat.com \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.