[PATCH 0/5] kdb: Don't implicitly change tasks; plus misc fixups

[PATCH 0/5] kdb: Don't implicitly change tasks; plus misc fixups

[Douglas Anderson]

This started out as just a follow-up to Daniel's post [1].  I wanted
to stop implicitly changing the current task in kdb.  ...but, of
course, everywhere you look in kdb there are things to cleanup, so
this gets a few misc cleanups I found along the way.  Enjoy.

[1] https://lore.kernel.org/r/20191010150735.dhrj3pbjgmjrdpwr@holly.lan


Douglas Anderson (5):
  MIPS: kdb: Remove old workaround for backtracing on other CPUs
  kdb: kdb_current_regs should be private
  kdb: kdb_current_task shouldn't be exported
  kdb: Gid rid of implicit setting of the current task / regs
  kdb: Get rid of confusing diag msg from "rd" if current task has no
    regs

 arch/mips/kernel/traps.c       |  5 -----
 include/linux/kdb.h            |  2 --
 kernel/debug/kdb/kdb_bt.c      |  8 +-------
 kernel/debug/kdb/kdb_main.c    | 31 ++++++++++++++-----------------
 kernel/debug/kdb/kdb_private.h |  2 +-
 5 files changed, 16 insertions(+), 32 deletions(-)

-- 
2.24.0.432.g9d3f5f5b63-goog

[PATCH 1/5] MIPS: kdb: Remove old workaround for backtracing on other CPUs

[Douglas Anderson]

As of commit 2277b492582d ("kdb: Fix stack crawling on 'running' CPUs
that aren't the master") we no longer need any special case for doing
stack dumps on CPUs that are not the kdb master.  Let's remove.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
I have no way to test this personally, so hopefully someone who uses
kdb/kgdb on MIPS can.

Ideally this patch should be Acked by MIPS folks and then land through
the kdb/kgdb tree since the next patch in the series, ("kdb:
kdb_current_regs should be private") depends on it.

 arch/mips/kernel/traps.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 342e41de9d64..46fbcfeaae9a 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -210,11 +210,6 @@ void show_stack(struct task_struct *task, unsigned long *sp)
 			regs.regs[29] = task->thread.reg29;
 			regs.regs[31] = 0;
 			regs.cp0_epc = task->thread.reg31;
-#ifdef CONFIG_KGDB_KDB
-		} else if (atomic_read(&kgdb_active) != -1 &&
-			   kdb_current_regs) {
-			memcpy(&regs, kdb_current_regs, sizeof(regs));
-#endif /* CONFIG_KGDB_KDB */
 		} else {
 			prepare_frametrace(&regs);
 		}
-- 
2.24.0.432.g9d3f5f5b63-goog

Re: [PATCH 1/5] MIPS: kdb: Remove old workaround for backtracing on other CPUs

[Daniel Thompson]

On Sat, Nov 09, 2019 at 11:16:40AM -0800, Douglas Anderson wrote:
> As of commit 2277b492582d ("kdb: Fix stack crawling on 'running' CPUs
> that aren't the master") we no longer need any special case for doing
> stack dumps on CPUs that are not the kdb master.  Let's remove.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> I have no way to test this personally, so hopefully someone who uses
> kdb/kgdb on MIPS can.

I took this as a hint to add mips support to kgdbtest ;-)

Support is added and working well. Unfortunately lack of familiarity
with mips means I have not yet figured out which mips defconfig gives
us working SMP (and what the corresponding qemu invocation should be).

I think that means I still can't (quite) exercise this code fully.
The most appropriate test is bta on an SMP system, right?


> Ideally this patch should be Acked by MIPS folks and then land through
> the kdb/kgdb tree since the next patch in the series, ("kdb:
> kdb_current_regs should be private") depends on it.

An Acked-by from a MIPS maintainer would be very welcome. Perhaps
with a bit of extra work on the above I might be able to provide
a Tested-by:.

I didn't see anything that particularly bothered me in the patches but
given we're already at -rc7 I'm inclined to target this patchset for 5.6
rather than 5.5.


Daniel.

Re: [PATCH 1/5] MIPS: kdb: Remove old workaround for backtracing on other CPUs

[Doug Anderson]

Hi,

On Thu, Nov 14, 2019 at 2:51 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Sat, Nov 09, 2019 at 11:16:40AM -0800, Douglas Anderson wrote:
> > As of commit 2277b492582d ("kdb: Fix stack crawling on 'running' CPUs
> > that aren't the master") we no longer need any special case for doing
> > stack dumps on CPUs that are not the kdb master.  Let's remove.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> > I have no way to test this personally, so hopefully someone who uses
> > kdb/kgdb on MIPS can.
>
> I took this as a hint to add mips support to kgdbtest ;-)
>
> Support is added and working well. Unfortunately lack of familiarity
> with mips means I have not yet figured out which mips defconfig gives
> us working SMP (and what the corresponding qemu invocation should be).

Nice!


> I think that means I still can't (quite) exercise this code fully.
> The most appropriate test is bta on an SMP system, right?

Yeah, or at least "btc".


> > Ideally this patch should be Acked by MIPS folks and then land through
> > the kdb/kgdb tree since the next patch in the series, ("kdb:
> > kdb_current_regs should be private") depends on it.
>
> An Acked-by from a MIPS maintainer would be very welcome. Perhaps
> with a bit of extra work on the above I might be able to provide
> a Tested-by:.
>
> I didn't see anything that particularly bothered me in the patches but
> given we're already at -rc7 I'm inclined to target this patchset for 5.6
> rather than 5.5.

That's fine.  This is all just cleanup stuff so targeting 5.6 is fine.

-Doug

Re: [PATCH 1/5] MIPS: kdb: Remove old workaround for backtracing on other CPUs

[Paul Burton]

Hi Daniel,

On Thu, Nov 14, 2019 at 10:51:25AM +0000, Daniel Thompson wrote:
> On Sat, Nov 09, 2019 at 11:16:40AM -0800, Douglas Anderson wrote:
> > As of commit 2277b492582d ("kdb: Fix stack crawling on 'running' CPUs
> > that aren't the master") we no longer need any special case for doing
> > stack dumps on CPUs that are not the kdb master.  Let's remove.
> > 
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> > I have no way to test this personally, so hopefully someone who uses
> > kdb/kgdb on MIPS can.
> 
> I took this as a hint to add mips support to kgdbtest ;-)

Wonderful! :)

> Support is added and working well. Unfortunately lack of familiarity
> with mips means I have not yet figured out which mips defconfig gives
> us working SMP (and what the corresponding qemu invocation should be).

You can build 64r6el_defconfig & boot it something like this:

$ qemu-system-mips64el \
    -M boston -cpu I6500 -smp 4 \
    -kernel arch/mips/boot/vmlinux.gz.itb \
    -serial stdio \
    -hda my-disk-image.bin \
    -append "root=/dev/sda"

Linux should see the system as a single core with 4 hardware threads
(VPs or Virtual Processors in MIPS terminology).

> > Ideally this patch should be Acked by MIPS folks and then land through
> > the kdb/kgdb tree since the next patch in the series, ("kdb:
> > kdb_current_regs should be private") depends on it.
> 
> An Acked-by from a MIPS maintainer would be very welcome. Perhaps
> with a bit of extra work on the above I might be able to provide
> a Tested-by:.

The patches look reasonable to me; I was hoping to test them before
giving an ack but haven't had the time yet. It seems you may be making
that easier :)

Thanks,
    Paul

Re: [PATCH 0/5] kdb: Don't implicitly change tasks; plus misc fixups

[Doug Anderson]

Hi

On Sat, Nov 9, 2019 at 11:17 AM Douglas Anderson <dianders@chromium.org> wrote:
>
> This started out as just a follow-up to Daniel's post [1].  I wanted
> to stop implicitly changing the current task in kdb.  ...but, of
> course, everywhere you look in kdb there are things to cleanup, so
> this gets a few misc cleanups I found along the way.  Enjoy.
>
> [1] https://lore.kernel.org/r/20191010150735.dhrj3pbjgmjrdpwr@holly.lan
>
>
> Douglas Anderson (5):
>   MIPS: kdb: Remove old workaround for backtracing on other CPUs
>   kdb: kdb_current_regs should be private
>   kdb: kdb_current_task shouldn't be exported
>   kdb: Gid rid of implicit setting of the current task / regs
>   kdb: Get rid of confusing diag msg from "rd" if current task has no
>     regs
>
>  arch/mips/kernel/traps.c       |  5 -----
>  include/linux/kdb.h            |  2 --
>  kernel/debug/kdb/kdb_bt.c      |  8 +-------
>  kernel/debug/kdb/kdb_main.c    | 31 ++++++++++++++-----------------
>  kernel/debug/kdb/kdb_private.h |  2 +-
>  5 files changed, 16 insertions(+), 32 deletions(-)

I noticed that this series doesn't seem to be in linux-next, but I
think it was supposed to target v5.6?  Do you know if there is
anything outstanding or if it'll be queued up sometime soon?

Thanks!


-Doug

Re: [PATCH 0/5] kdb: Don't implicitly change tasks; plus misc fixups

[Andrew Morton]

On Tue, 28 Jan 2020 08:59:01 -0800 Doug Anderson <dianders@chromium.org> wrote:

> Hi
> 
> On Sat, Nov 9, 2019 at 11:17 AM Douglas Anderson <dianders@chromium.org> wrote:
> >
> > This started out as just a follow-up to Daniel's post [1].  I wanted
> > to stop implicitly changing the current task in kdb.  ...but, of
> > course, everywhere you look in kdb there are things to cleanup, so
> > this gets a few misc cleanups I found along the way.  Enjoy.
> >
> > [1] https://lore.kernel.org/r/20191010150735.dhrj3pbjgmjrdpwr@holly.lan
> >
> >
> > Douglas Anderson (5):
> >   MIPS: kdb: Remove old workaround for backtracing on other CPUs
> >   kdb: kdb_current_regs should be private
> >   kdb: kdb_current_task shouldn't be exported
> >   kdb: Gid rid of implicit setting of the current task / regs
> >   kdb: Get rid of confusing diag msg from "rd" if current task has no
> >     regs
> >
> >  arch/mips/kernel/traps.c       |  5 -----
> >  include/linux/kdb.h            |  2 --
> >  kernel/debug/kdb/kdb_bt.c      |  8 +-------
> >  kernel/debug/kdb/kdb_main.c    | 31 ++++++++++++++-----------------
> >  kernel/debug/kdb/kdb_private.h |  2 +-
> >  5 files changed, 16 insertions(+), 32 deletions(-)
> 
> I noticed that this series doesn't seem to be in linux-next, but I
> think it was supposed to target v5.6?  Do you know if there is
> anything outstanding or if it'll be queued up sometime soon?
> 

I grabbed them.

Are there any updates on the testing status, particularly on MIPS?

Re: [PATCH 0/5] kdb: Don't implicitly change tasks; plus misc fixups

[Daniel Thompson]

On Tue, Jan 28, 2020 at 05:42:48PM -0800, Andrew Morton wrote:
> On Tue, 28 Jan 2020 08:59:01 -0800 Doug Anderson <dianders@chromium.org> wrote:
> 
> > Hi
> > 
> > On Sat, Nov 9, 2019 at 11:17 AM Douglas Anderson <dianders@chromium.org> wrote:
> > >
> > > This started out as just a follow-up to Daniel's post [1].  I wanted
> > > to stop implicitly changing the current task in kdb.  ...but, of
> > > course, everywhere you look in kdb there are things to cleanup, so
> > > this gets a few misc cleanups I found along the way.  Enjoy.
> > >
> > > [1] https://lore.kernel.org/r/20191010150735.dhrj3pbjgmjrdpwr@holly.lan
> > >
> > >
> > > Douglas Anderson (5):
> > >   MIPS: kdb: Remove old workaround for backtracing on other CPUs
> > >   kdb: kdb_current_regs should be private
> > >   kdb: kdb_current_task shouldn't be exported
> > >   kdb: Gid rid of implicit setting of the current task / regs
> > >   kdb: Get rid of confusing diag msg from "rd" if current task has no
> > >     regs
> > >
> > >  arch/mips/kernel/traps.c       |  5 -----
> > >  include/linux/kdb.h            |  2 --
> > >  kernel/debug/kdb/kdb_bt.c      |  8 +-------
> > >  kernel/debug/kdb/kdb_main.c    | 31 ++++++++++++++-----------------
> > >  kernel/debug/kdb/kdb_private.h |  2 +-
> > >  5 files changed, 16 insertions(+), 32 deletions(-)
> > 
> > I noticed that this series doesn't seem to be in linux-next, but I
> > think it was supposed to target v5.6?  Do you know if there is
> > anything outstanding or if it'll be queued up sometime soon?
> > 
> 
> I grabbed them.
> 
> Are there any updates on the testing status, particularly on MIPS?

I put in a fair bit of work earlier this dev cycle to add MIPS to the
kgdb test suite. Unfortunately that left me catching up on a few other
things... or putting it another way I am late putting together the kgdb
tree for v5.6. It will be done by the weekend.

The first patch never got a formal Acked-by from the MIPS folks but they
commented positively so we can probably go ahead.


Daniel.