2018-01-06 05:41:44

by Ravi Bangoria

[permalink] [raw]
Subject: [PATCH] trace_uprobe: Display correct offset in uprobe_events

Recently, how the pointers being printed with %p has been changed
by commit ad67b74d2469 ("printk: hash addresses printed with %p").
This is causing a regression while showing offset in the
uprobe_events file. Instead of %p, use %px to display offset.

Before patch:

# perf probe -vv -x /tmp/a.out main
Opening /sys/kernel/debug/tracing//uprobe_events write=1
Writing event: p:probe_a/main /tmp/a.out:0x58c

# cat /sys/kernel/debug/tracing/uprobe_events
p:probe_a/main /tmp/a.out:0x0000000049a0f352

After patch:

# cat /sys/kernel/debug/tracing/uprobe_events
p:probe_a/main /tmp/a.out:0x000000000000058c

Signed-off-by: Ravi Bangoria <[email protected]>
---
kernel/trace/trace_uprobe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 40592e7b3568..268029ae1be6 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -608,7 +608,7 @@ static int probes_seq_show(struct seq_file *m, void *v)

/* Don't print "0x (null)" when offset is 0 */
if (tu->offset) {
- seq_printf(m, "0x%p", (void *)tu->offset);
+ seq_printf(m, "0x%px", (void *)tu->offset);
} else {
switch (sizeof(void *)) {
case 4:
--
2.13.6



2018-01-08 05:19:34

by Srikar Dronamraju

[permalink] [raw]
Subject: Re: [PATCH] trace_uprobe: Display correct offset in uprobe_events


* Ravi Bangoria <[email protected]> [2018-01-06 11:12:46]:

> Recently, how the pointers being printed with %p has been changed
> by commit ad67b74d2469 ("printk: hash addresses printed with %p").
> This is causing a regression while showing offset in the
> uprobe_events file. Instead of %p, use %px to display offset.
>
> Signed-off-by: Ravi Bangoria <[email protected]>
> ---
> kernel/trace/trace_uprobe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 40592e7b3568..268029ae1be6 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -608,7 +608,7 @@ static int probes_seq_show(struct seq_file *m, void *v)
>
> /* Don't print "0x (null)" when offset is 0 */
> if (tu->offset) {
> - seq_printf(m, "0x%p", (void *)tu->offset);
> + seq_printf(m, "0x%px", (void *)tu->offset);
> } else {
> switch (sizeof(void *)) {
> case 4:

Looks good to me. Did you consider %pK instead of %px?

Acked-by: Srikar Dronamraju <[email protected]>

2018-01-08 06:29:55

by Ravi Bangoria

[permalink] [raw]
Subject: Re: [PATCH] trace_uprobe: Display correct offset in uprobe_events



On 01/08/2018 10:49 AM, Srikar Dronamraju wrote:
> * Ravi Bangoria <[email protected]> [2018-01-06 11:12:46]:
>
>> Recently, how the pointers being printed with %p has been changed
>> by commit ad67b74d2469 ("printk: hash addresses printed with %p").
>> This is causing a regression while showing offset in the
>> uprobe_events file. Instead of %p, use %px to display offset.
>>
>> Signed-off-by: Ravi Bangoria <[email protected]>
>> ---
>> kernel/trace/trace_uprobe.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
>> index 40592e7b3568..268029ae1be6 100644
>> --- a/kernel/trace/trace_uprobe.c
>> +++ b/kernel/trace/trace_uprobe.c
>> @@ -608,7 +608,7 @@ static int probes_seq_show(struct seq_file *m, void *v)
>>
>> /* Don't print "0x (null)" when offset is 0 */
>> if (tu->offset) {
>> - seq_printf(m, "0x%p", (void *)tu->offset);
>> + seq_printf(m, "0x%px", (void *)tu->offset);
>> } else {
>> switch (sizeof(void *)) {
>> case 4:
> Looks good to me. Did you consider %pK instead of %px?

Thanks Srikar,

Checked %pK. But I see same issue with that:

perf probe:
  Opening /sys/kernel/debug/tracing//uprobe_events write=1
  Writing event: p:probe_a/main /tmp/a.out:0x58c

cat /sys/kernel/debug/tracing/uprobe_events:
  p:probe_a/main /tmp/a.out:0x0000000014fd571e

> Acked-by: Srikar Dronamraju <[email protected]>

2018-01-08 06:36:07

by Tobin C. Harding

[permalink] [raw]
Subject: Re: [PATCH] trace_uprobe: Display correct offset in uprobe_events

On Mon, Jan 08, 2018 at 12:01:04PM +0530, Ravi Bangoria wrote:
>
>
> On 01/08/2018 10:49 AM, Srikar Dronamraju wrote:
> > * Ravi Bangoria <[email protected]> [2018-01-06 11:12:46]:
> >
> >> Recently, how the pointers being printed with %p has been changed
> >> by commit ad67b74d2469 ("printk: hash addresses printed with %p").
> >> This is causing a regression while showing offset in the
> >> uprobe_events file. Instead of %p, use %px to display offset.
> >>
> >> Signed-off-by: Ravi Bangoria <[email protected]>
> >> ---
> >> kernel/trace/trace_uprobe.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> >> index 40592e7b3568..268029ae1be6 100644
> >> --- a/kernel/trace/trace_uprobe.c
> >> +++ b/kernel/trace/trace_uprobe.c
> >> @@ -608,7 +608,7 @@ static int probes_seq_show(struct seq_file *m, void *v)
> >>
> >> /* Don't print "0x (null)" when offset is 0 */
> >> if (tu->offset) {
> >> - seq_printf(m, "0x%p", (void *)tu->offset);
> >> + seq_printf(m, "0x%px", (void *)tu->offset);
> >> } else {
> >> switch (sizeof(void *)) {
> >> case 4:
> > Looks good to me. Did you consider %pK instead of %px?
>
> Thanks Srikar,
>
> Checked %pK. But I see same issue with that:
>
> perf probe:
> ? Opening /sys/kernel/debug/tracing//uprobe_events write=1
> ? Writing event: p:probe_a/main /tmp/a.out:0x58c
>
> cat /sys/kernel/debug/tracing/uprobe_events:
> ? p:probe_a/main /tmp/a.out:0x0000000014fd571e

%pK behaves the same as %p (hashes address) when kpt_restrict==0

Hope this helps,
Tobin.

2018-01-08 13:58:11

by Srikar Dronamraju

[permalink] [raw]
Subject: Re: [PATCH] trace_uprobe: Display correct offset in uprobe_events

* Tobin C. Harding <[email protected]> [2018-01-08 17:35:59]:

> On Mon, Jan 08, 2018 at 12:01:04PM +0530, Ravi Bangoria wrote:
> >
> >
> > On 01/08/2018 10:49 AM, Srikar Dronamraju wrote:
> > > * Ravi Bangoria <[email protected]> [2018-01-06 11:12:46]:
> > >
> > >> Recently, how the pointers being printed with %p has been changed
> > >> by commit ad67b74d2469 ("printk: hash addresses printed with %p").
> > >> This is causing a regression while showing offset in the
> > >> uprobe_events file. Instead of %p, use %px to display offset.
> > >>
> > >> Signed-off-by: Ravi Bangoria <[email protected]>
> > >> ---
> > >> kernel/trace/trace_uprobe.c | 2 +-
> > >> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> > >> index 40592e7b3568..268029ae1be6 100644
> > >> --- a/kernel/trace/trace_uprobe.c
> > >> +++ b/kernel/trace/trace_uprobe.c
> > >> @@ -608,7 +608,7 @@ static int probes_seq_show(struct seq_file *m, void *v)
> > >>
> > >> /* Don't print "0x (null)" when offset is 0 */
> > >> if (tu->offset) {
> > >> - seq_printf(m, "0x%p", (void *)tu->offset);
> > >> + seq_printf(m, "0x%px", (void *)tu->offset);
> > >> } else {
> > >> switch (sizeof(void *)) {
> > >> case 4:
> > > Looks good to me. Did you consider %pK instead of %px?
> >
> > Thanks Srikar,
> >
> > Checked %pK. But I see same issue with that:
> >
> > perf probe:
> > ? Opening /sys/kernel/debug/tracing//uprobe_events write=1
> > ? Writing event: p:probe_a/main /tmp/a.out:0x58c
> >
> > cat /sys/kernel/debug/tracing/uprobe_events:
> > ? p:probe_a/main /tmp/a.out:0x0000000014fd571e
>
> %pK behaves the same as %p (hashes address) when kpt_restrict==0
>
> Hope this helps,

Thanks Tobin for the clarification. So %px is the right choice.

--
thanks and regards
Srikar

2018-01-19 16:05:46

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] trace_uprobe: Display correct offset in uprobe_events

On Sat, 6 Jan 2018 11:12:46 +0530
Ravi Bangoria <[email protected]> wrote:

> Recently, how the pointers being printed with %p has been changed
> by commit ad67b74d2469 ("printk: hash addresses printed with %p").
> This is causing a regression while showing offset in the
> uprobe_events file. Instead of %p, use %px to display offset.
>
> Before patch:
>
> # perf probe -vv -x /tmp/a.out main
> Opening /sys/kernel/debug/tracing//uprobe_events write=1
> Writing event: p:probe_a/main /tmp/a.out:0x58c
>
> # cat /sys/kernel/debug/tracing/uprobe_events
> p:probe_a/main /tmp/a.out:0x0000000049a0f352
>
> After patch:
>
> # cat /sys/kernel/debug/tracing/uprobe_events
> p:probe_a/main /tmp/a.out:0x000000000000058c
>

Thanks, I'll pull this into my tree.

-- Steve