public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: mingo@redhat.com, x86@kernel.org,
	Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H . Peter Anvin" <hpa@zytor.com>,
	"Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Alexei Starovoitov <ast@fb.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [BUGFIX PATCH] kprobes/x86: Remove IRQ disabling from jprobe handlers
Date: Thu, 5 Oct 2017 17:28:36 +0900	[thread overview]
Message-ID: <20171005172836.d084ef709be2e34037ccc0d6@kernel.org> (raw)
In-Reply-To: <20171005075704.2wv3vcg35s4g5jhq@gmail.com>

On Thu, 5 Oct 2017 09:57:04 +0200
Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > On Wed, 4 Oct 2017 12:41:01 +0200
> > Ingo Molnar <mingo@kernel.org> wrote:
> > 
> > > 
> > > * Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > 
> > > > Hmm, actually we can not disable jprobe, that has no separate Kconfig.
> > > > So we need to introduce new kconfig for that.
> > > > 
> > > > And, there are several network protocols using jprobe to trace events.
> > > > (e.g. NET_DCCPPROBE and NET_TCPPROBE)
> > > > I think they need to migrate to trace-event at first.
> > > > 
> > > > So, how about below idea?
> > > > 
> > > > 1. Introduce CONFIG_JPROBE_API which only separate jprobe general parts
> > > >      (no arch dependent code involves) and make it default n.
> > > > 2. Mark break_handler and jprobe APIs deprecated so that no new user comes up.
> > > > 3. migrate in-kernel jprobe user to trace-event or ftrace.
> > > >    (may take some time)
> > > 
> > > So my suggestion would be to just return from register_jprobe() and don't register 
> > > anything.
> > 
> > with CONFIG_JPROBE_API=n, is that right?
> 
> No, unconditionally off with a WARN_ON_ONCE() warning in the registry function and 
> the deactivation of all in-kernel uses (such as self-tests).
> 
> The point is to make people that _truly_ rely on it complain - not just make them 
> silently turn on a Kconfig option ...

Hmm, but in that case, anyway we have to remove or rename the function
like register_jprobe. Or would that "unconditionally" mean "#if 0"?

> > > Yes, there are usecases of jprobes in the kernel, but they all look 
> > > pretty ancient and unused.
> > 
> > Hmm, in that case, should we also remove those users? If we disable such way
> > those features are just useless.
> 
> My hypothesis is that those features are not used (hence useless), but we should 
> first test whether there's any reliance before we remove code.

Agreed.

Thank you,

> 
> Thanks,
> 
> 	Ingo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

  reply	other threads:[~2017-10-05  8:28 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-30 23:12 [kprobes/x86] a19b2e3d78: WARNING:at_kernel/locking/lockdep.c:#trace_hardirqs_off_caller kernel test robot
2017-10-02  7:33 ` Peter Zijlstra
2017-10-02 15:46   ` Masami Hiramatsu
2017-10-02 16:05     ` Peter Zijlstra
2017-10-03  1:10       ` Masami Hiramatsu
2017-10-02 16:19     ` Linus Torvalds
2017-10-03  2:26       ` Masami Hiramatsu
2017-10-03  7:18 ` [BUGFIX PATCH] kprobes/x86: Remove IRQ disabling from jprobe handlers Masami Hiramatsu
2017-10-03  9:33   ` Ingo Molnar
2017-10-03 15:24     ` Masami Hiramatsu
2017-10-03 17:11       ` Ingo Molnar
2017-10-04  6:18         ` Masami Hiramatsu
2017-10-04 10:41           ` Ingo Molnar
2017-10-04 14:08             ` Masami Hiramatsu
2017-10-05  7:57               ` Ingo Molnar
2017-10-05  8:28                 ` Masami Hiramatsu [this message]
2017-10-03 17:43   ` [tip:x86/urgent] " tip-bot for 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=20171005172836.d084ef709be2e34037ccc0d6@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=ananth@linux.vnet.ibm.com \
    --cc=ast@fb.com \
    --cc=ast@kernel.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox