Re: [PATCH 2/2] trace/kprobe: Remove limit on kretprobe maxactive

From: Masami Hiramatsu <mhiramat@kernel.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>,
	Anton Blanchard <anton@ozlabs.org>,
	linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH 2/2] trace/kprobe: Remove limit on kretprobe maxactive
Date: Thu, 17 Jun 2021 00:10:09 +0900	[thread overview]
Message-ID: <20210617001009.d5ae7b2edfdc34f4f8c19ab5@kernel.org> (raw)
In-Reply-To: <20210616112711.ce318fc66b389038203331d1@kernel.org>

On Wed, 16 Jun 2021 11:27:11 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Tue, 15 Jun 2021 21:03:51 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Wed, 16 Jun 2021 09:46:22 +0900
> > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > 
> > > 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.
> > 
> > What if you just made the max as 10 * number of possible cpus, or 4096,
> > which ever is greater? Why would a user need more?
> 
> It could be. But actually, that is not correct number because the
> number of instances depends on the number of processes and the possiblity
> of recursive. Thus the huge system which runs more than 64k processes,
> may need more than that.
> 
> > I'd still like to get a wrapper around function graph tracing so that
> > kretprobes could use it. I think that would get rid of the requirement
> > of maxactive, because isn't that just used to have a way to know the
> > original return value?
> 
> Hmm, yes, on some arch, it can be done. But on other arch we still need
> current implementation for generic solution.
> What I need is not fully wrapped by the function graph, but just share
> the per-task (software) shadow stack.

BTW, I have 2 ideas to fix this except for wrapper.

1. Use func-graph tracer API directly from dynamic event instead of
  kretprobes. This will be enabled only if the arch supports fgraph
  tracer and enable it. maxactive will be ignored if this is enabled,
  and tracefs user may not need except for the return value 
  (BTW, is that possible to access the stack? In some case, return
  value can be passed via stack)

2. Move the kretprobe instance pool from kretprobe to struct task.
  This pool will allocates one page per task, and shared among all
  kretprobes. This pool will be allocated when the 1st kretprobe
  is registered. maxactive will be kept for someone who wants to
  use per-instance data. But since dynamic event doesn't use it,
  it will be removed from tracefs and perf.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>