linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
Cc: Akanksha J N <akanksha@linux.ibm.com>,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org, rostedt@goodmis.org,
	shuah@kernel.org
Subject: Re: [PATCH] selftests/ftrace: Extend multiple_kprobes.tc to add multiple consecutive probes in a function
Date: Sat, 28 Jan 2023 10:16:22 +0900	[thread overview]
Message-ID: <20230128101622.ce6f8e64d929e29d36b08b73@kernel.org> (raw)
In-Reply-To: <1674629944.vwzovyd4lk.naveen@linux.ibm.com>

On Wed, 25 Jan 2023 12:39:36 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> Hi Masami,
> 
> Masami Hiramatsu wrote:
> >> > 
> >> > Yes, please make it separate, this test case is for checking whether
> >> > the ftrace can define/enable/disable multiple kprobe events. Not for
> >> > checking kprobe with different types, nor checking interactions among
> >> > different types of kprobes.
> >> > 
> >> > (BTW, if you want to test optprobe on x86, you can not put the probes
> >> >  within the jump instruction (+5 bytes). It will unoptimize existing
> >> >  optimized kprobe in that case)
> >> 
> >> Ok, I can see why we won't be able to optimize any of the probes on x86 
> >> with this approach. But, we should be able to do so on powerpc and arm, 
> >> the only other architectures supporting OPTPROBES at this time. For x86, 
> >> we may have to extend the test to check kprobes/list.
> > 
> > Are there any instruction type specific limitation on those arch for
> > using optprobe? I guess the 'call' (branch with link register) will not
> > able to be optimized because it leaves the trampoline address on the
> > stack.
> 
> Yes, at least on powerpc, we only optimize ALU instructions and do not 
> optimize load/store instructions, among many others. This is the reason 
> we try to put a probe uptil 256 offset into a function in the proposed 
> test, which will almost certainly catch an instruction that can be 
> optimized.
> 
> > 
> >> 
> >> Crucially, I think trying to place a probe at each byte can still 
> >> exercize interactions across KPROBES_ON_FTRACE and normal kprobes, so 
> >> this test is still a good start. In addition, we get to ensure that 
> >> kprobes infrastructure is rejecting placing probes at non-instruction 
> >> boundaries.
> > 
> > The interfere between probes can be happen between kprobes and optprobe
> > (*only on x86*), but not with KPORBES_ON_FTRACE. The ftrace replaced NOP
> > will be handled as one instruction. 
> 
> Yes.
> 
> > 
> >> > And do you really need to run "multiple" kprobes at once?
> >> > I think what you need is 'kprobe_opt_types.tc'.
> >> 
> >> Yes, enabling those probes is a good stress test to ensure we are only 
> >> accepting valid probe locations.
> >> 
> >> multiple_kprobe_types.tc ? :)
> > 
> > Please don't mixed it with the concept of 'multiple' probe test.
> > It is different that
> >  - kprobes can put probes on each instruction boundary.
> >  - kprobes can allocate and enable multiple probes at the same time.
> > 
> > What the multiple_kprobes.tc tests is the latter one.
> > (This is the reason why it chooses different functions so as not to
> >  interfere with each other.)
> 
> Ok, I was coming from the point of view that both tests end up 
> installing "multiple" kprobes, but I do see your point.
> 
> How about adding two new tests:
> 1. The same test as has been proposed in this thread: trying to add a 
> kprobe at every byte within $FUNCTION_FORK upto an offset of 256 bytes. 
> We can probably call it kprobe_insn_boundary.tc

OK.

> 2. A new test to ensure we can add different kprobe types 
> (kprobe_opt_types.tc). This test will need to enable and check if each 
> probe has been optimized or not and needs arch-specific knowledge so 
> that we can take care of x86.

OK, this should be only for x86. 

> 
> Would that be ok?

Yes, this sounds good to me. 

Thank you!

> 
> 
> Thanks,
> Naveen
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

      reply	other threads:[~2023-01-28  1:16 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20230112095600.37665-1-akanksha@linux.ibm.com>
2023-01-12 13:21 ` [PATCH] selftests/ftrace: Extend multiple_kprobes.tc to add multiple consecutive probes in a function Naveen N. Rao
2023-01-12 15:51   ` Masami Hiramatsu
2023-01-13  9:29     ` Naveen N. Rao
2023-01-13 15:21       ` Masami Hiramatsu
2023-01-16  8:32         ` Naveen N. Rao
2023-01-19 23:55           ` Masami Hiramatsu
2023-01-25  7:09             ` Naveen N. Rao
2023-01-28  1:16               ` Masami Hiramatsu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230128101622.ce6f8e64d929e29d36b08b73@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=akanksha@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=naveen.n.rao@linux.vnet.ibm.com \
    --cc=rostedt@goodmis.org \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).