linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Shuah Khan <shuah.kh@samsung.com>, Ingo Molnar <mingo@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH ftrace/for-next ] tracing/kprobes: Replace startup test with selftest script
Date: Tue, 7 Oct 2014 12:05:30 -0400	[thread overview]
Message-ID: <20141007120530.136312fd@gandalf.local.home> (raw)
In-Reply-To: <5433817C.3000206@hitachi.com>

On Tue, 07 Oct 2014 15:00:28 +0900
Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

> (2014/10/07 7:33), Steven Rostedt wrote:
> > On Mon, 06 Oct 2014 11:48:06 +0000
> > Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> > 
> >> Replace the kprobe_tracer's startup test with two selftest scripts.
> >> These test cases are testing that the kprobe_event can accept a
> >> kprobe event with $stack related arguments and a kretprobe event
> >> with $retval argument.
> > 
> > Can't we keep both? I have boxes I run my own tests with and enables
> > these start up tests in the kernel. I don't plan on testing on all
> > theses boxes using the scripts in the kernel.
> > 
> > Having a self test in the kernel itself can be useful too.
> 
> Hmm, deprecating the test is acceptable, but I think it is just
> a dead weight that if we have both of them forever in the kernel.
> Of course, if that feature is fundamentally related to booting up
> the kernel, we need to keep them in boot up code. But if it is
> possible to run after booting up, I think we'd better to move it
> under kselftest, since we can do more investigation after booting.
> 

I'm just saying that it is more likely to have this test run if it's in
the kernel than in userspace. But as you say, we can debug it better if
there's a userspace tool that can run too. This is why I'm saying we
should keep both. I think they are both useful for different reasons.
Keeping it in the kernel as a config option will give it more exposure,
and keeping it in the tools/testing directory gives us a way to debug
it if there an issue should arise.

Both of these have valid reasons staying in the kernel and I don't see
either as dead weight. Is there a maintenance issue with keeping it in
the kernel? There doesn't seem to be much done to it. It seems
untouched for over a year, and that was to add support for multiple
buffers.

-- Steve

  reply	other threads:[~2014-10-07 16:05 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-06 11:48 [PATCH ftrace/for-next ] tracing/kprobes: Replace startup test with selftest script Masami Hiramatsu
2014-10-06 22:33 ` Steven Rostedt
2014-10-07  6:00   ` Masami Hiramatsu
2014-10-07 16:05     ` Steven Rostedt [this message]
2014-10-08  1:59       ` Masami Hiramatsu
2014-10-08  2:20         ` Steven Rostedt
2014-10-08  4:02           ` Masami Hiramatsu

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=20141007120530.136312fd@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@kernel.org \
    --cc=shuah.kh@samsung.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 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).