4:04 a.m.
Hi,
I would like to suggest new ikconfig's API from defs.h.
The get_kernel_config("config name without CONFIG_ prefix", &val)
will 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).
API require CONFIG_IKCONFIG=y. Otherwise user is warned.
I think it is useful for such as extension module developers
to write kernel config switch code.
Although ifdef or symbol APIs can be used instead,
I believe using this API can write more impressive or flexible code
about kernel config depended parts (over my senses...).
Thanks,
Toshi.
-----------------------------------------------------------
Example usage: Resolve RADIX_TREE_MAP_SHIFT from config depends.
[ code of kernel-2.6.35 ]
#define RADIX_TREE_MAP_SHIFT (CONFIG_BASE_SMALL ? 4 : 6)
[ code of crash utility or extension modules ]
static int radix_tree_map_shift = 6;
#define RADIX_TREE_MAP_SHIFT radix_tree_map_shift
int _init(void)
{
read_in_kernel_config(IKCFG_SETUP);
if (get_kernel_config("BASE_SMALL", NULL) == IKCONFIG_Y)
radix_tree_map_shift = 4;
return 0;
}
int int _fini(void)
{
read_in_kernel_config(IKCFG_FREE);
return 0;
}
Toshikazu Nakayama (2):
new ikconfig API.
dump_kernel_table()
crash-5.1.1/defs.h | 13 ++++
crash-5.1.1/kernel.c | 166 +++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 178 insertions(+), 1 deletions(-)
--
1.7.4.rc2
10:59 a.m.
----- Original Message -----
Hi,
I would like to suggest new ikconfig's API from defs.h.
The get_kernel_config("config name without CONFIG_ prefix", &val)
will 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).
API require CONFIG_IKCONFIG=y. Otherwise user is warned.
I think it is useful for such as extension module developers
to write kernel config switch code.
Although ifdef or symbol APIs can be used instead,
I believe using this API can write more impressive or flexible code
about kernel config depended parts (over my senses...).
This does look like a good idea for extension modules. I'm off today,
but will check your patches out tomorrow.
Thanks,
Dave
Thanks,
Toshi.
-----------------------------------------------------------
Example usage: Resolve RADIX_TREE_MAP_SHIFT from config depends.
[ code of kernel-2.6.35 ]
#define RADIX_TREE_MAP_SHIFT (CONFIG_BASE_SMALL ? 4 : 6)
[ code of crash utility or extension modules ]
static int radix_tree_map_shift = 6;
#define RADIX_TREE_MAP_SHIFT radix_tree_map_shift
int _init(void)
{
read_in_kernel_config(IKCFG_SETUP);
if (get_kernel_config("BASE_SMALL", NULL) == IKCONFIG_Y)
radix_tree_map_shift = 4;
return 0;
}
int int _fini(void)
{
read_in_kernel_config(IKCFG_FREE);
return 0;
}
Toshikazu Nakayama (2):
new ikconfig API.
dump_kernel_table()
crash-5.1.1/defs.h | 13 ++++
crash-5.1.1/kernel.c | 166
+++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 178 insertions(+), 1 deletions(-)
--
1.7.4.rc2
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
11:12 a.m.
----- Original Message -----
Hi,
I would like to suggest new ikconfig's API from defs.h.
The get_kernel_config("config name without CONFIG_ prefix", &val)
will 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).
API require CONFIG_IKCONFIG=y. Otherwise user is warned.
I think it is useful for such as extension module developers
to write kernel config switch code.
Although ifdef or symbol APIs can be used instead,
I believe using this API can write more impressive or flexible code
about kernel config depended parts (over my senses...).
Thanks,
Toshi.
Hello Toshi,
I have a few issues/questions with your patch:
1. This should be impossible, because even if preload_extensions()
is used, that function is called long after the init-time call
to read_in_kernel_config(IKCFG_INIT) is done:
+ if (command == IKCFG_INIT && kt->ikconfig_setup) {
+ /* fastpath */
+ if (get_kernel_config("DEBUG_BUGVERBOSE", NULL) == IKCONFIG_N)
+ kt->flags |= BUGVERBOSE_OFF;
+
+ if (get_kernel_config("NR_CPUS", &val) == IKCONFIG_STR) {
+ kt->kernel_NR_CPUS = atoi(val);
+ if (CRASHDEBUG(1))
+ error(INFO, "CONFIG_NR_CPUS: %d\n",
+ kt->kernel_NR_CPUS);
+ }
+ if (get_kernel_config("PGTABLE_4", NULL) == IKCONFIG_Y) {
+ machdep->flags |= VM_4_LEVEL;
+ if (CRASHDEBUG(1))
+ error(INFO, "CONFIG_PGTABLE_4\n");
+ }
+ if (get_kernel_config("HZ", &val) == IKCONFIG_STR) {
+ machdep->hz = atoi(val);
+ if (CRASHDEBUG(1))
+ error(INFO, "CONFIG_HZ: %d\n", machdep->hz);
+ }
+
+ goto out1;
+ }
2. I didn't test this, but I think this will be a problem because it will
not return back to the _init() function in the extension module:
if ((sp = symbol_search("kernel_config_data")) == NULL) {
- if (command == IKCFG_READ)
+ if (command == IKCFG_READ || command == IKCFG_SETUP)
error(FATAL,
"kernel_config_data does not exist in this
kernel\n");
return;
It should do this:
If IKCONFIG_INIT: return quietly with no error message
If IKCONFIG_READ: error(FATAL, ...
If IKCONFIG_SETUP: error(WARNING, ...
2. Can you explain the need for the IKCONFIG_SORT_INTERVAL stuff?
3. When adding items to any global structure (such as to the kernel_table),
I always add them to the end of the structure so that any pre-existing
extension modules that use something in the unmodified structure will
still work even if additional entries are added to the end.
4. Just a preference on my part, but I would allow get_kernel_config()
to accept both "CONFIG_XXXX" and "XXX" as a "name"
argument, and then
just strip off "CONFIG_" if it's there.
Dave
-----------------------------------------------------------
Example usage: Resolve RADIX_TREE_MAP_SHIFT from config depends.
[ code of kernel-2.6.35 ]
#define RADIX_TREE_MAP_SHIFT (CONFIG_BASE_SMALL ? 4 : 6)
[ code of crash utility or extension modules ]
static int radix_tree_map_shift = 6;
#define RADIX_TREE_MAP_SHIFT radix_tree_map_shift
int _init(void)
{
read_in_kernel_config(IKCFG_SETUP);
if (get_kernel_config("BASE_SMALL", NULL) == IKCONFIG_Y)
radix_tree_map_shift = 4;
return 0;
}
int int _fini(void)
{
read_in_kernel_config(IKCFG_FREE);
return 0;
}
Toshikazu Nakayama (2):
new ikconfig API.
dump_kernel_table()
crash-5.1.1/defs.h | 13 ++++
crash-5.1.1/kernel.c | 166
+++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 178 insertions(+), 1 deletions(-)
--
1.7.4.rc2
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
10:57 p.m.
Hi Dave,
I shuold agree with your comments at all, thanks a lot.
Hello Toshi,
I have a few issues/questions with your patch:
1. This should be impossible, because even if preload_extensions()
is used, that function is called long after the init-time call
to read_in_kernel_config(IKCFG_INIT) is done:
You're right. I'll remove this condition because this never hit as TRUE.
+ if (command == IKCFG_INIT && kt->ikconfig_setup) {
+ /* fastpath */
+ if (get_kernel_config("DEBUG_BUGVERBOSE", NULL) == IKCONFIG_N)
+ kt->flags |= BUGVERBOSE_OFF;
+
+ if (get_kernel_config("NR_CPUS", &val) == IKCONFIG_STR) {
+ kt->kernel_NR_CPUS = atoi(val);
+ if (CRASHDEBUG(1))
+ error(INFO, "CONFIG_NR_CPUS: %d\n",
+ kt->kernel_NR_CPUS);
+ }
+ if (get_kernel_config("PGTABLE_4", NULL) == IKCONFIG_Y) {
+ machdep->flags |= VM_4_LEVEL;
+ if (CRASHDEBUG(1))
+ error(INFO, "CONFIG_PGTABLE_4\n");
+ }
+ if (get_kernel_config("HZ", &val) == IKCONFIG_STR) {
+ machdep->hz = atoi(val);
+ if (CRASHDEBUG(1))
+ error(INFO, "CONFIG_HZ: %d\n", machdep->hz);
+ }
+
+ goto out1;
+ }
2. I didn't test this, but I think this will be a problem because it will
not return back to the _init() function in the extension module:
if ((sp = symbol_search("kernel_config_data")) == NULL) {
- if (command == IKCFG_READ)
+ if (command == IKCFG_READ || command == IKCFG_SETUP)
error(FATAL,
"kernel_config_data does not exist in this
kernel\n");
return;
It should do this:
If IKCONFIG_INIT: return quietly with no error message
If IKCONFIG_READ: error(FATAL, ...
If IKCONFIG_SETUP: error(WARNING, ...
I'll change with this manner.
2. Can you explain the need for the IKCONFIG_SORT_INTERVAL stuff?
Example for CONFIG_X86(_64) which is probably referenced many times from
extensions, it shuold be in front position of config list for rapid loop break.
In my environs, kernel owned around 400 valid kernel configs.
However, I'll remove them except kt->ikconfig_refs.
Extensions may not repeat get_kernel_config(), it's only in their initialization.
Also such a major configs are already existing at front of kernel_config_data.
(In addition to these reasons, can not work for "is not set" configs.)
The kt->ikconfig_refs is remained as get_kernel_config() call (reference)
counters including the case of IKCONFIG_N.
3. When adding items to any global structure (such as to the
kernel_table),
I always add them to the end of the structure so that any pre-existing
extension modules that use something in the unmodified structure will
still work even if additional entries are added to the end.
I could not be aware of this issue, will fix my bad-mannered.
In my understanding, this is not indicating position
at dump_kernel_table(), correct?
4. Just a preference on my part, but I would allow
get_kernel_config()
to accept both "CONFIG_XXXX" and "XXX" as a "name"
argument, and then
just strip off "CONFIG_" if it's there.
I also prefer this idea, I'll update patch to support "CONFIG_name"
argument.
Thanks,
Toshi.
Dave
> -----------------------------------------------------------
>
> Example usage: Resolve RADIX_TREE_MAP_SHIFT from config depends.
>
> [ code of kernel-2.6.35 ]
>
> #define RADIX_TREE_MAP_SHIFT (CONFIG_BASE_SMALL ? 4 : 6)
>
> [ code of crash utility or extension modules ]
>
> static int radix_tree_map_shift = 6;
> #define RADIX_TREE_MAP_SHIFT radix_tree_map_shift
>
> int _init(void)
> {
> read_in_kernel_config(IKCFG_SETUP);
>
> if (get_kernel_config("BASE_SMALL", NULL) == IKCONFIG_Y)
> radix_tree_map_shift = 4;
>
> return 0;
> }
>
> int int _fini(void)
> {
> read_in_kernel_config(IKCFG_FREE);
>
> return 0;
> }
>
> Toshikazu Nakayama (2):
> new ikconfig API.
> dump_kernel_table()
>
> crash-5.1.1/defs.h | 13 ++++
> crash-5.1.1/kernel.c | 166
> +++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 178 insertions(+), 1 deletions(-)
>
> --
> 1.7.4.rc2
>
> --
> Crash-utility mailing list
> Crash-utility(a)redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
5059
days inactive
5062
days old
devel@lists.crash-utility.osci.io
3 comments
2 participants
participants (2)
-
Dave Anderson -
Toshikazu Nakayama