Subject: Re: [PATCH] LoongArch: Add kprobe on ftrace support
To:     Huacai Chen <chenhuacai@kernel.org>
References: <1675414264-28241-1-git-send-email-yangtiezhu@loongson.cn>
 <CAAhV-H4tMC+y9enyvWj9hu0ngin6ND4DPKbcFybVzU=A-7tLSA@mail.gmail.com>
Cc:     WANG Xuerui <kernel@xen0n.name>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        loongarch@lists.linux.dev, linux-kernel@vger.kernel.org
From:   Tiezhu Yang <yangtiezhu@loongson.cn>
Message-ID: <736bc691-feae-9d19-130e-4846cdd0a699@loongson.cn>
Date:   Fri, 3 Feb 2023 17:34:38 +0800
User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <CAAhV-H4tMC+y9enyvWj9hu0ngin6ND4DPKbcFybVzU=A-7tLSA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 02/03/2023 05:00 PM, Huacai Chen wrote:
> Hi, Tiezhu,
>
> On Fri, Feb 3, 2023 at 4:51 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>
>> Add kprobe_ftrace_handler() and arch_prepare_kprobe_ftrace() to support
>> kprobe on ftrace, the code is similar with x86 and riscv.
>>
>> Here is a simple example:
>>
>>   # echo 'p:myprobe kernel_clone' > /sys/kernel/debug/tracing/kprobe_events
>>   # echo 'r:myretprobe kernel_clone $retval' >> /sys/kernel/debug/tracing/kprobe_events
>>   # echo 1 > /sys/kernel/debug/tracing/events/kprobes/myprobe/enable
>>   # echo 1 > /sys/kernel/debug/tracing/events/kprobes/myretprobe/enable
>>   # echo 1 > /sys/kernel/debug/tracing/tracing_on
>>   # cat /sys/kernel/debug/tracing/trace
>>   # tracer: nop
>>   #
>>   # entries-in-buffer/entries-written: 2/2   #P:4
>>   #
>>   #                                _-----=> irqs-off/BH-disabled
>>   #                               / _----=> need-resched
>>   #                              | / _---=> hardirq/softirq
>>   #                              || / _--=> preempt-depth
>>   #                              ||| / _-=> migrate-disable
>>   #                              |||| /     delay
>>   #           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
>>   #              | |         |   |||||     |         |
>>               bash-488     [002] .....  2041.190681: myprobe: (kernel_clone+0x0/0x40c)
>>               bash-488     [002] .....  2041.190788: myretprobe: (__do_sys_clone+0x84/0xb8 <- kernel_clone) arg1=0x200
>>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> ---
>>  arch/loongarch/Kconfig             |  1 +
>>  arch/loongarch/kernel/ftrace_dyn.c | 65 ++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 66 insertions(+)
>>
>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
>> index 134a2f8..22a3e77 100644
>> --- a/arch/loongarch/Kconfig
>> +++ b/arch/loongarch/Kconfig
>> @@ -104,6 +104,7 @@ config LOONGARCH
>>         select HAVE_IRQ_EXIT_ON_IRQ_STACK
>>         select HAVE_IRQ_TIME_ACCOUNTING
>>         select HAVE_KPROBES
>> +       select HAVE_KPROBES_ON_FTRACE
>>         select HAVE_KRETPROBES
>>         select HAVE_MOD_ARCH_SPECIFIC
>>         select HAVE_NMI
>> diff --git a/arch/loongarch/kernel/ftrace_dyn.c b/arch/loongarch/kernel/ftrace_dyn.c
>> index 0f07591..7b074c2 100644
>> --- a/arch/loongarch/kernel/ftrace_dyn.c
>> +++ b/arch/loongarch/kernel/ftrace_dyn.c
>> @@ -6,6 +6,7 @@
>>   */
>>
>>  #include <linux/ftrace.h>
>> +#include <linux/kprobes.h>
>>  #include <linux/uaccess.h>
>>
>>  #include <asm/inst.h>
>> @@ -271,3 +272,67 @@ int ftrace_disable_ftrace_graph_caller(void)
>>  }
>>  #endif /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */
>>  #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
>> +
>> +#ifdef CONFIG_KPROBES_ON_FTRACE
>> +/* Ftrace callback handler for kprobes -- called under preepmt disabled */
>> +void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
>> +                          struct ftrace_ops *ops, struct ftrace_regs *fregs)
>> +{
>> +       struct kprobe *p;
>> +       struct pt_regs *regs;
>> +       struct kprobe_ctlblk *kcb;
>> +       int bit;
>> +
>> +       bit = ftrace_test_recursion_trylock(ip, parent_ip);
>> +       if (bit < 0)
>> +               return;
>> +
> In the old version I see preempt_disable_notrace() here and
> preempt_enable_notrace() at the end.

In the latest upstream kernel code, no need to do it, here is
the related commit:

ftrace: disable preemption when recursion locked
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce5e48036c9e

> And I prefer to add this one to
> the kprobes series rather than a separate patch.
>

Actually, this patch is based on the kprobe series, no conflicts,
sent it as a separate patch is to avoid resending the kprobe series.

Thanks,
Tiezhu