1:57 p.m.
Hello Toshi,
Here are my comments and further suggestions for this patchset:
0001-new-ikconfig-API.patch
0002-dump_kernel_table.patch
In the interest of simplification, let me suggest the following.
With your current scheme, this is required:
crash> extend module.so
...
the module's _init() function gets called, which does:
read_in_kernel_config(IKCFG_SETUP);
ret = get_kernel_config("XXX", str);
read_in_kernel_config(IKCFG_FREE);
...
crash>
And then you have a bunch of stuff for handling multiple extension
modules, reference counting, etc.
But extension modules should not have to worry about having to make
the two read_in_kernel_config() calls -- it should simply be a matter
of doing this to get a kernel configuration item:
ret = get_kernel_config("XXX", str);
This would typically be called one (or more) times in their _init()
function, but it could also be called in a registered function as well.
Anyway, let the read_in_kernel_config(IKCFG_SETUP) and (IKCFG_FREE)
mechanisms be handled by the static crash code:
- read_in_kernel_config(IKCFG_SETUP) should be called by
the exported get_kernel_config() function. (see below)
- read_in_kernel_config(IKCFG_FREE) should be called by the
restore_sanity() function, which gets called just before
the "crash>" prompt for the *next* crash command is displayed.
(see below)
That way, the ikconfig data is only available for the life-time of
a particular command, for example, while the crash "extend" command
is running, or perhaps later on when a module's registered command
is run.
That being the case, all of the stuff you have for handling multiple
extension modules can be removed, because that could never happen.
The kt->ikconfig_setup counter can be removed, and replaced by
a new kt->ikconfig_flags field, that has at least two flags:
#define IKCONFIG_AVAIL 0x1 /* kernel contains ikconfig data */
#define IKCONFIG_LOADED 0x2 /* ikconfig data is currently loaded */
And the would be set/cleared like so:
- When the initialization-time read_in_kernel_config(IKCFG_INIT) call is
made, IKCFG_AVAIL would get set if the ikconfig data is available in
the kernel.
- When read_in_kernel_config(IKCFG_SETUP) successfully reads the ikconfig
data, then IKCFG_LOADED could be be set (temporarily).
- When read_in_kernel_config(IKCFG_FREE) is called, IKCFG_LOADED gets cleared.
That being the case, get_kernel_config() could have this at the top:
switch (kt->ikconfig_flags & (IKCFG_LOADED|IKCFG_AVAIL))
{
case IKCFG_AVAIL:
read_in_kernel_config(IKCFG_SETUP);
if (!kt->config_flags & IKCFG_LOADED)
return IKCONFIG_N;
break;
case (IKCFG_LOADED|IKCFG_AVAIL): /* already loaded */
break;
default:
return IKCFG_N;
}
And restore_sanity() could just do this:
if (kt->ikconfig_flags & IKCFG_LOADED)
read_in_kernel_config(IKCFG_FREE);
And lastly, I see no need for kt->ikconfig_refs. You increment
it in get_kernel_config() each time it is called, and then set
it back to 0 when read_in_kernel_config(IKCFG_FREE) frees everything.
But nobody else reads it -- and I don't see why an extension module
would ever need to read it?
0003-load_module_symbols_helper.patch
Queued for the next release.
Thanks again,
Dave
----- Original Message -----
Declare get_kernel_config(), load_module_symbols_helper()
for crash outside extension users.
CRASH_MODULE_PATH valiable can apply to search non-standard module path
from load_module_symbols_helper() and "mod" command.
- get_kernel_config("config name", &val)
Return one of target kernel configurations.
If val == NULL, return value is poinited whether config is valid or
not.
IKCONFIG_N: kernel config is not set (not valid).
IKCONFIG_Y: kernel config is set y (valid).
IKCONFIG_M: kernel config is set m (valid).
IKCONFIG_STR: kernel config is values or strings, etc (valid).
"help -k" can display ikconfig setup state,
number of valid ikconfig entries, function calls.
Notes:
1. How to activate get_kernel_config().
read_in_kernel_config(IKCFG_SETUP)
-> get_kernel_config() become active.
read_in_kernel_config(IKCFG_FREE)
-> get_kernel_config() become iniactive.
2. This function require CONFIG_IKCONFIG=y. Otherwise user is warned.
Functional change from previous one.
"config name" can be allowed both with or without CONFIG_ prefix strings.
[ Dave's recommendation ]
- load_module_symbols_helper("module name")
Load specified kernel module symbols with argument.
This is simplified usage from original load_module_symbols().
Add "CRASH_MODULE_PATH" valiable which can use to resolve
non-standard module path.
Functional change from previous one.
"CRASH_MODULE_PATH" could be used by the "mod -s" command as well.
[ Dave's recommendation ]
Example usage:
< in .crashrc >
mod -s ext3
mod -s jbd
:
:
export CRASH_MODULE_PATH="your module's root directory"
/usr/sbin/crash
Even if target kernel is changed,
crash will load appropriate ext3 or jbd modules by this valiable.
Toshikazu Nakayama (3):
new ikconfig API.
dump_kernel_table()
load_module_symbols_helper().
crash-5.1.1/defs.h | 14 +++++
crash-5.1.1/kernel.c | 148
++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 162 insertions(+), 0 deletions(-)
--
1.7.4.rc2.3.g60a2e
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility