2023-08-12 05:45:28

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH v3 1/8] Documentation: probes: Add a new ret_ip callback parameter

From: Masami Hiramatsu (Google) <[email protected]>

Add a new ret_ip callback parameter description.

Fixes: cb16330d1274 ("fprobe: Pass return address to the handlers")
Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
---
Documentation/trace/fprobe.rst | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/trace/fprobe.rst b/Documentation/trace/fprobe.rst
index 40dd2fbce861..a6d682478147 100644
--- a/Documentation/trace/fprobe.rst
+++ b/Documentation/trace/fprobe.rst
@@ -91,9 +91,9 @@ The prototype of the entry/exit callback function are as follows:

.. code-block:: c

- int entry_callback(struct fprobe *fp, unsigned long entry_ip, struct pt_regs *regs, void *entry_data);
+ int entry_callback(struct fprobe *fp, unsigned long entry_ip, unsigned long ret_ip, struct pt_regs *regs, void *entry_data);

- void exit_callback(struct fprobe *fp, unsigned long entry_ip, struct pt_regs *regs, void *entry_data);
+ void exit_callback(struct fprobe *fp, unsigned long entry_ip, unsigned long ret_ip, struct pt_regs *regs, void *entry_data);

Note that the @entry_ip is saved at function entry and passed to exit handler.
If the entry callback function returns !0, the corresponding exit callback will be cancelled.
@@ -108,6 +108,10 @@ If the entry callback function returns !0, the corresponding exit callback will
Note that this may not be the actual entry address of the function but
the address where the ftrace is instrumented.

+@ret_ip
+ This is the return address of the traced function. This can be used
+ at both entry and exit.
+
@regs
This is the `pt_regs` data structure at the entry and exit. Note that
the instruction pointer of @regs may be different from the @entry_ip



2023-08-19 16:03:58

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH v3 1/8] Documentation: probes: Add a new ret_ip callback parameter

On Thu, 17 Aug 2023 10:57:18 +0200
Florent Revest <[email protected]> wrote:

> On Sat, Aug 12, 2023 at 7:36 AM Masami Hiramatsu (Google)
> <[email protected]> wrote:
> >
> > From: Masami Hiramatsu (Google) <[email protected]>
> >
> > Add a new ret_ip callback parameter description.
> >
> > Fixes: cb16330d1274 ("fprobe: Pass return address to the handlers")
> > Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
> > ---
> > Documentation/trace/fprobe.rst | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/trace/fprobe.rst b/Documentation/trace/fprobe.rst
> > index 40dd2fbce861..a6d682478147 100644
> > --- a/Documentation/trace/fprobe.rst
> > +++ b/Documentation/trace/fprobe.rst
> > @@ -91,9 +91,9 @@ The prototype of the entry/exit callback function are as follows:
> >
> > .. code-block:: c
> >
> > - int entry_callback(struct fprobe *fp, unsigned long entry_ip, struct pt_regs *regs, void *entry_data);
> > + int entry_callback(struct fprobe *fp, unsigned long entry_ip, unsigned long ret_ip, struct pt_regs *regs, void *entry_data);
> >
> > - void exit_callback(struct fprobe *fp, unsigned long entry_ip, struct pt_regs *regs, void *entry_data);
> > + void exit_callback(struct fprobe *fp, unsigned long entry_ip, unsigned long ret_ip, struct pt_regs *regs, void *entry_data);
> >
> > Note that the @entry_ip is saved at function entry and passed to exit handler.
> > If the entry callback function returns !0, the corresponding exit callback will be cancelled.
> > @@ -108,6 +108,10 @@ If the entry callback function returns !0, the corresponding exit callback will
> > Note that this may not be the actual entry address of the function but
> > the address where the ftrace is instrumented.
> >
> > +@ret_ip
> > + This is the return address of the traced function. This can be used
> > + at both entry and exit.
>
> Maybe that's just the lack of coffee but I had to think twice to
> understand what this paragraph meant :) On my first pass I thought
> this meant "the address of the return instruction", which made little
> sense since there can of course be multiple "ret"s in a function. I
> like the name in the fprobe code "parent_ip" because I find it conveys
> better that this is an address in the caller of the traced function.
> I'm also fine with this "ret_ip" but I propose we modify the paragraph
> a little bit to something like:
>
> This is the address that the traced function will return to, somewhere
> in its caller. This can be used at both entry and exit.

Thanks, that makes it more clear. I'll update it.


--
Masami Hiramatsu (Google) <[email protected]>

2023-08-19 23:37:20

by Florent Revest

[permalink] [raw]
Subject: Re: [PATCH v3 1/8] Documentation: probes: Add a new ret_ip callback parameter

On Sat, Aug 12, 2023 at 7:36 AM Masami Hiramatsu (Google)
<[email protected]> wrote:
>
> From: Masami Hiramatsu (Google) <[email protected]>
>
> Add a new ret_ip callback parameter description.
>
> Fixes: cb16330d1274 ("fprobe: Pass return address to the handlers")
> Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
> ---
> Documentation/trace/fprobe.rst | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/trace/fprobe.rst b/Documentation/trace/fprobe.rst
> index 40dd2fbce861..a6d682478147 100644
> --- a/Documentation/trace/fprobe.rst
> +++ b/Documentation/trace/fprobe.rst
> @@ -91,9 +91,9 @@ The prototype of the entry/exit callback function are as follows:
>
> .. code-block:: c
>
> - int entry_callback(struct fprobe *fp, unsigned long entry_ip, struct pt_regs *regs, void *entry_data);
> + int entry_callback(struct fprobe *fp, unsigned long entry_ip, unsigned long ret_ip, struct pt_regs *regs, void *entry_data);
>
> - void exit_callback(struct fprobe *fp, unsigned long entry_ip, struct pt_regs *regs, void *entry_data);
> + void exit_callback(struct fprobe *fp, unsigned long entry_ip, unsigned long ret_ip, struct pt_regs *regs, void *entry_data);
>
> Note that the @entry_ip is saved at function entry and passed to exit handler.
> If the entry callback function returns !0, the corresponding exit callback will be cancelled.
> @@ -108,6 +108,10 @@ If the entry callback function returns !0, the corresponding exit callback will
> Note that this may not be the actual entry address of the function but
> the address where the ftrace is instrumented.
>
> +@ret_ip
> + This is the return address of the traced function. This can be used
> + at both entry and exit.

Maybe that's just the lack of coffee but I had to think twice to
understand what this paragraph meant :) On my first pass I thought
this meant "the address of the return instruction", which made little
sense since there can of course be multiple "ret"s in a function. I
like the name in the fprobe code "parent_ip" because I find it conveys
better that this is an address in the caller of the traced function.
I'm also fine with this "ret_ip" but I propose we modify the paragraph
a little bit to something like:

This is the address that the traced function will return to, somewhere
in its caller. This can be used at both entry and exit.