All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Joerg Roedel <jroedel@suse.de>, Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Borislav Petkov <bp@alien8.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Shile Zhang <shile.zhang@linux.alibaba.com>,
	Andy Lutomirski <luto@amacapital.net>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
Subject: [RFC][PATCH] x86/mm: Sync all vmalloc mappings before text_poke()
Date: Wed, 29 Apr 2020 05:48:57 -0400	[thread overview]
Message-ID: <20200429054857.66e8e333@oasis.local.home> (raw)

From: Steven Rostedt (VMware) <rostedt@goodmis.org>

Tzvetomir was adding a feature to trace-cmd that would allow the user
to specify filtering on process IDs within a tracing instance (or
buffer). When he added this feature and tested it on tracing PIDs 1 and
2, it caused his kernel to hang.

He sent me his code and I was able to reproduce the hang as well. I
bisected it down to this commit 763802b53a42 ("x86/mm: split
vmalloc_sync_all()"). It was 100% reproducible. With the commit it
would hang, and reverting the commit, it would work.

Adding a bunch of printk()s, I found where it locked up. It was after
the recording was finished, and a write of "0" to
tracefs/instance/foo/events/enable. And in the code, it was:

(you may skip to the end of the chain)

system_enable_write() {
  __ftrace_set_clr_event() {
    __ftrace_set_clr_event_nolock() {
      ftrace_event_enable_disable() {
        __ftrace_event_enable_disable() {
          call->class->reg() <trace_event_reg()> {
            trace_point_probe_unregister() {
              tracepoint_remove_func() {
                static_key_slow_dec() {
                  __static_key_slow_dec() {

    <continued>

  __static_key_slow_dec_cpus_locked() {
    jump_label_update() {
      __jump_label_update()
        arch_jump_label_transform() {
          jump_label_transform() {
            __jump_label_transform() {
              text_poke_bp() {
                text_poke_bp_batch() {
                  text_poke() {
                    __text_poke() {

    <continued> (This is where you want to see)

  use_temporary_mm() {
    switch_mm_irqs_off() {
      load_new_mm_cr3() {
        write_cr3() <<--- Lock up!

The really strange part about this, is that this only crashes if we
filter the PIDs on an instance (and via trace-cmd, which does some
initial clean ups). But it works fine when doing from the top level
tracing buffer, or by doing this manually. I'm not sure how vmalloc
gets involved with this. Anyway, I tried the following patch, and it
fixes the lockup for both myself and Tzvetomir. But since I'm not 100%
sure what broke, I'm sending this out as an RFC.

Fixes: 763802b53a42 ("x86/mm: split vmalloc_sync_all()")
Reported-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 7867dfb3963e..015dd30a2260 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -802,6 +802,8 @@ static void *__text_poke(void *addr, const void *opcode, size_t len)
 	 */
 	BUG_ON(!after_bootmem);
 
+	vmalloc_sync_mappings();
+
 	if (!core_kernel_text((unsigned long)addr)) {
 		pages[0] = vmalloc_to_page(addr);
 		if (cross_page_boundary)

             reply	other threads:[~2020-04-29  9:49 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-29  9:48 Steven Rostedt [this message]
2020-04-29 10:59 ` [RFC][PATCH] x86/mm: Sync all vmalloc mappings before text_poke() Joerg Roedel
2020-04-29 12:28   ` Steven Rostedt
2020-04-29 14:07     ` Steven Rostedt
2020-04-29 14:10       ` Joerg Roedel
2020-04-29 14:32         ` Steven Rostedt
2020-04-29 15:44           ` Peter Zijlstra
2020-04-29 16:17       ` Joerg Roedel
2020-04-29 16:20         ` Joerg Roedel
2020-04-29 16:52           ` Steven Rostedt
2020-04-29 17:29             ` Mathieu Desnoyers
2020-04-29 18:51               ` Peter Zijlstra
2020-04-30 14:11       ` Joerg Roedel
2020-04-30 14:50         ` Joerg Roedel
2020-04-30 15:20           ` Mathieu Desnoyers
2020-04-30 16:16             ` Steven Rostedt
2020-04-30 16:18               ` Mathieu Desnoyers
2020-04-30 16:30                 ` Steven Rostedt
2020-04-30 16:35                   ` Mathieu Desnoyers
2020-04-30 15:23         ` Mathieu Desnoyers
2020-04-30 16:12           ` Steven Rostedt
2020-04-30 16:11         ` Steven Rostedt
2020-04-30 16:16           ` Mathieu Desnoyers
2020-04-30 16:25             ` Steven Rostedt
2020-04-30 19:14           ` Joerg Roedel
2020-05-01  1:13             ` Steven Rostedt
2020-05-01  2:26               ` Mathieu Desnoyers
2020-05-01  2:39                 ` Steven Rostedt
2020-05-01 10:16                   ` Joerg Roedel
2020-05-01 13:35                   ` Mathieu Desnoyers
2020-05-04 15:12                   ` [PATCH] percpu: Sync vmalloc mappings in pcpu_alloc() and free_percpu() Joerg Roedel
2020-05-04 15:28                     ` Mathieu Desnoyers
2020-05-04 15:31                       ` Joerg Roedel
2020-05-04 15:38                         ` Mathieu Desnoyers
2020-05-04 15:51                           ` Joerg Roedel
2020-05-04 17:04                           ` Steven Rostedt
2020-05-04 17:40                     ` Steven Rostedt
2020-05-04 18:38                       ` Joerg Roedel
2020-05-04 19:10                         ` Steven Rostedt
2020-05-05 12:31                           ` [PATCH] tracing: Call vmalloc_sync_mappings() after alloc_percpu() Joerg Roedel
2020-05-06 15:17                             ` Steven Rostedt
2020-05-08 14:42                               ` Joerg Roedel
2020-05-04 20:25                     ` [PATCH] percpu: Sync vmalloc mappings in pcpu_alloc() and free_percpu() Peter Zijlstra
2020-05-04 20:43                       ` Steven Rostedt
2020-05-01  4:20                 ` [RFC][PATCH] x86/mm: Sync all vmalloc mappings before text_poke() Steven Rostedt
2020-05-01 13:22                   ` Mathieu Desnoyers

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=20200429054857.66e8e333@oasis.local.home \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=jroedel@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=shile.zhang@linux.alibaba.com \
    --cc=tglx@linutronix.de \
    --cc=tz.stoyanov@gmail.com \
    /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.