Hi Doug, On Tue, 2 Mar 2021 at 00:10, Doug Anderson <diand...@chromium.org> wrote: > > Hi, > > On Tue, Feb 23, 2021 at 11:08 PM Sumit Garg <sumit.g...@linaro.org> wrote: > > > > Simplify kdb commands registration via using linked list instead of > > static array for commands storage. > > > > Signed-off-by: Sumit Garg <sumit.g...@linaro.org> > > --- > > > > Changes in v5: > > - Introduce new method: kdb_register_table() to register static kdb > > main and breakpoint command tables instead of using statically > > allocated commands. > > > > Changes in v4: > > - Fix kdb commands memory allocation issue prior to slab being available > > with an array of statically allocated commands. Now it works fine with > > kgdbwait. > > - Fix a misc checkpatch warning. > > - I have dropped Doug's review tag as I think this version includes a > > major fix that should be reviewed again. > > > > Changes in v3: > > - Remove redundant "if" check. > > - Pick up review tag from Doug. > > > > Changes in v2: > > - Remove redundant NULL check for "cmd_name". > > - Incorporate misc. comment. > > > > kernel/debug/kdb/kdb_bp.c | 81 ++++-- > > kernel/debug/kdb/kdb_main.c | 472 ++++++++++++++++++++------------- > > kernel/debug/kdb/kdb_private.h | 3 + > > 3 files changed, 343 insertions(+), 213 deletions(-) > > This looks good to me, thanks! > > Random notes: > > * We no longer check for "duplicate" commands for any of these > statically allocated ones, but I guess that's fine.
Yeah, I think that check is redundant for static ones. > > * Presumably nothing outside of kdb/kgdb itself needs the ability to > allocate commands statically. The only user I see now is ftrace and > it looks like it runs late enough that it should be fine. Agree. > > Reviewed-by: Douglas Anderson <diand...@chromium.org> > Thanks, -Sumit > > -Doug