kdb: Fix incomplete usage help of md, mds and btc commands

[PATCH] kdb: Fix incomplete usage help of md, mds and btc commands

Posted by Nir Lichtman 3 weeks, 2 days ago

Fix kdb usage help to document some currently missing CLI commands options

Signed-off-by: Nir Lichtman <nir@lichtman.org>
---
 kernel/debug/kdb/kdb_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index f5f7d7fb5936..0bdffb17b163 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2667,7 +2667,7 @@ EXPORT_SYMBOL_GPL(kdb_unregister);
 static kdbtab_t maintab[] = {
 	{	.name = "md",
 		.func = kdb_md,
-		.usage = "<vaddr>",
+		.usage = "<vaddr> [lines] [radix]",
 		.help = "Display Memory Contents, also mdWcN, e.g. md8c1",
 		.minlen = 1,
 		.flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
@@ -2686,7 +2686,7 @@ static kdbtab_t maintab[] = {
 	},
 	{	.name = "mds",
 		.func = kdb_md,
-		.usage = "<vaddr>",
+		.usage = "<vaddr> [lines] [radix]",
 		.help = "Display Memory Symbolically",
 		.flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
 	},
@@ -2743,8 +2743,8 @@ static kdbtab_t maintab[] = {
 	},
 	{	.name = "btc",
 		.func = kdb_bt,
-		.usage = "",
-		.help = "Backtrace current process on each cpu",
+		.usage = "[<cpu>]",
+		.help = "Backtrace current process on each cpu, or a specific one if cpu is passed",
 		.flags = KDB_ENABLE_INSPECT,
 	},
 	{	.name = "btt",
-- 
2.39.2

Re: [PATCH] kdb: Fix incomplete usage help of md, mds and btc commands

Posted by Doug Anderson 3 weeks, 2 days ago

Hi,

On Thu, Oct 31, 2024 at 1:40 PM Nir Lichtman <nir@lichtman.org> wrote:
>
> Fix kdb usage help to document some currently missing CLI commands options
>
> Signed-off-by: Nir Lichtman <nir@lichtman.org>
> ---
>  kernel/debug/kdb/kdb_main.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Some of this is a bit similar to what I tried to do at :

https://lore.kernel.org/r/20240617173426.2.I5621f286f5131c84ac71a212508ba1467ac443f2@changeid

...but that series kinda fell on the floor because my end goal was to
try to get it so I could access IO memory and Daniel pointed out that
what I was doing was unsafe. The earlier patches in the series are
overall good cleanups, though. If you're interested feel free to
iterate on any of them.

> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index f5f7d7fb5936..0bdffb17b163 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -2667,7 +2667,7 @@ EXPORT_SYMBOL_GPL(kdb_unregister);
>  static kdbtab_t maintab[] = {
>         {       .name = "md",
>                 .func = kdb_md,
> -               .usage = "<vaddr>",
> +               .usage = "<vaddr> [lines] [radix]",

In my patch, I said:

.usage = "<vaddr> [<lines> [<radix>]]",

...so I had the <> characters and nested the []. I think that the <>
is supposed to signify that you're not supposed to write the text
"lines" but that it's a variable.

>                 .help = "Display Memory Contents, also mdWcN, e.g. md8c1",
>                 .minlen = 1,
>                 .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
> @@ -2686,7 +2686,7 @@ static kdbtab_t maintab[] = {
>         },
>         {       .name = "mds",
>                 .func = kdb_md,
> -               .usage = "<vaddr>",
> +               .usage = "<vaddr> [lines] [radix]",

From my prior research, "mds" doesn't support <radix>. However, some
of the other "md" commands that you didn't modify do support
lines/radix. Let me know if I got that wrong.

-Doug

Re: [PATCH] kdb: Fix incomplete usage help of md, mds and btc commands

Posted by Nir Lichtman 3 weeks ago

On Fri, Nov 01, 2024 at 08:42:43AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Thu, Oct 31, 2024 at 1:40 PM Nir Lichtman <nir@lichtman.org> wrote:
> >
> > Fix kdb usage help to document some currently missing CLI commands options
> >
> > Signed-off-by: Nir Lichtman <nir@lichtman.org>
> > ---
> >  kernel/debug/kdb/kdb_main.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Some of this is a bit similar to what I tried to do at :
> 
> https://lore.kernel.org/r/20240617173426.2.I5621f286f5131c84ac71a212508ba1467ac443f2@changeid
> 
> ...but that series kinda fell on the floor because my end goal was to
> try to get it so I could access IO memory and Daniel pointed out that
> what I was doing was unsafe. The earlier patches in the series are
> overall good cleanups, though. If you're interested feel free to
> iterate on any of them.

Interesting, I'll take a look at that.

> 
> 
> > diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> > index f5f7d7fb5936..0bdffb17b163 100644
> > --- a/kernel/debug/kdb/kdb_main.c
> > +++ b/kernel/debug/kdb/kdb_main.c
> > @@ -2667,7 +2667,7 @@ EXPORT_SYMBOL_GPL(kdb_unregister);
> >  static kdbtab_t maintab[] = {
> >         {       .name = "md",
> >                 .func = kdb_md,
> > -               .usage = "<vaddr>",
> > +               .usage = "<vaddr> [lines] [radix]",
> 
> In my patch, I said:
> 
> .usage = "<vaddr> [<lines> [<radix>]]",
> 
> ...so I had the <> characters and nested the []. I think that the <>
> is supposed to signify that you're not supposed to write the text
> "lines" but that it's a variable.

Sounds reasonable, just used the [lines] as it is currently in the help of
the dmesg command but the reasoning you point out makes sense.

> 
> 
> >                 .help = "Display Memory Contents, also mdWcN, e.g. md8c1",
> >                 .minlen = 1,
> >                 .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
> > @@ -2686,7 +2686,7 @@ static kdbtab_t maintab[] = {
> >         },
> >         {       .name = "mds",
> >                 .func = kdb_md,
> > -               .usage = "<vaddr>",
> > +               .usage = "<vaddr> [lines] [radix]",
> 
> From my prior research, "mds" doesn't support <radix>. However, some
> of the other "md" commands that you didn't modify do support
> lines/radix. Let me know if I got that wrong.

"mds" does support radix, I'll check and make sure about the other variants.

Thanks,
Nir