Messages in this thread Patch in this message | | | Date | Tue, 8 Nov 2022 10:50:01 +0800 | Subject | Re: [PATCH] kretprobe events missing on 2-core KVM guest | From | wuqiang <> |
| |
On 2022/11/7 21:36, Solar Designer wrote: > On Wed, Oct 26, 2022 at 12:33:15AM +0900, Masami Hiramatsu wrote: >> On Tue, 25 Oct 2022 18:01:17 +0800 >> wuqiang <wuqiang.matt@bytedance.com> 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. >>> >>> This patch increases the minimum of maxactive to 10. >> >> This looks reasonable to me. >> >> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Reasonable yes, but: > > Is 10 enough? How exactly do those instances get used up without > preemption? Perhaps because execve can sleep? If so, perhaps we should > use the same logic without preemption that we do with preemption? So > maybe just make this line unconditional? - > > rp->maxactive = max_t(unsigned int, 10, 2*num_possible_cpus());
I agree to make it unconditional, though it could cost a bit more memory.
Here's my testcase: a shell script was added to crontab, and the content of the script is:
#!/bin/sh do_something_with_magic `tr -dc a-z < /dev/urandom | head -c 10`
cron will trigger a series of program executions (4 times every hour). Then we noticed events loss after 3-4 hours of testings.
The issue is caused by a burst of series of execve requests. The best number of instances could be different case by case, and should be the user's duty to decide, but num_possible_cpus() as a default value is inadequate. For my testcase, 8 is enough as verified, and 10 is chosen to keep it identical.
The handling of execve syscall can be suspended or voluntarily yield up cpu due to i/o or memory resources and then a new execve gets its time slice to start. It could be worse for scenarios of resource throttling or routines that are heavier than execve (though rare as I think).
> Also, the behavior was documented in Documentation/trace/kprobes.rst, so > perhaps that file should be updated at the same time with the code.
Right, will update in next version.
>>> Signed-off-by: wuqiang <wuqiang.matt@bytedance.com> >>> --- >>> kernel/kprobes.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/kernel/kprobes.c b/kernel/kprobes.c >>> index 3220b0a2fb4a..b781dee3f552 100644 >>> --- a/kernel/kprobes.c >>> +++ b/kernel/kprobes.c >>> @@ -2211,7 +2211,7 @@ int register_kretprobe(struct kretprobe *rp) >>> #ifdef CONFIG_PREEMPTION >>> rp->maxactive = max_t(unsigned int, 10, 2*num_possible_cpus()); >>> #else >>> - rp->maxactive = num_possible_cpus(); >>> + rp->maxactive = max_t(unsigned int, 10, num_possible_cpus()); >>> #endif >>> } >>> #ifdef CONFIG_KRETPROBE_ON_RETHOOK >>> -- >>> 2.34.1 >>> >> >> >> -- >> Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Thanks, > > Alexander
Best regards, wuqiang
| |