2018-09-14 12:36:10

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

On a powerpc 8xx, 'btc' fails as follows:

Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
kdb> btc
btc: cpu status: Currently on cpu 0
Available cpus: 0
kdb_getarea: Bad address 0x0

when booting the kernel with 'debug_boot_weak_hash', it fails as well

Entering kdb (current=0xba99ad80, pid 284) due to Keyboard Entry
kdb> btc
btc: cpu status: Currently on cpu 0
Available cpus: 0
kdb_getarea: Bad address 0xba99ad80

On other platforms, Oopses have been observed too, see
https://github.com/linuxppc/linux/issues/139

This is due to btc calling 'btt' with %p pointer as an argument.

This patch replaces %p by %px to get the real pointer value as
expected by 'btt'

Signed-off-by: Christophe Leroy <[email protected]>
Cc: <[email protected]> # 4.15+
---
kernel/debug/kdb/kdb_bt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/debug/kdb/kdb_bt.c b/kernel/debug/kdb/kdb_bt.c
index 6ad4a9fcbd6f..7921ae4fca8d 100644
--- a/kernel/debug/kdb/kdb_bt.c
+++ b/kernel/debug/kdb/kdb_bt.c
@@ -179,14 +179,14 @@ kdb_bt(int argc, const char **argv)
kdb_printf("no process for cpu %ld\n", cpu);
return 0;
}
- sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
+ sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
kdb_parse(buf);
return 0;
}
kdb_printf("btc: cpu status: ");
kdb_parse("cpu\n");
for_each_online_cpu(cpu) {
- sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
+ sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
kdb_parse(buf);
touch_nmi_watchdog();
}
--
2.13.3



2018-09-16 19:06:49

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
> On a powerpc 8xx, 'btc' fails as follows:
>
> Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
> kdb> btc
> btc: cpu status: Currently on cpu 0
> Available cpus: 0
> kdb_getarea: Bad address 0x0
>
> when booting the kernel with 'debug_boot_weak_hash', it fails as well
>
> Entering kdb (current=0xba99ad80, pid 284) due to Keyboard Entry
> kdb> btc
> btc: cpu status: Currently on cpu 0
> Available cpus: 0
> kdb_getarea: Bad address 0xba99ad80
>
> On other platforms, Oopses have been observed too, see
> https://github.com/linuxppc/linux/issues/139
>
> This is due to btc calling 'btt' with %p pointer as an argument.
>
> This patch replaces %p by %px to get the real pointer value as
> expected by 'btt'
>
> Signed-off-by: Christophe Leroy <[email protected]>
> Cc: <[email protected]> # 4.15+

Would a Fixes: be better here?
Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")

No blame attached to Tobin, but the fixes makes it super clear what
changed and why this breaks kdb (which was not explicitly called out
the patch description).


Daniel.

> ---
> kernel/debug/kdb/kdb_bt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/debug/kdb/kdb_bt.c b/kernel/debug/kdb/kdb_bt.c
> index 6ad4a9fcbd6f..7921ae4fca8d 100644
> --- a/kernel/debug/kdb/kdb_bt.c
> +++ b/kernel/debug/kdb/kdb_bt.c
> @@ -179,14 +179,14 @@ kdb_bt(int argc, const char **argv)
> kdb_printf("no process for cpu %ld\n", cpu);
> return 0;
> }
> - sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
> + sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
> kdb_parse(buf);
> return 0;
> }
> kdb_printf("btc: cpu status: ");
> kdb_parse("cpu\n");
> for_each_online_cpu(cpu) {
> - sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
> + sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
> kdb_parse(buf);
> touch_nmi_watchdog();
> }
> --
> 2.13.3
>

2018-09-16 22:19:06

by Tobin C. Harding

[permalink] [raw]
Subject: Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

On Sun, Sep 16, 2018 at 12:06:10PM -0700, Daniel Thompson wrote:
> On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
> > On a powerpc 8xx, 'btc' fails as follows:
> >
> > Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
> > kdb> btc
> > btc: cpu status: Currently on cpu 0
> > Available cpus: 0
> > kdb_getarea: Bad address 0x0
> >
> > when booting the kernel with 'debug_boot_weak_hash', it fails as well
> >
> > Entering kdb (current=0xba99ad80, pid 284) due to Keyboard Entry
> > kdb> btc
> > btc: cpu status: Currently on cpu 0
> > Available cpus: 0
> > kdb_getarea: Bad address 0xba99ad80
> >
> > On other platforms, Oopses have been observed too, see
> > https://github.com/linuxppc/linux/issues/139
> >
> > This is due to btc calling 'btt' with %p pointer as an argument.
> >
> > This patch replaces %p by %px to get the real pointer value as
> > expected by 'btt'
> >
> > Signed-off-by: Christophe Leroy <[email protected]>
> > Cc: <[email protected]> # 4.15+
>
> Would a Fixes: be better here?
> Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")
>
> No blame attached to Tobin, but the fixes makes it super clear what

:)

> changed and why this breaks kdb (which was not explicitly called out
> the patch description).
>
>
> Daniel.

2018-09-26 11:11:57

by Daniel Thompson

[permalink] [raw]
Subject: Re: Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

On 16/09/2018 20:06, Daniel Thompson wrote:
> On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
>> On a powerpc 8xx, 'btc' fails as follows:
>>
>> Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
>> kdb> btc
>> btc: cpu status: Currently on cpu 0
>> Available cpus: 0
>> kdb_getarea: Bad address 0x0
>>
>> when booting the kernel with 'debug_boot_weak_hash', it fails as well
>>
>> Entering kdb (current=0xba99ad80, pid 284) due to Keyboard Entry
>> kdb> btc
>> btc: cpu status: Currently on cpu 0
>> Available cpus: 0
>> kdb_getarea: Bad address 0xba99ad80
>>
>> On other platforms, Oopses have been observed too, see
>> https://github.com/linuxppc/linux/issues/139
>>
>> This is due to btc calling 'btt' with %p pointer as an argument.
>>
>> This patch replaces %p by %px to get the real pointer value as
>> expected by 'btt'
>>
>> Signed-off-by: Christophe Leroy <[email protected]>
>> Cc: <[email protected]> # 4.15+
>
> Would a Fixes: be better here?
> Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")

Christophe, When you add the Fixes: could you also add my

Reviewed-by: Daniel Thompson <[email protected]>


Thanks.


>
> No blame attached to Tobin, but the fixes makes it super clear what
> changed and why this breaks kdb (which was not explicitly called out
> the patch description).
>
>
> Daniel.
>
>> ---
>> kernel/debug/kdb/kdb_bt.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/debug/kdb/kdb_bt.c b/kernel/debug/kdb/kdb_bt.c
>> index 6ad4a9fcbd6f..7921ae4fca8d 100644
>> --- a/kernel/debug/kdb/kdb_bt.c
>> +++ b/kernel/debug/kdb/kdb_bt.c
>> @@ -179,14 +179,14 @@ kdb_bt(int argc, const char **argv)
>> kdb_printf("no process for cpu %ld\n", cpu);
>> return 0;
>> }
>> - sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
>> + sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
>> kdb_parse(buf);
>> return 0;
>> }
>> kdb_printf("btc: cpu status: ");
>> kdb_parse("cpu\n");
>> for_each_online_cpu(cpu) {
>> - sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
>> + sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
>> kdb_parse(buf);
>> touch_nmi_watchdog();
>> }
>> --
>> 2.13.3
>>


2018-09-26 11:20:13

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'



Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
> On 16/09/2018 20:06, Daniel Thompson wrote:
>> On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
>>> On a powerpc 8xx, 'btc' fails as follows:
>>>
>>> Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
>>> kdb> btc
>>> btc: cpu status: Currently on cpu 0
>>> Available cpus: 0
>>> kdb_getarea: Bad address 0x0
>>>
>>> when booting the kernel with 'debug_boot_weak_hash', it fails as well
>>>
>>> Entering kdb (current=0xba99ad80, pid 284) due to Keyboard Entry
>>> kdb> btc
>>> btc: cpu status: Currently on cpu 0
>>> Available cpus: 0
>>> kdb_getarea: Bad address 0xba99ad80
>>>
>>> On other platforms, Oopses have been observed too, see
>>> https://github.com/linuxppc/linux/issues/139
>>>
>>> This is due to btc calling 'btt' with %p pointer as an argument.
>>>
>>> This patch replaces %p by %px to get the real pointer value as
>>> expected by 'btt'
>>>
>>> Signed-off-by: Christophe Leroy <[email protected]>
>>> Cc: <[email protected]> # 4.15+
>>
>> Would a Fixes: be better here?
>> Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")
>
> Christophe, When you add the Fixes: could you also add my
>
> Reviewed-by: Daniel Thompson <[email protected]>


Ok, thanks for the review, but do I have to do anything really ?

The Fixes: and now your Reviewed-by: appear automatically in patchwork
(https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
so I believe they'll be automatically included when Jason or someone
else takes the patch, no ?

Christophe

>
>
> Thanks.
>
>
>>
>> No blame attached to Tobin, but the fixes makes it super clear what
>> changed and why this breaks kdb (which was not explicitly called out
>> the patch description).
>>
>>
>> Daniel.
>>
>>> ---
>>>   kernel/debug/kdb/kdb_bt.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/debug/kdb/kdb_bt.c b/kernel/debug/kdb/kdb_bt.c
>>> index 6ad4a9fcbd6f..7921ae4fca8d 100644
>>> --- a/kernel/debug/kdb/kdb_bt.c
>>> +++ b/kernel/debug/kdb/kdb_bt.c
>>> @@ -179,14 +179,14 @@ kdb_bt(int argc, const char **argv)
>>>                   kdb_printf("no process for cpu %ld\n", cpu);
>>>                   return 0;
>>>               }
>>> -            sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
>>> +            sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
>>>               kdb_parse(buf);
>>>               return 0;
>>>           }
>>>           kdb_printf("btc: cpu status: ");
>>>           kdb_parse("cpu\n");
>>>           for_each_online_cpu(cpu) {
>>> -            sprintf(buf, "btt 0x%p\n", KDB_TSK(cpu));
>>> +            sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu));
>>>               kdb_parse(buf);
>>>               touch_nmi_watchdog();
>>>           }
>>> --
>>> 2.13.3
>>>

2018-09-27 11:09:55

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

Christophe LEROY <[email protected]> writes:
> Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
>> On 16/09/2018 20:06, Daniel Thompson wrote:
>>> On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
>>>> On a powerpc 8xx, 'btc' fails as follows:
>>>> Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
...
>>>>
>>>> Signed-off-by: Christophe Leroy <[email protected]>
>>>> Cc: <[email protected]> # 4.15+
>>>
>>> Would a Fixes: be better here?
>>> Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")
>>
>> Christophe, When you add the Fixes: could you also add my
>>
>> Reviewed-by: Daniel Thompson <[email protected]>
>
> Ok, thanks for the review, but do I have to do anything really ?
>
> The Fixes: and now your Reviewed-by: appear automatically in patchwork
> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
> so I believe they'll be automatically included when Jason or someone
> else takes the patch, no ?

patchwork won't add the Fixes tag from the reply, it needs to be in the
original mail.

See:
https://github.com/getpatchwork/patchwork/issues/151


cheers

2018-09-27 11:31:43

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'



Le 27/09/2018 à 13:09, Michael Ellerman a écrit :
> Christophe LEROY <[email protected]> writes:
>> Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
>>> On 16/09/2018 20:06, Daniel Thompson wrote:
>>>> On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
>>>>> On a powerpc 8xx, 'btc' fails as follows:
>>>>> Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
> ...
>>>>>
>>>>> Signed-off-by: Christophe Leroy <[email protected]>
>>>>> Cc: <[email protected]> # 4.15+
>>>>
>>>> Would a Fixes: be better here?
>>>> Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")
>>>
>>> Christophe, When you add the Fixes: could you also add my
>>>
>>> Reviewed-by: Daniel Thompson <[email protected]>
>>
>> Ok, thanks for the review, but do I have to do anything really ?
>>
>> The Fixes: and now your Reviewed-by: appear automatically in patchwork
>> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
>> so I believe they'll be automatically included when Jason or someone
>> else takes the patch, no ?
>
> patchwork won't add the Fixes tag from the reply, it needs to be in the
> original mail.
>
> See:
> https://github.com/getpatchwork/patchwork/issues/151
>

Ok, so it accounts it and adds a '1' in the F column in the patches
list, but won't take it into account.

Then I'll send a v2 with revised commit text.

Christophe

2018-09-28 12:58:28

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

Christophe LEROY <[email protected]> writes:
> Le 27/09/2018 à 13:09, Michael Ellerman a écrit :
>> Christophe LEROY <[email protected]> writes:
>>> Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
>>>> On 16/09/2018 20:06, Daniel Thompson wrote:
>>>>> On Fri, Sep 14, 2018 at 12:35:44PM +0000, Christophe Leroy wrote:
>>>>>> On a powerpc 8xx, 'btc' fails as follows:
>>>>>> Entering kdb (current=0x(ptrval), pid 282) due to Keyboard Entry
>> ...
>>>>>>
>>>>>> Signed-off-by: Christophe Leroy <[email protected]>
>>>>>> Cc: <[email protected]> # 4.15+
>>>>>
>>>>> Would a Fixes: be better here?
>>>>> Fixes: ad67b74d2469d9b82 ("printk: hash addresses printed with %p")
>>>>
>>>> Christophe, When you add the Fixes: could you also add my
>>>>
>>>> Reviewed-by: Daniel Thompson <[email protected]>
>>>
>>> Ok, thanks for the review, but do I have to do anything really ?
>>>
>>> The Fixes: and now your Reviewed-by: appear automatically in patchwork
>>> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
>>> so I believe they'll be automatically included when Jason or someone
>>> else takes the patch, no ?
>>
>> patchwork won't add the Fixes tag from the reply, it needs to be in the
>> original mail.
>>
>> See:
>> https://github.com/getpatchwork/patchwork/issues/151
>>
>
> Ok, so it accounts it and adds a '1' in the F column in the patches
> list, but won't take it into account.

Yes. The logic that populates the columns is separate from the logic
that scrapes the tags, which is a bug :)

> Then I'll send a v2 with revised commit text.

Thanks.

cheers

2018-10-01 19:53:32

by Jason Wessel

[permalink] [raw]
Subject: Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

On 09/28/2018 07:57 AM, Michael Ellerman wrote:
> Christophe LEROY <[email protected]> writes:
>> Le 27/09/2018 à 13:09, Michael Ellerman a écrit :
>>> Christophe LEROY <[email protected]> writes:
>>>> Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
>>>>
>>>> The Fixes: and now your Reviewed-by: appear automatically in patchwork
>>>> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
>>>> so I believe they'll be automatically included when Jason or someone
>>>> else takes the patch, no ?
>>>
>>> patchwork won't add the Fixes tag from the reply, it needs to be in the
>>> original mail.
>>>
>>> See:
>>> https://github.com/getpatchwork/patchwork/issues/151
>>>
>>
>> Ok, so it accounts it and adds a '1' in the F column in the patches
>> list, but won't take it into account.
>
> Yes. The logic that populates the columns is separate from the logic
> that scrapes the tags, which is a bug :)
>
>> Then I'll send a v2 with revised commit text.
>


No need. https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git/commit/?h=kgdb-next

Since it is a regression fix, we'll try and get it merged as soon as we can.

Cheers,
Jason.

2018-11-10 21:44:11

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

Hi Jason,

On Mon, Oct 1, 2018 at 12:52 PM Jason Wessel <[email protected]> wrote:
>
> On 09/28/2018 07:57 AM, Michael Ellerman wrote:
> > Christophe LEROY <[email protected]> writes:
> >> Le 27/09/2018 à 13:09, Michael Ellerman a écrit :
> >>> Christophe LEROY <[email protected]> writes:
> >>>> Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
> >>>>
> >>>> The Fixes: and now your Reviewed-by: appear automatically in patchwork
> >>>> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
> >>>> so I believe they'll be automatically included when Jason or someone
> >>>> else takes the patch, no ?
> >>>
> >>> patchwork won't add the Fixes tag from the reply, it needs to be in the
> >>> original mail.
> >>>
> >>> See:
> >>> https://github.com/getpatchwork/patchwork/issues/151
> >>>
> >>
> >> Ok, so it accounts it and adds a '1' in the F column in the patches
> >> list, but won't take it into account.
> >
> > Yes. The logic that populates the columns is separate from the logic
> > that scrapes the tags, which is a bug :)
> >
> >> Then I'll send a v2 with revised commit text.
> >
>
>
> No need. https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git/commit/?h=kgdb-next
>
> Since it is a regression fix, we'll try and get it merged as soon as we can.

Looks like this didn't make it in yet, even with a merge window inbetween? :(

I know first-hand that time to do upstream work can sometimes be hard
to find. I also know that Daniel has shown interest in helping out
here, and is listed as a maintainer. May I suggest that he starts a
tree to collect patches and submit pull requests for a while, until
you find more time for it?

Having a tag-team maintainer setup like we have had for arm-soc has
been very useful especially when one of us get too busy for a while,
etc.


-Olof