Messages in this thread | | | Date | Mon, 14 Nov 2022 14:45:48 +0900 | From | Masami Hiramatsu (Google) <> | Subject | Re: [PATCH v2] kprobes: kretprobe events missing on 2-core KVM guest |
| |
On Thu, 10 Nov 2022 15:52:23 +0100 Solar Designer <solar@openwall.com> 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@kernel.org>
Thanks,
> > > > Signed-off-by: wuqiang <wuqiang.matt@bytedance.com> > > Reviewed-by: Solar Designer <solar@openwall.com> > > 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@kernel.org>
| |