public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed

From: Ingo Molnar <mingo@kernel•org>
To: Masami Hiramatsu <mhiramat@kernel•org>
Cc: Steven Rostedt <rostedt@goodmis•org>,
	Ingo Molnar <mingo@redhat•com>,
	Alban Crequy <alban.crequy@gmail•com>,
	Alban Crequy <alban@kinvolk•io>,
	Alexei Starovoitov <ast@kernel•org>,
	Jonathan Corbet <corbet@lwn•net>,
	Arnaldo Carvalho de Melo <acme@redhat•com>,
	Omar Sandoval <osandov@fb•com>,
	linux-doc@vger•kernel.org,	netdev@vger•kernel.org,
	linux-kernel@vger•kernel.org,	iago@kinvolk•io,
	michael@kinvolk•io,	Dorau Lukasz <lukasz.dorau@intel•com>,
	systemtap@sourceware•org
Subject: Re: [RFC PATCH tip/master 2/3] kprobes: Allocate kretprobe instance if its free list is empty
Date: Thu, 30 Mar 2017 08:53:32 +0200	[thread overview]
Message-ID: <20170330065332.GA30148@gmail.com> (raw)
In-Reply-To: <20170329172510.e012406497fd38a54d5069b3@kernel.org>


* Masami Hiramatsu <mhiramat@kernel•org> wrote:

> > So this is something I missed while the original code was merged, but the concept 
> > looks a bit weird: why do we do any "allocation" while a handler is executing?
> > 
> > That's fundamentally fragile. What's the maximum number of parallel 
> > 'kretprobe_instance' required per kretprobe - one per CPU?
> 
> It depends on the place where we put the probe. If the probed function will be
> blocked (yield to other tasks), then we need a same number of threads on
> the system which can invoke the function. So, ultimately, it is same
> as function_graph tracer, we need it for each thread.

So then put it into task_struct (assuming there's no kretprobe-inside-kretprobe 
nesting allowed). There's just no way in hell we should be calling any complex 
kernel function from kernel probes!

I mean, think about it, a kretprobe can be installed in a lot of places, and now 
we want to call get_free_pages() from it?? This would add a massive amount of 
fragility.

Instrumentation must be _simple_, every patch that adds more complexity to the 
most fundamental code path of it should raise a red flag ...

So let's make this more robust, ok?

Thanks,

	Ingo

next prev parent reply	other threads:[~2017-03-30  6:53 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-29  5:20 [RFC PATCH tip/master 0/3] kprobes: tracing: kretprobe_instance dynamic allocation Masami Hiramatsu
2017-03-29  5:22 ` [RFC PATCH tip/master 1/3] trace: kprobes: Show sum of probe/retprobe nmissed count Masami Hiramatsu
2017-03-31  9:45   ` Alban Crequy
2017-03-29  5:23 ` [RFC PATCH tip/master 2/3] kprobes: Allocate kretprobe instance if its free list is empty Masami Hiramatsu
2017-03-29  6:30   ` Ingo Molnar
2017-03-29  8:25     ` Masami Hiramatsu
2017-03-29 17:18       ` Josh Stone
2017-03-30  0:39         ` Masami Hiramatsu
2017-03-30  6:53       ` Ingo Molnar [this message]
2017-03-30 13:01         ` Masami Hiramatsu
2017-04-12  6:42           ` Ingo Molnar
2017-03-30 13:03         ` Alban Crequy
2017-03-29  5:24 ` [RFC PATCH tip/master 3/3] kprobes: Limit kretprobe maximum instances Masami Hiramatsu
2017-03-29 13:27 ` [RFC PATCH tip/master 0/3] kprobes: tracing: kretprobe_instance dynamic allocation Frank Ch. Eigler

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=20170330065332.GA30148@gmail.com \
    --to=mingo@kernel$(echo .)org \
    --cc=acme@redhat$(echo .)com \
    --cc=alban.crequy@gmail$(echo .)com \
    --cc=alban@kinvolk$(echo .)io \
    --cc=ast@kernel$(echo .)org \
    --cc=corbet@lwn$(echo .)net \
    --cc=iago@kinvolk$(echo .)io \
    --cc=linux-doc@vger$(echo .)kernel.org \
    --cc=linux-kernel@vger$(echo .)kernel.org \
    --cc=lukasz.dorau@intel$(echo .)com \
    --cc=mhiramat@kernel$(echo .)org \
    --cc=michael@kinvolk$(echo .)io \
    --cc=mingo@redhat$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=osandov@fb$(echo .)com \
    --cc=rostedt@goodmis$(echo .)org \
    --cc=systemtap@sourceware$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox