2020-05-21 07:25:10

by liwei (GF)

Subject: [PATCH v3] kdb: Remove the misfeature 'KDBFLAGS'

Currently, 'KDBFLAGS' is an internal variable of kdb, it is combined
by 'KDBDEBUG' and state flags. It will be shown only when 'KDBDEBUG'
is set, and the user can define an environment variable named 'KDBFLAGS'
too. These are puzzling indeed.

After communication with Daniel, it seems that 'KDBFLAGS' is a misfeature.
So let's replace 'KDBFLAGS' with 'KDBDEBUG' to just show the value we
wrote into. After this modification, we can use `md4c1 kdb_flags` instead,
to observe the state flags.

Suggested-by: Daniel Thompson <[email protected]>
Signed-off-by: Wei Li <[email protected]>
---
v2 -> v3:
- Change to replace the internal env 'KDBFLAGS' with 'KDBDEBUG'.
v1 -> v2:
- Fix lack of braces.

kernel/debug/kdb/kdb_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4fc43fb17127..392029287083 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -418,8 +418,7 @@ int kdb_set(int argc, const char **argv)
argv[2]);
return 0;
}
- kdb_flags = (kdb_flags &
- ~(KDB_DEBUG_FLAG_MASK << KDB_DEBUG_FLAG_SHIFT))
+ kdb_flags = (kdb_flags & ~KDB_DEBUG(MASK))
| (debugflags << KDB_DEBUG_FLAG_SHIFT);

return 0;
@@ -2081,7 +2080,8 @@ static int kdb_env(int argc, const char **argv)
}

if (KDB_DEBUG(MASK))
- kdb_printf("KDBFLAGS=0x%x\n", kdb_flags);
+ kdb_printf("KDBDEBUG=0x%x\n",
+ (kdb_flags & KDB_DEBUG(MASK)) >> KDB_DEBUG_FLAG_SHIFT);

return 0;
}
--
2.17.1

2020-05-21 15:50:12

by Daniel Thompson

Subject: Re: [Kgdb-bugreport] [PATCH v3] kdb: Remove the misfeature 'KDBFLAGS'

On Thu, May 21, 2020 at 03:21:25PM +0800, Wei Li wrote:
> Currently, 'KDBFLAGS' is an internal variable of kdb, it is combined
> by 'KDBDEBUG' and state flags. It will be shown only when 'KDBDEBUG'
> is set, and the user can define an environment variable named 'KDBFLAGS'
> too. These are puzzling indeed.
>
> After communication with Daniel, it seems that 'KDBFLAGS' is a misfeature.
> So let's replace 'KDBFLAGS' with 'KDBDEBUG' to just show the value we
> wrote into. After this modification, we can use `md4c1 kdb_flags` instead,
> to observe the state flags.
>
> Suggested-by: Daniel Thompson <[email protected]>
> Signed-off-by: Wei Li <[email protected]>
> ---
> v2 -> v3:
> - Change to replace the internal env 'KDBFLAGS' with 'KDBDEBUG'.
> v1 -> v2:
> - Fix lack of braces.
>
> kernel/debug/kdb/kdb_main.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 4fc43fb17127..392029287083 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -418,8 +418,7 @@ int kdb_set(int argc, const char **argv)
> argv[2]);
> return 0;
> }
> - kdb_flags = (kdb_flags &
> - ~(KDB_DEBUG_FLAG_MASK << KDB_DEBUG_FLAG_SHIFT))
> + kdb_flags = (kdb_flags & ~KDB_DEBUG(MASK))
> | (debugflags << KDB_DEBUG_FLAG_SHIFT);
>
> return 0;
> @@ -2081,7 +2080,8 @@ static int kdb_env(int argc, const char **argv)
> }
>
> if (KDB_DEBUG(MASK))
> - kdb_printf("KDBFLAGS=0x%x\n", kdb_flags);
> + kdb_printf("KDBDEBUG=0x%x\n",
> + (kdb_flags & KDB_DEBUG(MASK)) >> KDB_DEBUG_FLAG_SHIFT);

For this expression to work correctly, kdb_flags, need to be unsigned
(otherwise we get an arithmetic right shift and mis-report when
KDBDEBUG == 0xfff).

This is just FYI, I think I can fix this up when applying...


Daniel.

2020-05-28 11:05:41

by Daniel Thompson

Subject: Re: [Kgdb-bugreport] [PATCH v3] kdb: Remove the misfeature 'KDBFLAGS'

On Thu, May 21, 2020 at 03:21:25PM +0800, Wei Li wrote:
> Currently, 'KDBFLAGS' is an internal variable of kdb, it is combined
> by 'KDBDEBUG' and state flags. It will be shown only when 'KDBDEBUG'
> is set, and the user can define an environment variable named 'KDBFLAGS'
> too. These are puzzling indeed.
>
> After communication with Daniel, it seems that 'KDBFLAGS' is a misfeature.
> So let's replace 'KDBFLAGS' with 'KDBDEBUG' to just show the value we
> wrote into. After this modification, we can use `md4c1 kdb_flags` instead,
> to observe the state flags.
>
> Suggested-by: Daniel Thompson <[email protected]>
> Signed-off-by: Wei Li <[email protected]>

Applied. Thanks.


Daniel.

> ---
> v2 -> v3:
> - Change to replace the internal env 'KDBFLAGS' with 'KDBDEBUG'.
> v1 -> v2:
> - Fix lack of braces.
>
> kernel/debug/kdb/kdb_main.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 4fc43fb17127..392029287083 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -418,8 +418,7 @@ int kdb_set(int argc, const char **argv)
> argv[2]);
> return 0;
> }
> - kdb_flags = (kdb_flags &
> - ~(KDB_DEBUG_FLAG_MASK << KDB_DEBUG_FLAG_SHIFT))
> + kdb_flags = (kdb_flags & ~KDB_DEBUG(MASK))
> | (debugflags << KDB_DEBUG_FLAG_SHIFT);
>
> return 0;
> @@ -2081,7 +2080,8 @@ static int kdb_env(int argc, const char **argv)
> }
>
> if (KDB_DEBUG(MASK))
> - kdb_printf("KDBFLAGS=0x%x\n", kdb_flags);
> + kdb_printf("KDBDEBUG=0x%x\n",
> + (kdb_flags & KDB_DEBUG(MASK)) >> KDB_DEBUG_FLAG_SHIFT);
>
> return 0;
> }
> --
> 2.17.1
>
>
>
> _______________________________________________
> Kgdb-bugreport mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport