All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Anton Blanchard <anton@ozlabs.org>,
	linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH 2/2] trace/kprobe: Remove limit on kretprobe maxactive
Date: Fri, 18 Jun 2021 18:49:06 +0530	[thread overview]
Message-ID: <1624005747.j7xp8o9byl.naveen@linux.ibm.com> (raw)
In-Reply-To: <20210618151714.3ae6528eba99eea39771b859@kernel.org>

Masami Hiramatsu wrote:
> 
>> To address this, as a first step, we should probably consider parsing 
>> kprobe_profile and printing a warning with 'perf' if we detect a 
>> non-zero miss count for a probe -- both a regular probe, as well as a 
>> retprobe.
> 
> Yeah, it is doable. Note that perf-probe only set up the event and
> perf-trace or other commands will use it.
> 
> 
>> If we do this, the nice thing with kprobe_profile is that the probe miss 
>> count is available, and can serve as a good way to decide what a more 
>> reasonable maxactive value should be. This should help prevent users 
>> from trying with arbitrary maxactive values.
> 
> Such feedback loop is an interesting idea.
> Note that nmissed count is an accumulate value, not the max number of
> the instance which will be needed.

Yes, we will have to factor-in the duration during which the event was 
active. This will still be an approximation, but serves as a good 
starting point. It may need a few tries to get this right, but more
importantly, the user knows instantly that there are missed probes.

> 
>> For perf_event_open(), perhaps we can introduce an ioctl to query the 
>> probe miss count.
> 
> Or, maybe we can expand the maxactive in runtime. e.g. add a shortage
> counter on the kretprobe, and run a monitor kernel thread (or kworker).
> If the shortage counter is incremented, the monitor allocates instances
> (2x counter) and give it to the kretprobe. And it resets the shortage
> counter. This adaptive maxactive may cause mis-hit in the beginning,
> but finally find the optimal maxactive value automatically.

I like this idea and I have been thinking along these lines too. If we 
start with a better default (rather than just num_possible_cpus() used 
today), I suspect we may be able to get this to work well enough to not 
have to miss any probes. Specifying 'maxactive' can still serve as a 
workaround to allocate a larger initial set of kretprobe_instances in 
case this doesn't work.

> 
> 
>> > To avoid such trouble, I had set the 4096 limitation for the maxactive
>> > parameter. Of course 4096 may not enough for some use-cases. I'm 
>> > welcome
>> > to expand it (e.g. 32k, isn't it enough?), but removing the limitation
>> > may cause OOM trouble easily.
>> 
>> Do you have suggestions for how we can determine a better limit? As you 
>> point out in the other email, there could very well be 64k or more 
>> processes on a large machine. Since the primary concern is memory usage, 
>> we probably need to decide this based on total memory. But, memory usage 
>> will vary depending on system load...
> 
> This is very good question. IMHO, it might better to calculate the total
> maxactive from the system memory size. For example, 1% of system memory
> can be used for the kretprobes, 16GB system will allow using 160MB for
> kretprobes, which means about "30M" is the max number of maxactive, or
> multiple kretprobes can share it. Doesn't it sound enough? Of course
> this will need to show the current usage of the kretprobe instance objects
> via tracefs or debugfs. But this total cap seems reasonable for me to
> avoid OOM trouble.
> 
>> Perhaps we can start by making maxactive limit be a tunable with a 
>> default value of 4096, with the understanding that users will be careful 
>> when bumping up this value. Hopefully, scripts won't simply start 
>> writing into this file ;)
> 
> Yeah, that's what I suggested at first, because the best maxactive will
> depend on the max number of the *processes* and the probed function.
> 
> If the probed function will NOT be preempted or slept, maxactive will be
> the number of *processor cores*. Or, if it can be preempted or slept, it
> will be the max number of *processes*. If the probed function can
> recursively called (Note: this is rare case), the maxactive has to
> be multiplied.
> 
> It is hard to estimate the max number of processes, since it depends
> on the system. Small embedded systems don't run thousands of processes,
> but big servers will run more than ten thousands of processes.
> Thus make it tunable will be a good idea.

Agree.


Thanks,
Naveen


      reply	other threads:[~2021-06-18 13:19 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-14 18:03 [PATCH 0/2] trace/kprobe: Two fixes for kretprobes Naveen N. Rao
2021-06-14 18:03 ` [PATCH 1/2] trace/kprobe: Fix count of missed kretprobes in kprobe_profile Naveen N. Rao
2021-06-15  5:47   ` Masami Hiramatsu
2021-06-14 18:03 ` [PATCH 2/2] trace/kprobe: Remove limit on kretprobe maxactive Naveen N. Rao
2021-06-15  9:35   ` Masami Hiramatsu
2021-06-15 17:41     ` Naveen N. Rao
2021-06-16  0:46       ` Masami Hiramatsu
2021-06-16  1:03         ` Steven Rostedt
2021-06-16  2:27           ` Masami Hiramatsu
2021-06-16 15:10             ` Masami Hiramatsu
2021-06-17 16:34               ` Naveen N. Rao
2021-06-17 17:07                 ` Steven Rostedt
2021-06-18  4:26                   ` Masami Hiramatsu
2021-06-18  8:41                     ` Naveen N. Rao
2021-06-17 16:19         ` Naveen N. Rao
2021-06-18  6:17           ` Masami Hiramatsu
2021-06-18 13:19             ` Naveen N. Rao [this message]

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=1624005747.j7xp8o9byl.naveen@linux.ibm.com \
    --to=naveen.n.rao@linux.vnet.ibm.com \
    --cc=anton@ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.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 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.