[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20220731234140.ad23f7d8c7c389112ac65487@kernel.org>
Date: Sun, 31 Jul 2022 23:41:40 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: zhangduo <zduo006@....com>
Cc: anil.s.keshavamurthy@...el.com, davem@...emloft.net,
linux-kernel@...r.kernel.org, mhiramat@...nel.org,
naveen.n.rao@...ux.ibm.com, zhangduo@...inos.cn
Subject: Re: [PATCH 2/2] kprobes: modify the copy_kprobe to make more
clearly
On Thu, 28 Jul 2022 20:45:16 +0800
zhangduo <zduo006@....com> wrote:
> From: zhangduo <zhangduo@...inos.cn>
>
> copy_kprobe() are called in init_aggr_kprobe() to copy opcode
> and insn from 'p' to 'ap',but in copy_kprobe(p,ap) implemtation
> 'p' pass to 'struct kprobe *ap' , 'ap' pass to 'struct kprobe *p',
> it looks strange, because p means orig kprobe instance, ap
> means aggregator kprobe, not the same. and before the add_new_kprobe()
> calling used the opposite copy from 'ap' to 'p', so modify copy_kprobe's
> parameter name as 'p_src' and 'p_dst' to fit all.
Agreed, that was confusing.
This doesn not change the behavior, but better to be read
(the arguments are sorted as same as memcpy(dst, src))
Acked-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
Thanks!
>
> Signed-off-by: zhangduo <zhangduo@...inos.cn>
> ---
> kernel/kprobes.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 4f3e84343..8bcef7d3c 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -405,10 +405,10 @@ static inline bool kprobe_unused(struct kprobe *p)
> }
>
> /* Keep all fields in the kprobe consistent. */
> -static inline void copy_kprobe(struct kprobe *ap, struct kprobe *p)
> +static inline void copy_kprobe(struct kprobe *p_dst, struct kprobe *p_src)
> {
> - memcpy(&p->opcode, &ap->opcode, sizeof(kprobe_opcode_t));
> - memcpy(&p->ainsn, &ap->ainsn, sizeof(struct arch_specific_insn));
> + memcpy(&p_dst->opcode, &p_src->opcode, sizeof(kprobe_opcode_t));
> + memcpy(&p_dst->ainsn, &p_src->ainsn, sizeof(struct arch_specific_insn));
> }
>
> #ifdef CONFIG_OPTPROBES
> @@ -1277,7 +1277,7 @@ static int add_new_kprobe(struct kprobe *ap, struct kprobe *p)
> static void init_aggr_kprobe(struct kprobe *ap, struct kprobe *p)
> {
> /* Copy the insn slot of 'p' to 'ap'. */
> - copy_kprobe(p, ap);
> + copy_kprobe(ap, p);
> flush_insn_slot(ap);
> ap->addr = p->addr;
> ap->flags = p->flags & ~KPROBE_FLAG_OPTIMIZED;
> @@ -1350,7 +1350,7 @@ static int register_aggr_kprobe(struct kprobe *orig_p, struct kprobe *p)
> }
>
> /* Copy the insn slot of 'ap' to 'p'. */
> - copy_kprobe(ap, p);
> + copy_kprobe(p, ap);
> ret = add_new_kprobe(ap, p);
>
> out:
> --
> 2.25.1
>
--
Masami Hiramatsu (Google) <mhiramat@...nel.org>
Powered by blists - more mailing lists