LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@elte.hu>, Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH 0/2] ftrace: handle NMIs safely
Date: Thu, 30 Oct 2008 16:08:31 -0400	[thread overview]
Message-ID: <20081030200831.467420488@goodmis.org> (raw)

The robustness of ftrace has been the focus of the code modification in
2.6.28. There is one remaining issue that needed to be addressed.
This was the case of NMIs.

To state the problem: if a process on one CPU is modifying code that is
being executed on another CPU, that CPU will have undefined results,
and possibly issue a GPF.  To cover this with dynamic ftrace, it calls
kstop_machine to prevent the other CPUs from issuing interrupts or
executing any other process. The problem we have is that this does not
protect us against NMIs.

Discussing this with Arjan and Ingo, we came up with this RCU like solution.
When the patcher wants to modify code, it must do the following steps:

1) load the instruction pointer to modify into an IP buffer and the
   contents that will be modified into a "code" buffer.
2) set a write_bit flag.
3) wait for any running NMIs to finish.
4) modify the code.
5) clear the write bit flag.
6) wait for any running NMIs to finish
7) return the status of the write.

In the NMI handler, the first thing it will do is to check if the
write_bit is set, and if so, the NMI performs the write.
We do not worry about multiple writers writing to the code at the
same time, they are all writing the same thing.

The idea is that executing code will not have a problem if another
CPU has a process writing to that code with the same value as
what is in the code. In-other-words, the code does not change.

To help verify this, I wrote this module and let it run for a while:

-----------------------------------------------
#include <linux/module.h>
#include <linux/kthread.h>

#define WRITE_SIZE 50

static int write_thread(void *arg)
{
	unsigned char *ptr = (unsigned char *)schedule;
	static unsigned char write_buf[WRITE_SIZE];


	memcpy(write_buf, ptr, WRITE_SIZE);

	while (!kthread_should_stop()) {
		memcpy(ptr, write_buf, WRITE_SIZE);
		msleep(1);
	}

	return 0;
}

static struct task_struct *writer;

static int __init writer_torture_init(void)
{
	int ret;

	writer = kthread_run(write_thread, NULL, "write_tortured");
	ret = PTR_ERR(writer);

	if (IS_ERR(writer)) {
		writer = NULL;
		return ret;
	}

	return 0;
}

static void writer_torture_exit(void)
{
	if (writer)
		kthread_stop(writer);
}

module_init(writer_torture_init);
module_exit(writer_torture_exit);

MODULE_AUTHOR("Steven Rostedt");
MODULE_DESCRIPTION("Write code torture");
MODULE_LICENSE("GPL");
-----------------------------------------------

Do not run the dynamic ftrace while running this module ;-)

The second patch adds stats to the dyn_ftrace_total_info that adds
 NR-times-nmi-detect NR-times-NMI-wrote

-- Steve


             reply	other threads:[~2008-10-30 20:11 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-30 20:08 Steven Rostedt [this message]
2008-10-30 20:08 ` [PATCH 1/2] ftrace: nmi safe code modification Steven Rostedt
2008-10-30 20:32   ` Andrew Morton
2008-10-30 20:38     ` Ingo Molnar
2008-10-30 20:58     ` Steven Rostedt
2008-10-30 21:10       ` Andrew Morton
2008-10-31  4:03         ` [PATCH] ftrace: nmi safe code clean ups Steven Rostedt
2008-10-31  4:16           ` [PATCH] hardirq.h clean up Steven Rostedt
2008-10-31  9:30             ` Ingo Molnar
2008-10-31 13:36               ` [PATCH] ftrace: fix hardirq header for non ftrace archs Steven Rostedt
2008-11-03 10:04                 ` Ingo Molnar
2008-10-31  9:28           ` [PATCH] ftrace: nmi safe code clean ups Ingo Molnar
2008-10-30 20:08 ` [PATCH 2/2] ftrace: nmi update statistics Steven Rostedt
2008-10-30 20:38   ` Andrew Morton
2008-10-30 21:14     ` Steven Rostedt
2008-10-30 20:34 ` [PATCH 0/2] ftrace: handle NMIs safely Ingo Molnar
2008-10-30 21:15   ` Steven Rostedt
2008-10-30 22:26     ` Ingo Molnar

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=20081030200831.467420488@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: [PATCH 0/2] ftrace: handle NMIs safely' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).