Re: [PATCH v2] kprobes: kretprobe events missing on 2-core KVM guest

From: Google
Date: Mon Nov 14 2022 - 00:46:55 EST


On Thu, 10 Nov 2022 15:52:23 +0100
Solar Designer <solar@xxxxxxxxxxxx> wrote:

> On Thu, Nov 10, 2022 at 04:15:02PM +0800, wuqiang wrote:
> > Default value of maxactive is set as num_possible_cpus() for nonpreemptable
> > systems. For a 2-core system, only 2 kretprobe instances would be allocated
> > in default, then these 2 instances for execve kretprobe are very likely to
> > be used up with a pipelined command.
> >
> > Here's the testcase: a shell script was added to crontab, and the content
> > of the script is:
> >
> > #!/bin/sh
> > do_something_magic `tr -dc a-z < /dev/urandom | head -c 10`
> >
> > cron will trigger a series of program executions (4 times every hour). Then
> > events loss would be noticed normally after 3-4 hours of testings.
> >
> > The issue is caused by a burst of series of execve requests. The best number
> > of kretprobe instances could be different case by case, and should be user's
> > duty to determine, but num_possible_cpus() as the default value is inadequate
> > especially for systems with small number of cpus.
> >
> > This patch enables the logic for preemption as default, thus increases the
> > minimum of maxactive to 10 for nonpreemptable systems.

Hm, OK. I think the enough value depends on where you put the probes on.
But maybe just making it NR_CPU -> 2 * NR_CPUS is reasonable.

Acked-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

Thanks,

> >
> > Signed-off-by: wuqiang <wuqiang.matt@xxxxxxxxxxxxx>
>
> Reviewed-by: Solar Designer <solar@xxxxxxxxxxxx>
>
> Thank you!
>
> > ---
> > Documentation/trace/kprobes.rst | 3 +--
> > kernel/kprobes.c | 10 +++-------
> > 2 files changed, 4 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/trace/kprobes.rst b/Documentation/trace/kprobes.rst
> > index 48cf778a2468..fc7ce76eab65 100644
> > --- a/Documentation/trace/kprobes.rst
> > +++ b/Documentation/trace/kprobes.rst
> > @@ -131,8 +131,7 @@ For example, if the function is non-recursive and is called with a
> > spinlock held, maxactive = 1 should be enough. If the function is
> > non-recursive and can never relinquish the CPU (e.g., via a semaphore
> > or preemption), NR_CPUS should be enough. If maxactive <= 0, it is
> > -set to a default value. If CONFIG_PREEMPT is enabled, the default
> > -is max(10, 2*NR_CPUS). Otherwise, the default is NR_CPUS.
> > +set to a default value: max(10, 2*NR_CPUS).
> >
> > It's not a disaster if you set maxactive too low; you'll just miss
> > some probes. In the kretprobe struct, the nmissed field is set to
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index a8b202f87e2d..1e80bddf2654 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -2212,11 +2212,7 @@ int register_kretprobe(struct kretprobe *rp)
> > rp->kp.post_handler = NULL;
> >
> > /* Pre-allocate memory for max kretprobe instances */
> > - if (rp->maxactive <= 0) {
> > -#ifdef CONFIG_PREEMPTION
> > + if (rp->maxactive <= 0)
> > rp->maxactive = max_t(unsigned int, 10, 2*num_possible_cpus());
> > -#else
> > - rp->maxactive = num_possible_cpus();
> > -#endif
> > - }
> > +
> > #ifdef CONFIG_KRETPROBE_ON_RETHOOK
> > --
> > 2.34.1
>
> Alexander


--
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>