| Message ID | ff05bd36b338f4f914780176e9eba8138e1b254b.1536927765.git.christophe.leroy@c-s.fr (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | kdb: use correct pointer when 'btc' calls 'btt' | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | warning | Test checkpatch on branch next |
| snowpatch_ozlabs/build-ppc64le | success | Test build-ppc64le on branch next |
| snowpatch_ozlabs/build-ppc64be | success | Test build-ppc64be on branch next |
| snowpatch_ozlabs/build-ppc64e | success | Test build-ppc64e on branch next |
| snowpatch_ozlabs/build-ppc32 | success | Test build-ppc32 on branch next |
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 <christophe.leroy@c-s.fr> > Cc: <stable@vger.kernel.org> # 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 >
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 <christophe.leroy@c-s.fr> > > Cc: <stable@vger.kernel.org> # 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.
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 <christophe.leroy@c-s.fr> >> Cc: <stable@vger.kernel.org> # 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 <daniel.thompson@linaro.org> 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 >>
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 <christophe.leroy@c-s.fr> >>> Cc: <stable@vger.kernel.org> # 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 <daniel.thompson@linaro.org> 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 >>>
Christophe LEROY <christophe.leroy@c-s.fr> 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 <christophe.leroy@c-s.fr> >>>> Cc: <stable@vger.kernel.org> # 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 <daniel.thompson@linaro.org> > > 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
Le 27/09/2018 à 13:09, Michael Ellerman a écrit : > Christophe LEROY <christophe.leroy@c-s.fr> 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 <christophe.leroy@c-s.fr> >>>>> Cc: <stable@vger.kernel.org> # 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 <daniel.thompson@linaro.org> >> >> 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
Christophe LEROY <christophe.leroy@c-s.fr> writes: > Le 27/09/2018 à 13:09, Michael Ellerman a écrit : >> Christophe LEROY <christophe.leroy@c-s.fr> 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 <christophe.leroy@c-s.fr> >>>>>> Cc: <stable@vger.kernel.org> # 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 <daniel.thompson@linaro.org> >>> >>> 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
On 09/28/2018 07:57 AM, Michael Ellerman wrote: > Christophe LEROY <christophe.leroy@c-s.fr> writes: >> Le 27/09/2018 à 13:09, Michael Ellerman a écrit : >>> Christophe LEROY <christophe.leroy@c-s.fr> 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.
Hi Jason, On Mon, Oct 1, 2018 at 12:52 PM Jason Wessel <jason.wessel@windriver.com> wrote: > > On 09/28/2018 07:57 AM, Michael Ellerman wrote: > > Christophe LEROY <christophe.leroy@c-s.fr> writes: > >> Le 27/09/2018 à 13:09, Michael Ellerman a écrit : > >>> Christophe LEROY <christophe.leroy@c-s.fr> 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
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(); }
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 <christophe.leroy@c-s.fr> Cc: <stable@vger.kernel.org> # 4.15+ --- kernel/debug/kdb/kdb_bt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)