All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Will Deacon <will@kernel.org>
Cc: Jianlin Lv <iecedge@gmail.com>,
	corbet@lwn.net, catalin.marinas@arm.com, rostedt@goodmis.org,
	mingo@redhat.com, naveen.n.rao@linux.ibm.com,
	anil.s.keshavamurthy@intel.com, davem@davemloft.net,
	mhiramat@kernel.org, arnd@arndb.de, zhengzengkai@huawei.com,
	jianlv@ebay.com, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64/kprobes: Add support for KPROBES_ON_FTRACE
Date: Mon, 7 Nov 2022 16:06:26 +0000	[thread overview]
Message-ID: <Y2ktAisfFAr0aU2V@FVFF77S0Q05N> (raw)
In-Reply-To: <20221107153506.GA21157@willie-the-truck>

On Mon, Nov 07, 2022 at 03:35:07PM +0000, Will Deacon wrote:
> On Mon, Nov 07, 2022 at 03:32:24PM +0000, Mark Rutland wrote:
> > On Mon, Nov 07, 2022 at 02:49:31PM +0000, Will Deacon wrote:
> > > [+Mark R]
> > > 
> > > On Thu, Jul 28, 2022 at 02:02:50AM +0000, Jianlin Lv wrote:
> > > > This is the arm64 version of ftrace-based kprobes to avoid the overhead
> > > > with regular kprobes, by using the ftrace infrastructure.
> > > > 
> > > > Signed-off-by: Jianlin Lv <iecedge@gmail.com>
> > > > ---
> > > >  .../debug/kprobes-on-ftrace/arch-support.txt  |  2 +-
> > > >  arch/arm64/Kconfig                            |  1 +
> > > >  arch/arm64/kernel/probes/Makefile             |  1 +
> > > >  arch/arm64/kernel/probes/kprobes-ftrace.c     | 81 +++++++++++++++++++
> > > >  include/linux/kprobes.h                       |  2 +
> > > >  kernel/kprobes.c                              |  4 +-
> > > >  6 files changed, 88 insertions(+), 3 deletions(-)
> > > >  create mode 100644 arch/arm64/kernel/probes/kprobes-ftrace.c
> > > 
> > > Sorry for the slow reply on this, but I think this deserved to be split
> > > into two patches: the first one reworking the core check_ftrace_location()
> > > logic to work properly with branch-and-link style architectures, and the
> > > second one adding support for arm64.
> > 
> > I'd prefer we don't do this at all; there a bunch of issues with kprobes *not*
> > taking an exception, since we get a dodgy not-quite-real pt_regs, and to clean
> > up the existing issues the plan is:
> > 
> > 1) Move ftrace over to ftrace_regs
> > 2) Implement fprobes using ftrace_regs
> > 3) Remove kretprobes
> > 
> > ... and regular kprobes will need to take an exception (via BRK) to get a real
> > pt_regs, so that can't be optimized to use ftrace.
> 
> OKey doke. Does that mean that other architectures will follow the same
> approach of taking an exception,

I think once everyone has FPROBE, KPROBES_ON_FTRACE becomes redundant, and
could be removed (leaving kprobes to always follow a take-an-exception flow on
all architectures).

> or do they somehow work by magic?

Some architectures don't need to take an exception to be able to create a full
pt_regs (e.g. x86's flags are accessible in a way arm64's PSTATE isn't), but
that needs to be generated / restored differently to exception entry/return,
and so even where it's possible it can be painful to maintain (and slower than
using ftrace_regs), so I suspect KPROBES_ON_FTRACE would be removed.

So different constaints more than magic.

Thanks,
Mark.

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Will Deacon <will@kernel.org>
Cc: Jianlin Lv <iecedge@gmail.com>,
	corbet@lwn.net, catalin.marinas@arm.com, rostedt@goodmis.org,
	mingo@redhat.com, naveen.n.rao@linux.ibm.com,
	anil.s.keshavamurthy@intel.com, davem@davemloft.net,
	mhiramat@kernel.org, arnd@arndb.de, zhengzengkai@huawei.com,
	jianlv@ebay.com, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64/kprobes: Add support for KPROBES_ON_FTRACE
Date: Mon, 7 Nov 2022 16:06:26 +0000	[thread overview]
Message-ID: <Y2ktAisfFAr0aU2V@FVFF77S0Q05N> (raw)
In-Reply-To: <20221107153506.GA21157@willie-the-truck>

On Mon, Nov 07, 2022 at 03:35:07PM +0000, Will Deacon wrote:
> On Mon, Nov 07, 2022 at 03:32:24PM +0000, Mark Rutland wrote:
> > On Mon, Nov 07, 2022 at 02:49:31PM +0000, Will Deacon wrote:
> > > [+Mark R]
> > > 
> > > On Thu, Jul 28, 2022 at 02:02:50AM +0000, Jianlin Lv wrote:
> > > > This is the arm64 version of ftrace-based kprobes to avoid the overhead
> > > > with regular kprobes, by using the ftrace infrastructure.
> > > > 
> > > > Signed-off-by: Jianlin Lv <iecedge@gmail.com>
> > > > ---
> > > >  .../debug/kprobes-on-ftrace/arch-support.txt  |  2 +-
> > > >  arch/arm64/Kconfig                            |  1 +
> > > >  arch/arm64/kernel/probes/Makefile             |  1 +
> > > >  arch/arm64/kernel/probes/kprobes-ftrace.c     | 81 +++++++++++++++++++
> > > >  include/linux/kprobes.h                       |  2 +
> > > >  kernel/kprobes.c                              |  4 +-
> > > >  6 files changed, 88 insertions(+), 3 deletions(-)
> > > >  create mode 100644 arch/arm64/kernel/probes/kprobes-ftrace.c
> > > 
> > > Sorry for the slow reply on this, but I think this deserved to be split
> > > into two patches: the first one reworking the core check_ftrace_location()
> > > logic to work properly with branch-and-link style architectures, and the
> > > second one adding support for arm64.
> > 
> > I'd prefer we don't do this at all; there a bunch of issues with kprobes *not*
> > taking an exception, since we get a dodgy not-quite-real pt_regs, and to clean
> > up the existing issues the plan is:
> > 
> > 1) Move ftrace over to ftrace_regs
> > 2) Implement fprobes using ftrace_regs
> > 3) Remove kretprobes
> > 
> > ... and regular kprobes will need to take an exception (via BRK) to get a real
> > pt_regs, so that can't be optimized to use ftrace.
> 
> OKey doke. Does that mean that other architectures will follow the same
> approach of taking an exception,

I think once everyone has FPROBE, KPROBES_ON_FTRACE becomes redundant, and
could be removed (leaving kprobes to always follow a take-an-exception flow on
all architectures).

> or do they somehow work by magic?

Some architectures don't need to take an exception to be able to create a full
pt_regs (e.g. x86's flags are accessible in a way arm64's PSTATE isn't), but
that needs to be generated / restored differently to exception entry/return,
and so even where it's possible it can be painful to maintain (and slower than
using ftrace_regs), so I suspect KPROBES_ON_FTRACE would be removed.

So different constaints more than magic.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-11-07 16:07 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-28  2:02 [PATCH] arm64/kprobes: Add support for KPROBES_ON_FTRACE Jianlin Lv
2022-07-28  2:02 ` Jianlin Lv
2022-11-07 14:49 ` Will Deacon
2022-11-07 14:49   ` Will Deacon
2022-11-07 15:32   ` Mark Rutland
2022-11-07 15:32     ` Mark Rutland
2022-11-07 15:35     ` Will Deacon
2022-11-07 15:35       ` Will Deacon
2022-11-07 16:06       ` Mark Rutland [this message]
2022-11-07 16:06         ` Mark Rutland
2022-11-08  5:10         ` Masami Hiramatsu
2022-11-08  5:10           ` Masami Hiramatsu
2022-11-08  5:53           ` Jianlin Lv
2022-11-08  5:53             ` Jianlin Lv

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=Y2ktAisfFAr0aU2V@FVFF77S0Q05N \
    --to=mark.rutland@arm.com \
    --cc=anil.s.keshavamurthy@intel.com \
    --cc=arnd@arndb.de \
    --cc=catalin.marinas@arm.com \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=iecedge@gmail.com \
    --cc=jianlv@ebay.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=naveen.n.rao@linux.ibm.com \
    --cc=rostedt@goodmis.org \
    --cc=will@kernel.org \
    --cc=zhengzengkai@huawei.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.