Messages in this thread | | | Subject | Re: [PATCH v5] arm64: kprobe: Enable OPTPROBE for arm64 | From | "liuqi (BA)" <> | Date | Tue, 4 Jan 2022 11:50:58 +0800 |
| |
On 2022/1/4 10:35, Masami Hiramatsu wrote: > Hi Jianhua, > > On Mon, 3 Jan 2022 17:03:33 +0800 > Jianhua Liu <jianhua.ljh@gmail.com> wrote: > >> Hi Qi, >> I have tested your patch on UNISOC s9863a. >> Test case "kprobe_example & kretprobe_example" is OK. >> >> Two point: >> 1. backtrace is not perfect. >> optprobe_common does not saved frame pointer, >> backtrace lacks two calls. >> such as for dup_mm: lack copy_process-->dup_mm >> dup_mm backtrace from your patch: >> [ 832.387066] CPU: 0 PID: 296 Comm: sh Not tainted 5.16.0-rc5+ #8 >> [ 832.387078] Hardware name: Spreadtrum SP9863A-1H10 Board (DT) >> [ 832.387083] Call trace: >> [ 832.387086] dump_backtrace+0x0/0x1e0 >> [ 832.387103] show_stack+0x24/0x30 >> [ 832.387112] dump_stack_lvl+0x68/0x84 >> [ 832.387123] dump_stack+0x18/0x34 >> [ 832.387131] handler_pre+0x40/0x50 [kprobe_example] >> [ 832.387143] opt_pre_handler+0x84/0xc0 >> [ 832.387154] optprobe_optimized_callback+0xec/0x164 >> [ 832.387164] optprobe_common+0x70/0xc4 >> [ 832.387173] kernel_clone+0x98/0x440 >> [ 832.387182] __do_sys_clone+0x54/0x80 >> [ 832.387191] __arm64_sys_clone+0x2c/0x40 >> [ 832.387199] invoke_syscall+0x50/0x120 >> [ 832.387208] el0_svc_common.constprop.0+0x4c/0xf4 >> [ 832.387217] do_el0_svc+0x30/0x9c >> [ 832.387225] el0_svc+0x20/0x60 >> [ 832.387235] el0t_64_sync_handler+0xe8/0xf0 >> [ 832.387242] el0t_64_sync+0x1a0/0x1a4 >> >> >> dup_mm backtrace from other: >> [ 173.352294] CPU: 6 PID: 309 Comm: sh Not tainted 5.16.0-rc5+ #19 >> [ 173.352301] Hardware name: Spreadtrum SP9863A-1H10 Board (DT) >> [ 173.352304] Call trace: >> [ 173.352307] dump_backtrace+0x0/0x1d4 >> [ 173.352319] show_stack+0x18/0x24 >> [ 173.352326] dump_stack_lvl+0x68/0x84 >> [ 173.352333] dump_stack+0x18/0x34 >> [ 173.352338] handler_pre+0x38/0x48 [kprobe_example] >> [ 173.352347] opt_pre_handler+0x74/0xb0 >> [ 173.352354] optimized_callback+0x108/0x130 >> [ 173.352361] optinsn_slot+0x258/0x1000 >> [ 173.352366] dup_mm+0x4/0x4b0 >> [ 173.352373] copy_process+0x1284/0x1360 >> [ 173.352378] kernel_clone+0x5c/0x3c0 >> [ 173.352384] __do_sys_clone+0x54/0x80 >> [ 173.352390] __arm64_sys_clone+0x24/0x30 >> [ 173.352396] invoke_syscall+0x48/0x114 >> [ 173.352402] el0_svc_common.constprop.0+0x44/0xec >> [ 173.352408] do_el0_svc+0x24/0x90 >> [ 173.352413] el0_svc+0x20/0x60 >> [ 173.352420] el0t_64_sync_handler+0xe8/0xf0 >> [ 173.352427] el0t_64_sync+0x1a0/0x1a4 > > Is the second one with your patch? > >> >> 2. The reserve memory "OPT_SLOT_SIZE - PAGE_SIZE" is waste. >> kernel/kprobe.c used only one PAGE_SIZE slot memory. > Hi Masami, > Good catch! > Qi, can you make an array (or bit map) of usage flags and > manage the reserved memory? >
Sure, I'll add and test this in bext version, thanks. Qi > #define OPT_INSN_PAGES (OPT_SLOT_SIZE/PAGE_SIZE) > static bool insn_page_in_use[OPT_INSN_PAGES]; > > void *alloc_optinsn_page(void) > { > int i; > > for (i = 0; i < OPT_INSN_PAGES; i++) > if (!insn_page_in_use[i]) > goto found; > return NULL; > found: > insn_page_in_use[i] = true; > return (void *)((unsigned long)optinsn_slot + PAGE_SIZE * i); > } > > void free_optinsn_page(void *page) > { > unsigned long idx = (unsigned long)page - (unsigned long)optinsn_slot; > > WARN_ONCE(idx & (PAGE_SIZE - 1)); > idx >>= PAGE_SHIFT; > if (WARN_ONCE(idx >= OPT_INSN_PAGES)) > return; > insn_page_in_use[idx] = false; > } > > Thank you, > > > >
| |